Skip to content

The shared clock follows a lasting path-delay step - #8749

Merged
kixelated merged 4 commits into
mainfrom
quest/m2/audio-playout/clock-step
Nov 14, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m2/audio-playout/clock-step

Conversation

@kixelated

@kixelated kixelated commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

After a lasting increase in path delay, the shared clock follows it, in the browser (js/watch Sync) and natively (moq_mux::sync).

Until now a track's anchor (the earliest arrival minus timestamp it has seen) only ever moved earlier, while the estimator measures each frame against the fastest of the last 2 s. After a step, the estimate forgot the step within its window but the clock kept the old reference, so every frame stayed late by the step. Under the lag rule (moq-dev/moq#8722) that showed up as underruns.

The rule, for both players:

  • A track's arrivals fall into back-to-back 2 s windows of arrival time (the estimator's WINDOW). The arrival that closes a window counts in it and opens the next.
  • When a window closes and every arrival in it trailed the track's anchor by more than half the delay, the anchor moves later to the window's earliest arrival.
  • An earlier arrival still moves the anchor earlier at once. The reference is still the earliest anchor, so it moves fully only once every arriving track has followed. A track that stops arriving keeps its anchor.
  • A move is lag like any other: audio slows down within its stretch threshold or pads silence past it, and video holds a frame.

Both players read one shared vector, doc/concept/audio-jitter/clock/corpus.json, with five cases: step, shallow, spike, tracks, and silent. doc/concept/audio-jitter.md gains "A lasting step". This finishes quest/m2/audio-playout/clock-step.md and deletes it.

Decisions

Each was taken by the agent under /quest-afk, for the maintainer's review.

  • Deadband: half the resolved delay. Picked by replaying the 11-13 Nightly's captured browser arrivals (run 43447849289) through each candidate rule. On chromium-opus-step, today's clock leaves 43% of the frames after 35 s late. With half the delay none are late, after one 56 ms move, and no other profile moves at all. Rejected:
    • A fixed 10 or 20 ms deadband: it churned on wide, 2 to 7 moves per minute, each one a skip or slowdown.
    • The whole delay: it never fires on Opus step, whose 55 ms step is about the size of its 60 ms target.
  • Window: 2 s tumbling, the estimator's WINDOW, so the clock and the target measure against the same fastest frame. Rejected:
    • 4 s: a slower follow for no gain in the replay.
    • A sliding minimum: it needs a deque per track where two numbers do.
  • Each track follows its own anchor, and the reference stays the earliest anchor. Rejected: dropping a track that has gone quiet from the reference. That would change how a stalled track holds the clock, which this quest does not own.
  • A congestion that keeps every frame past half the delay for a whole window counts as a step for as long as it lasts. The first faster frame moves the anchor back. In the replay and the live congestion rows, the 1.5 s collapse and its backlog never filled a whole window.
  • Merge, after the native Opus step reruns under Validation showed no regression. Decided 2026-11-14 (maintainer, /quest-complete): ✅ merge

Validation

  • Corpus: cargo test -p moq-mux --lib sync:: (33 pass) and bun test js/watch/src/sync.test.ts (43 pass). Against main's sync.ts, the corpus fails its step and tracks cases.
  • Replays: just test audio-quality --replays --enforce. All six rows are unchanged and within their exact budgets.
  • Live rows on a loaded developer host (x86_64 Linux, debug build, seed 7, 60 s a row), branch against main:
row underruns/min (main → branch) A/V lag p50 / p95 ms (main → branch)
native-aac step 27.1 → 5.4 5.3 / 31.6 → 3.5 / 28.1
native-opus step 1.1 → 20.6 -0.9 / 32.6 → -5.1 / 17.2
chromium-aac step 0 → 0 33.2 / 59 → 31.9 / 67.5
chromium-opus step 0 → 0 (player underruns 2.2 → 0) 38 / 66.2 → 32.7 / 57.3
native-aac wide 11.9 → 16.3 -2.9 / 14.5 → -6.8 / 13.2
native-opus wide 5.4 → 9.8 -4.8 / 9.8 → 0.4 / 20.9
chromium-aac wide 2.2 → 0 33.2 / 54.2 → 29.1 / 51
chromium-opus wide 0 → 0 33.8 / 58.1 → 34.9 / 59.8
  • Timing on the native step rows:
    • On main, the AAC row underruns repeatedly after the step against the stale reference: at 30.7, 31.3, 48.8, 50.8 and 59.5 s.
    • On the branch, quiet quanta occur only at the step itself (29.75 s on Opus, one 18-quantum episode, and 30.0 s on AAC). That is about 2.5 s before the move: the lag jumps from +13 to -45 ms at 32.5 s as the reference moves later, then audio rests within half a frame of the clock (about -10 ms on Opus).
    • The Opus row's higher per-minute count is that single episode at the step, before any move. The reruns below show it is noise.
  • Native Opus step reruns, 13 runs interleaved on one host (fedora, debug build, seed 7, 60 s, 10:26 to 10:40): main is the PR's merge base 77c9ca5, the branch is 777470d. The step is the lag's jump up, the move is the lag's drop as the reference moves later, and episodes are the analyzer's quiet-quanta gaps, on the viewer clock.
run underruns/min A/V lag p50 / p95 ms step s move s episodes (s, quanta)
main 1 0 6.1 / 41.3 30.0 none none
main 2 18.5 5.8 / 45.3 30.0 none 29.81, 17
main 3 91.9 (over budget) 7.7 / 41.9 30.0 none 28.56, 10; then 10 more from 32.6 to 54.7 s
main 4 0 5.9 / 44.5 30.0 none none
main 5 13 2.8 / 40.4 30.0 none 51.15 to 51.27, 65; 54.6, 1; 58.1, 3
main 6 0 4 / 43.3 29.75 none none
branch 1 0 -5.3 / 12.8 29.75 32.25 none
branch 2 0 -7 / 5.6 30.0 32.25 none
branch 3 15.2 -3.9 / 19.7 30.0 32.5 15.58 to 15.71, 26
branch 4 20.5 -4.4 / 1.4 30.0 32.25 7.41 to 7.58, 31
branch 5 0 -3.8 / 8.7 30.0 32.25 none
branch 6 0 -6 / 16.7 30.0 32.25 none
branch 7 0 -6.6 / 8.2 30.0 32.25 none
  • Mean underruns per minute: main 20.6 (median 6.5), branch 5.1 (median 0). Both sides swing from 0 to about 20 per minute run to run, so a single run of this row is noise.
  • The episode at the step is not the change's. main 2 has the same one, 17 quanta at 29.81 s, beside the earlier branch run's 18 at 29.75 s. Until the first window closes with every arrival past half the delay (the move, 2.25 to 2.5 s after the step), the branch's clock is main's, so it cannot cause anything at the step.
  • No branch episode falls at or after the step or the move. Both of the branch's episodes come in the first 16 s, before the step.
  • On main, audio slips further behind the stale reference after the step. The median lag goes from -8 ms before the step to +12 ms at 34 to 45 s and +40 ms at 45 to 60 s (main 2). The late episodes in main 3 and 5 come from that. On the branch the lag returns to about -5 to -10 ms after the move, and its p95 drops from 40 to 45 ms to 1 to 20 ms.
  • Per the replay, no reference moved on wide. The wide underrun counts differ by a few quanta in each direction, within run-to-run noise on this host.
  • Branch mild and congestion rows ran too; none fell outside its budget except the A/V lag keys.
  • Every browser row's A/V lag p50 sits at 27 to 39 ms on this host, on main as well, which the budget note already records. That is why those keys exceed their Nightly-measured budgets here.
  • After merging main (with moq-dev/moq#8730's join discount): cargo test -p moq-mux --lib (2109 pass), bun test js/watch/src js/hang/src/container doc/concept (929 pass), and just test audio-quality --replays --enforce (all six rows within budget).
  • Not run: the Nightly runner itself. The first Nightly after merge grades it.

Impact

  • No public API or wire change. Both players' clocks now move later after a lasting path-delay step.

Caused by

No single PR. The anchor has only ever moved earlier since the shared clock was written. The lag rule (moq-dev/moq#8722) turned the stale reference from a resting offset into underruns.

Follow-ups

None.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-opus-5-5, unknown effort (iterate, XS)

(Written by Claude Opus 5.5)

Review

Rewritten by quest review after each round.

  • Head: b714fed

  • Base: origin/main at 8b93ab2

  • Reviewer: Codex, gpt-6.1-sol, high effort

  • Session: 01a1da7f-5392-7d21-a96b-6d0a6ec10a10, round 2

  • Score: 8/10, The new head only merges main, leaves the previously reviewed implementations unchanged, and introduces no actionable findings; builds and tests were not run as instructed.

  • Rounds: 2; findings 0: fixed 0, refuted 0, open 0

  • Findings: none

kixelated and others added 3 commits November 14, 2026 05:15
Each track's anchor moves later once a whole 2 s window of its arrivals
trailed it by more than half the delay, in js/watch Sync and moq_mux::sync,
held to one corpus.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 14, 2026 14:42
@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

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 8 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: 8eb0bb95-ff1f-433d-9499-2b73fa0dfda3
Commits

Reviewing files that changed from the base of the PR and between 8b93ab2 and b714fed.

Files selected for processing: 7

  • doc/concept/audio-jitter.md
  • doc/concept/audio-jitter/clock/corpus.json
  • js/watch/src/sync.test.ts
  • js/watch/src/sync.ts
  • quest/m2/audio-playout/README.md
  • quest/m2/audio-playout/clock-step.md
  • rs/moq-mux/src/sync.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

pullfrog[bot]
pullfrog Bot previously approved these changes Nov 14, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

I reviewed the clock-step rule in both players, the shared corpus, and the docs. Both corpus suites pass locally: bun test js/watch/src/sync.test.ts (43 pass) and cargo test -p moq-mux --lib sync:: (33 pass).

  • Lasting-step follow in js/watch Sync: Clock.follow closes 2 s tumbling windows per track and moves the anchor later once the window's least ref trails it by more than half the resolved delay. When the owner moves, handoff passes the reference to the earliest anchor left. While startup is in progress, nothing moves.
  • Same rule in moq_mux::sync: Part::follow uses the same window and the same strict half-delay bound (Anchor::earlier against anchor + half). The reference is still the minimum over parts, so a move shows up through moved.
  • Window lifecycle: reset, end, unanchor, and release all clear the window, so a restarted timeline never inherits stale evidence.
  • Shared vector clock/corpus.json: five hand-written cases (step, shallow, spike, tracks, silent) run in both languages, following the comfort/ corpus precedent.
  • Docs and quest: audio-jitter.md gains "A lasting step", and clock-step.md is retired.

On the JS handoff: the reference only moves when owner === part. I checked the case where the owner is not the track with the earliest anchor, which can happen after the startup shift or under a lookahead cap. It still converges, because each track measures the trail against its own anchor. A shared step therefore makes the owner follow too, and each handoff passes ownership to the earliest anchor.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

# Conflicts:
#	quest/m2/audio-playout/README.md

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

The only change since the last review is a merge of origin/main. The clock code, the corpus and the docs are unchanged.

  • Quest README after the merge: In quest/m2/audio-playout/README.md, the merge keeps the PR's sentence that the clock step is done. It also brings in main's sentence about the join's intervals (moq-dev/moq#8730).

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 14, 2026
Merged via the queue into main with commit 7b9f664 Nov 14, 2026
6 checks passed
@kixelated
kixelated deleted the quest/m2/audio-playout/clock-step branch November 14, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant