Skip to content

fix(timeline): quantise the scene timeline once instead of per scene - #412

Merged
LeadcodeDev merged 1 commit into
mainfrom
fix/cumulative-frame-rounding
Sep 29, 2026
Merged

LeadcodeDev merged 1 commit into
mainfrom
fix/cumulative-frame-rounding

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Closes #372 (the half that #391 left open).

The defect

Two 1.05 s scenes rendered 64 frames where 2.1 s at 30 fps is 63: each scene
rounded 31.5 up on its own and the error accumulated. Twenty 0.35 s beats ended
10 frames past their soundtrack.

The model

The timeline is defined in seconds first and quantised once. quantised_spans
walks the scene durations and the transition overlaps as real numbers, then rounds
each cumulative boundary. A scene's frame count is the difference of two rounded
boundaries, so the half frame is paid once by whichever side the boundary rounds
toward — never twice.

A transition's overlap falls out of the same arithmetic: it is
spans[i].end - spans[i+1].start, not a separately rounded figure that could
disagree with the scenes around it.

Four sites, and they did not agree

Site Was Now
encode/video/tasks.rs (v1) round(d·fps) per scene, minus separately rounded transitions derives from the spans
encode/video/tasks.rs (v2) cursor accumulated rounded frames cursor carries seconds, so at resolves on the same grid as automatic placement
encode/video_audio.rs rounded each scene again to place audio reads the spans
cli/commands/migrate.rs computed the v2 at/duration from its own per-scene rounding derives both from quantised_spans
cli/commands/info.rs printed per-scene counts that summed past its own total (32 + 32 against 63) prints the spans

video_audio.rs mattering here is not cosmetic: it places an embedded video's
audio slice, so it was drifting from that video's own picture by a frame per scene.

migrate caught itself

migrate's own frame-identity check is what surfaced the fourth site — migration
of the six-scene fixture went 405 → 404 as soon as the render side was corrected.
It now derives from the same function, and the check passes:

Duration: 13.500s -> 13.500s (405 -> 405 frames @ 30fps) — frame-identical

One expected value changed, and why

snap_beat_on_top_of_the_migrated_file_moves_cuts_and_changes_duration asserted
15.0 s. That number was an artifact of the old chain: scene 4 was given 51 frames
where its window at the snapped start is 52 (round(379.27) - round(327.27)).

Recomputed by hand from the migrated file's own at values (0, 1.9, 4.2, 6.5, 9.3667, 11.1) against the bpm-11 beat grid (0 s, 5.4545 s, 10.9091 s), with the
three clamped-forward cuts, the answer is 451 frames. The test now pins that,
and states the structure it comes from rather than a bare literal.

The issue's repros, end to end

info rendered (ffprobe)
two 1.05 s scenes 2.1 s, 63 frames 63
v2 at: 0 / at: 1.0, both 2.0 s 3.0 s, 90 frames 90
two 1.0 s scenes, 0.6 s iris 1.4 s, 42 frames 42

Reverts that bite

Restoring the per-scene round(d·fps) inside quantised_spans fails three of the
six new tests:

2.1s at 30fps is 63 frames; rounding each 1.05s scene on its own gives 32 + 32
20 beats of 0.35s is 7.0s exactly; rounding 10.5 up twenty times ends 10 frames late
the half frame is paid once, by whichever scene the boundary rounds toward — it is never paid twice

Gate

cargo fmt --all --check clean · cargo clippy --workspace --all-targets --features rustmotion/studio -D warnings clean · cargo test --workspace 1777 passed.

Two 1.05s scenes rendered 64 frames where 2.1s at 30fps is 63: each
scene rounded 31.5 up on its own and the error accumulated. Twenty
0.35s beats ended 10 frames past their soundtrack.

The timeline is now defined in seconds first and quantised once:
quantised_spans walks the scene durations and the transition overlaps as
real numbers, then rounds each cumulative boundary. A scene's frame
count is the difference of two rounded boundaries, so the half frame is
paid once by whichever side the boundary rounds toward and never twice.
A transition's overlap falls out of the same arithmetic -- it is
spans[i].end - spans[i+1].start, not a separately rounded figure that
could disagree with the scenes around it.

Four sites were computing frames from a duration independently, and they
did not agree:

- encode/video/tasks.rs, v1 and v2, now both derive from the spans.
  v2's cursor carries seconds rather than accumulated frames, so `at`
  resolves on the same grid as the automatic placement.
- encode/video_audio.rs rounded each scene again to place audio, so an
  embedded video's sound drifted from its own picture by a frame per
  scene. It now reads the spans.
- cli/commands/migrate.rs computed the v2 `at`/`duration` values from its
  own per-scene rounding. Its own frame-identity check caught this:
  migration of the six-scene fixture went 405 -> 404. It now derives
  both from quantised_spans, and the check passes.
- cli/commands/info.rs printed per-scene frame counts that summed past
  the total it announced (32 + 32 against 63). It now prints the spans.

The migrate snap test's expected 15.0s was itself an artifact of the old
chain: scene 4 was given 51 frames where its window at the snapped start
is 52. Recomputed by hand from the migrated file's own `at` values and
the bpm-11 beat grid, the answer is 451 frames.
@LeadcodeDev LeadcodeDev added the bug Something isn't working label Sep 29, 2026
@LeadcodeDev LeadcodeDev self-assigned this Sep 29, 2026
@LeadcodeDev
LeadcodeDev merged commit 4f4cdbc into main Sep 29, 2026
4 checks passed
@LeadcodeDev
LeadcodeDev deleted the fix/cumulative-frame-rounding branch September 29, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duration accounting: info ignores transitions and at, and per-scene rounding adds a frame

1 participant