Skip to content

Gate the frame-rate limit on sustained cadence, not the largest gap between two frames - #69

Merged
smb merged 1 commit into
mainfrom
fix/issue-68-sustained-cadence
Sep 5, 2026
Merged

smb merged 1 commit into
mainfrom
fix/issue-68-sustained-cadence

Conversation

@smb

@smb smb commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

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_FPS move together: the probe, the
declared-rate check in _source_info, and proof verification. A file that opens
now also survives every later decode instead of failing at render time.

peak_rate becomes sustained_rate on both carriers, 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.

Tests

  • test_millisecond_quantized_60fps_source_opens_and_proof_verifies — 1800
    frames, gaps alternating 17/17/16 ms. Fails before the change with
    SOURCE_FPS_UNSUPPORTED, passes after, and exercises proof verification on
    the positive path.
  • test_short_millisecond_quantized_60fps_source_opens — four frames, gaps
    16/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_rates
    replaces 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, 4
skipped) and scripts/check_guardrails.py are green.

This is the fourth consecutive fix: commit touching jobs/source.py; the
maintainer agreed to it explicitly, with an observed trigger and a
previously-failing test.

Closes #68

🤖 Generated with Claude Code

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>
@smb
smb merged commit d32cf3b into main Sep 5, 2026
17 of 18 checks passed
@smb
smb deleted the fix/issue-68-sustained-cadence branch September 5, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ordinary 60 fps video is rejected as exceeding the 60 fps limit, because the check measures the peak gap between two frames

1 participant