Repository navigation
Conversation
#5082 made catalog::Producer::anchor private and moved importers onto a per-import Timebase, while #5003 added the TS passthrough importer on the old anchor, so main stopped compiling. Passthrough now takes a timebase from its reservation and shifts every object, like the other importers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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 11 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
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 |
|
Automated review of #5118 at Small, correct port of the TS passthrough importer onto #5082's per-import Blocking: none. Non-blocking
Verdict: MERGE (once CI is green, choosing this or #5117) This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: e996aaa
No actionable correctness or security defects found in the diff and relevant surrounding code. The direction is sound: passthrough.rs:299,610–612 reuses the reservation’s existing Timebase, fixes the obsolete private anchor call, and anchors before the reservation is released. Applying one constant offset after the PCR monotonicity guard preserves ordering and intervals; public API, framing, and TS payload bytes stay unchanged.
Non-blocking verification gap: rs/moq-mux/src/container/ts/passthrough_test.rs:64–68 constructs a fresh catalog, so these tests do not exercise the newly relevant nonzero offset. Building on the existing overlap note, retain #5117’s an_import_shifts_onto_a_clock_already_in_use regression if this implementation is selected; it checks a fixed catalog clock, constant offset across every object, and a flagged jump.
Verification limits: GitHub source review only; I did not run builds/tests or independently reproduce the reported 1,180-test pass. Check/Test and platform CI are still running on this SHA. State, head, and existing reviews were rechecked before posting.
|
Closing as a duplicate of #5117, which merged the same Timebase shift with a regression test ( (Written by Claude Opus 5.5) |
Problem
maindoesn't compile. #5082 madecatalog::Producer::anchorpub(super)and moved every importer onto a per-importcatalog::Timebase. #5003 landed the TS passthrough importer on the oldself.catalog.anchor(timestamp)call, somoq-muxfails withE0624: method anchor is private(container/ts/passthrough.rs:622). Every CI job on PRs that mergemainfails, for example #4039.Fix
Passthrough takes a timebase from its reservation (
reserved.timebase()) and shifts every object's timestamp through it, the same port #5082 made for FLV, fMP4, and MKV. The first object always starts a group, so it anchors the import before the reservation publishes, as before.Impact
No public API change and no wire change. Passthrough timestamps now follow #5082's timebase rules, verbatim when the import places the catalog clock, like the other importers.
Verification
cargo test -p moq-mux --lib(1180 passed) andjust check.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code