fix(moq-mux): importers publish the catalog at their first frame

[kixelated]

Completes the quest/m1/import-first-frame quest.

Summary

The fMP4, MKV, and MPEG-TS importers now hold their initial catalog reservation until the first frame anchors the clock. FLV already did this. Before, they published the catalog at the init segment (moov, Tracks, or the first PMT) on a provisional clock and re-anchored it at the first frame. Readers that copy the clock once, such as moq-hls export's EXT-X-PROGRAM-DATE-TIME, kept the stale value.

• fMP4: the reservation is released at the first fragment. A duplicate moov is now detected from the parsed moov, not from the reservation being gone.
• MKV: the reservation is released at the first block.
• MPEG-TS: the reservation is released at the first PES with a PTS, including a clock-only video PES. This covers Opus-only streams, whose config comes from the PMT, and programs made only of verbatim PIDs.
• All four importers: finish() releases the reservation, so input with no frames still publishes its declared tracks. An init that declares no track releases it right away.
• PMT changes: a PMT version change before the first PES does not touch the held handle. Each stream reserves a fresh Reserved from the catalog, so a stream added before the first PES joins the initial snapshot.

The quest is deleted, and its references in other quests are updated.

Tests

Each one fails on main except the FLV guard:

• fmp4, mkv, flv first_catalog_carries_the_anchored_clock: nothing publishes at the init, and every snapshot carries the anchored clock. The FLV test passes on main and guards against regressions.
• ts::import_test::opus_catalog_carries_the_anchored_clock: an Opus-only stream an hour into its PTS.
• ts::import::test::verbatim_only_program_publishes_at_the_first_pes
• fmp4::import_test::a_second_moov_is_refused and a_moov_with_nothing_selected_publishes
• fmp4::import_test::a_moov_after_finish_publishes: a moov decoded after finish() released the reservation declares its tracks on a fresh reservation instead of panicking.
• moq-hls fmp4_import_times_playlists_by_the_anchored_clock: end to end. The viewer watches before the first fragment, and EXT-X-PROGRAM-DATE-TIME matches the anchored clock.
• The moq-srt multi-program tests fed PSI only. They now add a PES per program, since a PMT alone no longer publishes.

just check passes.

Public API / wire

• Public API: no new items. A catalog fed by an fMP4, MKV, or MPEG-TS importer is first published at the first frame instead of the init segment. A reader waiting for the catalog now waits for media. This is noted in doc/setup/upgrade.md under Unreleased.
• Wire: none. The hang draft already says the mapping is fixed for the broadcast.

Decisions

• An MPEG-TS program whose only streams are section-framed (SCTE-35 or private sections with no PES) does not anchor on the PCR. Its catalog appears at finish() until a real stream needs otherwise. This was not seen in practice. A program whose only PES streams are verbatim private data carrying no PTS behaves the same way, since nothing in it anchors the clock.

Follow-ups

• Shared import clock still owns the case where a container joins after an earlier publish.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Status: this was resumed from an interrupted run and rebased onto main. just check passes. The new fMP4, MKV, Opus-TS, verbatim-TS, and moq-hls tests fail on the old importer code. The moq-srt multi-program tests now feed one PES per program, because a PMT alone no longer publishes the catalog.

Open question: should a program with only section-framed streams (no PES) anchor on its PCR? Right now it publishes only at finish(). I recommend leaving it until a real stream needs it.

(Written by Claude Opus 5.5)

[coderabbitai]

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: 21e20be0-07a3-4c97-ad6a-dc7054b495eb

📥 Commits

Reviewing files that changed from the base of the PR and between da3f068 and 3b8ad8d.

📒 Files selected for processing (21)

• doc/lib/rs/moq-mux.md
• doc/setup/upgrade.md
• quest/m1/README.md
• quest/m1/hls-first-catalog.md
• quest/m1/hop-aligned-import.md
• quest/m1/import-first-frame.md
• quest/m1/shared-clock.md
• quest/m2/rtsp-import.md
• rs/moq-hls/src/export/mod.rs
• rs/moq-mux/src/catalog/producer.rs
• rs/moq-mux/src/container/flv/import.rs
• rs/moq-mux/src/container/flv/import_test.rs
• rs/moq-mux/src/container/fmp4/import.rs
• rs/moq-mux/src/container/fmp4/import_test.rs
• rs/moq-mux/src/container/fmp4/mod.rs
• rs/moq-mux/src/container/mkv/import.rs
• rs/moq-mux/src/container/mkv/import_test.rs
• rs/moq-mux/src/container/test_util.rs
• rs/moq-mux/src/container/ts/import.rs
• rs/moq-mux/src/container/ts/import_test.rs
• rs/moq-srt/src/ts.rs

💤 Files with no reviewable changes (3)

• quest/m1/README.md
• quest/m2/rtsp-import.md
• quest/m1/import-first-frame.md

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

---

Walkthrough

Container importers now withhold catalog publication until a media frame anchors the clock. They release the reservation after anchoring, or during finish if no frame arrives. Tests cover publication timing for FLV, fMP4, MKV, and MPEG-TS, plus playlist timestamp mapping and MPEG-TS fixtures. Documentation and quest records now describe the updated timing.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3b8ad

The catalog timing change has no identified merge-blocking issue. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3b8ad

The change improves clock consistency, and the reviewed paths do not establish a new security defect. Some uncertainty remains around interrupted imports and failure recovery when several inputs share one broadcast.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated propagation scope is the caller-assigned shared catalog and its downstream readers. HLS can place multiple importer generations on that catalog, so withholding by one unresolved importer can delay initial discovery for the shared broadcast. Deployment-wide or cross-tenant exposure is not established.

Trust Boundaries and Controls

• observed — The inspected SRT path feeds bytes from an accepted socket into the existing publisher, which creates the catalog using a caller-provided origin and path. This PR changes publication timing rather than introducing a new identity or privilege transition in that path.

Resilience and Maintainability Implications

• observed — SRT receive or demux failure explicitly aborts the importer and closes the broadcast. Reservation drop can still flush pending catalog state, while HLS releases failed importer generations through eviction. These cleanup paths do not prove that every interrupted import publishes an anchored clock.

Hardening Proposals

• proposed — Specify and validate the externally visible catalog state for malformed first payloads and pre-anchor cancellation in a shared broadcast. Explicit terminal-state expectations would distinguish deliberate fallback publication from failure-containment regressions.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 13 files. (5 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: importers publish the catalog at the first frame.
Description check	✅ Passed	The description explains the importer changes, their reason, related tests, and documentation updates.

Full details: Docstring Coverage

Explanation

Docstring coverage is 78.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 13 files. (5 skipped: 5 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 595eac1

No actionable correctness findings in the full 21-file diff and catalog/TS reservation context. The direction addresses the copy-once clock bug: fMP4 (rs/moq-mux/src/container/fmp4/import.rs:916-924), MKV (rs/moq-mux/src/container/mkv/import.rs:434-442), and TS (rs/moq-mux/src/container/ts/import.rs:740-747) anchor before releasing the initial catalog hold. Duplicate-moov detection is appropriately independent of reservation lifetime, and the tests inspect actual published snapshots, including the HLS consumer. The section-only TS program's finish-time publication and an importer joining an already-published catalog remain explicit scope limitations, already discussed, rather than new findings.

Verification limits: static review only; I did not run builds or tests or reproduce the reported negative controls. GitHub currently reports merge conflicts; this is not confirmation of merge readiness.

[kixelated]

Review of the PR diff after merging origin/main (#4883, #4894). This is an adversarial Codex review plus a manual read, posted because CodeRabbit is rate limited.

Merge: one conflict, in doc/setup/upgrade.md under Unreleased. Both entries are kept: this PR's "importers publish the catalog at their first frame" and #4883's "fMP4 export fixes its track set at the init segment". doc/lib/rs/moq-mux.md and doc/bin/cli.md merged cleanly.

Findings

1. Codex [high]: TS programs that never carry a PES PTS hold their catalog until finish(). This covers section-only programs, which match the accepted decision in the description, and verbatim private PES with no PTS, which is the same case: nothing in the program anchors the clock. On a live SRT ingest, such a program's catalog never publishes before EOF. Because Program::All gives each program its own broadcast, other programs are not blocked. Handling: kept as the accepted decision, and the description now names the PTS-less verbatim case too. If a real stream needs it, the cheapest follow-up is to release the reservation at a PMT that declares no PES stream, since nothing can anchor there and the provisional clock is final.
2. Manual: finish() followed by a moov panicked. init called .expect() on the initial reservation, but finish() now drops it. On main, the same call sequence returned an error instead. Fixed in 3b8ad8d: init uses a fresh catalog.reserve() when the initial one is gone. The regression test a_moov_after_finish_publishes panics without the fix.

Nothing else came up. The release points are right in each importer: they anchor before they drop the reservation, a declaration with no tracks releases right away, and finish() releases on every importer. For the duplicate-moov check, self.moov replaces the reservation as the signal.

Checks: just check passes. just test interop --all passes 32/32.

(Written by Claude Opus 5.5)

[kixelated]

Automated review: head 3b8ad8d1

This moves the fMP4, MKV, and MPEG-TS importers' initial-reservation release from the init segment to the first anchored frame, matching FLV. Each frame path now anchors before it releases, finish() and track-less inits release right away, and fMP4's duplicate-moov check now keys on the parsed moov. I traced all four importers and found no blocking issues. CI on the previous head 595eac1a was green, and CI on this head is still running.

Non-blocking

1. A drop or abort before the first frame still publishes once on the provisional clock. In fmp4/import.rs, initial_reservation is declared before tracks. So when an import is dropped after its moov but before any fragment (say, an ingest that disconnects), the reservation is released first. That publishes a snapshot that lists every track on the provisional clock, and only then are the tracks retired. abort() has the same problem in reverse: it retires the tracks, then self drops the reservation and publishes an empty snapshot on that clock. MKV is the same. This isn't a regression, since main already published at the moov. Still, it's the one remaining path where a copy-once reader like moq-hls export can see a provisional clock. Possible fixes are to drop tracks before the reservation in abort and in a Drop impl, or to give Reserved a way to cancel without publishing.
2. The hold now depends on a frame from a selected track. fMP4 extract skips trafs of tracks dropped by select before it calls anchor, and MKV handle_block returns early for unknown or skipped tracks. Take an input whose kept tracks are silent for a while, like --no-video on a source that sends video first or an audio track that starts late. Its catalog now stays unpublished until a kept track delivers, where before it appeared at the init. This is probably the right behavior, but it's worth one line in the upgrade.md entry ("waits for media of a selected track"). A test with with_select that drops the first-arriving track would also pin it down.
3. a_moov_after_finish_publishes makes "declare tracks after finish()" a supported behavior. Tracks created by a moov after finish() are never finished, so they stay open until the importer drops. Matching main instead of panicking is fine. If nothing relies on this, though, returning an error for a moov after finish() would be stricter, and it would make the fresh-reserve() fallback in init unnecessary.
4. Small test gaps. The MKV test checks that video.renditions has length 1 but never checks that the Opus track is there. In the moq-hls test, let _ = throws away both the next_event and the broadcaster.ready() timeouts. If a regression meant nothing published until finish, the test would fail at the playlist timeout with a less obvious message.
5. Nit: the edited sentence in quest/m1/hop-aligned-import.md (around line 85) wasn't rewrapped and is now one very long line.

Verdict: MERGE (once CI on 3b8ad8d1 is green)

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