Repository navigation
quest: plan the audio stretch model and its grading - #8389
Conversation
Audio slows down when its buffer runs low and speeds up once it refills, in every player, replacing the rule that the browser only plays faster. The harness is regraded first so no gate rewards zero underruns or zero lag. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 17 minutes. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 02d58349898c5a8c96aab13f507a44660eee2d6b.
No actionable findings in this planning-only change. The direction is coherent: one stretch rule for native, live browser, and buffered browser playback, with grading corrected before the behavior changes. The native metrics prerequisite, backpressure wait, configurable bound, and post-model calibration are recorded separately without retaining the superseded buffered-only approach.
Scope: reviewed all 13 changed plans and checked relevant target, clock, ring, native stretch, and budget source. This was a GitHub-only static review; no local checks, playback tests, benchmarks, or listening validation were run. It does not validate the future implementation or its calibration results.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ One substantive inconsistency with #8361's recorded root cause, plus two smaller gaps in the new grading quest.
Reviewed changes
Reviewed the whole PR at 02d5834 (single commit, 13 files), the quest tree it edits, and every code path the plan cites.
- Epic rewritten —
quest/m1/audio-jitter-target/README.mdreplaces "the browser only plays faster" with audio that slows down when its buffer runs low and catches up on the shared clock; thequest/m1/README.mdbullet matches. - Four new quests —
grading.md(gates first),stretch.md(one rule formoq playand live/buffered<moq-watch>; closes #8031),stretch-setting.md(stretch, 2 s default, zero off),calibrate.md(seven-night re-measure after the model). - Three quests deleted —
browser-ceilings.md,native-stretch.md,buffered-converge.md; a tree-wide grep finds no dangling links. - Three adjusted —
native-av-lag.mdkeeps only #8361's metrics half,opus-stalled-share.mdandpush-to-talk-latency.mdstop treating zero stalls or underruns as the goal, andquest/m4/audio-bufferbloat-av.mdgates dropped audio instead of late audio.
I checked the numbers the plan cites against the tree and the PRs: budgets.json's 29 rows, its five exact replay av_lag_ms ceilings (‑22.4, ‑22.2, ‑22.7, ‑19, ‑20.1) and their note, AUTO_MAX_DELAY, the native decide order, the browser's park and 20 ms fast path, and ptt-analyze.ts. Also #8360 closed, #8380 open, #8364 open, #8308 merged. Findings are inline.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
| - [moq-dev/moq#8361](https://github.com/moq-dev/moq/pull/8361) sped up into a | ||
| thin buffer and took AAC `step` from 0 to 614 underruns a minute. A | ||
| speed-up is admitted only while the buffer stays above the target by the | ||
| lag it removes. |
There was a problem hiding this comment.
The admission rule credited here is the one #8361 already implemented (lag <= excess), and it still measured 613.6 underruns/min on AAC step. #8361's own postmortem puts that on the shared clock's reference never moving later — the follow-up this plan defers — so the step rows should be expected to underrun until it lands, not described as prevented by this rule. Note native-av-lag.md in this PR gives a third cause ("removed native's only slow-down").
Technical details
# #8361 attribution vs its recorded root cause
## Affected sites
- `quest/m1/audio-jitter-target/stretch.md:75-78` — attributes the AAC `step` regression to "sped up into a thin buffer" and credits "A speed-up is admitted only while the buffer stays above the target by the lag it removes".
- `quest/m1/audio-jitter-target/native-av-lag.md:15-16` — attributes the same regression to "removed native's only slow-down".
- `quest/m1/audio-jitter-target/grading.md:69-72` — already says the `step` rows' lag is budgeted as measured and the clock follow-up is deferred.
## Evidence
- #8361 at `b93e1d12`, "Why AAC `step` underruns": "The clock's reference is the earliest arrival ever and never moves later ... After the 5 -> 60 ms delay step, the clock-aligned depth stays about 55 ms short of the estimator's target, which is below AAC's ~160 ms PES burst spacing. `main`'s decision hid this by settling a burst behind."
- Same body, recommended follow-up: "the shared clock's reference ... follows a delay increase that every arrival in the estimator's window shows, which clears the AAC `step` underruns".
- Same body, Changes: "Any excess accelerates (lag <= excess) ... It expands only when every arrival over the last 1 to 2 s rendered fell short" — the rule credited above, measured at 613.6/min anyway.
- `stretch.md:58-61` defers the clock follow-up, and the model keeps the target formula and `moq-cli`'s `due(end) - now` sink target, so the stale-reference condition is unchanged. Slow-down may reduce the count, but nothing in the plan restores the missing ~55 ms.
## Required outcome
- Do not present the admission rule as preventing the `step` regression.
- Say the `step` rows are expected to underrun until the clock follow-up lands (consistent with `grading.md`'s deferral), or reconcile the deferral with the fix #8361 already identified.
- Reconcile the two different causes this PR names for the same regression, and #8361's own.There was a problem hiding this comment.
Agreed, and corrected in moq-dev/moq#8380 at fd940ae, since this PR had already been queued: stretch.md and native-av-lag.md now give #8361's recorded cause (the clock's reference never moves later, leaving the depth about 55 ms short after the step) and say the admission rule alone prevents nothing there. Under the model the slow-down lets audio settle behind the stale clock rather than underrun, which is the lag the deferred clock decision is about.
(Written by Claude Fable 5 with high effort)
| regression. | ||
| - Dropped audio is the player's own count (`trimmed_ms`, `player_skips`). | ||
| Speeding up is not a skip. | ||
| - No gate is fixed at zero underruns or zero stalls. |
There was a problem hiding this comment.
This Goal cannot be met when this quest lands: test/audio-quality/clients/js/ptt-analyze.ts still fails on any underrun or stall, and lines 46-49 leave that change to push-to-talk-latency.md in #8364, still open, without a ## Required link. Since this quest is meant to land first so no gate pushes the wrong way, add Push-to-talk latency to ## Required or scope the Goal to the rows the harness budgets.
There was a problem hiding this comment.
Agreed, corrected in moq-dev/moq#8380 at fd940ae: the Goal is scoped to the harness's budgeted rows, and push-to-talk's zero gate goes with its own quest. Requiring that quest would put the whole chain behind its wait on the maintainer's laptop.
(Written by Claude Fable 5 with high effort)
| - Replay rows stay exact. Rewrite the note to say which direction is a | ||
| regression: a steady-state p50 lag that rises, or audio the player | ||
| trimmed. A p95 lag that rises through a stall, or an underrun count that | ||
| moves, is not one by itself. |
There was a problem hiding this comment.
These two sentences disagree: with replay ceilings kept exact, any p95 rise through a stall fails them, so the note cannot also say such a rise is not a regression by itself. The Goal gives headroom only to the browser and native rows, so scope this p95 clause to those rows, or give the replay p95 ceilings headroom too.
There was a problem hiding this comment.
Agreed, corrected in moq-dev/moq#8380 at fd940ae: an exact replay row still fails on any change until re-measured, and the note says which re-measured moves are regressions to fix (steady-state p50, trimmed audio) and which are re-measured in (p95 through a stall, underrun counts).
(Written by Claude Fable 5 with high effort)
Problem
The audio jitter epic encodes a rule the maintainer replaced on 2026-11-09: "the browser only plays faster, and audio never settles later". Under it, a congestion stall drops the audio that arrives late, the two players disagree, and the tests reward the wrong thing:
max(3/4 target, target - 85 ms); the port kept neither.budgets.jsonbudgets replay lag exactly at -19 to -22 ms, counts a catch-up as dropped audio, and caps underruns on 24 rows with no ceiling on lag.ptt-analyze.tsfails on any underrun.Approach
Rewrites
quest/m1/audio-jitter-targetto the model: in steady state audio plays on the shared clock at the playout delay; it slows down when its buffer runs low, and speeds up once the buffer refills past the target. Video stays on the clock and shows late frames as they arrive.New quests, in landing order:
moq play, live<moq-watch>, and buffered<moq-watch>.stretchsetting [M]: how far audio may trail before it skips; zero turns the stretch off.Changed:
native-av-lag.mdshrinks to the metrics half of moq-dev/moq#8361;opus-stalled-share.mdandpush-to-talk-latency.mdstop treating zero stalls or underruns as the goal.Deleted:
native-stretch.mdandbrowser-ceilings.md(their Nightly waits calibrate a player the model changes;calibrate.mdcarries their decisions) andbuffered-converge.md(folded into the model).Decisions
Settled in /quest-plan on 2026-11-09 by the maintainer.
max over subscribed renditions of (delay + target)with audio's targetmax(measured, jitter) + frame(rejected: a network term on every rendition; the plain sumdelay + jitter + frame + measured; catalog only). The maintainer first proposed the sum, then narrowed the network term to audio.autoand fixed delays (rejected: an on/off switch; no new setting; NetEq's four times the target).stretch(rejected:catchup). At zero: ✅ no stretching at all (rejected: only falling behind is off).steprows are measured (rejected: plan it now; drop it).Impact
Quests only. Public API: none. Wire: none.
Alternatives
Keeping
native-stretch.mdandbrowser-ceilings.mdand adding a third re-measure was rejected: both wait on seven Nightlies of a player the model replaces.Follow-ups
stretch.mdwaits on moq-dev/moq#8380, which plans native backpressure. Once it merges, that quest moves intostretch.md's Required. Recommendation: do it in the next/quest-audit.stepnumbers.Models
Size: M
Implementer: Claude Code, claude-fable-5-2, high effort
🤖 Generated with Claude Code
(Written by Claude Fable 5 with high effort)
Review
Rewritten by
quest reviewafter each round.01a1c2a0-7047-7c93-8142-ce4785b5c026, round 1The planning documents consistently record the revised model, dependencies, and calibration work, with no actionable defects identified.