Conversation
This branch has not been 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.
What
Snapshots select the source video frame whose presentation interval contains the resolved source time. For a 24 fps source sampled at 0.4, 22/30, and 10/30 seconds, that means frames 9, 17, and 8.
Why
Seeking FFmpeg directly to an off-grid time discards the containing frame and returns the next frame. This shifts a baked video plate by one source frame relative to other renderers.
Related work
Fixes #4763. Implements the source-presentation-timestamp approach approved in the issue discussion.
PR #4767 addresses a separate VFX post-injection capture issue (#4762); this change addresses source-frame selection.
How
Probe integer presentation timestamps and the source time base, accounting for the container start time. Select the last timestamp at or before the resolved source time, with a small floating-point tolerance, then decode that frame by its index. The probe reads the source prefix through one second after the requested time. Both rendered-video injection and
--againstreference extraction use this rule; their previous fast/accurate seek split is removed.Trim, playback-rate, automation, loop, and clip visibility time mapping remain in their existing shared runtime paths. A held tail samples the actual last containing frame.
This requires FFprobe alongside FFmpeg. Accurate selection decodes from the source start, so late samples can cost more than a keyframe seek. The existing 30-second extraction bound remains, and probing has its own 30-second/32 MiB output bound.
Test plan
Snapshot and captureCompositionFrame suites: 80 tests passed, including real FFmpeg extraction for frames 9/17/8, exact boundaries, an irregular VFR interval, and held tails.
Tests cover floating-point round-off and confirm
--againstcalls the same extractor.CLI typecheck passed.
Repository pre-commit checks passed: core/studio/scripts typechecks, lint/format, fallow (no new findings), tracked artifacts, large files, and commitlint. Comment-citation and comment-ratchet checks passed.
Whole-repository lint and formatting checks passed.
No browser pixel reproduction is claimed for this source-extractor change.
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments"