Repository navigation
fix(mux)!: a broadcast stamps everything from one clock - #7218
Conversation
Quest-Claim: 4c9bf704a218d58f
…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>
|
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 47 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
left a comment
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
ℹ️ 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 installedManual, a tokio runtime, or the host, andnow()/wall()read that source from any thread or runtime.moq_mux::Clock— now holds akio::clock::Clockand readsnow/capturethrough it;arrival/placed/adoptedbecame methods that preserve the captured source; no longerCopy.- Catalog stamps —
Outputs::emitstamps the hang and MSF groups withstate.clock.now()instead of ambientTimestamp::now();adoptreturnsOption<Clock>andGuard::retimere-bases the HLS tracks while the catalog is unanchored. - HLS records —
hls::Producer/hls::Trackstake the broadcast clock, andstampedfreezes 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; deletesquest/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?deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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 ambientTimestamp::now()instead. TheClockalso 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 withTimestampMismatch. moq-ffi'scontainers_of_one_source_keep_their_relative_media_timehit 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 ordernow()checks it: an enteredDeadlinesqueue, a simulator run, an installedManualclock, a tokio runtime handle, or the host clock.now()andwall()keep reading that clock from any thread or runtime. Equality means the same clock.moq_mux::Clockholds one, captured atClock::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 isClone, no longerCopy.Outputs::emitstamps the hang and MSF groups with the broadcast clock'snow().hls::Producerandhls::Trackstake 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.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):
moq_mux::Clockis no longerCopy. Callers clone it.moq_mux::hls::Producer::new(track, clock)takes the clock to stamp on. Only tests called it.kio::clock::Clockis new, which is additive.kio::clock, which replaced moq-clock in moq-dev/moq#7027, and reviewed afresh.Impact
!): moq-mux losesClock: Copy, andhls::Producer::newchanges signature. kio gainsclock::Clock.Validation
kiotests (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.moq-muxtesta_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-fficontainers_of_one_source_keep_their_relative_media_timeruns through the public FFI entry point.cargo nextestfor 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 withruntime,sim, andmanual: clean.cargo fmt --check,cargo doc -D warnings(kio, moq-mux),sh/rs/stats-docs.py,quest check --base origin/main: clean.Review
gpt-6.1-sol, medium effort,quest review --base origin/main, round 1.956f7d32ada87cd63b947887c45cc96ecab64eb8, with 0 findings.origin/mainthat touches quest files only, giving heade4b93ae2a86bc2d37fed2936d5c09ce8d71cbb30. The simple-review rule did not apply, because the change touches public API.e3850e01154a12aedbd5fb5ca1d719a0ac963c74addsdocs(mux): rewrap the broadcast clock paragraph, which rewraps one paragraph ofdoc/lib/rs/moq-mux.mdper Pullfrog's thread. The local review is skipped for it under the simple-review rule: it touches docs only.Suggested follow-ups
kio::clock::Clockcaptured 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)