Repository navigation
fix(watch,publish): keep audio across declared ends; repair browser media tests - #7716
Merged
Merged
Conversation
Quest-Claim: d4ebdf7a8466fdb1
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
kixelated
changed the title
This was referenced Nov 3, 2026
A video codec change re-ran the publisher's per-rendition track gate for every rendition. The gate's cleanup cleared the audio rendition's track a microtask before the rerun set it again, so the audio encoder ended its epoch and wrote a discontinuity marker on every video rename. The player then treated the later audio group as a new timeline: it reset the shared clock and flushed the audio ring, discarding up to a 500 ms floor of received audio and refilling it in silence. That is the Firefox `audio tone` failure in Nightly's codec rename and coded-size windows. Publishers keep timestamps moving forward across a declared end, so the decoder now only restarts the codec there, as it already does for a gap. The publisher sets each rendition's track directly instead of clearing it on every rerun, and clears every handle when the publication ends. Finishes quest/m0/test-flakes-2/browser-media-flakes.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
kixelated
marked this pull request as ready for review
November 5, 2026 07:14
Contributor
|
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 5 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
commented
Nov 5, 2026
kixelated
left a comment
kixelated
left a comment
Collaborator
Author
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 1029a6f
No actionable introduced defects found. Preserving the decoder's PCM and clock across declared ends while resetting on an actual broadcast replacement fits the existing lifecycle; the publisher cleanup paths remain explicit. Added coverage exercises the declared-end gap and stable audio identity through video changes.
Verification: GitHub-only static review. I did not run tests; the reported load trials still expose other mechanisms and do not establish general flake elimination.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — read the full diff, the touched sources, the signals Effect lifecycle, and the Rust audio consumer contract.
- Audio player (
js/watch/src/audio/decoder.ts) — a later group after a declared end now restarts only the codec (#reprime) instead ofsync.reset()+ ring flush, so retained PCM and the shared clock survive; matchesrs/moq-audio's forward-only timeline contract and the removed rewind test's out-of-contract premise. - Publisher (
js/publish/src/broadcast.ts) — the per-rendition gate setshandle.trackdirectly, clearing explicitly on closed/idle and via an outer publication cleanup, removing the microtaskundefined→track flap that ended audio's epoch on another rendition's rename. - Tests — new broadcast test asserts a video rename never notifies audio's track signal; legacy and CMAF decoder tests assert codec-only restart with ring, clock, and PCM retained across a 40 ms gap.
- Browser harness — Firefox child scoped to the Nix ffmpeg/libpulse path; buffering test polls visibility through
waitForso fixture page errors surface immediately, with a 30 s deadline; fixture gets a data favicon. - Quest —
browser-media-flakes.mddeleted with its epic bullet; no dangling references.
Both changed test files pass at head (bun test: 44 and 56 tests).
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Collaborator
Author
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Nov 5, 2026
Contributor
|
quest land watch: the merge queue removed this pull request at 1029a6f because its The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
… run [850.46ms]
moq-doc check:
moq-doc check: ::endgroup::
moq-doc check:
moq-doc check: ::group::concept/audio-jitter/corpus.test.ts:
@moq/loc check: bun test v1.4.2 (744846f84)
@moq/loc check:
@moq/loc check: ::group::src/index.test.ts:
@moq/receive-credit-test check: Exited with code 0
@moq/p2p-test check: Exited with code 0
@moq/json check: Exited with code 0
moq-doc check: (pass) audio jitter corpus > paced > matches the checked-in file [0.29ms]
moq-doc check: (pass) audio jitter corpus > paced > is a trace of equal-length clocks [0.05ms]
moq-doc check: (pass) audio jitter corpus > paced > arrives in a monotonic order [2.91ms]
moq-doc check: (pass) audio jitter corpus > paced > names a target for the first frame [0.12ms]
moq-doc check: (pass) audio jitter corpus > paced > reports every target as a change [0.14ms]
moq-doc check: (pass) audio jitter corpus > flush > matches the checked-in file [0.05ms]
moq-doc check: (pass) audio jitter corpus > flush > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > flush > arrives in a monotonic order [0.34ms]
moq-doc check: (pass) audio jitter corpus > flush > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > flush > reports every target as a change [0.02ms]
moq-doc check: (pass) audio jitter corpus > advertised > matches the checked-in file [0.06ms]
moq-doc check: (pass) audio jitter corpus > advertised > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > advertised > arrives in a monotonic order [0.44ms]
moq-doc check: (pass) audio jitter corpus > advertised > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > advertised > reports every target as a change
moq-doc check: (pass) audio jitter corpus > buildup > matches the checked-in file [0.07ms]
moq-doc check: (pass) audio jitter corpus > buildup > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > buildup > arrives in a monotonic order [0.43ms]
moq-doc check: (pass) audio jitter corpus > buildup > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > buildup > reports every target as a change [0.03ms]
moq-doc check: (pass) audio jitter corpus > reorder > matches the checked-in file [0.12ms]
moq-doc check: (pass) audio jitter corpus > reorder > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > reorder > arrives in a monotonic order [0.36ms]
moq-doc check: (pass) audio jitter corpus > reorder > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > reorder > reports every target as a change
moq-doc check: (pass) audio jitter corpus > backlog > matches the checked-in file [0.08ms]
moq-doc check: (pass) audio jitter corpus > backlog > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > backlog > arrives in a monotonic order [0.60ms]
moq-doc check: (pass) audio jitter corpus > backlog > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > backlog > reports every target as a change
moq-doc check: (pass) audio jitter corpus > idle > matches the checked-in file [0.04ms]
moq-doc check: (pass) audio jitter corpus > idle > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > idle > arrives in a monotonic order [0.20ms]
moq-doc check: (pass) audio jitter corpus > idle > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > idle > reports every target as a change [0.01ms]
moq-doc check: (pass) audio jitter corpus > tunein > matches the checked-in file [0.06ms]
moq-doc check: (pass) audio jitter corpus > tunein > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > tunein > arrives in a monotonic order [0.35ms]
moq-doc check: (pass) audio jitter corpus > tunein > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > tunein > reports every target as a change
moq-doc check: (pass) audio jitter corpus > spike > matches the checked-in file [0.08ms]
moq-doc check: (pass) audio jitter corpus > spike > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > spike > arrives in a monotonic order [1.00ms]
moq-doc check: (pass) audio jitter corpus > spike > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > spike > reports every target as a change [0.01ms]
moq-doc check: (pass) audio jitter corpus > straggler > matches the checked-in file [0.07ms]
moq-doc check: (pass) audio jitter corpus > straggler > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > straggler > arrives in a monotonic order [0.34ms]
moq-doc check: (pass) audio jitter corpus > straggler > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > straggler > reports every target as a change [0.01ms]
moq-doc check: (pass) audio jitter corpus > fixed > matches the checked-in file [0.03ms]
moq-doc check: (pass) audio jitter corpus > fixed > is a trace of equal-length clocks
moq-doc check: (pass) audio jitter corpus > fixed > arrives in a monotonic order [0.15ms]
moq-doc check: (pass) audio jitter corpus > fixed > names a target for the first frame
moq-doc check: (pass) audio jitter corpus > fixed > reports every target as a change
moq-doc check:
moq-doc check: ::endgroup::
moq-doc check:
moq-doc check: 134 pass
moq-doc check: 0 fail
moq-doc check: 15965 expect() calls
moq-doc check: Ran 134 tests across 3 files. [6.24s]
@moq/loc check: (pass) Format treats an empty payload as a duration marker [100.67ms]
@moq/loc check: (pass) Format preserves empty data payloads [0.22ms]
@moq/loc check: (pass) Format decodes no frame from a data track's empty object and throws on a media track's [0.18ms]
@moq/loc check: (pass) Format decodes timestamp at default microseconds timescale [0.11ms]
@moq/loc check: (pass) Format honors per-frame timescale property [0.11ms]
@moq/loc check: (pass) Format skips unknown odd-typed properties [0.11ms]
@moq/loc check: (pass) Format throws when the timestamp property is missing [0.21ms]
@moq/loc check: (pass) Format rejects zero per-frame timescale [0.14ms]
@moq/loc check: (pass) Format throws when properties_length exceeds frame size [0.09ms]
@moq/loc check: (pass) Format decodes the draft-03 timestamp property [0.07ms]
@moq/loc check: (pass) Format skips an unknown property whose value needs all 62 bits [0.46ms]
@moq/loc check: (pass) Format rejects a timestamp past 2^53 - 1 instead of rounding [3.23ms]
@moq/loc check:
@moq/loc check: ::endgroup::
@moq/loc check:
@moq/loc check: 12 pass
@moq/loc check: 0 fail
@moq/loc check: 19 expect() calls
@moq/loc check: Ran 12 tests across 1 file. [738.00ms]
@moq/loc check: Exited with code 0
@moq/msf check: Exited with code 0
@moq/crypto check: Exited with code 0
@moq/hang check: Exited with code 0
@moq/interop-native check: Exited with code 0
@moq/watch check: Exited with code 0
@moq/publish check: Exited with code 0
@moq/demo-carla check: Exited with code 0
@moq/boy check: Exited with code 0
@moq/video-test check: Exited with code 0
@moq/room check: Exited with code 0
@moq/bench-browser check: Exited with code 0
@moq/audio-quality check: Exited with code 0
@moq/interop-browser check: Exited with code 0
@moq/demo check: Exited with code 0
moq-doc check: API reference: 17 packages in 83.4s
moq-doc check:
moq-doc check: �[32m�[1mvitepress�[22m v1.6.4�[39m
moq-doc check:
moq-doc check: - building client + server bundles...
moq-doc check: �[34mllmstxt�[39m�[2m » �[22m �[1mvitepress-plugin-llms�[22m initialized �[2m(client build)�[22m with workDir: �[36m/var/cache/github-runner-work/moq/moq/doc�[39m
moq-doc check: �[34mllmstxt�[39m�[2m » �[22m Build started, file collection cleared
moq-doc check: �[34mllmstxt�[39m�[2m » �[22m Processing �[1m126�[22m markdown files from �[36m/var/cache/github-runner-work/moq/moq/doc�[39m
error: recipe `check-changed` was terminated on line 32 by signal 15
##[endgroup]
##[error]ci: check obs_compile failed
ci: seconds per module
139 check obs_compile (failed)
9 check biome
130 check js (cancelled)
71 check flake
8 check quest
13 check obs
21 check shell
1 check large_files
1 check text
3 check toml
3 check nix
0 check justfile
140 wall time
error: recipe `ci` failed on line 115 with exit code 2
##[error]Process completed with exit code 2.
##[group]Run actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9
with:
name: cargo-timings
path: target/cargo-timings/cargo-timing-*.html
if-no-files-found: ignore
retention-days: 14
compression-level: 6
overwrite: false
include-hidden-files: false
archive: true
env:
BASE: d0c73e35844c65bdd27bd43cba37431de14fee60
CHECK_FAIL_FAST: 1
NEXTEST_TEST_THREADS: 24
MBX_CACHE_LINKS: 1
MBX_REMOTE_URL:
##[endgroup]
With the provided path, there will be 1 file uploaded
Artifact name is valid!
Root directory input is valid!
Uploading artifact: cargo-timings.zip
Beginning upload of artifact content to blob storage
Uploaded bytes 78245
Finished uploading artifact content to blob storage!
SHA256 digest of uploaded artifact is 05b5c8f1345b9fe4389135570549dd756f8d4fa9facbc70eb2b5dd3f0c2bf8b8
Finalizing artifact upload
Artifact cargo-timings successfully finalized. Artifact ID 13437435936
Artifact cargo-timings has been successfully uploaded! Final size is 78245 bytes. Artifact ID is 13437435936
Artifact download URL: https://github.com/moq-dev/moq/actions/runs/42072368255/artifacts/13437435936
Post job cleanup.
##[start-action display=Run jdx/mr-boxington-action@88acf2bb50552c7c508cbac13e5f8c576b95b1de;id=__9a665453-1541-4725-a102-44f8b46644a1.mbx]
##[end-action id=__9a665453-1541-4725-a102-44f8b46644a1.mbx;outcome=skipped;conclusion=skipped;duration_ms=0]
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/fb52d934-a68f-40ab-aae8-5d9a00c04048' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/moq/moq
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 0 objects and 0 action results (0 B logical); 49.9 GiB logical remain
Cleaning up orphan processes
(written by quest land watch) |
Contributor
|
codex / Codex land fixer note:
(written by codex / Codex) |
Contributor
|
codex / Codex land fixer: error: couldn't create a temp dir: No space left on device (os error 28) at path "/var/cache/github-runner-work/moq/moq/target/debug/deps/rmetanKcmWa" error: could not compile `moq-ffi` (lib) due to 1 previous error (written by codex / Codex) |
kixelated
added a commit
that referenced
this pull request
Nov 5, 2026
Take #7716's forward-only declared-end handling: a later group restarts the codec on the same timeline, so the per-end audio timeline reset and its test are dropped; audio keeps its own Track on the shared clock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 5, 2026
kixelated
added a commit
that referenced
this pull request
Nov 5, 2026
Take #7716's audio decoder: a later group after a declared end restarts only the codec and keeps the ring and the shared clock. The PR's hook keeps just the ring's declared end and next utterance, so buffered playback no longer pauses the tail behind a clock reset. A short declared pause between two buffered utterances now plays as comfort noise: the ring reports the span it zero-filled and the worklet fills it. Running dry between the next utterance's arrival and its first decoded write still counts as declared silence. The replay harness no longer resets its ring on a gap, matching the decoder; traces record no declared ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Finishes the browser media flake quest.
The Firefox player's
audio tonechecks failed under load in Nightly (2026-11-01:codec rename: 69/78), always around a video transition. Reproduced locally (Firefox player,--media-cases rename,pause, pinned to 2 CPUs):codec rename: 71/81,70/80, andcoded-size growth: 63/75, matching that trace.Instrumenting the player and publisher showed one chain:
js/publish/src/broadcast.tsgates every rendition'shandle.trackand reruns when any rendition registers, renames, or fails. Its nested gate cleared the handle on cleanup and set it again a microtask later, so a video codec change flapped the audio trackundefinedand back. The audio encoder ends its epoch on any track change, so every video rename wrote an audio discontinuity marker (end == next, zero real gap). This happened once in every run.sync.reset()and flushed the ring. That discarded all received PCM (up to the 500 ms floor) and re-anchored on the next frame, so the floor refilled in silence. With a ~150 ms delay the window tolerated it. At CI's 560-835 ms delay it failed.The buffering fixture's missing
video.source.out.configsignal from the earlier attempt landed on main through moq-dev/moq#7672. The page-error surfacing and Firefox library scoping from this PR remain.Approach
js/watch/src/audio/decoder.ts): a later group after a declared end now restarts the codec, as a gap already does, and keeps decoded PCM and the shared clock. Publishers keep timestamps moving forward across a discontinuity (doc/concept/hang.md and every SDK's "Timeline breaks" section). Rust's audio consumer also assumes this, and its player drains the old sink first. The inlined#onReadhelper is gone.js/publish/src/broadcast.ts): the gate setshandle.trackdirectly rather than clearing it on cleanup. It clears it explicitly when the track closes, when on-demand video is idle, or when a failed rendition waits to recover. A cleanup on the publication clears every handle when the publication itself ends.sync.reset()call.undefinedand back.Choices made without the maintainer:
Impact
@moq/watch: a discontinuity marker on audio no longer silences playback for the buffered floor or re-anchors video's clock.@moq/publish: registering, renaming, or failing one rendition no longer restarts the others' encoders (audio flush and discontinuity marker).Alternatives
Validation
All local, Firefox player against a Chromium publisher.
pin=taskset -c 0-1. The host was shared with other sessions' builds the whole time.end == next), 0 ring resets. Failures in other windows, at 1-19% host idlebun testinjs/watch(496) andjs/publish(347): pass. The new tests fail on the old code as described above.quest gate nix develop --command just check: pass. Strictcheck flakeis skipped by its namespace probe.just ci: every module passes exceptcheck flake, which fails building the unchanged runner image withunshare: write failed /proc/self/uid_map: Operation not permitted. That is this host's limit, which the recorded decision below leaves to hosted CI.quest check --base origin/main: pass.Review
quest review, gpt-6.1-sol, medium effort, round 1 (new reviewer session for this iteration).1029a6f0c005d587d09edbfe3adb603e098c6632.Maintainer decision
2026-11-04 (/quest-unblock): ✅ Hosted CI suffices for the strict Nix gate. Local
check flakeskipping on a host that refuses user-namespace mapping is expected since moq-dev/moq#7763. The strict check runs in hosted CI on the ready PR. Requiring a namespace-capable local builder was not chosen.Follow-ups
#onBreak) still callssync.reset(), under the same "new timeline" assumption this PR drops for audio. It is reached after a video disable or demand gap. Not reproduced here, and thedisablemedia case passes. Worth a quest to align it with the forward-only contract.D-Bus name org.PulseAudio1 already taken.(Written by Claude Opus 5.5)