Skip to content

fix(mux): data tracks stamp on the catalog's current clock - #4668

Merged
kixelated merged 3 commits into
moq-dev:devfrom
Dryvnt:quest/m1/data-track-clock
Oct 1, 2026
Merged

kixelated merged 3 commits into
moq-dev:devfrom
Dryvnt:quest/m1/data-track-clock

Conversation

@Dryvnt

@Dryvnt Dryvnt commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Completes quest/m1/data-track-clock.md.

Problem

A JSON or binary data producer copied the catalog's Clock when it was created. Since #4543, a container importer re-anchors the catalog clock on its first frame, so a data track created before that frame kept stamping on the catalog's starting clock (reads 10 s at creation) while the media read the stream's own PTS. A test with a stream starting at 3600 s saw the data track stamp at 10.002 s.

Fix

catalog::Listing (already owned by every data producer) now stamps payloads: it reads the catalog's current clock on each write, the way media tracks already do through Rendition::timestamp. The four producers (json::{Snapshot,Stream}, binary::{Snapshot,Stream}) drop their cached clock.

Following the anchor exposed a second problem. When the clock steps backwards (common, since fMP4 and FLV sources start near PTS 0), a data track that wrote captured payloads before and after the anchor would measure the step as lateness. Its advertised jitter would then latch at about 10 s, because jitter.max never decays. Listing therefore resets its estimator (Estimator::discontinuity) when the clock's wall mapping changes.

Scope notes

  • Payloads written before the anchor stay on the catalog's starting clock. They were stamped before the mapping existed, so nothing can retime them. This is documented in doc/lib/rs/moq-mux.md.
  • moq-audio capture (encode/capture.rs) and moq-video encode (encode/producer.rs) also copy catalog.clock() once at startup. That is the same stale-copy problem for a local encoder sharing a catalog with an importer. It is out of scope here and suggested as a follow-up.

Public API and wire impact

  • Public API: none. Listing is crate-private, and the removed fields were private.
  • Wire: none. Data-track frame timestamps now carry the anchored clock's values on broadcasts with a container importer.

Tests

  • binary::test::a_write_follows_the_anchored_clock: snapshot and stream tracks created before anchor(3600 s) stamp on the anchored clock. Fails without the fix (10002 ms vs 3600000 ms).
  • json::test::a_write_follows_the_anchored_clock: the same check for json::{Snapshot,Stream}, so a later split of the JSON path stays covered.
  • binary::test::an_anchor_is_not_jitter: captured writes across an anchor back to 0 advertise no jitter. Fails with the clock fix but without the estimator reset.
  • cargo test -p moq-mux (945 tests) and just check pass.

Follow-ups

  • Local encoders (moq-audio capture, moq-video encode) read the catalog clock per frame instead of a startup copy.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

Dryvnt and others added 2 commits October 1, 2026 11:38
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A JSON or binary producer copied the catalog clock at creation, so one
created before an importer's first frame never saw the anchor and kept
stamping on the catalog's starting clock. Listing now reads the
catalog clock on every write, and resets its estimator when the wall
mapping changes so a re-anchor is not measured as jitter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Dryvnt

Dryvnt commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Outcome of the quest-start session:

  • Done: data tracks stamp on the catalog's current clock, and a re-anchor resets the track's estimator, which keeps a backwards clock step from latching about 10 s of jitter. Both regressions are covered by tests that fail without their half of the fix. just check passes.
  • Decision: payloads written before an importer's first frame stay on the catalog's starting clock. This is documented in doc/lib/rs/moq-mux.md. The maintainer chose not to plan a quest to hold or refuse those writes.
  • Follow-up: the moq-audio capture and moq-video encode clock copies are being scoped as a quest.
  • Note: this account has no push access to moq-dev/moq, so the claim branch lives on the fork (Dryvnt:quest/m1/data-track-clock), not on origin.

(Written by Claude Opus 5.5)

@Dryvnt
Dryvnt marked this pull request as ready for review October 1, 2026 10:27

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: 9638803

No actionable bugs found in this diff.

Direction: centralizing current-clock stamping in the crate-private Listing is a focused simplification for all four data producers. Recording against the same clock snapshot keeps each write internally consistent, and reusing Estimator::discontinuity is justified to avoid measuring the re-anchor as jitter. I don’t see a simpler alternative that preserves those properties.

Verification: static inspection of the full diff, clock/catalog/estimator integration, downstream JSON/binary producers, and the new regression tests. I did not execute tests or just check. GitHub’s Test, Check, Windows, and macOS checks are still in progress for this commit; Android succeeded.

@Dryvnt

Dryvnt commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Merge status for 9638803:

  • CI is green (Android, Check, Test, Windows, macOS). The automated OpenAI review found no actionable bugs.
  • Changes: data producers stamp through catalog::Listing on the catalog's current clock, and a changed wall mapping resets the track's estimator. There are two regression tests and a doc note on pre-anchor writes. No public API or wire impact.
  • Decisions: payloads written before the anchor stay on the starting clock, as documented. The local-encoder copies are planned separately in quest: plan one timeline for every source on a catalog #4669 (m2).
  • Waiting on: a Codex review of this head. Codex skips fork PRs, so @kixelated, could you request one? The author has no write access, so a maintainer needs to merge.

(Written by Claude Opus 5.5)

@kixelated

kixelated commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

fix(mux): data tracks re-read the catalog clock on each stamp and reset the estimator when an importer re-anchors, so JSON/binary tracks created before the first frame share the media timeline.

CI: Android, Check, Test, Windows, macOS all green on 96388033b8f8386f475fb59705a1567513e32eeb.

Findings: No concrete issues found.

Listing::stamp (rs/moq-mux/src/catalog/data.rs) re-reads via Rendition::clock() → Producer::clock() (live Arc<Mutex<State>>), compares wall() for a mapping break, calls Estimator::discontinuity(), then stamps. That matches how media already follows a post-#4543 first-frame anchor, and the two new binary tests (a_write_follows_the_anchored_clock, an_anchor_is_not_jitter) lock the regression and the jitter latch. Crate-private API; pre-anchor payloads staying on the starting clock is documented in doc/lib/rs/moq-mux.md.

Non-blocking notes:

  • Regression coverage is on binary::{Snapshot,Stream} only. JSON producers share Listing::stamp, so the path is covered, but a thin JSON twin of a_write_follows_the_anchored_clock would guard against a future split.
  • Same stale-copy class remains in moq-audio / moq-video capture (quest: plan one timeline for every source on a catalog #4669 plans that follow-up); correctly out of scope here.

MERGE on 96388033b8f8386f475fb59705a1567513e32eeb

This is an automated review, not the maintainer's decision
(Written by Grok)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Dryvnt

Dryvnt commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Added the JSON version of a_write_follows_the_anchored_clock from the Grok review's non-blocking note, in c45a5ea. It covers json::{Snapshot,Stream} and fails without the fix (10001 ms vs 3600 s). Since that push restarts CI and the reviews, the merge should wait on the new head.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: c45a5ea

No new actionable bugs found. The only change since my previous review (9638803) adds the JSON anchored-clock regression test in rs/moq-mux/src/json.rs:516–551. It checks both an untimed snapshot and a captured stream record, creates both producers before anchoring, and verifies the actual frame timestamps with millisecond-safe bounds.

Direction: this is useful coverage of the JSON entry points without changing production behavior. The shared Listing::stamp design remains focused; a new test abstraction would add little value here.

Verification: static inspection of the delta and surrounding JSON producer/clock paths. I did not run tests or just check. GitHub's Android, Check, and Platform workflows are still in progress for this head.

@kixelated

Copy link
Copy Markdown
Collaborator

Merging c45a5ea.

  • Changes: JSON and binary data producers stamp through catalog::Listing on the catalog's current clock, so a track created before an importer's first frame follows the anchored clock. A changed wall mapping resets the track's estimator so a re-anchor does not latch as jitter. Three regression tests (binary and JSON anchored-clock, anchor-is-not-jitter) and a doc note on pre-anchor writes. No public API or wire impact.
  • Reviews: two automated reviews cover this head (no findings); the Grok note about JSON coverage was addressed in c45a5ea. CI green.
  • Decisions: payloads written before the anchor stay on the starting clock, as documented. The local-encoder clock copies (moq-audio capture, moq-video encode) are tracked in quest: plan one timeline for every source on a catalog #4669.

Auto-merge (squash) enabled on this head.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit e2a1f31 into moq-dev:dev Oct 1, 2026
6 checks passed
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.

2 participants