Repository navigation
fix(rendering): stop the AVAssetReader decoder answering with frames from the wrong time - #2638
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
richiemcilroy
added a commit
that referenced
this pull request
Oct 16, 2026
…hanism An expected failure used to excuse its whole (case, class) pair at any size, so a flaky or glob entry could hide a regression of the same class (every Windows export timestamp, the Instant mic behind a known system-audio loss). - Records carry measurements per failing class: frames per stream, output frame range and largest source-frame error for frame classes; count plus the check's own value (offset, spread, missing and lost markers, colour error, frame periods off the grid, failing share of seeded sequences) otherwise. - Every entry in crates/verify/expectations.toml states the bound of its documented mechanism (require_bound). A failure outside it, or with a nonzero metric the bound does not name, is a new failure, with the reason in the case summary. `starved` entries apply only when the case's sentinel saw the machine stall; `hosts` scopes an entry to a machine class (CAP_VERIFY_HOST). - Audio and sync are per source (audio-mic, audio-system, sync-mic, sync-system; real capture per pair), and the Instant oracle's frames missing from the file are frames-missing, never the wall-clock dropped class. - The export/* Windows timestamps entry is now starved-only with the reorder signature (every off-grid frame exactly one period off); the decode-deadline entries link #2636, the decoder entries #2638. - #2632 merged: the Instant processing-clock timestamps and the camera-clock entries are removed. - Real capture checks start and end coverage against the stimulus truth (a first picture later than 0.3 s after the start, or pictures lost before the stop, fail) with self-check injections for both; a skipped system-only case reports unsupported instead of nothing. - The recorded Studio fixture is checked against the frames its generator emitted, so a lost start or final segment cannot just shorten the model. - Cases labelled by decoder confirm the decoder that ran (real-time playback and colour cases on Media Foundation); CAP_VERIFY_UNSUPPORTED_ALLOW lists the cases a runner may skip when CAP_VERIFY_REQUIRE_SUPPORTED is set. - The decoder model keeps each sequence running after a wrong answer, so a seed's recorded classes are its full failure set.
richiemcilroy
added a commit
that referenced
this pull request
Oct 16, 2026
Blessed from the decoder model records of Engine verify 38851482697 (fast) and 38851482675 (full), Blacksmith macOS (AVAssetReader) and Linux (FFmpeg), now that each sequence keeps running after its first wrong answer: 89 entries added or widened with the later classes of listed seeds (resets, overread, grid-rounding, future, stale). Entries for a case and class that already existed keep their reason; new ones name the class and the decoder's mechanisms (AVAssetReader ones link #2638).
richiemcilroy
force-pushed
the
fix/avf-decoder-requests
branch
from
October 16, 2026 04:59
0d63cd0 to
ae94a87
Compare
Member
Author
|
hey @greptileai, please re-review the PR |
1 similar comment
Member
Author
|
hey @greptileai, please re-review the PR |
richiemcilroy
force-pushed
the
fix/avf-decoder-requests
branch
from
October 16, 2026 05:57
ae94a87 to
de46f23
Compare
…y from a fresh seek when it is gone
…ame from an older seek
richiemcilroy
force-pushed
the
fix/avf-decoder-requests
branch
from
October 16, 2026 14:17
de46f23 to
6188bc5
Compare
This comment has been minimized.
This comment has been minimized.
CI on this branch reports the camera runaway, keyframe-seek, seek-storm, VFR-hold and late-camera cases passing on macOS and the jitter-camera FFmpeg cases passing on Linux; the FFmpeg and AVAssetReader decoder models no longer produce 159 seed failures. Flaky entries for the same mechanisms are removed where the seek and jitter benchmarks (4 pairs each) and the VFR-hold run no longer show them.
Member
Author
|
@greptileai review |
This was referenced Oct 16, 2026
This branch was successfully deployed
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
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.
The AVAssetReader decoder (macOS) answers some frame requests with the wrong source frame: a jittery camera runs up to 74 s ahead in a 120 s export, a camera whose first sample is late plays ahead from the first frame, seeks land in the previous GOP, and reused readers answer from an older seek. The pts-hole map that causes the camera runaway is shared with the FFmpeg decoder, so the same 120 s export is also wrong on Linux and Windows builds that decode with FFmpeg.
This PR fixes six request-handling root causes in
crates/rendering/src/decoderand the AVAssetReader start time incrates/video-decode. It supersedes #2595 (its decoder commits) and #2603. The IOSurface part of #2595 already shipped in #2625.flowchart TD R[frame request] --> P{before the track's first frame?} P -- yes --> F[answer with the first frame from the cache] P -- no --> S[pick a pooled reader] S --> X{reader already vended past the request,<br/>or its newest cached frame comes from an older seek?} X -- yes --> Z[reset: seek to the exact keyframe tick] X -- no --> V[read forward] Z --> V V --> G{request falls in a pts gap} G -- previous vend still cached --> H[answer with the previous vend] G -- previous vend evicted --> T[retry once from a fresh seek]1. Pts-hole map drops the hole it just recorded
record_pts_hole(crates/rendering/src/decoder/mod.rs:683) kept 64 holes and evicted the narrowest one. A camera that delivers about 12 fps into a 30 fps track creates a narrow hole per sample, so after 64 holes every new hole was evicted as soon as it was recorded. Requests inside the forgotten holes read one sample further ahead each time, and the camera drifted up to 2232 source frames (74 s) ahead on AVAssetReader. FFmpeg uses the same map and showed 1551 stale camera frames instead.MAX_TRACKED_HOLES,mod.rs:677), enough for every hole of a realistic recording.2. A pooled reader that already passed the request is reused
After a read, a reader's pool position is capped to the newest request, so a reader that decoded past a frame still looks usable for it. It can never vend that frame again, and its next sample is a later frame (seek 0, then 8.8 s, then 0.017 s shows source 1 for source 0).
reader_passed_request(crates/rendering/src/decoder/avassetreader.rs:1384) makesselect_best_decoderreset such a reader before reading.3. The reader's previous vend is no longer held
When a request falls in a gap between two vended frames, the only correct answer is the frame vended just before it. Cross-fades read one decoder at two positions, and a reset for one position purged the other position's frames from the cache; a backward jump hit the same eviction. The nearest older cached frame then answered (1048 for 1049).
cache_eviction_victim,avassetreader.rs:1402, andpinned_frames,avassetreader.rs:968).answer_misses_previous_vend,avassetreader.rs:1390), which re-vends that frame.4. Requests before a late track's first frame step the reader
A camera whose first sample is 2 s into the recording should hold that first frame until then. Every earlier request read one more sample instead, so the camera ran up to 53 frames ahead from output frame 8.
avassetreader.rs:879), with no reset and no passed-reader seek.5. AVAssetReader starts on a keyframe rounded to 10 ms
get_reader_track_outputstarted the reader atcm::Time::with_secs(t, 100). When the keyframe time rounds down below the keyframe pts, the reader decodes the whole previous GOP and vends its last frame, so the frame after a seek showedkeyframe - 1.KeyframeIndex::exact_time,crates/video-decode/src/avassetreader.rs:384, used byreader_start_time,:411). Without a keyframe index it keeps the old start.6. A reused reader answers from an older, unrelated seek
A request the reader has passed, or that falls in a gap, is answered from the newest cached frame at or before it. That frame is only valid if this reader vended it in its current run; otherwise it is a leftover from an earlier seek (seek storm frame 349 showed 237).
reuse_would_serve_stale_frame(avassetreader.rs:1430) tracks the start of the current vend run and resets the reader when the cached answer predates it, or when nothing at or before the request is cached.Wrong frames per case
Oracle suite, full tier, wrong frames per stream (checked frames in brackets). macOS on Blacksmith
macos-15runners, 3 full runs each on main and on this branch; Linux onubuntu-24.04.export/jitter-camera-120s/full/mp460/avfplayback/jitter-camera-120s/full/seq60-from0/avfexport/jitter-camera-120s/full/mp460/ffmpegplayback/jitter-camera-120s/full/seq60-from0/ffmpegexport/jitter-camera-40s/full/mp460/avf(fast tier)export/camera-pts-late/full/mp430/avfexport/backward-hold/backward-jumps/mp460/avfexport/basic/transition/mp460/avfplayback/basic/transition/seq60-from0/avfplayback/basic/cuts-speeds-reorder/realtime30-from25/avfseek/seek-gop/keyframes60/avfseek/seek-native/keyframes60/avfseek/seek-gop/storm30/avfseek/seek-native/storm30/avfseek/vfr-holds/storm60/avfseek/vfr-holds/into-holds60/avfseek/vfr-holds/revisit60/avfThe scripted decoder model (
decoder::model, 3024 AVAssetReader and 2864 FFmpeg sequences, about 3.7 M requests) agrees: 27 AVAssetReader and 8 FFmpeg cases no longer fail, and 4 more lose failure classes. No case changes status on Windows, and no case that passes on main changes on any platform.Exports that main already gets right are byte-identical (ffmpeg
framemd5, decoded PCM SHA-256) across 53 MP4, GIF and MOV outputs (20,740 frames) of the fast export tier, both decoders. The only differing outputs are the four exports above that are wrong on main, plusexport/backward-hold/backward-jumps/mp430/avf, where one output frame lands exactly on a camera sample boundary: main shows sample 1048, this PR shows 1049, the exact slot. The 60 fps export of the same timeline flags main's 1048 as stale.Performance
Wall time per suite run on Apple silicon, both arms built on main with #2648; 4 interleaved pairs, median:
export/basic/*export/jitter-camera-120s/*seek/*/storm30/*, both decodersseek/vfr-holds/storm60/*, both decoders

Confidence Score: 5/5
The PR appears safe to merge; no actionable new issues were found.Summary
Fixes source-frame selection during macOS playback, seeking, and export.
Review was based on code inspection; no runtime tests were run.
Reviews (4) · Last reviewed commit: "test: remove the expectations the AVAsse..." · Reviewed by Greptile