Repository navigation
quest(m1): plan the audio-quality and wire-compat follow-ups #5445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # [M] AAC under bursty delivery plays without a re-stall loop | ||
|
|
||
| ## Goal | ||
|
|
||
| The browser player plays AAC under the `bursty` network profile without | ||
| repeatedly running dry and refilling: the nightly audio-quality rows' | ||
| `stalled_share`, skip-aheads, and discarded audio fall to what the Opus rows | ||
| on the same profile show, on both the isolated and the postMessage ring paths. | ||
|
|
||
| ## Plan | ||
|
|
||
| Measured by [#5442](https://github.com/moq-dev/moq/pull/5442) over the 7 | ||
| scheduled nightlies of 2026-10-07 to 2026-10-13: both AAC bursty rows stalled | ||
| 19 to 43% of the window every night, with about 17 skip-aheads a minute and | ||
| about 2 s a minute of audio discarded. AAC wide stalled 6 to 12%. The | ||
| budgets carry these numbers as measured so the job is green; this quest fixes | ||
| the player and tightens them. | ||
|
|
||
| Start after #5442 merges: it uploads the nightly artifacts and carries the | ||
| measured budgets this quest tightens. | ||
|
|
||
| Decided 2026-10-13 (maintainer): a child of the audio jitter target line, | ||
| reproducible from the harness now, so Watch's manual proof runs after it. | ||
|
|
||
| Reproduce before fixing. Start from the nightly's `audio-quality-run` | ||
| artifacts (uploaded every run since #5442), then capture the case as a | ||
| replay trace so the failure runs deterministically under | ||
| `js/watch/src/audio/replay.ts` on a simulated clock. AAC's 1024-sample frames | ||
| (23.2 ms at 44.1 kHz) against Opus's 20 ms are the first suspect: the | ||
| estimator's frame term, the ring sizing, and the skip-ahead threshold each | ||
| see frame duration. Watch already records that a ring running dry re-stalls | ||
| and refills to the target; find why AAC under bursts keeps re-entering that | ||
| loop. Fix the layer that is wrong, including `doc/concept/audio-jitter.md` | ||
| and its conformance corpus if the estimator itself is at fault, mirrored in | ||
| `rs/moq-audio`. | ||
|
|
||
| Tighten the replay budget for the new trace and the AAC browser ceilings in | ||
| `test/audio-quality/budgets.json` from re-measured nightly runs, with the | ||
| reason in review. | ||
|
|
||
| Public API: none expected. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Watch](/quest/m1/audio-jitter-target/watch.md) - the manual browser proof that follows this fix | ||
| - [Opus isolated-ring target](/quest/m1/audio-jitter-target/opus-isolated-target.md) - the other defect the same measurement found |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # [S] The isolated ring holds the target at near-zero jitter | ||
|
|
||
| ## Goal | ||
|
|
||
| On the `near-zero` network profile, Opus on the isolated (SharedArrayBuffer) | ||
| ring settles at the same playout target as the postMessage ring, with no | ||
| underrun episodes, in the nightly audio-quality rows. | ||
|
|
||
| ## Plan | ||
|
|
||
| Measured by [#5442](https://github.com/moq-dev/moq/pull/5442) over the 7 | ||
| scheduled nightlies of 2026-10-07 to 2026-10-13: Opus near-zero on the | ||
| isolated ring reached a `target_ms_p95` of 100 to 800 ms with underrun | ||
| episodes of about 1 s, while the postMessage ring on the same profile held at | ||
| 60 ms. A network with almost no jitter should not grow the target, so the | ||
| first suspect is the isolated page's path rather than the estimator. | ||
|
|
||
| Start after #5442 merges: it uploads the nightly artifacts and carries the | ||
| measured budgets this quest tightens. | ||
|
|
||
| Decided 2026-10-13 (maintainer): a child of the audio jitter target line, | ||
| reproducible from the harness now, so Watch's manual proof runs after it. | ||
|
|
||
| Reproduce it first, from the nightly artifacts and then in | ||
| `js/watch/src/audio/replay.ts` or a ring-level test on a simulated clock. | ||
| The target and the underruns may have separate causes. The target follows | ||
| `consumer.spread`, which `js/hang/src/container/consumer.ts` records from | ||
| transport arrivals before the ring, so a ring defect cannot inflate it: | ||
| compare the captured arrivals and configuration of the two rows first, for | ||
| something on the cross-origin isolated page that delays when frames are | ||
| observed. A ring that under-reports its fill, reads the worklet's position | ||
| late, or misses a resize explains underruns only. Fix each at the source with | ||
| a regression that fails without it, then tighten the isolated near-zero | ||
| ceilings in `test/audio-quality/budgets.json` from re-measured nightly runs. | ||
|
|
||
| Public API: none expected. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Watch](/quest/m1/audio-jitter-target/watch.md) - the manual browser proof that follows this fix | ||
| - [AAC bursty re-stall](/quest/m1/audio-jitter-target/aac-bursty-restall.md) - the other defect the same measurement found |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # [S] Wire compatibility always exercises the compat reader's media path | ||
|
|
||
| ## Goal | ||
|
|
||
| `just test wire-compat` runs at least one current-to-current and one | ||
| released-to-released media cell through the compat reader | ||
| (`test/interop/compat/client.ts`, selected by `INTEROP_COMPAT_CLIENT`), so a | ||
| regression in that reader's media path fails the nightly even while planned | ||
| breaks skip every cross-version media cell. | ||
|
|
||
| ## Plan | ||
|
|
||
| Found on [#5444](https://github.com/moq-dev/moq/pull/5444): the | ||
| `catalog-archive` planned break skips all 24 media cells until a release | ||
| carries #4034, so the new reader's media path ran only by hand. A planned | ||
| break is about a version pairing, so same-version cells are never covered by | ||
| one and stay meaningful. | ||
|
|
||
| Start after #5444 merges: it adds the reader's `subscribe` command and the | ||
| `INTEROP_COMPAT_CLIENT` selector this quest relies on. | ||
|
|
||
| Decided 2026-10-13 (maintainer): m1. Add the two control cells to the | ||
| matrix in `test/interop/compat.sh` and `resolve.ts`, reusing the existing | ||
| lanes rather than a new harness. Pick the lite version the released packages | ||
| and binaries both negotiate. Prove each cell fails when the reader's media | ||
| path is broken (for example, a reader that never decodes a frame), then | ||
| restore it. Runs in the nightly Interop's wire-compat step; never dispatch the | ||
| nightly on a branch. | ||
|
|
||
| Rejected 2026-10-13: a separate tsc pass of the compat reader. `just js check` | ||
| already typechecks `test/interop/compat/*.ts` against the checkout's packages | ||
| through `test/tsconfig.json`, and running a released lane catches API the | ||
| released packages lack. | ||
|
|
||
| Public API: none. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [#5444](https://github.com/moq-dev/moq/pull/5444) - the compat reader and the re-measured planned breaks | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither this selector nor the described reader exists:
test/interop/compat/client.tshas no media-subscribe command, and the harness selects the staged media reader attest/interop/clients/js-native/subscribe.tsviaINTEROP_NATIVE_CLIENT. Implementing the quest literally therefore cannot exercise the reader's media path; name the existing subscriber and selector, or explicitly plan the new selector and reader wiring.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disagree on the premise: #5444 (open) adds
subscribetotest/interop/compat/client.tsand replacesINTEROP_NATIVE_CLIENTwithINTEROP_COMPAT_CLIENTininterop.sh, so the names are right once it lands. bd34293 states that the quest starts after #5444 merges.(Written by Claude Opus 5.5)