Quicktime fixes - #4463
Open
Darlokt wants to merge 4 commits into
Open
Conversation
QTReader treated the stsz sample-count field as the first sample size and the stco entry-count field as the first chunk offset. It then advanced a one-chunk movie by the following sample's size instead of the preceding sample's size. Together these errors shifted variable-size sample boundaries and could make valid compressed frames unreadable. Read both tables from the first actual entry and derive each subsequent offset from the size of the sample immediately before it. Also keep the mdat end as an absolute file offset and apply the first-sample scale to the final sample boundary, so leading ftyp/wide atoms and the last frame are handled consistently. Add a six-frame QTRLE fixture with a non-zero mdat offset, one stco entry, and variable stsz entries. The regression test verifies that all six samples decode to the expected image.
QuickTime RLE delta frames depend on the pixels decoded for the preceding frame. A newly initialized or deserialized QTReader has sample-table metadata but no previous pixel buffer, so opening a later plane directly can return null or incorrect pixels. Memoizer makes that state especially visible because each fresh reader can load initialized metadata and immediately request an arbitrary plane. Track the absence of a decoded plane with -1 and, before decoding a non-sequential QTRLE request, replay the preceding frames to reconstruct the required reference image. Sequential reads and repeated reads retain the existing fast path; codecs without QTRLE frame dependencies are unchanged. Extend the QTRLE regression fixture to seed a memo, create a fresh Memoizer for every plane, assert each reader loaded from the memo, and compare every randomly requested plane with sequential direct-reader output. Fixes ome#4091.
melissalinkert
requested changes
Aug 3, 2026
Keep pixelBytes as a payload length while deriving the final sample boundary from its absolute end. Index variable sample sizes directly when constructing frame offsets, and close test Memoizers on every path with try-with-resources.
Author
|
This should address it all, I saw that I also forgot the closeable in #4464 so i will clean it up. |
Closed
Replace deferred deleteOnExit cleanup with immediate recursive deletion in always-run teardown. Ensure temporary fixtures created before setup failures are tracked and cleanup errors are reported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hej,
while investigating weird memoization behaviors, I came upon the old issue #4091. It was on the way, and only partially related to the memoization problems.
These commits now fix QuickTime parsing for variable-sized samples and random access to QTRLE delta frames after memoization, by which it fixes #4091
Changes