Skip to content

feat(mux): export clock::Anchor and clock::Lane - #4667

Merged
kixelated merged 3 commits into
moq-dev:mainfrom
Dryvnt:quest/m1/clock-anchor-public
Oct 1, 2026
Merged

kixelated merged 3 commits into
moq-dev:mainfrom
Dryvnt:quest/m1/clock-anchor-public

Conversation

@Dryvnt

@Dryvnt Dryvnt commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Makes moq_mux::clock::{Anchor, Lane} public, so an application running its own demuxer can put several tracks of one source on the broadcast Clock with one shared offset, as the built-in TS, fMP4, and FLV importers do.

One SourceMap per track lets tracks drift apart by their first-PTS difference, and one shared SourceMap reads interleaving (beyond MAX_REORDER) as a reset. Anchor plus one Lane per track is what the importers already use internally.

Public API

Additive, so it targets main.

  • pub mod clock (was private). The root re-exports of Clock and SourceMap stay.
  • clock::Anchor: new(Clock), translate(&mut Lane, Timestamp), translate_at(&mut Lane, Timestamp, u64), extend(Timestamp). Derives Debug.
  • clock::Lane: Default, restart(). Derives Debug.

No wire impact.

Changes

  • rs/moq-mux/src/clock.rs: visibility, docs, and a two-track doctest on Anchor.
  • rs/moq-mux/src/lib.rs: pub mod clock and a crate-doc bullet.
  • doc/lib/rs/moq-mux.md: when to use Anchor/Lane over SourceMap, with a two-track example.
  • Removes the quest file.

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

Dryvnt and others added 2 commits October 1, 2026 10:39
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>
@Dryvnt

Dryvnt commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Outcome: implemented as scoped. Anchor/Lane exported under moq_mux::clock (maintainer-chosen over root re-exports), documented with a two-track doctest and a section in doc/lib/rs/moq-mux.md. just check passes. Opened from a fork because the author lacks push access, so the quest claim lives on Dryvnt/moq rather than origin. No open decisions. /quest/m1/source-map-removal.md is now unblocked.

(Written by Claude Opus 5.5)

@Dryvnt
Dryvnt marked this pull request as ready for review October 1, 2026 09:18

@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: 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.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 44945fe0-be92-48d0-9f89-89fa5e232c1f

📥 Commits

Reviewing files that changed from the base of the PR and between 7608fe5 and f04f743.

📒 Files selected for processing (1)
  • quest/m1/README.md
💤 Files with no reviewable changes (1)
  • quest/m1/README.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.


Walkthrough

The clock module and its Anchor and Lane types and methods are now public. The crate documentation and examples describe translating timestamps from multiple tracks through one shared Anchor with a separate Lane per track. The library guide adds restart-handling guidance. The related public clock anchor quest and planning references are removed.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to f04f7

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 Review

Security architecture risk: 🔵 Low · up to 7608f

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Timestamp-driven failures can affect tracks sharing the caller’s Anchor. The inspected API provides no path from that timing state to another tenant, service, credential, or data store; downstream application exposure remains unassessed.

Trust Boundaries and Controls

  • observed — The new methods retain exclusive mutable borrowing of translator state. Existing public Clock and SourceMap constructors already allowed applications to create and translate their own timelines; the export adds shared-track coordination rather than access to a privileged resource.

Resilience and Maintainability Implications

  • observed — Generation checks prevent lanes from applying the same shared restart twice. Source tests assert shared timestamp spacing and repeated restart behavior, but the inspected error tests do not establish recovery after failure.

Hardening Proposals

  • proposed — Define whether failed translations preserve state or require rebuilding the Anchor and its lanes, and clarify that extended ends must belong to the same broadcast timeline. Targeted recovery checks would help external callers maintain shared-source failure containment.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: exporting clock::Anchor and clock::Lane from the mux crate.
Description check ✅ Passed The description accurately explains the public API, intended use, documentation updates, quest-file removal, and lack of wire impact.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

Comment @coderabbitai help to get the list of available commands.

@kixelated

kixelated commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Review (head 7608fe5ae3fa1e3164d9b8f076b7143372c439da)

Additive export of the multi-track clock mapping the TS/fMP4/FLV importers already use. pub mod clock with Anchor/Lane (root Clock/SourceMap re-exports kept), docs + two-track doctest, quest file closed. No wire impact; fields stay private; reanchor/extend_micros stay private.

Issues

No concrete bugs found in the export itself. Existing muxed_lanes_share_one_mapping / muxed_restart_reanchors_once tests already cover the shared-offset and single-reanchor behavior the doctest demonstrates, and the public method set matches importer usage (new, translate/translate_at, extend, Lane::restart + Default).

CI (Check red, not introduced here): Check fails on pre-existing quest lint for quest/m1/admission-bench.md and quest/m1/cluster-shims.md (plain-text Required lines that are not quest docs). This PR only deletes clock-anchor-public.md / clears its README + source-map-removal dependency; it does not touch those two files. Open #4666 (fix/quest-outside-conditions) is the fix. Test, Android, Windows, and macOS are green on this head.

Verdict

MERGE — clean additive API for a real external need; merge once Check is unblocked by #4666 (or equivalent) on main.

Reviewed head: 7608fe5ae3fa1e3164d9b8f076b7143372c439da

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

@kixelated

Copy link
Copy Markdown
Collaborator

Merge summary

  • Merged main in (f04f743) to pick up quest: link the two outside conditions main rejects #4666, which fixes the pre-existing quest lint that failed Check. No conflicts.
  • API reviewed: only clock::Anchor and clock::Lane are newly reachable; every exported item has a one-line doc. Clock/SourceMap are now also reachable as moq_mux::clock::* alongside the existing root re-exports, which is fine.
  • No review findings to address. Locally: quest check, the Anchor doctest, clippy -D warnings, and cargo doc pass for moq-mux.
  • Follow-up: /quest/m1/source-map-removal.md is now unblocked.

Enabling auto-merge on f04f743bbb7fd7d8167fec5d70e67d92c8dc35f8.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 1, 2026 21:08
@kixelated
kixelated merged commit 85971a7 into moq-dev:main Oct 1, 2026
6 checks passed
@moq-bot moq-bot Bot mentioned this pull request Oct 1, 2026
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