[kixelated]

Finishes quest/m1/push-to-talk/native-comfort-noise.md, the last child of the push-to-talk epic.

Summary

moq play now treats a publisher's declared silence the way @moq/watch has since moq-dev/moq#7942. When playback runs dry at a declared end, it plays comfort noise at the level of the quietest audio heard before it, at most -60 dBFS. It counts no gap or stall, and it keeps the noise going through the refill before the next utterance.

• moq-audio ring (rs/moq-audio/src/ring.rs): learns the browser ring's end/next pair.
  • end() counts a declared end. Running dry while one is outstanding fills the read with comfort noise and returns a new Read::Comfort, which the mixer counts as supplied and playing.
  • next() makes the next push start a new utterance one latency out. It queues a virtual comfort-noise span that tops the queue up to the target, placed behind whatever tail is still queued. The span is counted in written, so buffered() matches what a fresh sink reports. When the reader reaches the span, the ends it covers are cleared.
  • A real underrun after that is a gap again. A finished sink never plays comfort noise, so its drain completes.
• Comfort generator (rs/moq-audio/src/comfort.rs): a port of js/watch/src/audio/comfort.ts, written for the audio thread with no allocation and an xorshift PRNG. It measures only received audio, never comfort noise or the startup cushion.
• playback::Sink::end() / Sink::next(): new public methods. A device rebuild carries a pending declaration over to the new ring.
• moq play (rs/moq-cli/src/play/media.rs):
  • At an end marker it calls sink.end().
  • At the resume it keeps the sink and calls sink.next(), instead of draining the old sink and opening a new one. The old tail, then comfort noise, play while role.resume waits for the old media to drain.
  • A resume whose timeline already has a deferred end declares it again after its first frame.
  • A lost group right after a declared end also starts the next utterance (sink.next() plus a timeline reset). Before, the whole mute counted as a hole: either filled with silence or, past fill_max, a new sink and a stall.
• Shared level rule: a new "Comfort noise" section in doc/concept/audio-jitter.md, plus doc/concept/audio-jitter/comfort/corpus.json. Both comfort.rs and a new js/watch/src/audio/comfort.test.ts check against the corpus. ComfortNoise gains a level getter for that test.

Public API and wire

• moq-audio: adds playback::Sink::end() and playback::Sink::next(). Not breaking. There is a note in MIGRATE.md.
• @moq/watch: ComfortNoise.level getter. The class is internal to the package.
• Wire: none.

Choices made without asking

• One sink across a declared end. The quest left the mechanism open. The native ring has no media timeline, so comfort noise that spans the silence, the refill, and the level measurement has to live in one ring on the audio thread. Handing state from an old sink to a new one would have needed a cross-sink API. Keeping the sink also removed the stall that the drain-and-reopen left at every resume.
• The end is declared at the marker, not at the end timestamp minus the Opus pre-skip. The browser subtracts the pre-skip because its ring is a timeline that waits for samples up to the end. The native ring is a FIFO, so running dry after end() is the silence whatever timestamp the decoded audio stopped at, and the pre-skip question does not arise. The stopped encoder's final frames, which follow the marker, still play before the silence.
• No fades. The browser fades audio into and out of the noise. Native playback never faded around an underrun, and the mixer already ramps a sink's gain in, so this leaves fades out rather than adding a new behaviour in this PR.
• Two declared silences within one latency. If the reader has not yet reached the previous resume, that resume also clears the newer end (a compare-and-swap on one atomic). Its span already topped the queue up to the target, so the newer resume adds nothing, or plain zeros only if the target rose in between. This saves a queue of resume positions.
• The resampler tail. It is flushed at next(), not at end(). Flushing at end() would put zero padding between the last speech and the codec drain frames that follow the marker. This keeps today's behaviour: up to 20 ms of tail plays just before the next utterance. It applies only when the device rate differs from the stream rate.
• Coverage. Unit tests cover this end to end, not a harness row. just test push-to-talk drives only Chromium today, and the native audio-quality rows never mute.
  • The moq-audio sink test runs the real mixer and Observer: comfort RMS matches the background, the next utterance starts exactly one latency after its first write, and there is no gap or stall.
  • Ring edge-case tests: a queued tail, two silences within one latency, a cold join during a mute, and a trim across a resume.
  • Two moq play tests: a 2 s mute keeps the sink and the next press plays exactly one delay after it arrives, and the same holds after a lost group.

Validation

• just rs play (moq-cli with play): 263 passed.
• cargo nextest run -p moq-audio --features playback,bus,capture --lib: 339 passed, including the new comfort, ring, and sink tests.
• just rs capture and just rs media-defaults (clippy with the playback features): clean.
• bun test js/watch/src/audio/comfort.test.ts: 6 passed.
• quest check: ok.
• just check: passed at 6b569f1. The review fixes after it only touch rs/moq-audio/src/ring.rs; the moq-audio, just rs play, just rs capture and cargo fmt --check runs above are at the head.

Review

Codex (gpt-6.1-sol, medium), round 2 on head 47f63f8: no findings.

Round 1 on 6b569f1 raised two findings, both fixed:

• ring.rs resampler flush counted its zero padding as output but not as input, so a second flush could leave the next utterance's tail pending. Fixed: the padding counts as input. Regression test: every_utterance_through_the_resampler_is_heard.
• An emergency refill that started before the end marker held the queued tail through the whole mute. Fixed: a declared end releases the refill. Regression test: a_declared_end_releases_an_emergency_refill.

Both tests fail without their fix.

Suggested follow-ups

• Let just test push-to-talk drive a native moq play --metrics listener next to the browser one, so the epic's acceptance proof covers native playback too.

(Written by Claude Opus 5.5)

[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 54 seconds.

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: ebc057a4-6194-447e-898b-cbb695df02bc

Commits

Reviewing files that changed from the base of the PR and between 0672ad5 and 47f63f8.

Files selected for processing: 16

• doc/bin/cli.md
• doc/concept/audio-jitter.md
• doc/concept/audio-jitter/comfort/corpus.json
• js/watch/src/audio/comfort.test.ts
• js/watch/src/audio/comfort.ts
• quest/m1/push-to-talk/README.md
• quest/m1/push-to-talk/native-comfort-noise.md
• rs/moq-audio/MIGRATE.md
• rs/moq-audio/src/comfort.rs
• rs/moq-audio/src/lib.rs
• rs/moq-audio/src/playback/mixer.rs
• rs/moq-audio/src/playback/sink.rs
• rs/moq-audio/src/ring.rs
• rs/moq-cli/src/play/fake.rs
• rs/moq-cli/src/play/media.rs
• rs/moq-cli/src/play/output.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

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head 47f63f8 against base 0672ad5.

Findings: No actionable regression found in this review.

Coverage: Reviewed all 16 changed files and the relevant surrounding code. This included:

• Ring end/next state, publication and clearing of pending resumes, separate physical-position and stretch-adjusted consumption counters, virtual cushion playback/discard, and buffered-depth accounting.
• Resampler flushing across successive utterances, the padding-accounting fix, emergency-refill release at a declared end, sink finish/drain, and device-rebuild carry.
• Mixer gap/stall accounting and the allocation-free comfort generator against the documented level rule and shared Rust/JavaScript corpus.
• CLI End/Gap/Resume handling, deferred ends, terminal codec frames, retained-sink completion, and the mute/lost-group/quick-resume tests.

Direction: Keeping the sink across a declared silence puts the noise and received-audio level measurement in the same playback state. The migration notes and shared corpus describe the added behavior clearly. The documented delayed resampler tail and coalescing of multiple pending resumes were treated as intentional choices. No code change is requested from this pass.

Validation limits: GitHub-only static review. I did not execute quest context, just check, Cargo/Bun tests, browser/native playback, or concurrency testing. The author's reported checks were not independently reproduced; the full just check result in the PR predates the final ring fixes. The exact-head Check workflow is still in progress. The native push-to-talk harness coverage remains the documented follow-up. This is a comment review, not merge approval.

[pullfrog]

✅ No new issues found.

Reviewed changes

• moq-audio ring (rs/moq-audio/src/ring.rs): adds end()/next()/carry(), the ends/resume state, and Read::Comfort. Running dry at a declared end now fills with comfort noise and counts as supplied; a resume publishes a cushion so the next utterance starts one latency after its write. The trim and refill paths account for the pending cushion, and the resampler flush now counts its padding as admitted input.
• Comfort generator (rs/moq-audio/src/comfort.rs): a no-allocation port of js/watch/src/audio/comfort.ts that measures the quietest 20 ms block played since the last resume and caps the level at -60 dBFS.
• Playback sink (rs/moq-audio/src/playback/sink.rs, mixer.rs): exposes Sink::end() / Sink::next(), carries a pending declaration across a device rebuild, and treats a comfort read as supplied so the observer reports no gap or stall.
• moq play (rs/moq-cli/src/play/media.rs, fake.rs, output.rs): calls sink.end() at an end marker and sink.next() at the resume instead of draining and replacing the sink; a declared end followed by a lost group starts the next utterance and resets the timeline.
• Shared conformance: a new "Comfort noise" section in doc/concept/audio-jitter.md and doc/concept/audio-jitter/comfort/corpus.json, consumed by both the Rust and JS tests; ComfortNoise.level exposes the measured level to the JS test.

The declared-silence state machine is covered by the new ring, sink, and moq play tests, including the resampler-flush regression and the emergency-refill release. I found no behavioral regressions in this review.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}