Skip to content

fix(studio): a media file's sound plays with its pictures - #93

Merged
kixelated merged 9 commits into
mainfrom
quest/m0/media-av-sync
Nov 7, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m0/media-av-sync

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Finishes quest/m0/media-av-sync.md, found in moq-dev/studio#81.

A media file source's sound trailed its own pictures by about 45 ms in every output: the program, published scenes, recordings, and Listen. The source showed each picture when it was due, and wrote each sound to the bus when it was due too. But the bus plays a write 50 ms later (moq-audio's bus::LATENCY). Broadcast sources already write a bus latency ahead through playout::Writer. Media sources didn't.

Approach

Mostly in rs/moq-scene/src/source.rs (media and play_clip), plus the own-track stamping in rs/moq-scene/src/publish.rs.

  • Sound goes through playout::Writer, written BUS_LATENCY ahead of its moment, so the bus sounds it at that moment.
  • Pictures are handed over at the same lead, to a small presenter (Shows) that shows each one at its moment. The presenter lives across plays: a loop's next pass hands its first sound and picture over a bus latency before the current pass ends, while the current pass's last pictures still wait to show. A cut (play, stop) clears the presenter's queue along with the channel.
  • A fresh play begins a bus latency after its first picture and sound decode. Nothing can be heard sooner, so its first picture now shows 50 ms after the play is decoded instead of at once. Run::begun is that moment.
  • A loop's next pass begins exactly where the last ended, as before, now handed over 50 ms early. The bus gets the same 50 ms margin to open the next pass's decoders that the old channel buffer gave it. Without the lead, the next pass's first write would land as the channel runs dry and could click.
  • A non-looping play waits out its end before it reads as ended.
  • A stinger switches the scenes at its cut point only once its clip has begun (compositor::stinger). With Run::begun now 50 ms after the play decodes, a stinger cut at zero would otherwise switch before its clip's first picture showed.
  • The source's own tracks (its picture and sound tracks in the studio's local broadcast, for replays and recordings) are encoded at hand-over, 50 ms early, but stamped with when each picture shows and each sound sounds. New publish::Video::anchor starts a picture track's timeline at the source's origin instead of at its first encode. publish::Audio::write counts a stamp that is ahead of the studio clock on from now, instead of clamping it to now.

Tests:

  • New media_tests::a_media_files_sound_plays_with_its_pictures. It steps a paused clock and the bus together a block at a time, reading the program mix by bus position. The first picture shows in the block it is due, and the first sound is heard within one bus block of the moment the play began. On main this fails: the sound is heard 50 ms after the picture.
  • New media_tests::a_looping_files_sound_runs_on. A looping tone's second pass is heard as loud, from its own beginning, as its first.
  • engine::tests::the_program_and_a_portrait_cut_together (nightly): the tone is heard as it plays, no longer "50 ms later, give or take a block".
  • New compositor::tests::a_stinger_cuts_no_sooner_than_its_clip_begins: a stinger cut at zero does not switch between its clip's decode and its beginning, and does at the beginning. Fails with the fix reverted.
  • publish::tests::a_media_file_is_published_on_one_timeline and a_looping_media_file_keeps_its_sound_on_time check that the own tracks' first picture and first sound are stamped with when the play began. With the stamping fix reverted, both read 50 ms early.
  • The media test helpers (source::media_tests::shown, publish::tests::pictures) wait for a play to begin, rather than for the moment they asked for it.

Impact

  • Users: a media file's sound and pictures are in sync in every output. A media source (a stinger, a restart on program, a manual play) starts 50 ms after it is asked, which is when its sound can start.
  • Public API: none outside the crate. publish::Video::anchor is crate-private. publish::Audio::write now keeps a microphone's stamp that runs a moment ahead of the studio clock instead of clamping it to now. Run::begun and Run::ended keep their types. Their docs now say a play begins a bus latency after it decodes, and that a looping play's ended is known 50 ms ahead.
  • Wire: none.

Choices made without asking

  • Delay the pictures, not drop the sound. A play could instead show its first picture at once and drop the first 50 ms of sound. A stinger's or intro's sound starts with that 50 ms, so the play waits for it.
  • The presenter holds up to 50 ms of decoded pictures. That is 2 to 4 frames between 30 and 60 fps. A broadcast source already holds up to 3 (QUEUED). Holding them lets a loop's next pass be handed over before the last pass's pictures have all shown.
  • Own tracks encoded at hand-over, stamped at play. The source's own tracks are encoded 50 ms before each picture shows and each sound sounds, carrying when they play. Encoding in the presenter would mean sharing the encoder between two futures across an await. On a cut or a stop, the own picture track may carry up to 50 ms of pictures the studio never showed. Its sound track already did, from what the bus dropped on a cut.
  • Replay sources are left alone. They have the same 50 ms trail (replay writes sound when due), but they are not this quest's source. Suggested as a follow-up below.
  • The engine test's bound is a block late to two blocks early. A first write after silence goes in as the bus last mixed it, which places it up to a block early. A late tick of the studio's audio clock can add another. Measured on lavapipe over 8 runs: heard 1.6 to 8.7 ms before played. Before this change it was 41 to 45 ms after.

Validation

On nixos, through quest gate:

  • just ci at 3285a47 (the reviewed head, merged with main at a8ca56f): passes, 284 tests. Also at 2622772: 283 tests.
  • The nightly's moq-scene GPU set and engine set (just render-sw's first two commands, lavapipe and OpenH264) at 735b0a8: 49 and 6 tests pass.
  • The nightly sets again at 2622772: the engine set passes 6 of 6. The moq-scene GPU set passed 16 more runs, but in the first, compositor::tests::a_stinger_stops_its_clip timed out at 60 s with no panic or output, on a host at load average 36 to 40. It passed 6 of 6 alone and 16 of 16 in the set after, including 12 with diagnostics on its wait loops, so the hang was not reproduced or pinned to this change. Worth watching in the nightly.
  • engine::tests::the_program_and_a_portrait_cut_together with the final bound at 08a6383, with cargo fmt --check and cargo clippy -p moq-studio --all-targets --features openh264: passes 3 of 3.
  • Reproduced on main (d01819803 moq pin, origin/main 79c60b9 with the new tests added): a_media_files_sound_plays_with_its_pictures hears the onset 50 ms after the play began, and fails. With the fix, 0 ms.
  • Not run: macOS. The change is platform-independent scheduling, and macOS decodes on the runtime rather than on threads, so its paused-clock tests are no harder.

Follow-ups

  • Worth a quest: replay sources write their sound when due, so a replay's sound trails its pictures by the bus latency too. The same fix applies, minus the loop.
  • Worth a quest (moq-dev/moq): bus::Channel cannot say how much it holds, so playout::Writer places a first write after silence relative to the block the bus last mixed, up to a block early. The same applies to broadcast sources. A channel that reported its fill, or the bus position, would let the writer place it to the sample.

Review

  • Reviewer: Codex (quest review), gpt-6.1-sol, medium effort. Session of this branch, rounds 1 and 2.
  • Round 1 at 08a6383, 1 finding: [P2] the source's own tracks took their stamps at hand-over, 50 ms before the media played, and the picture track's epoch came from its first encode. Fixed in dc44da4 (Video::anchor, Audio::write keeps a future stamp), with the two publish test assertions above, which fail with the fix reverted.
  • Round 2 at 2622772: no findings.
  • OpenAI online review, P2 of 2622772: a zero-offset stinger switched before its clip began. Fixed in 2141f09, with the compositor test above.
  • Round 3 at 3285a47 (the fix and a merge of main): no findings.
  • Reviewed head: 3285a47.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

Quest-Claim: 1924528555cce7fd
@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

kixelated and others added 3 commits November 7, 2026 07:06
A media source wrote each sound to the bus when it was due, and the bus
sounds a write its 50 ms latency later, so the sound trailed the
pictures in every output. It now writes through playout::Writer a bus
latency ahead, as broadcast sources do, and hands each picture over
then too, to a presenter that shows it at its moment. A play begins a
bus latency after it decodes, and a loop hands its next pass over a bus
latency before the one playing ends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A media play now begins a bus latency after its first picture decodes,
so tests walk its pictures from when it began rather than from when it
was asked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A media file's sound plays with its pictures fix(studio): a media file's sound plays with its pictures Nov 7, 2026
kixelated and others added 2 commits November 7, 2026 07:54
Handed over a bus latency ahead, a media source's own pictures and sound
took their stamps from when they were encoded, 50 ms before they showed
and sounded. The picture track now anchors its timeline at the source's
origin, and the sound track counts a stamp ahead of the clock on from
now, so both carry when they play.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 7, 2026 16:07
@coderabbitai

coderabbitai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5cd5fced-6a33-4322-af9e-ec177c616963
Commits

Reviewing files that changed from the base of the PR and between a461c84 and b9e9a5e.

Files selected for processing: 7

  • quest/m0/README.md
  • quest/m0/media-av-sync.md
  • rs/moq-scene/src/compositor.rs
  • rs/moq-scene/src/playout.rs
  • rs/moq-scene/src/publish.rs
  • rs/moq-scene/src/source.rs
  • rs/moq-studio/src/engine.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head 2622772 against base 79c60b9.

P2: Keep zero-offset stingers on the outgoing scene until playback begins. At rs/moq-scene/src/source.rs:1627–1629, Run.begun is now recorded 50 ms in the future. Its consumer at rs/moq-scene/src/compositor.rs:1068–1069 still tests now.saturating_duration_since(begun) >= cut. For the valid Stinger { cut: 0s, ... }, that is true throughout the pre-roll, while the new presenter has not displayed the first clip frame. The incoming scene therefore appears uncovered for up to 50 ms before the stinger appears. Require the beginning to have arrived before comparing the cut offset, and cover a zero-offset stinger rendered between decoding and begun.

Direction: reusing the playout writer and stamping tracks with presentation times addresses the audio delay coherently; update the transition consumer for the newly future-valued start time before landing.

Verification: GitHub-only source review of the changes, surrounding playback/publication code and stinger caller. No build, runtime/GPU tests or quest context execution; repository guidance and pinned quest context were read as source.

(Written by review (OpenAI))

kixelated and others added 2 commits November 7, 2026 08:29
A media play now begins a bus latency after it is asked, so a stinger
cut at zero switched the scenes while its clip's first picture was still
on its way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head 3285a47 against base a8ca56f, following review 5813516880.

The prior P2 is addressed: rs/moq-scene/src/compositor.rs:1067–1071 now rejects times before Run.begun using checked_duration_since. The added paused-clock test at lines 2861–2895 checks a zero-offset stinger before and at its scheduled beginning.

No new actionable findings in this follow-up. The remaining changes since the last reviewed head come from merging the updated base; the other PR patches retain their previous added/removed lines. Direction: the focused consumer fix preserves the audio/video scheduling approach.

Verification: GitHub-only static review of the fix, regression test and surrounding playback/transition code. No build, runtime/GPU tests or quest context execution; repository guidance and pinned quest context were read as source.

(Written by review (OpenAI))

@kixelated

Copy link
Copy Markdown
Contributor Author

Fixed the zero-offset stinger finding in 2141f09. compositor::stinger now compares the cut only once now has reached Run::begun. The new test compositor::tests::a_stinger_cuts_no_sooner_than_its_clip_begins reads Some(false) between the decode and the beginning and Some(true) at the beginning; with the fix reverted it fails. Reviewed locally at 3285a47 with no findings.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Contributor Author

Pick (/quest-complete, maintainer away under /afk, recommended option): ✅ Merge, keeping the choices made without asking (delay the pictures 50 ms rather than drop sound; the own tracks encoded 50 ms early). Both OpenAI P2s are fixed, and Codex round 3 and the OpenAI follow-up review found nothing. I'm planning the two follow-ups (replay sources, and the bus channel's fill in moq).

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit 47c4b3d into main Nov 7, 2026
3 checks passed
@kixelated
kixelated deleted the quest/m0/media-av-sync branch November 7, 2026 17:11
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.

1 participant