[kixelated]

Finishes quest/m0/nightly/firefox-resume-audio.md.

Summary

Nightly's Firefox after resume window stayed silent for about half a second while the picture played. The cause is in how the player waits for missing audio after a pause:

1. While no one is subscribed, the on-demand publisher holds its captured audio. When the viewer resumes, it replays the last max-delay of it (about 2 s), and those groups arrive newest first.
2. The relay first serves the groups it cached before the pause. That cache was current: the publisher had stopped producing, so the relay serves nothing stale.
3. The player's audio consumer reads those pre-pause groups. It then waits for the replay's oldest group, which lands last. On a CPU-starved host that takes longer than the hole wait (delay plus lookahead), so audio stays silent for the whole wait and then skips anyway. The decoder would have dropped that backlog regardless, because it rejoins the live edge on resume.

Fix: @moq/hang's container Consumer takes an optional playhead, the player's playout clock. A missing group whose successor starts at or before the media time presenting now could only arrive too late to play. So the consumer skips the hole then, and the hole's timer also wakes when the clock reaches it. Both @moq/watch decoders pass their Sync.

Decisions

• Taken on my recommendation while the user was away: fix both hops, the relay and the consumers (rejected: player-only rejoin; larger runner). Implementing it showed the relay hop isn't real. The model already holds an idle copy's cache until its route answers (Feed::Idle, answered). The "stale" tail was simply the newest thing the publisher had, because the publisher had stopped producing during the pause. A consumer starting at the live edge less its reach still starts on that tail. So I changed neither the relay nor the consumer's start. The fix lands where the time actually went: the hole wait. I made this further call myself.
• The video decoder passes its clock too, from the lane driving the clock only: the max-delay warmup decoder reads older groups on purpose (review round 1). A video group behind the clock is also dropped, and the trace showed the same kind of hole on video (31 -> 36).
• JS only. Rust's moq-mux consumer has no caller with a pause and rejoin today, and the repository rule is that API needs a consumer. Parity is a follow-up quest.

Validation

• Reproduced 4/5 before the fix, by pinning the whole run to two cores: taskset -c 4-5 nix develop --command just test media --media-engines firefox --media-cases rename,pause. It passes on three or four cores, and pause alone never reaches the bug, because without rename the publisher stops producing.
• Before the fix, the after resume window opened with 6, 12, 9, and 14 silent samples while video advanced, the CI signature. After the fix, 8 of 10 runs reached the window. 6 of them started audible. Two started silent: fix-3, whose picture was frozen at the same moment (the whole page starved), and fix-9, where audio stalled for about 0.3 s on smaller holes near the live edge. Neither showed CI's wait-out of the large hole. Separately, some two-core runs fail coded-size growth audio/video sync, unchanged from before the fix, on a host starved harder than CI.
• Second, independent check: a unit test with a mocked clock in consumer.holes.test.ts. The hole ends exactly when the clock reaches the group past it (100 ms), still at the 500 ms wait when the clock trails or when there is no clock. Removing the rule fails the first case.
• just check passes. After the round-1 fixes, three more two-core runs: every resume window started audible; one failed later in the window on scattered underflows from the starved host.
• Not run: Nightly itself. The hosted ARM job confirms after merge.

Related: the interop-green replacement case (moq-dev/moq#8797) is likely not covered. Its failing join reads a backlog about a second behind the clock with no hole after delivery, which this change does not skip. Three local Chromium replacement runs passed, but that case fails only about 1 in 6 runs on a loaded host.

Public API / wire impact

• @moq/hang: new optional ConsumerProps.playhead and exported Playhead interface (now() plus an anchor getter that changes when the clock jumps). Additive, no migration.
• @moq/watch: Sync.anchor, so a Sync is a Playhead. Additive.
• Wire: none.

Caused by

No single PR. The publisher's on-demand replay (retained audio replayed on rejoin) meets the consumer's in-order hole wait. Each is correct on its own.

Follow-ups

• quest/m2/audio-playout/rust-playhead-holes.md: native players stop waiting on a hole their clock passed (Rust parity). No open decisions.

Models

Size: L
Implementer: Claude, claude-opus-5-5, unknown effort (arm A)

(Written by Claude Opus 5.5)

Review

Rewritten by quest review after each round.

• Head: 6a4d109

• Base: origin/main at 250a13a

• Risk: medium, the base has no RISK.md to name what is always high or customer-facing, so every change is medium

• Classified: medium by default

• Reviewer: Codex, gpt-6.1-sol, high effort

• Session: 01a1df3b-42a9-7da2-8c07-7323c233728c, round 2

• Score: 10/10, Both earlier findings are fixed, and source review of the intervening changes found no new issues; builds and tests were not run.

• Rounds: 2; first round 2 (medium 2); findings 2 (medium 2): fixed 2, refuted 0, open 0

• Findings: none

• The last round's findings, at 984f024:

  • Fixed: medium, js/watch/src/video/decoder.ts:728: Preserve the warmup decoder's allowance for older groups; js/watch/src/video/decoder.ts:730 passes the playhead only when #drives is true, preserving the secondary decoder's warmup allowance.
  • Fixed: medium, js/hang/src/container/consumer.ts:1076: Recompute the hole deadline when the clock reference changes; js/watch/src/sync.ts:480 derives anchor from reference, delay, and playing, and js/hang/src/container/consumer.ts:1073 subscribes to it so clock jumps reschedule the hole deadline.

[coderabbitai]

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 30 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews 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: f6f174fb-2247-4638-b2d7-7eaa08a21610

Commits

Reviewing files that changed from the base of the PR and between cbb742c and 6a4d109.

Files selected for processing: 12

• doc/concept/audio-jitter.md
• js/hang/api/container.api.md
• js/hang/src/container/consumer.holes.test.ts
• js/hang/src/container/consumer.ts
• js/hang/src/container/index.ts
• js/watch/api/index.api.md
• js/watch/src/audio/decoder.ts
• js/watch/src/sync.ts
• js/watch/src/video/decoder.ts
• quest/m0/nightly/README.md
• quest/m0/nightly/firefox-resume-audio.md
• quest/m2/audio-playout/rust-playhead-holes.md

• 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

• X
• Mastodon
• Reddit
• LinkedIn

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the full PR at 6a4d109: the container consumer now stops waiting for a missing group once the player's clock passes it, and both @moq/watch decoders pass their clock in.

• ConsumerProps.playhead / Playhead: Adds a passed condition to the missing-group skip in #next. It fires once playhead.now() reaches #bound(), the start of the first timestamped group in the buffer.
• Hole timer: The timer re-runs when playhead.anchor changes. It wakes at whichever comes first: the end of the remaining wait, or the moment the clock reaches the bound. It now also arms when the wait itself is unlimited.
• Sync.anchor: A Derived over reference, delay, and playing, so Sync satisfies Playhead. Derived holds no resources, so nothing needs closing.
• Decoders: Audio always passes this.sync. Video passes it only on the lane that drives the clock (#drives, fixed in the constructor), so the warmup lane can still read older groups.
• Docs and quests: Updates the prose in audio-jitter.md, removes the finished quest, and adds the Rust follow-up quest.

Checked: group.start (microseconds of the frame timestamp) and Sync.now() (media time) use the same timeline. Sync.now() is undefined during buffered startup, so the start-up path is unchanged. The zero-length timer case uses the same bound and clock as the skip check, so it adds no busy loop. bun test src/container/consumer.holes.test.ts passes, 14 of 14. The JS hole rule now differs from Rust's moq-mux consumer until quest/m2/audio-playout/rust-playhead-holes.md lands, as the PR states.

  ^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}