[kixelated]

Problem

Native playback corrects jitter and clock drift with silence and trims. The callback also lacks the changing playout target that moq play computes. This is the native half of the maintainer's NetEq choice in moq-dev/moq#7679.

Approach

• Port NetEq's received-PCM accelerate / preemptive expand: 30 ms windows, 4 kHz pitch search, shared multichannel seams, original correlation and passive-speech thresholds, 20 ms decision band, and original cooldowns. Retain WebRTC's BSD notice and patent grant.
• Specify the floating-point adaptation once in doc/concept/audio-jitter.md, with literal input/output vectors in audio-jitter/stretch/corpus.json. This gives the browser its conformance contract without waiting for its implementation. The jitter estimator is unchanged.
• Preallocate the callback's input/output storage. Count staged audio in buffered/drain accounting. A true underrun emits emergency silence while received PCM refills; startup silence, short blocks, and finished streams bypass stretch. No history is repeated after input ends.
• Pass the changing target into the callback and remove the CLI's obsolete refill controller. Keep only emergency trims. Extend the callback benchmark over sink count and callback size, with stretch on/off; the nightly rust-test benchmark job (sh/gh/nightly.sh) already runs it.

Decisions

Decided 2026-11-04 (maintainer): accept the additive latency setter and the shared floating-point NetEq adaptation, with its documented arithmetic differences from bit-exact int16 WebRTC. Recorded in the quest Plan.

• ✅ The additive Sink::set_latency setter and the shared float adaptation
• An int16 port of WebRTC's DSP
• A different target handoff without the setter

Impact

• Public API: additive playback::Sink::set_latency(Duration) -> Result<(), Error>. It preserves the target across device rebuilds and refuses a target that cannot fit the ring allocated at open. Migration notes are under ## Unreleased.
• Wire and bindings: none. Fixed-latency callers need no code change; native playback now stretches received PCM by default.

Validation

On 629f13be3e (merged with main at ab6bb2865f):

• just rs test -p moq-audio: 219 passed, including stretch::tests::shared_pcm_vectors, the corpus conformance tests, and the allocation tests. a_target_near_capacity_still_has_emergency_headroom now reaches an accelerate seam; with Accelerate disabled it fails ("removed 0"). just rs play: 255 passed. Both compile under clippy with warnings denied; the only skips are the existing #[ignore] benchmarks and device test.
• quest check --base origin/main (freshly built pinned quest) and sh/text.sh: pass.
• The previous head 97d4154a6d passed the full just ci gate (1,614 Rust tests). It also ran all 12 native quality rows for 60 s each: none voided, and none had a gap after its target converged.

Opus bursty target breach, resolved. One developer run reported a native Opus bursty target p95 of 360 ms, against the provisional 350 ms ceiling. It was a startup interval, not the stretch. That run's target jumped from the 120 ms prior to 360 ms at 2 s and held until the 20 s half-life brought it to 180 ms at 23 s; its other two attempts read 180 ms. The reported target is the estimator's (consumer.delay() in rs/moq-cli/src/play/media.rs), timed when the receive loop reads each frame. This PR changes neither the estimator nor that loop. I ran 7 interleaved runs each of main (635dd54f3c) and this branch on one loaded 24-core host (load 13 to 55, seed 7, 60 s):

	target p95 (ms)	median	underruns/min
main	140, 180, 180, 180, 160, 240, 160	180	0 to 14.1
this PR	180, 200, 200, 180, 180, 180, 180	180	0 in every run

The worst run was on main: a second late interval at 7 s. The 2026-10-29 to 11-03 scheduled nightlies on main read 160 to 280 ms for this row. The ceiling stays as it is, as Capped budgets directs for a startup breach, until the seven-nightly native calibration.

Remaining work, kept in the quest

This PR leaves quest/m1/audio-jitter-target/native-stretch.md with what only an outside condition or the maintainer can finish:

• Maintainer listening check for 2 ms/s drift and the measured expansions on a real device. The quest's ## Waiting gives the exact command for kixtop. Its source is macOS say reading LICENSE-APACHE (no download), published with ffmpeg -readrate 1.002 (then 0.998), and played both directly and through moq-shaper --profile bursty. I smoke-tested that pipeline here, with a sine in place of say and a PulseAudio null sink in place of the speaker: both paths connected, played, and wrote metrics. No listening result is claimed.
• Seven scheduled native nightlies, then the existing budget calibration rule. These run on main, so they follow the merge.
• just rs bench-playback on a quiet host. On 2026-11-04, nixos, nixos2, and kixtop all ran agents and desktop refused SSH. The nightly rust-test job already runs the benchmark. No performance acceptance claim is made.

Review

Codex, gpt-6.1-sol, medium effort, quest review --base origin/main, round 2: no findings on 629f13be3ec1893d20748968add8c0c5415c0b5a. Its responses covered the two pullfrog threads, both fixed:

• .github/workflows/nightly.yml: the added rs bench-playback matrix entry duplicated sh/gh/nightly.sh:99's nightly benchmark run. Reverted.
• rs/moq-audio/src/ring.rs: the near-capacity test never left the startup prime, so it never reached a stretch decision. It now drains the prime and cooldown, then asserts one pitch period is accelerated away below the trim watermark.

Round 1 had no findings on 11590627f3, reviewing the whole branch. Before that, Claude Code reviewed the previous author's implementation: all seven round-1 findings were fixed, and round 2 had no findings on 97d4154a6d.

Alternatives

• Keep the opening latency as the callback target: it would fight automatic playout delay and preserve the current pad/trim controller.
• Port the int16 DSP and its rate-specific machinery verbatim: stronger bit-level WebRTC parity, but substantially more fixed-point code across Rust and JS. Not chosen (2026-11-04).

Follow-ups

None new. The native-stretch quest keeps the remaining proof above. moq-dev/moq#7853 touches playback platform compilation only and does not duplicate this work.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Design change: Buffer depth uses admitted output-rate frames and one atomic consumption position. Local review found that adding ring occupancy to a staged-output snapshot could temporarily hide a 30 ms window during callback handoff. This accounting keeps the callback allocation-free and lock-free; the jitter estimator and NetEq thresholds are unchanged.

(Written by GPT-6.1 Sol)

[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 28 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: 9f17cdcc-ee57-474d-90d2-44a0e0df7331

Commits

Reviewing files that changed from the base of the PR and between ab6bb28 and 629f13b.

Files selected for processing: 16

• doc/concept/audio-jitter.md
• doc/concept/audio-jitter/stretch/corpus.json
• quest/m1/audio-jitter-target/native-stretch.md
• rs/moq-audio/MIGRATE.md
• rs/moq-audio/WEBRTC-LICENSE
• rs/moq-audio/WEBRTC-PATENTS
• 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-audio/src/stretch.rs
• rs/moq-cli/src/play/fake.rs
• rs/moq-cli/src/play/media.rs
• rs/moq-cli/src/play/output.rs
• rs/moq-cli/src/play/timeline.rs
• test/audio-quality/README.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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 1159062 against base 635dd54.

• [P2] Make slow-drift correction work for supported short targets (rs/moq-audio/src/ring.rs:450–462, rs/moq-audio/src/stretch.rs:66–69, 120–121). Input::validate and Sink::set_latency accept a 10 ms target, but expansion requires both a full 30 ms input window and buffered < target. Those conditions cannot both hold at any target of 30 ms or less. For a direct playback caller using a 10 ms target and 10 ms audio callbacks, short windows are passed unchanged, so a slightly slow source drains the cushion until an underrun and refill instead of converging through expansion. Please reconcile the supported latency range with the analysis lookahead, and add a sustained slow-drift test at a short accepted target. The CLI's 50 ms floor avoids this particular case.

The selected floating-point adaptation and the outstanding listening and nightly measurements are documented choices and remaining validation, respectively.

Verification limits: GitHub-only static review of the callback, target validation and DSP paths; I did not run tests, listen on a device, or measure callback cost.

(Written by review (OpenAI))

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes

• NetEq time stretch: rs/moq-audio/src/stretch.rs ports accelerate and preemptive expand with the documented floating-point adaptation; ring.rs drives it from buffered/target, preallocated per sink, never in the callback.
• Changing playout target: additive playback::Sink::set_latency forwards moq play's moving target into the callback and survives device rebuilds; the CLI's write-time pad controller is replaced with an emergency-only fit.
• Accounting: Producer::buffered now reports written - consumed, so staged stretch output counts in buffered(), available(), trim, and drain.
• Docs and vectors: normative algorithm in doc/concept/audio-jitter.md, literal PCM vectors in stretch/corpus.json, plus WebRTC BSD/patent notices and migration notes.
• Tests/CI: new ring, sink, fake, and timeline tests; the mixer benchmark sweeps stretch; rs bench-playback added to the nightly test matrix.

ℹ️ The stretch corpus has no generator or drift check

The estimator corpus's contract is enforced by reference.ts + corpus.test.ts in just check, which is what stops a checked-in vector file from being hand-edited or generated from the implementation under test. The new doc/concept/audio-jitter/stretch/corpus.json has neither, so nothing prevents the circularity the new doc section warns against. I independently reproduced all 12 vectors from the written spec, so the corpus is correct today, but a stretch reference plus drift test (or an explicit note that it is deferred with the browser half) would make that guarantee durable.

Technical details

# Stretch corpus conformance is unguarded

## Affected sites
- `doc/concept/audio-jitter/stretch/corpus.json` — 12 vectors, no generator committed.
- `doc/concept/audio-jitter/corpus.test.ts` — only iterates `cases` from `corpus.ts` (estimator), so the stretch directory is outside `just check`.
- `test/audio-quality/README.md` / `doc/concept/audio-jitter.md` — describe the vectors as the shared cross-language contract.

## Required outcome
- A change to the stretch vectors is either regenerated from a committed reference implementation or flagged by a drift test, matching the estimator corpus's guarantee.
- If the browser implementation (quest `watch.md`) is expected to supply the reference, say so explicitly in the new doc section so the gap is intentional and tracked.

## Suggested approach (optional)
- Add a `stretch/reference.ts` (or a Rust-side generator outside the callback) and a `corpus.test.ts` sibling that re-runs it, mirroring `corpus.ts`/`corpus.test.ts`.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 629f13b against base ab6bb28.

The near-capacity test now drains startup padding and cooldown before asserting an actual pitch-period removal, and the duplicate nightly benchmark entry has been removed while the existing nightly job remains. No new actionable finding in this update.

The prior short-target expansion finding remains unchanged: targets of 30 ms or less are still accepted but cannot satisfy both the full-window and below-target conditions.

Verification limits: GitHub-only static review; I did not run tests, listen on a device, or measure callback cost.

(Written by review (OpenAI))

[pullfrog]

✅ No new issues found.

Reviewed changes

• Nightly revert: Removed the duplicate rs bench-playback matrix entry; .github/workflows/nightly.yml now matches main, and the quest note points at the nightly rust-test job (sh/gh/nightly.sh:99) that already runs the benchmark.
• Ring test fix: Rewrote a_target_near_capacity_still_has_emergency_headroom to drain the startup prime and cooldown before pushing a 200 Hz tone between NetEq's high limit and the trim watermark, so the next read now takes an accelerate seam and asserts one pitch period is removed.
• Merge: main merged in; no conflicts affecting the reviewed paths.

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