fix(audio): a world-only scenario lasts the sum of its scenes - #347
Merged
Merged
Conversation
`resolved_scenario_duration` read "last scene's offset + its duration" from
`scene_start_offsets`. For a `World` view that function deliberately gives every
scene the same offset — the view's own window start — because world scenes share
one window for embedded-video audio. So the total collapsed to
`0 + last_scene.duration`, and a synthesised score sized from it went silent
after the last scene's duration and stayed silent to the end. The video rendered
at the right length and the muxed track was padded, so only listening or
measuring showed it.
The end cursor was already being computed for both view types; it just was not
returned. `timeline_offsets` now exposes `view_ends` beside `scene_starts` from
one walk, so the two cannot drift, and `scene_start_offsets` stays as a wrapper
with its existing signature and semantics. World scenes keep their shared window
start, which is what embedded-video extraction wants — only the total changes.
Measured on the reproduction from the issue, three world scenes of 3 s + 3 s +
2 s with a kick on every beat:
before after
t=0.5s -8.0 dB -8.0 dB
t=1.5s -8.0 dB -8.0 dB
t=2.5s -91.0 dB -8.0 dB
t=4.0s -91.0 dB -8.0 dB
t=6.0s -91.0 dB -8.0 dB
t=7.5s -91.0 dB -8.0 dB
Four tests. The first fails against the old computation with `got 2` for a
scenario that lasts 8 s. One pins that world scenes still share their window
start; one covers a world view followed by a slide view, which passed before the
fix too and is exactly why the bug survived — appending a slide view was the
known workaround; and one covers a trailing view with no scenes, which must
neither add to the total nor shorten it.
Closes #341
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.
Closes #341.
The fault
resolved_scenario_durationread "last scene's offset + its duration" fromscene_start_offsets. For aWorldview that function deliberately gives everyscene the same offset — the view's own window start — because world scenes
share one window for embedded-video audio extraction. So the total collapsed to
0 + last_scene.duration.That total sizes the synth buffer, so a score event with no
to("play until theend") stopped at the wrong end. The video rendered at the right length and the
muxed track was padded, so
ffprobeshowed nothing wrong — only listening, ormeasuring, did.
The fix
The end cursor was already computed for both view types. It simply was not
returned.
timeline_offsetsnow yieldsview_endsalongsidescene_startsfromone walk, so the two cannot drift, and
scene_start_offsetsremains a wrapperwith its existing signature and semantics.
World scenes keep their shared window start — that is what embedded-video
extraction wants. Only the total changes.
Measured, not reasoned
The reproduction from the issue: three world scenes of 3 s + 3 s + 2 s, a kick on
every beat, rendered with the binary before and after.
−91 dB is silence. It began at 2.0 s, the last scene's duration, exactly as
reported.
Four tests
a_world_only_scenario_lasts_the_sum_of_its_scenes— fails against the oldcomputation with
got 2for a scenario that lasts 8 s.world_scenes_keep_their_shared_window_start— pins the semantics the fixmust not disturb.
a_world_view_followed_by_a_slide_view_still_totals_correctly— passesbefore the fix too, which is precisely why this survived: appending a slide
view was the known workaround, so any scenario that had one looked fine.
a_trailing_empty_view_does_not_shorten_the_total— an empty view mustneither add to nor subtract from the total. The old code
continued past it;the new one filters it out, and this pins that they agree.
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warningsand
cargo test --workspace(1518) all clean.Note on comments
Written comment-free, per the codebase-wide rule being applied in #345. The
reasoning that would have been a comment is in the commit message and above.