Repository navigation
fix(rendering): stop exported camera frames running ahead on long recordings - #2595
Closed
richiemcilroy wants to merge 10 commits into
Closed
richiemcilroy wants to merge 10 commits into
richiemcilroy wants to merge 10 commits into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
|
hey @greptileai, please re-review the PR |
…ap is no longer held
…n rules and test them
…tream's first frame
…t frame from the cache
Member
Author
|
@greptileai review |
richiemcilroy
added a commit
that referenced
this pull request
Oct 15, 2026
Full tier on all three platforms had unexpected failures, and several AVF entries cited the wrong mechanism. Each entry now states a measured cause: - Pool position capping in the AVAssetReader decoder (a reader that decoded past a frame is reused for it without a reset) explains the one-frame-ahead answers. seek-native/keyframes60 is deterministic and now strict, with a three-seek repro. The storm30 cases, the 60 fps transition, backward-hold, camera-pts-late, the vfr storm and the 120 s jitter export and playback all pass with the decoder changes of #2595, measured on Apple silicon. - The decoder's 250 ms idle timeout makes several outcomes depend on request timing: a 300 ms pause flips decoder-model seeds 28, 53 and 59 from future to stale, removes the seek-native storm's ahead answers, makes the 60 fps transition correct and reproduces the last-frame stale answer of the 120 s jitter playback. Those entries are flaky with that mechanism. - editing seed 10 is the FFmpeg first-frame shortcut, reproduced with the FFmpeg decoder on macOS. - The 120 s jitter export's nondeterminism and torn frame, the 30 s audio cross-fades and the recorded-Studio isolated ahead frame are the IOSurface source lifetime bug (absent with sources retained). - Full-tier variants with identical failures share the existing entries (30 s long cross-fade, 120 s jitter playback on FFmpeg).
richiemcilroy
added a commit
that referenced
this pull request
Oct 16, 2026
…ra runaway On the Blacksmith macOS runner one of three full runs exported the 120 s jitter-camera project differently twice (4001 of 7200 frames, all inside the camera runaway). After the pts-hole map loses the hold, the frame a request gets depends on how far the AVAssetReader has decoded, so the wrong answers vary. #2595's decoder changes remove the runaway and this class with it; flaky until then.
Member
Author
|
Superseded by #2638 (merged), which fixes the same decoder request handling with the suite's oracle proof. |
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.
Exports and editor playback can show the wrong camera or screen frame. On a 180 s Studio recording with a 720p camera that delivered ~15 fps with jitter, the 1080p60 export served a camera frame from the future for 9,023 of 10,799 output frames, up to 117 s ahead. A real 9 min recording served the wrong screen frame for 23,884 of 32,402 output frames. Separately, the camera bubble could differ between two exports of the same project, because the GPU sometimes sampled a decoder buffer that had already been recycled.
The correct answer to a frame request is the newest source frame at or before the requested time. Every change below makes the macOS decoder (
crates/rendering/src/decoder/avassetreader.rs) give that answer, or seek again until it can.1. Pts holes are kept
record_pts_hole(crates/rendering/src/decoder/mod.rs) records gaps in a track's timestamps, so a request inside a gap is answered with the frame before it without decoding. It kept 64 gaps and evicted the narrowest, which meant every new 2-frame gap was dropped as soon as it was inserted.2. A pooled reader only answers what it can prove
The decoder keeps a pool of readers. Each one vends samples in pts order, so the frame it vended just before a jump is the answer for every request inside that jump. Three ways to break that are now closed:
sequenceDiagram participant R as Request participant D as Decoder participant P as Pooled reader R->>D: frame 1054 (not cached) D->>P: pool position 1053 (capped), but the reader already vended 1054 alt reader passed the request D->>P: seek back first (was: read on and answer 1052, stale) else reader steps over the request P-->>D: vends 1057; frame vended before it = 1054 D-->>R: 1054 if still held, otherwise retry once from a fresh seek endreader_passed_request).answer_misses_previous_vend).cache_eviction_victim).3. IOSurface sources held until the GPU reads them
convert_nv12_from_iosurface_to_encoder(crates/rendering/src/yuv_converter.rs) wraps the decoded IOSurface in Metal textures without retaining theCVPixelBuffer, and the owningDecodedFramecould be dropped before the command buffer ran, letting VideoToolbox decode the next picture into the same buffer. The converter now keeps its last 4 source buffers in a ring (one pipelined frame plus a transition's two conversions). Nothing is copied.Results
Every decoder answer was logged and checked against the file's packet timestamps, at 960x540
cap export, for 30 and 60 fps output on both decoders (AVF = AVAssetReader, FF =--force-ffmpeg-decoder). "Wrong where main right" counts answers this PR gets wrong thatmaingets right.mainwrongmainrightmainright" is 0 in every one, and the remaining FFmpeg misses are a subset ofmain's.ci/engine-verify(export, playback, seek; 2 runs): every casemainpasses also passes here. This PR additionally passesexport/jitter-camera-40s/full/mp460/avf,export/backward-hold/backward-jumps/mp460/avfandseek/seek-gop/{keyframes30,storm30}/avf, which fail onmain.playback-hash: sequential, 150 random seeks, drag back/forward, and play-jump-back lists on the 180 s, 9 min, synthetic and static-screen projects, both decoders): 0 answers wrong wheremainis right, andmain's 47 wrong seek answers on the 9 min recording are now 0.mainmainavg (ms)

Confidence Score: 5/5
The PR appears safe to merge; no new blocking issue was found.Summary
The PR fixes camera timing on long recordings and keeps decoded macOS buffers alive while the GPU reads them.
Review used source inspection; no runtime checks were performed.
Reviews (3) · Last reviewed commit: "fix(rendering): answer requests before a..." · Reviewed by Greptile