Repository navigation
feat(catalog)!: measure jitter and delay from timestamps, not the wall clock - #6595
Merged
Merged
Conversation
…l clock The catalog estimator measured each frame's flush time against its media timestamp, so any capture clock drifting from the wall clock read as ever-rising lateness. A physical camera on moq.dev/publish ratcheted audio jitter and delay without bound, republishing the catalog and driving hundreds of SUBSCRIBE_UPDATEs a minute. jitter now covers only reorder delay and batch spans. delay is how far a frame's timestamp trails the highest one another track of the catalog already wrote, so importers measure it too. flush, moq_publish_media_flush, the binding wrappers, and moqsink's encoder pad property are removed. The camera's audio still drifts behind video by PTS; the subscribe update quest becomes the audio capture clock fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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 15 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58d410a0a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8fee30cdd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…m-timestamps # Conflicts: # go/wrapper/MIGRATE.md # py/moq-rs/MIGRATE.md # rs/moq-audio/src/encode/producer.rs # rs/moq-c/MIGRATE.md # rs/moq-c/src/publish.rs # rs/moq-ffi/MIGRATE.md # rs/moq-gst/src/sink/imp.rs # rs/moq-gst/src/sink/pad.rs # rs/moq-mux/MIGRATE.md # rs/moq-video/src/encode/producer.rs # swift/MIGRATE.md
…sees them A buffered container::Producer takes its place in the write order when its buffer flushes, a finished producer or fMP4 track stops holding the others back, and a stopped JS video rendition leaves the order after its black keyframe instead of before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Folds in #6599's measurement: release's DTX made Chromium's sample-counted Opus stamps fall behind, and main already removed DTX. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#6610 writes MIGRATE.md at the breaking cut, so this PR's per-package entries become its Impact section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
added this pull request to the merge queue
Oct 24, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 24, 2026
# Conflicts: # rs/moq-mux/src/import/track.rs
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
The catalog
jitteranddelayestimates came from the wall clock: each frame'sflushtime minus its media timestamp, a lifetime maximum. Any clock that does not track the wall clock exactly looks like ever-growing lateness. Every rise republishes the catalog, and the viewer turns each republish into a SUBSCRIBE_UPDATE on every track.I measured this with a physical camera and mic published from
moq.dev/publish(release) and watched from a second tab. These are the viewer'ssubscribe update: id=lines:Over 110 s, the advertised audio
jitterwent from about 3 s to 7.6 s, and the audiodelayreached 26 s: audio output fell about 0.25 s/s behind the wall clock, and the estimator reported that as lateness.Cause of the camera lag
#6599 measured the same camera and mic once a second (worklet stamp minus
now(), AudioContext clock minus wall clock, empty and dropped quanta,encodeQueueSize, encoder input and output counts):moq.dev/publish(6f204fd): input stamps track the clock andencodeQueueSizestays 0, so no samples are lost and there is no encoder backlog. Release configures voice withusedtx: true, and Chromium stamps Opus output by counting the samples it emits, so every frame DTX suppresses pulls later audio 20 ms earlier. (frames in - frames out) x 20 ms equals the gap exactly (1860, 4460, then 9380 ms at 175, 190, and 202 s), and DTX's 400 ms keep-alive matches the 400 ms catalog steps.delaywas 13 ms.mainalready removed DTX in #4808 and #4919 (a test checks it never reaches the encoder), and #6598 backports that torelease. So this PR deletes the camera check quest,quest/m1/subscribe-update-churn.md, instead of replacing it: its chatter came from DTX's timestamps plus the wall-clock estimator, and both are gone. Thanks to #6599 for the measurement; its quest deletion is folded in here.Approach
Jitter and delay are now measured only from media timestamps, in Rust (
moq-mux) and JS (@moq/publish):jitter: only reorder delay (PTS minus DTS) and the media span of a batch or fragment written together.delay: the most a frame's timestamp has trailed the highest timestamp another track of the same catalog has already written. For example, video at 0/20/40/60 and audio at 0/15/30/45/60, written in the orderv0 v20 a0 a15 v40 a30 v60 a45 a60, gives audiodelay20 and video none.moq-ffikeeps a finished media track) counts as ended.container::Producer(with_buffer) takes its place in the order when the buffer flushes, since that is when the transport has its frames. Rates are still counted on the way in.catalog_write_orderbench sweeps 1, 16, and 256 tracks.flushand the gstencoderpad property are removed, and importers now measuredelaytoo.Impact
drafts/draft-lcurley-moq-hang.mdfield-jitter, field-delay, and the data estimates are rewritten, with the moq-hang-04 changelog updated.flushthat reported a frame's write time is removed, since nothing reads the wall clock any more. To migrate, delete the call (and regenerate bindings formoq-ffi);delayis now measured from the order of the timestamps you write, so importers advertise it too.moq-mux:catalog::Estimator::flush,container::Producer::flush,import::Track::flush, and every codec importer'sflush.moq-c:moq_publish_media_flush.moq-ffi:MoqMediaTrackProducer::flush. The checked-in Dart bindings are regenerated.media.TrackProducer.flush,Media.TrackProducer.flush(timestampUs:), andmedia.TrackProducer.Flush.moq-gst): themoqsinksink_%u::encoderpad property is removed. To migrate, dropencoder=truefrom pipeline descriptions; every pad now advertisesdelayfrom its buffer timestamps against the other pads.jitternow covers only reorder delay and batch spans, so a source whose catalog advertised wall-clock lateness advertises much less.@moq/publish: internal only.Broadcast.baseline(@internal) becomesBroadcast.order.Per #6610, these migration notes land in each package's
MIGRATE.mdat the breaking cut rather than here.Alternatives
delayagainst the minimum lateness across tracks (the previousBaseline). That still needs a wall clock per frame, which is what failed.Follow-ups
None from this PR. #6599 suggests an hour-long run to see whether the AudioContext clock keeps stepping behind the wall clock; it no longer affects the catalog, since nothing here reads that clock.
Checks
just check: passes after mergingmain.bun testin js/publish: passes. The new black-keyframe assertion fails without the video teardown fix.just test interop --all: every pairing passes, including thejs-native-node/js-native-bunones that failed on the previous worktree.quest check --base origin/main: passes.delay13 ms, no drift).(Written by Claude Opus 5.5)
🤖 Generated with Claude Code