feat(moq-mux): own the TS demux and reassemble PSI across packets

[t0ms]

Implements the TS PSI reassembly quest.

Problem

The TS importer read the PAT and PMT through mpeg2ts's TsPacketReader, which parses PSI from one packet and rejects a nonzero pointer_field, and its errors end the import. A PMT longer than one packet, a PAT with more than about 40 programs, or one flipped CRC byte on a PAT repetition stopped moq import ts.

Approach

• The importer owns the packet demux; import no longer uses TsPacketReader. The Feed mutex and the pmt_pids/streams gate go with it.
• PID 0 and each PMT PID go through the existing SectionReassembler. The PAT (every section of a version), the PMT, and the PES header are parsed in a new ts/psi.rs.
• Each PAT and PMT section's CRC-32/MPEG-2 is checked with the crc crate. A bad section is dropped whole and the last good table stays. Drops are counted in a new Stats::crc_error, which ts::stats::Log reports.
• ts::Programs finds the PAT through the same path and hands each importer the whole section.
• A malformed adaptation field on a section PID drops the partial section, like a continuity gap.

Impact

• decode on kyrion_mpeg2av_ac3.ts (release, 1316-byte chunks, best of 200 imports, three runs each): main 876–894 MB/s, this branch 1780–1815 MB/s.
• test/ts/run.sh default, --open-gop, and --source with an 11 Mb/s broadcast capture pass with the same hard-check results as main.
• An ffmpeg clip with one bit flipped in its 20th PAT's CRC: on main the publisher exits after 0.5 s (CRC32 mismatch); this branch drops one section and every check passes.
• The same clip with its PMT padded to two packets: main exits at the first PMT; this branch imports to the end.
• Stricter-than-spec checks go away with the reader: a stream_type mpeg2ts has no name for is carried verbatim, PES headers with ES rate, trick mode, copy info, CRC, or extension fields are accepted, and a bounded PES with header stuffing gets its true length. A payload that isn't a PES header still ends the import.
• Stats gains crc_error (#[non_exhaustive], so additive); is_empty counts it.

Alternatives

Covered in the quest: working around the reader from outside, feeding it synthesized one-packet PMTs, fixing sile/mpeg2ts upstream, or moving to mpeg2ts-reader.

Follow-ups

• Export writes the PMT through mpeg2ts, which can't emit a section longer than one packet, so a long PMT now imports but doesn't round-trip.
• Sections dropped for other reasons (malformed adaptation field, parse failure) are left for PAT_error/PMT_error in TS import health.

[coderabbitai]

Warning

Review limit reached

You'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 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1a43a4c2-db23-4f78-9a5c-b5b49d22c83d

📥 Commits

Reviewing files that changed from the base of the PR and between 50f5459 and 0d62369.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• rs/moq-mux/Cargo.toml
• rs/moq-mux/src/container/ts/import.rs
• rs/moq-mux/src/container/ts/mod.rs
• rs/moq-mux/src/container/ts/programs.rs
• rs/moq-mux/src/container/ts/psi.rs
• rs/moq-mux/src/container/ts/stats.rs

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 136f613

Three P2 findings in rs/moq-mux/src/container/ts/programs.rs:

• L179-L185: Preserve every section of the startup PAT. For section 0 and section 1 in separate PUSI packets, PatScan assembles both but resets opened to section 1; L193 discards section 0 before feeding the fresh importers. They cannot complete their own PAT and ignore the following PMTs/media until a repetition. A finite capture with only that PAT never publishes. Seed the importers with the assembled table or replay all its sections, and add a multi-section Programs test; the new two-packet test uses only one section.
• L196-L199: Bound retained startup data. Once a PAT section is partial, opened keeps every later multiplex packet in pending. If its continuation and later PID-0 packets never arrive, otherwise ongoing traffic grows this buffer indefinitely. Retain bounded PSI data rather than the entire intervening multiplex, and test a partial PAT followed by sustained non-PAT traffic.
• L120-L122: Avoid counting replayed CRC failures twice. A packet containing a corrupt PAT section followed by a valid one increments scan.crc_error, then is replayed unchanged into each importer. Their maximum already includes that same failure, so adding the scanner count reports 2 for one dropped section. Exclude overlapping replay counts and test both sections packed into one packet.

Direction: owning the demux and reusing SectionReassembler fits the quest and removes the reader workarounds; keep that approach, but repair the scanner-to-importer handoff.

Verification: reviewed all seven changed files and surrounding demux/parser context. Synthetic Python byte-fixture traces reproduced these bookkeeping outcomes; they are not Rust integration tests. Head CI, including Check/Test, succeeded. Rust/Nix are unavailable locally, so I did not run repository tests or reproduce the throughput claims.

(Written by OpenAI)

[kixelated]

Review: feat(moq-mux): own the TS demux and reassemble PSI across packets

Head: 136f613749b2d9967b2cd7b405b80f99b8b5ea77
Mergeability: MERGEABLE / clean. Required checks green (Check, Test, Platform, WASM, OBS, Swift, Android).
Scope: Drop TsPacketReader/Feed; own framer + SectionReassembler for PAT/PMT; new psi.rs (CRC-32/MPEG-2, PAT assembler, PMT/PES parse); Stats::crc_error; Programs uses the same PAT path.

Findings

No blocking defects found. Bounds checks look careful (section_length > 4093 drop, malformed adaptation clears partial, pointer_field past payload resyncs). Tests cover multi-packet PMT, nonzero pointer_field, 50-program PAT, corrupt PAT/PMT between good repetitions, and Programs startup on a spanning PAT.

1. Non-blocking — multi-section PMT still refused (psi.rs Pmt::parse requires number == last == 0)
   Spec-rare; the quest’s pain was multi-packet single-section PMTs, which this fixes. A true multi-section PMT would still not map streams (silent None after a good CRC). Mentioned in spirit by the export follow-up (long PMT cannot round-trip); worth a log line if it ever shows up in the wild.

2. Non-blocking — CRC-valid but structurally odd PAT body (PatAssembler::section)
   After CRC passes, a body that is not a multiple of 4 returns None with no crc_error bump. Harmless for well-formed feeds; only matters for pathological generators.

3. Note — additive Stats::crc_error
#[non_exhaustive] keeps this API-safe. Cross-check when stacking with feat(mux): report each TS elementary stream's access units at export #4577: export Stats snapshots leave crc_error at 0 (Default), which is correct.

Cross-PR

Import-side; small overlap with #4577 (Liveness visibility / shared Stats). #4579 is export schedule — orthogonal. Landing this first is the least conflict-prone of the three mux PRs.

Verdict

MERGE at 136f613749b2d9967b2cd7b405b80f99b8b5ea77 — clear win (multi-packet PSI, CRC resilience, large demux speedup) with CI green.

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

[kixelated]

Addressed the three OpenAI P2 findings in programs.rs with one change: PatScan no longer replays held bytes into the fresh importers. It returns the assembled psi::Pat, each importer is seeded with it via handle_pat, and only bytes after the packet that completed the PAT are fed.

• Multi-section startup PAT: importers get the whole table, not just the completing section. Test: a_pat_in_two_sections_starts_every_program.
• Unbounded retention: the scanner keeps the partial PAT in its own reassembler, so pending holds only a trailing partial packet. Test: a_partial_pat_does_not_hold_the_multiplex.
• Double-counted CRC failures: importers never see the scanner's packets, so the scanner and importer counts are disjoint. Test: a_corrupt_pat_beside_a_good_one_counts_once.

All three tests fail against the previous scanner. Also merged main (Opus opus_config conflict, adapted to catalog::Descriptor) and dropped the now-unused PatReader::in_progress. Tradeoff: PMTs between the PAT's first and completing packet are dropped until their next repetition, consistent with "input ahead of that PAT is dropped".

No public API or wire change.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 0d62369

The three original cases in the previous review are addressed by the new handoff and targeted tests. One new P2 remains:

• Preserve the PAT reader state at handoff — programs.rs:86–88. handle_pat seeds the table, but each importer's PatReader starts empty. The packet completing the initial PAT can also begin the next section; that partial section remains in the abandoned scanner. For a complete 16-byte PAT followed by the first 167 bytes of a longer next-version PAT, the importer ignores the following non-PUSI continuation. If the new table moves the selected program's PMT PID, its PMT and media are dropped until another PAT repetition. Preserve the bounded reassembly/continuity state without replaying already-counted bytes, and add a packed, spanning PAT-transition regression test.

Direction: keep the owned demux and table-seeding approach; the remaining gap is the parser-state handoff.

Verification: inspected the fix, regression tests, surrounding parser/framer code, and main-merge resolution. The Opus resolution matches the current base apart from the descriptor type. A narrow Python translation with byte fixtures reproduces this edge; this did not execute the Rust implementation. Rust/Nix are unavailable here, and head Check/Test are still pending.

(Written by OpenAI)