Skip to content

fix(mux)!: a broadcast stamps everything from one clock - #7218

Merged
moq-bot[bot] merged 6 commits into
mainfrom
quest/m1/broadcast-clock
Nov 2, 2026
Merged

moq-bot[bot] merged 6 commits into
mainfrom
quest/m1/broadcast-clock

Conversation

@kixelated

@kixelated kixelated commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m1/broadcast-clock.md and deletes it.

Problem

A moq-mux broadcast stamped its media on moq_mux::Clock. Its hang and MSF catalog groups (catalog/producer.rs) and its HLS records (hls.rs) used the ambient Timestamp::now() instead. The Clock also read its own instant from the ambient clock. When a call came from another runtime or clock context, a track got stamps from two clocks and moq-net refused it with TimestampMismatch. moq-ffi's containers_of_one_source_keep_their_relative_media_time hit this and worked around it by driving the private importers.

Approach

The maintainer decided on 2026-10-29, as a follow-up of moq-dev/moq#7161, that the whole broadcast uses one clock. Stamping with max(now, last) would only hide the second clock.

  • kio::clock::Clock (new): Clock::current() captures the ambient clock, checked in the same order now() checks it: an entered Deadlines queue, a simulator run, an installed Manual clock, a tokio runtime handle, or the host clock. now() and wall() keep reading that clock from any thread or runtime. Equality means the same clock.
  • moq_mux::Clock holds one, captured at Clock::new() / Clock::at(). Every later reading goes through it: now, capture, and the re-basings (arrival, placed, adopted, which are now methods that keep the captured source). It is Clone, no longer Copy.
  • Catalog groups: Outputs::emit stamps the hang and MSF groups with the broadcast clock's now().
  • HLS records: hls::Producer and hls::Tracks take the broadcast clock. If the catalog re-bases its clock before anything fixes it, the record tracks follow. A record track that has already stamped keeps its clock, so its timestamps never go back.
  • moq-ffi test: it now writes through the exported MoqMediaContainerProducer::write, which enters the unpaused FFI worker. It checks that media and the catalog both land on the paused clock the broadcast was built under.

Choices made without asking (please review):

  • The catalog moves onto the media timeline. The quest asked to check this against Address groups by timestamp. Once groups are addressed by their start time, a catalog group stamped on the media timeline answers "which catalog applies at media time T". The old local-clock milliseconds answered nothing, and nobody reads them as a clock today. Nothing in the time-addressing quests needed to change.
  • Public API changes (the quest expected none). They were needed because the clock has to carry a runtime handle or an installed clock:
    • moq_mux::Clock is no longer Copy. Callers clone it.
    • moq_mux::hls::Producer::new(track, clock) takes the clock to stamp on. Only tests called it.
    • kio::clock::Clock is new, which is additive.
  • The stale claim's dead session had left uncommitted work in another worktree. This PR is that design, ported onto kio::clock, which replaced moq-clock in moq-dev/moq#7027, and reviewed afresh.

Impact

  • Breaking (!): moq-mux loses Clock: Copy, and hls::Producer::new changes signature. kio gains clock::Clock.
  • Wire: no format change. Hang and MSF catalog group timestamps and HLS record timestamps now carry broadcast-clock values, the same timeline as the media, instead of local monotonic milliseconds.
  • Behavior: in production, every unpaused runtime already shares the host clock, so the stamps only move onto the media timeline. A paused, manual, or simulated clock now stays one timeline wherever a broadcast is written from.

Validation

  • New kio tests (clock::captured): a paused runtime, an installed manual clock, the simulator, and an entered queue are each read the same from other runtimes and threads.
  • New moq-mux test a_broadcast_written_from_two_clock_contexts_never_rewinds: a broadcast is built under one paused runtime and written from a second paused runtime and from a plain thread. Media, hang, MSF, and HLS records all stay non-decreasing on the clock it was built under, and media group starts land exactly on it.
  • moq-ffi containers_of_one_source_keep_their_relative_media_time runs through the public FFI entry point.
  • cargo nextest for kio, moq-mux, moq-hls, moq-ffi, and moq-transcode with all features: 2710 passed. moq-video and moq-audio with capture: 550 passed.
  • cargo clippy --workspace --all-targets --all-features -D warnings (excluding fuzz), the capture-feature clippy, and kio clippy/check without features and with runtime, sim, and manual: clean.
  • cargo fmt --check, cargo doc -D warnings (kio, moq-mux), sh/rs/stats-docs.py, quest check --base origin/main: clean.

Review

  • Reviewer: Codex gpt-6.1-sol, medium effort, quest review --base origin/main, round 1.
  • Reviewed head: 956f7d32ada87cd63b947887c45cc96ecab64eb8, with 0 findings.
  • Since then, the only change is a conflict-free merge of origin/main that touches quest files only, giving head e4b93ae2a86bc2d37fed2936d5c09ce8d71cbb30. The simple-review rule did not apply, because the change touches public API.
  • Head e3850e01154a12aedbd5fb5ca1d719a0ac963c74 adds docs(mux): rewrap the broadcast clock paragraph, which rewraps one paragraph of doc/lib/rs/moq-mux.md per Pullfrog's thread. The local review is skipped for it under the simple-review rule: it touches docs only.

Suggested follow-ups

  • An HLS record pushed before anything fixes the broadcast clock keeps the pre-re-base clock. Stamping a record could fix the clock instead, the way a catalog snapshot on the wire already does.
  • kio::clock::Clock captured in an unpaused tokio runtime enters the runtime handle on every read. That is cheap but avoidable if kio can tell that a runtime is unpaused.

(Written by Claude Opus 5.5)

kixelated and others added 3 commits November 1, 2026 15:03
…under

A moq-mux broadcast stamped media on its Clock but the hang and MSF catalog
groups and the HLS records with the ambient Timestamp::now(), and the Clock
itself read the ambient clock. A call from another runtime or clock context
could then move a track's timestamps backwards.

kio::clock::Clock::current() captures the ambient clock (host, tokio runtime,
installed manual clock, or simulator run) and keeps reading it from anywhere.
moq_mux::Clock holds one, so it is Clone and no longer Copy. The catalog's hang
and MSF groups and the HLS records are stamped on the broadcast clock, so they
share the media timeline. hls::Producer::new takes the clock to stamp on.

The moq-ffi container test now writes through the exported entry point, which
enters the FFI worker, and checks the catalog lands on the media timeline.

Finishes quest/m1/broadcast-clock.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A broadcast stamps everything from one clock fix(mux)!: a broadcast stamps everything from one clock Nov 1, 2026
@kixelated
kixelated marked this pull request as ready for review November 1, 2026 23:24
@coderabbitai

coderabbitai Bot commented Nov 1, 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 47 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: 6baa8f5b-019e-4d7f-a837-d9a55e11a790
Commits

Reviewing files that changed from the base of the PR and between 88e6990 and e3850e0.

Files selected for processing: 22
  • doc/lib/rs/kio.md
  • doc/lib/rs/moq-mux.md
  • quest/m1/README.md
  • quest/m1/broadcast-clock.md
  • rs/kio/src/clock/captured.rs
  • rs/kio/src/clock/installed.rs
  • rs/kio/src/clock/mod.rs
  • rs/kio/src/clock/runtime.rs
  • rs/kio/src/clock/sim.rs
  • rs/moq-audio/src/encode/capture.rs
  • rs/moq-ffi/src/media.rs
  • rs/moq-hls/src/export/archive_tests.rs
  • rs/moq-hls/src/export/mod.rs
  • rs/moq-hls/src/server/routes.rs
  • rs/moq-mux/src/catalog/data.rs
  • rs/moq-mux/src/catalog/producer.rs
  • rs/moq-mux/src/clock.rs
  • rs/moq-mux/src/container/fmp4/import_test.rs
  • rs/moq-mux/src/container/producer.rs
  • rs/moq-mux/src/hls.rs
  • rs/moq-transcode/src/catalog.rs
  • rs/moq-video/src/encode/producer.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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

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: e4b93ae

No actionable bugs found in this review. Capturing the clock source once and using it for media, catalog and HLS stamps addresses the underlying cross-context mismatch more directly than clamping timestamps. Preserving that source through rebases and keeping already-stamped HLS tracks on their existing timeline are coherent with the monotonicity constraints; the documented pre-anchor HLS limitation remains explicit.

Verification: static review of the changes and relevant clock, catalog and HLS code, including the cross-context regression coverage. I did not run tests or builds, and have not independently verified the test results in the PR description.

(Written by OpenAI)

@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 critical issues — minor suggestions inline.

Reviewed changes

  • kio::clock::Clock — new ambient-clock capture (rs/kio/src/clock/captured.rs); Clock::current() picks the entered queue's installed clock, a sim run, an installed Manual, a tokio runtime, or the host, and now()/wall() read that source from any thread or runtime.
  • moq_mux::Clock — now holds a kio::clock::Clock and reads now/capture through it; arrival/placed/adopted became methods that preserve the captured source; no longer Copy.
  • Catalog stamps — Outputs::emit stamps the hang and MSF groups with state.clock.now() instead of ambient Timestamp::now(); adopt returns Option<Clock> and Guard::retime re-bases the HLS tracks while the catalog is unanchored.
  • HLS records — hls::Producer/hls::Tracks take the broadcast clock, and stamped freezes a track's clock after its first record so it never rewinds.
  • Tests — new kio capture tests (paused runtime, installed manual, simulator, entered queue), a_broadcast_written_from_two_clock_contexts_never_rewinds, and the moq-ffi test now writes through the exported producer and asserts the catalog lands on the broadcast clock.
  • Docs/quest — doc/lib/rs/kio.md, doc/lib/rs/moq-mux.md, quest/m1/README.md; deletes quest/m1/broadcast-clock.md.

ℹ️ doc/concept/hang.md still says to pass the same moq_mux::Clock

moq_mux::Clock is no longer Copy, so doc/concept/hang.md:76's "pass the same moq_mux::Clock to each catalog::Config::with_clock" now needs a clone per config. The PR updated doc/lib/rs/kio.md and doc/lib/rs/moq-mux.md but not this line.

Technical details
# Stale `Copy` instruction in doc/concept/hang.md

## Affected sites
- `doc/concept/hang.md:76` — "In one Rust publisher, pass the same `moq_mux::Clock` to each `catalog::Config::with_clock`".

## Required outcome
- State that each config takes a clone (clones still share one timeline), so a reader following the sentence compiles against the non-`Copy` `Clock`.

## Suggested approach
- Replace "pass the same `moq_mux::Clock`" with "pass a clone of one `moq_mux::Clock`" or equivalent.

ℹ️ An entered Deadlines queue is captured as the host clock, not the queue

For an entered queue with no installed or simulated clock, Clock::current() returns Source::Host while now() returns the queue's instant unconditionally. This is intentional per the elaborated doc sentence ("the installed or simulated one, else the host's") and matches the io_uring worker, whose queue is advanced from the host clock; flagging only that the first doc line ("in the order now picks it") overclaims for a queue advanced from some other clock.

Technical details
# Entered queue captures Host, not the queue

## Affected sites
- `rs/kio/src/clock/captured.rs:44-46` — `if entered { return Self(Source::Host); }`.
- `rs/kio/src/clock/mod.rs:68-71` — `now()` returns the entered queue's instant first.

## Detail
- For an entered queue with no installed/simulated clock, `Clock::current()` yields `Source::Host` while `now()` reads the queue's last `advance` value. They agree to within sampling latency for a queue advanced from the host clock (the in-repo `moq-uring` worker), and diverge for a queue advanced from another clock. `Deadlines::enter` and `Clock` are both public, so this is reachable by a direct `kio` user.

## Open question for the human
- Is an entered queue always a host/installed/sim view, or should `current()` capture or refuse an unsupported queue?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread doc/lib/rs/moq-mux.md Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 23:49
@moq-bot
moq-bot Bot merged commit d8288e6 into main Nov 2, 2026
1 check passed
@moq-bot
moq-bot Bot deleted the quest/m1/broadcast-clock branch November 2, 2026 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant