Repository navigation
Gate the frame-rate limit on sustained cadence, not the largest gap between two frames - #69
Merged
Merged
Conversation
Trigger: test test_millisecond_quantized_60fps_source_opens_and_proof_verifies An ordinary 60 fps recording could not be opened. Its presentation timestamps are quantised to whole milliseconds, and 1/60 s is 16.667 ms, so the muxer alternates 17 and 16 ms gaps; the 16 ms gap reads as 62.5 fps instantaneously. _derive_timeline gated on the maximum instantaneous rate between two consecutive frames, so one short gap rejected the whole file and told the user to convert it to 60 fps or less, which is what it already was. Every declared rate on such a file is within the limit; the check simply measured something else. That breaks G4: the precondition cannot be met from the interface, and the advice given is false about the user's file. The gate now measures sustained cadence - frames over the duration they span. Two ticks of the stream time base absorb the floor error that quantisation leaves in that duration. That is the error bound itself, not a chosen tolerance: a fixed frame-rate allowance cannot work here, because the error lives in the time domain and scales with 1/frames, reaching 60.6 fps for a four-frame clip while a 30-second one stays under 60.002. An earlier draft of this change used a fixed 1/400 allowance and would have rejected every quantised 60 fps clip shorter than about 27 seconds. All three comparisons against MAX_SOURCE_FPS move together - the probe, the declared-rate check in _source_info, and proof verification - so a file that opens also survives every later decode rather than failing at render time. The carried field is renamed peak_rate to sustained_rate, along with its readers in core/timeline.py, ui/source_presentation.py and the string lookup in scripts/screenshots.py. The per-pair maximum is removed; nothing read it. Sources whose sustained cadence genuinely exceeds 60 fps are still rejected, which a 120 fps companion fixture covers alongside the four-frame and 1800-frame quantised fixtures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
An ordinary 60 fps clip could not be opened. The probe gated on the maximum
instantaneous rate between two consecutive frames, and on a millisecond
timestamp grid every 60 fps file alternates 17 and 16 ms gaps because 16.667 ms
cannot be represented. The 16 ms gap reads as 62.5 fps, so one short gap
rejected the file — and told the user to convert it to 60 fps or less, which is
what it already was.
What changed
The gate measures sustained cadence: frames over the duration they span. Two
ticks of the stream time base absorb the floor error quantisation leaves in that
duration — the error bound itself rather than a chosen tolerance.
A fixed frame-rate allowance cannot work here. The error is in the time domain
and scales with 1/frames: 60.6 fps for a four-frame clip, 60.06 at one second,
60.002 at thirty. The first draft of this change used a fixed 1/400 allowance
and would have rejected every quantised 60 fps clip shorter than about 27
seconds — the bug it set out to fix, for the clip lengths a loop tool sees most.
All three comparisons against
MAX_SOURCE_FPSmove together: the probe, thedeclared-rate check in
_source_info, and proof verification. A file that opensnow also survives every later decode instead of failing at render time.
peak_ratebecomessustained_rateon both carriers, with its readers incore/timeline.py,ui/source_presentation.pyand the string lookup inscripts/screenshots.py. The per-pair maximum is removed — nothing read it.Tests
test_millisecond_quantized_60fps_source_opens_and_proof_verifies— 1800frames, gaps alternating 17/17/16 ms. Fails before the change with
SOURCE_FPS_UNSUPPORTED, passes after, and exercises proof verification onthe positive path.
test_short_millisecond_quantized_60fps_source_opens— four frames, gaps16/17/16. The case a fixed tolerance could not cover.
test_sustained_120fps_source_is_rejected— the limit still holds.test_vfr_timestamp_scan_uses_sustained_cadence_even_with_declared_ratesreplaces the test that asserted the old statistic, and still proves the
timestamp scan runs and governs the outcome.
uv run ruff check .,uv run mypy src, the full suite (1786 passed, 4skipped) and
scripts/check_guardrails.pyare green.This is the fourth consecutive
fix:commit touchingjobs/source.py; themaintainer agreed to it explicitly, with an observed trigger and a
previously-failing test.
Closes #68
🤖 Generated with Claude Code