feat(mux): export clock::Anchor and clock::Lane - #4667
Conversation
An application running its own demuxer can now put several tracks of one source on the broadcast clock with one shared offset, as the TS, fMP4, and FLV importers do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: implemented as scoped. (Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 7608fe5.
No actionable bugs found in the current diff. The direction is appropriately small and additive: exposing clock::{Anchor, Lane} reuses the existing per-source/per-track mapping without changing its algorithm or removing the root Clock/SourceMap exports. The two-track example and explicit-arrival documentation support the intended external-demuxer use case without adding another abstraction.
Verification: inspected all six changed files, the clock implementation and existing tests, and TS/fMP4/FLV call sites. I did not run tests or the new doctest locally. CI is not fully green: the Check job fails on quest prerequisite formatting at quest/m1/admission-bench.md:23 and quest/m1/cluster-shims.md:21; both offending entries are already present in base commit eb3e971 and are unchanged by this PR.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughThe Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The public clock translators and usage guidance are consistent with the implementation. No actionable merge risk remains; merging is appropriate after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new API operates on application-owned timing state and does not demonstrate expanded privileges or cross-service access. Risk is bounded, but recovery after failed translation and the validity of caller-supplied published-end timestamps are not fully established for external consumers. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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 |
Review (head
|
|
Merge summary
Enabling auto-merge on (Written by Claude Opus 5.5) |
Makes
moq_mux::clock::{Anchor, Lane}public, so an application running its own demuxer can put several tracks of one source on the broadcastClockwith one shared offset, as the built-in TS, fMP4, and FLV importers do.One
SourceMapper track lets tracks drift apart by their first-PTS difference, and one sharedSourceMapreads interleaving (beyondMAX_REORDER) as a reset.Anchorplus oneLaneper track is what the importers already use internally.Public API
Additive, so it targets
main.pub mod clock(was private). The root re-exports ofClockandSourceMapstay.clock::Anchor:new(Clock),translate(&mut Lane, Timestamp),translate_at(&mut Lane, Timestamp, u64),extend(Timestamp). DerivesDebug.clock::Lane:Default,restart(). DerivesDebug.No wire impact.
Changes
rs/moq-mux/src/clock.rs: visibility, docs, and a two-track doctest onAnchor.rs/moq-mux/src/lib.rs:pub mod clockand a crate-doc bullet.doc/lib/rs/moq-mux.md: when to useAnchor/LaneoverSourceMap, with a two-track example.Export path picked as
moq_mux::clock::{Anchor, Lane}(namespaced short names) over root re-exports.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code