Skip to content

M.4 T1: MMTP-over-MoQ — Mapping-B publisher, catalog conformance, transport subgroups, live Shaka E2E - #20

Merged
kkroo merged 50 commits into
mainfrom
blo-4020-m4-t1
Jun 13, 2026
Merged

M.4 T1: MMTP-over-MoQ — Mapping-B publisher, catalog conformance, transport subgroups, live Shaka E2E#20
kkroo merged 50 commits into
mainfrom
blo-4020-m4-t1

Conversation

@kkroo

@kkroo kkroo commented Jun 13, 2026

Copy link
Copy Markdown

What this collapses

Single integration PR for the M.1 → M.4 T1 MMTP-over-MoQ epic, opened directly to main to supersede the incremental stack (blo-4020-m1blo-4020-m4-t1, ex-PRs #1/#6). The branch already fully contains main (merge-base = main HEAD), so it merges clean — no rebase, no conflicts.

48 commits · 65 files · +12,720 / −59 (33 of the 65 are .planning/ handoff docs).

Code (subsystem-grouped)

moq-pub-mmtp — the Mapping-B publisher

  • MMTP publisher on IETF moq-transport draft-14+ (3101aca); multicast UDP listener (526f0e9); raw per-track verification smoke (5e7310c).
  • Mapping B: subgroup-per-MFU dispatch keyed by the per-sample MMTP timestamp (a348ea3) — survives first-fragment loss because the timestamp rides every fragment, unlike the MFU DU sample_number (first-fragment only). Hard-errors on aggregated MPUs.
  • Subgroup history window wired config-or-throw (47ac1b3); real moq_mmt capture replayed through dispatch with a frozen (group,subgroup,priority) oracle (ea1a5b5, 5174596).
  • Parity-vector generators: MMTP packet (15cae33), MFU reassembler (97ceba1), MPU-sequence vectors (80c24cc).
  • Vendored mmt-core re-pinned to the post-libmmt-fix(docker): fail closed on missing SOURCE_REVISION/BASE_REVISION #68 FEC-trailer contract (03cfd7b, e145cf2); drift tracked in VENDOR.md (5617221, ex-docs(moq-mmtp): document vendored mmt-core drift #16).

moq-catalog — format conformance

  • Container enum + multicast extension (6ee40ff), then replaced by packaging + mmtpMode per draft-ramadan-moq-mmt §12.1 (8b05d83, ex-BLO-10088: replace container catalog field #18); mmtp packaging value (8a3b705); validation rejects inherited mmtp packaging without mmtpMode, accepts fec-repair tracks.

moq-transport — subgroup delivery

  • Latest-wins → deliver-all-subgroups-per-group rewrite with group-window prune + set_history_window (553a92d).

moq-sub-raw — raw per-track payload subscriber (509c5c8).

deps — web-transport stack bumped for Chrome 146 WebTransport compat (eea732e).

E2E

T1.7 staged live Shaka MMTP: observe-dump smoke (614123d), Karma vehicle wrapper + cross-origin CORS (d6b23ff), playable-stream contract assertion (afe82a4), CI FFmpeg resolution via FFMPEG_PATH/submodule (48b2c14).

Review provenance

Every code change in this range already landed through a reviewed sub-PR (#8#18) on the stack. Net-new here is only the collapse to a single main-targeted unit. cargo test -p moq-catalog -p moq-pub-mmtp green at HEAD; cargo fmt clean.

Note for the operator

Opening this to main makes PR #1 (m1main) and PR #6 (m4-t1m1) redundant. I have not touched them — say the word and I'll close them with a pointer here.

kkroo and others added 30 commits May 28, 2026 17:52
T1 clock pub/sub + T2 fMP4 BBB pub/sub with --catalog both pass on
Cloudflare moq-rs v0.7.17 (commit f9f51dc) over draft-14 wire.
Establishes the working baseline for the BLO-4020 MMTP migration.

Findings captured:

- dev/pub vs dev/sub scope-mismatch: dev scripts connect to different
  URLs (root vs /<name>), landing in different multi-tenant scopes on
  moq-relay-ietf and surfacing as `namespace not found`. Not a spec
  bug — Cloudflare's tenant-scope feature layered on draft-14. Fix is
  one-line in dev/sub.

- file-coordinator state leak: /tmp/moq-coordinator.json persists
  registrations across abrupt publisher exits, causing `duplicate`
  rejection on next announce. Local-dev annoyance; production uses
  Redis-backed moq-api.

What this confirms for the migration:
- G2 (publisher container) is the main work — moq-pub is firmly
  fMP4-coupled, MMTP needs a new container module.
- G1 (catalog Container::Mmtp) is the smallest first change.
- G6 (libmoq vs moq-rs draft-14 wire diff) remains open.
- M.4 is "replace, not extend": moq-lite is a different wire and not
  worth bridging to draft-14.

Logs captured under .planning/m0-logs/ for reproducibility.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds two optional shapes to moq_catalog::Root for the MMTP-on-MoQ
migration (BLO-4020). Both fields are
#[serde(skip_serializing_if = "Option::is_none")] so existing
catalogs without the extensions round-trip without diff.

1. Container enum (Track.container field) per
   draft-ramadan-moq-mmt-00 §11.1 — values: isobmff | mmtp | mfu |
   fec-repair. Distinguishes how media is encapsulated inside MoQ
   objects from TrackPackaging (which is the IETF draft-01 catalog's
   cmaf vs loc streaming format).

2. multicast: Option<MulticastConfig> field per
   draft-ramadan-moq-multicast-00 §4.1 + §4.2.3 — describes one or
   more multicast endpoints with the MMTP packet_id → MoQ track map
   used by the multicast send-side. NetworkSource carries AMT relay
   discovery (type=amt, discovery=driad per §4.2.1). The
   OneOrMany<T> carrier preserves the "one object OR array" input
   form per §4.2.3 (single object does NOT become a one-element
   array on re-serialize).

moq-pub/src/media.rs gets `multicast: None` to satisfy the new
field on Root.

19 unit tests:
  - 4 Container enum (spec values, optional, default, accepts/omits)
  - 5 multicast spec alignment (§4.1 protocol-optional,
    networkSource at endpoint level, no simple-form fields, §4.2.3
    OneOrMany single/array forms)
  - 5 serde round-trips
  - 4 multicast-extension parsing on Root
  - 1 TrackPackaging round-trip pin

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Captures the M.1 architecture decisions for MMTP-on-moq-transport
(draft-14+) per umbrella BLO-4020. Option A locked: new sibling
crate `moq-pub-mmtp` rather than patching upstream moq-pub or
rewriting cast's bridge (Option B / Option C). Minimal blast
radius, zero upstream-coordination latency.

Decisions:
  A1 — Object 0 of each new MPU MUST be FragmentType::Init
       (publisher errors on violation; does NOT synthesize).
  A2 — MPU sequence numbers strictly non-decreasing per track
       (moq-transport's SubgroupsWriter::create silently drops
       group_id ≤ latest — A2 monotonicity check catches this).
  A3 — Unknown packet_id hard-errors.
  A4 — Smoke test verifies per-track sha256 (not concatenated)
       and mlog framing (not qlog).
  A5 — Vendor mmt-core under moq-pub-mmtp/vendor/ at pinned
       libmmt commit so the crate builds standalone.
  C1 — Stdin (length-prefix) AND UDP (datagram) input both
       supported; UDP for the multicast path.

Implementation Tasks T1-T9 derived from the decisions; T6 + T1-T4
land Lane A (publisher path). T5 (catalog validation), T7
(moq-sub-raw), T8 (ffmpeg fork stdout mode), T9 (smoke) remain.

GSTACK REVIEW REPORT footer: /gstack-plan-eng-review CLEARED;
Codex outside-voice flagged 12 findings, 9 folded into the
Implementation Tasks, 3 acknowledged as M.1b TODOs (object_id_delta
verification, MMTP fragmentation reassembly, FEC source-block
grouping).

Handoff prompt (m1-next-session-prompt.md) captures the resume
context for picking up mid-task.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New sibling crate to moq-pub. Reads MMTP packets (from stdin with
4-byte BE length-prefix framing, or from a bound UDP socket) and
publishes them as raw MoQ object payloads per draft-ramadan-moq-mmt
§3.1+§4.1. Catalog is loaded from --catalog-json; per-track state
keyed by MMTP packet_id from the multicast extension's
endpoints[].tracks[] map.

Implements M.1 ADR Implementation Tasks T6 + T1-T4:

  T6 — Vendor mmt-core under vendor/
       libmmt commit 929e5b0c7a14f6ffe0ecd50d792fff7cdc44ba0a
       vendored so moq-pub-mmtp builds without a sibling libmmt
       checkout. Refresh procedure in vendor/mmt-core/VENDOR.md.
       Vendored Cargo.toml uses explicit versions (no workspace
       deps). Per ADR A5.

  T1 — Publisher loop with spec-true grouping
       Dispatch fn abstracted over TrackSubgroups + SubgroupWrite
       traits so the core logic is unit-testable without a real
       moq-transport session. Enforces three hard invariants:
         A1 — first packet of a new MPU is FragmentType::Init
              (publisher errors on violation; caller's
              responsibility to send MPU metadata first)
         A2 — MPU sequence numbers strictly non-decreasing per
              track. moq-transport's SubgroupsWriter::create
              silently drops subgroups whose group_id ≤ latest
              (subgroup.rs:116-128) — A2 is what catches the bug.
         A3 — unknown packet_id hard-errors (no silent drop)
       SubgroupsWriter::create({group_id: mpu_seq, subgroup_id: 0,
       priority}) — NOT append() — per Codex #5.
       Equal MPU sequence appends to the open subgroup.

  T2 — `.catalog` track posted at startup
       Full catalog JSON as one object on group 0 at priority 127.
       Caller retains the returned SubgroupsWriter so the track
       stays alive for the session.

  T3 — FEC repair routing on `<name>/repair` siblings
       build_state_map auto-creates a `<source>/repair` track for
       every catalog source track. Repair packets land at priority
       7 (per draft-ramadan-moq-mmt §7.2). Repair group_id mirrors
       source MPU group_id so the receiver can correlate repair
       symbols with the source data they protect. Per-FEC-block
       grouping (parsing Source/Repair FEC Payload ID) is M.1b.

  T4 — UDP input mode
       tokio::net::UdpSocket::bind(--mmtp-udp-bind); each datagram
       is one MMTP packet (no length prefix — the datagram boundary
       IS the framing). recv_one_udp_packet() extracted as a
       testable helper.

CLI parity with moq-pub for session setup: positional URL (no
path), --name, --catalog-json, --mmtp-input stdin|udp,
--mmtp-udp-bind, --bind, and the flattened
moq_native_ietf::tls::Args (--tls-cert/--tls-key/--tls-root/
--tls-disable-verify).

30 unit tests:
  - 5 dispatch invariants (A1/A2/A3 + advance + equal)
  - 5 repair routing (sibling, priority 7, group correlation,
    unknown packet_id, before-source-MPU, missing-repair-sibling)
  - 6 framing (length-prefix encode/decode + EOF/partial/oversize)
  - 4 MMTP/MPU header parsing (route fn)
  - 6 build_state_map (no-multicast, no-endpoints, duplicate-
    packet_id, missing-track-reference, happy-path, repair-track-
    registration)
  - 1 priority_for_container pin
  - 1 publish_catalog_track registration pin
  - 1 UDP recv → dispatch integration
  - 1 mmtp_parse short-packet rejection

Out of scope for M.1 (deferred): MPU metadata synthesis from MFU
(caller's job), MMTP fragmentation reassembly (M.1b), per-FEC-block
grouping (M.1b), receiver decode/render (M.4),
moq-transport object_id_delta verification (Codex #6 follow-up
landing in T9 mlog).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reflects post-rebase + post-PR state:
- T6 + T1-T4 marked complete with the four commit SHAs landed on
  blo-4020-m1 (dbf5ee1, 6ee40ff, bde975d, 3101aca on base f0a709a
  after a clean 47-commit upstream rebase).
- PR #1 link: #1.
- Remotes documented (origin = cloudflare/moq-rs untouched;
  blockcast = Blockcast/moq-rs where local main tracks
  blockcast/blo-4020-m1).
- Workflow notes added: new commits land on PR #1 automatically;
  branch off if T5/T7 want separate PRs.
- Obsolete "uncommitted on main, ask before non-test edits"
  branch warning removed (no longer applicable).
- Remaining T5/T7/T8/T9 scope carried forward unchanged.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Promotes catalog invariants from publisher-only runtime checks to
library-level validation so subscribers can reject malformed
catalogs without re-implementing the guards.

moq-catalog:
  - New `CatalogValidationError` enum (Display + std::error::Error,
    no external deps). Three variants:
      DuplicatePacketId       — T5a (draft-ramadan-moq-multicast §4.1)
      UnknownTrackReference   — T5b
      FecRepairInCatalog      — T5e (repair tracks are
                                publisher-derived per ADR T3)
  - `Root::validate()` runs all three checks (first-error-wins).
  - `Root::expand_common_fields()` promotes commonTrackFields into
    each track entry (track-level overrides win). Replaces the
    previously-dead `Track::with_common` private helper. T5c.
  - `CommonTrackFields` gains `Clone` (needed for expand).

moq-pub-mmtp:
  - `check_namespace_consistency(&Root, name)` — if
    commonTrackFields.namespace is Some(X) and X != --name, hard
    error. Catches a class of publisher misconfigurations. T5d.
  - main(): wired in order
      1. catalog.validate()              (lib-level guards)
      2. check_namespace_consistency()   (publisher CLI consistency)
      3. catalog.expand_common_fields()  (in-memory normalization;
                                          on-wire .catalog bytes
                                          remain the original
                                          authorial form)
  - `build_state_map` runtime guards (duplicate packet_id, unknown
    track ref) kept as defense in depth.

Tests:
  - moq-catalog: +5 (3 validate + 2 expand_common_fields) → 24 total
  - moq-pub-mmtp: +3 (check_namespace_consistency) → 33 total

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New sibling crate to moq-sub. Subscribes to named tracks on a MoQ
broadcast and dumps each track's concatenated raw object payloads
to its own file. Used by the M.1 smoke test (T9) for per-track
sha256 verification — by dumping object payloads with NO separators
(no length prefix, no timestamp), the publisher's input bytes and
the subscriber's output bytes match byte-for-byte per track.

CLI: paired `--track NAME --output PATH` arguments, repeatable.

  moq-sub-raw --name BBB \
      --track v --output v.bin \
      --track a --output a.bin \
      https://localhost:4443

Implementation:
  - `drain_track_to_writer<W>` — extracted as a testable async fn
    so unit tests can drive it with in-process Tracks::produce
    pairs (no live relay required). Walks TrackReaderMode::Subgroups
    → groups.next → group.next → object.read, writing every chunk
    to W in arrival order. Returns total bytes written.
  - `validate_track_output_pairs` — surfaces CLI misconfigurations
    (empty args, mismatched counts) BEFORE opening a session.
  - main(): mirrors moq-sub's session template. Spawns one
    subscriber.subscribe task per track AND one drain task per
    track; tokio::select! over session.run + drain JoinSet.

Tests (6 new):
  - validate_track_output_pairs: empty / mismatched / matched.
  - drain_concatenates_object_payloads_in_arrival_order — single
    subgroup, 3 objects, verify concat.
  - drain_concatenates_across_multiple_groups — producer/consumer
    run concurrently via tokio::join! with a 100ms gap between
    groups (SubgroupsReader surfaces only the latest subgroup —
    documented gotcha pinned by the test).
  - drain_writes_zero_bytes_on_empty_track — subgroups mode set
    but never written; drain returns Ok(0).

Workspace entry added; Cargo.lock updated.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Blockcast FFmpeg fork's `moqenc_mmt` muxer already emits MMTP
packets through standard AVIOContext to a `udp://group:port?multicast=1`
URL. The proper way to wire moq-pub-mmtp as the consumer is to have
its existing `--mmtp-input udp` listener auto-join when the bind
target is a multicast address — no FFmpeg changes, no new flags on
moq-pub-mmtp, just smarter UDP open.

New `udp` module:
  - `is_multicast(SocketAddr) -> bool` — IPv4 and IPv6.
  - `open_udp_socket(target) -> UdpSocket`:
      * unicast target: bind directly.
      * multicast target: bind wildcard on target.port(), join the
        group on all interfaces, enable multicast loopback so a
        single-host smoke test can both send and receive.

`run_udp_loop` swapped its raw `UdpSocket::bind` for
`udp::open_udp_socket`, so `--mmtp-udp-bind 239.255.1.1:5004`
now actually receives multicast.

T8 ADR plan ("add stdout flag to ffmpeg muxer") superseded: the
muxer's existing avio_write path through AVIOContext IS the proper
FFmpeg interface. The dirty state in the FFmpeg fork is unrelated
to this work (FEC algorithm FFI extension + IPv6 multicast egress
fix in progress on a separate workstream).

Tests (+6):
  - is_multicast: 4 (ipv4 multicast / unicast, ipv6 multicast / unicast).
  - open_unicast_binds_directly_to_target.
  - open_multicast_binds_wildcard_and_recvs_loopback — full
    end-to-end: open multicast listener, send via second socket,
    verify recv. Gracefully skips if sandboxed network lacks
    multicast loopback (treats timeout / open-error as skip).

Total moq-pub-mmtp tests: 33 → 39.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the M.1 ADR Definition-of-Done item: an end-to-end smoke test
proving MMTP packets flow byte-for-byte through the new draft-14+
pipeline.

Pipeline (5 processes, all out of this repo):
  synth_mmtp (Rust example, deterministic MPU sequences)
    → UDP loopback (127.0.0.1:5004)
    → moq-pub-mmtp --mmtp-input udp
    → moq-relay-ietf --dev --mlog-dir
    → moq-sub-raw (per-track payload dump)
    → per-track sha256 vs synth's expected files

Verdict: PASS. Both tracks' sha256 match between publisher input
and subscriber output; --mlog-dir captures SUBGROUP/OBJECT framing
per draft-14. Full results recorded in
.planning/moq-rs-m1-results.md.

New files:
  - moq-pub-mmtp/examples/synth_mmtp.rs — deterministic MMTP
    packet generator. Emits valid MPU Init packets for two
    packet_ids (1=video, 2=audio) with predictable payloads.
    Outputs to UDP (one datagram = one packet) OR stdin
    (length-prefixed) AND writes expected per-track files for
    sha256 comparison.
  - .planning/m1-smoke.sh — orchestrator. Builds binaries, writes
    catalog, starts relay → pub → sub → synth, compares hashes,
    shows mlog, reports pass/fail. Idempotent across reruns.
  - .planning/moq-rs-m1-results.md — verdict, evidence, repro
    instructions, and the open M.1b items (FEC source-block
    grouping, fragmentation reassembly, libmoq G6 byte-diff).

Pacing note: SubgroupsReader surfaces only the latest subgroup —
the smoke uses --packet-delay-ms so the subscriber drains each
MPU before the next supersedes. Already covered by a unit test
in moq-sub-raw; documented in the results doc as a real-world
constraint (real publishers are network-rate-limited).

T8 status: superseded. The Blockcast FFmpeg fork's moqenc_mmt
muxer already uses AVIOContext for UDP emission — no fork
changes needed. The architecture pushback (no special stdout
flag) is the correct call. T8.5 (moq-pub-mmtp multicast join,
already landed) is the matching consumer-side support.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Updates the in-tree handoff prompt to reflect the post-T9 state:
9 commits on blo-4020-m1, 69 tests + smoke green, T1-T9 all landed
(T8 N/A per ADR architecture pushback). Lists the four candidate
follow-up tracks in priority order — A (PR merge prep), B (M.1b
sub-tasks), C (M.2 cast bridge — ADR-gated), D (M.4 receiver inventory).

No code changes; the doc tracks the closed state of the milestone
and the open M.1b sub-tasks (fragmentation reassembly, per-FEC-block
grouping, object_id_delta correctness, G6 byte-diff vs libmoq).
…M.2/M.4 ahead

Refreshes the handoff prompt to reflect the 2026-05-28 M.1b session:
- M.1b §B1=C closed by raw-passthrough fragmentation contract (PR #2).
- M.1b §B3 forensics confirmed object_id_delta bug at
  moq-transport/src/session/subscribed.rs:281 (PR #3); upstream issue
  drafted in BLO-8047 awaiting external filing.
- M.1b §B4 confirms wire-format divergence between cast/moq_lite and
  moq-pub-mmtp/IETF (PR #4); B2 re-scoped from "high operational value"
  → "M.4 prerequisite, deferred".

Four stacked PRs on blo-4020-m1. Next session picks:
- PR review / merge prep (likely fastest)
- Upstream object_id_delta issue filing at cloudflare/moq-rs (use BLO-8047 draft)
- M.2 cast bridge port (biggest blast radius; needs ADR + plan-phase first)
- M.4 receiver migration (multi-object subgroup decoder, tier-switching fallback)

Also documents the new FRAGMENT=N smoke env knob, the four follow-up
branch names with detached upstreams, and the raw-passthrough contract
as a carry-forward constraint.

No code changes; doc tracks the post-M.1b state of the milestone.
Reflects 2026-05-28 session continuation:
- M.4 ADR drafted on branch blockcast/blo-4020-m4-adr (commits ec8e4b7,
  5ffd5e1, 1b0c577). A0-A3 locked; Q3-Q8 tactical questions still open.
- Scope discovery: moqtail already on IETF draft-16 with MMTP container
  wired; Shaka MSF speaks drafts 14+16 with ALPN negotiation; hang-mmt-fec
  is the actual migration burden but Track 3 is INCLUDED per session sign-off.
- Updates the "PICK ONE FOR NEXT SESSION" list: D becomes "M.4 ADR sign-off +
  T0 (publisher draft-16 bump)"; adds new E for "M.4 Track 1 (Shaka MMTP)
  implementation" as the post-T0 starting point.

No code changes.
Reflects 2026-05-28 session continuation:
- M.4 ADR Q3-Q8 signed off; Track 3 reshaped per Q7 (use moqtail-ts;
  skip Rust IETF subscriber fix). Total M.4 scope ~5-6 weeks.
- T0 (publisher draft-16 bump) implemented + smoke-validated on
  branch blo-4020-m4-t0 (commit 9336e4a), PR #5 opened on the fork.
  Per-track sha256 IDENTICAL to M.1 baseline at FRAGMENT=0; mlog
  confirms selected_version DRAFT_16 in both QUIC sessions.
- 113 moq-transport tests pass (+3 negotiation regression tests).

Five PRs now stacked on blo-4020-m1; one fork-only branch with M.4 ADR.

Pick list restructured:
- D = M.4 Track 1 (Shaka MMTP container) — RECOMMENDED next, smallest
- E = M.4 Track 2 (moqtail tier-switching)
- F = M.4 Track 3 (MoqWatch port onto moqtail-ts; T3.1/T3.2 SKIPPED
      per Q7)
- G/H/I = PR review, upstream issue, M.2

Track 1 start files: shaka-player/lib/msf/loc_parser.js and
lib/transmuxer/loc_transmuxer.js (templates to mirror for MMTP).
M.4 ADR §T1.1-T1.7 has the full task breakdown.

No code changes; doc tracks the post-T0 state.
…/Q3)

Adds examples/reassembler_vectors.rs which feeds canonical MFU fragment
sequences through mmt_core::MfuReassembler and serialises each
(input fragments -> reassembled output) pair to JSON. The Shaka pure-JS
MfuReassembler port (M.4 Track 1) loads this fixture and asserts
byte-for-byte equality, pinning the JS impl to the Rust source-of-truth
per ADR Q3.

Vectors mirror reassembler.rs::tests deterministic cases (complete /
two-in-order / out-of-order / three-fragment) plus one matching the M.1b
B1 smoke wire (Mfu fragments only; Init objects bypass the reassembler).

Verified: out-of-order reassembles to XXXZZZ (counter-sorted, not arrival
order); timestamps propagate from the first fragment; RAP from FI=1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds examples/mmtp_packet_vectors.rs which builds raw MMTP packets with the
canonical mmt_core header encoders (MmtpHeader / MmtpHeaderExt / MpuHeader /
SourceFecPayloadId) and serialises each (packet bytes -> expected parsed
fields) pair to JSON. The Shaka MMTPParser loads this fixture and asserts it
recovers exactly those fields, pinning the JS bit-layout decode to the Rust
encoder and avoiding the circular risk of hand-encoding the same layout in
both the parser and its test.

Vectors: Init MPU packet, first MFU fragment, an Mfu packet with a
SourceFecPayloadId (fec_type=1, 4 B between MMTP and MPU headers), and a
non-MPU Repair packet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…M.4 T1.5a)

Extends the packet-vector generator with a full Init+MFU-fragment sequence
(packets_hex + expected init payload + reassembled MFU bytes), built with the
canonical mmt_core encoders. Drives the Shaka MmtpTrackProcessor observe-first
integration test non-circularly (real encoder bytes in, classified records out).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…B-MIG-pub next

Captures this session: T1.1/T1.3/T1.CAP/T1.4/T1.5a built + tested, eng review +
codex absorption, Draft Wave-1 (mapping B), B-MIG-recv done. Records branches
(none pushed), decisions (D1-D4, B mapping, codex absorptions), the spec-leads-
impl B pivot, and the pending queue (B-MIG-pub, staged T1.7, T1.2/Init/timing,
T1.5b, B-MIG-relay, FEC, version-pin, Wave-2, T1.6).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rune

Subgroups was latest-subgroup-wins: SubgroupsState kept a single
latest_subgroup_reader slot, create() dropped any subgroup that wasn't the
new latest within a group, and next() only ever yielded that one slot. This
silently dropped all-but-latest subgroups per group before they reached the
wire — incompatible with Mapping B (draft-ramadan-moq-mmt §4.3), which emits
subgroup 0 = MPU metadata and subgroups 1..M = one MFU each.

Mirror the object-level SubgroupState (Vec + per-reader cursor) one layer up:
- SubgroupsState: subgroups: Vec<SubgroupReader> + pruned: usize.
- create(): append unconditionally (drop the latest-wins guard and the
  Duplicate check); prune the front by group window.
- next(): absolute read_index mapped to subgroups[read_index - pruned];
  a reader behind the window skips to pruned.
- Opt-in SubgroupsWriter::set_history_window(groups) bounds memory; prune
  condition group_id + window <= newest (additive, no unsigned underflow).

The egress (session/subscribed.rs::serve_subgroups) already spawns a task +
uni stream per subgroup, so concurrent subgroups now reach the wire. Public
signatures (create/append/next/latest) unchanged.

NOTE: set_history_window is opt-in; unset retains all subgroups (was O(1)
under latest-wins). Live writers (publisher main.rs, relay receive path)
MUST set a window from config or grow unbounded — wiring deferred to
B-MIG-pub. See .planning/m4-b-mig-transport-subgroups-design.md.

TDD: 5 tests (deliver-all, window prune, keeping-up reader, cloned-reader
fanout, append compat). moq-transport 114 pass / 0 fail, clippy clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…esign

- m4-b-mig-transport-subgroups-design.md: design note + implementation status
  for the concurrent-subgroup transport fix (the B-MIG-pub prerequisite).
- m4-t1-next-session-prompt.md: Session-2 update — T1.7 done (real moq_mmt
  wire format: Init+MFU only, MFU header first-frag-only, per-sample MMTP
  timestamp is the loss-robust MFU key, AVCC NAL payload), B-MIG-pub design
  settled, corrected the prereq ordering (transport fix was mis-ranked #6).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…yed)

Migrate the publisher from the interim flat mapping (all packets of an MPU on
subgroup 0) to draft-ramadan-moq-mmt §4.3 Mapping B: group = MPU sequence,
subgroup 0 = Init (MPU metadata), subgroups 1..M = one MFU each, object =
MMTP packet in FI order.

The MFU subgroup key is the per-sample MMTP timestamp, not sample_number:
the moq_mmt muxer writes the MFU DU header (sample_number) only on the first
fragment AND emits one sample per movie fragment (so sample_number is always 1),
whereas the MMTP timestamp is constant across an MFU's fragments and present on
every packet. Keying off the timestamp survives first-fragment loss (T1.7).

- mmtp_parse: surface MMTP `timestamp` in PacketRouting (TDD).
- publish: TrackState now holds subgroup 0 (Init) + a per-group
  HashMap<timestamp, Group> for MFU subgroups + a next-subgroup-id counter,
  reset on group advance; TrackState::new constructor. dispatch routes
  FT=Init -> subgroup 0, FT=Mfu -> timestamp-keyed subgroup (>=1),
  FT=Fragment -> hard error (muxer never emits moof on the multicast wire).
- A1 (Init-must-be-first) relaxed: group boundaries come from MPU sequence,
  not Init (which may be lost; it also rides the reliable catalog). A2
  monotonicity still enforced.
- main: construct TrackState via ::new.

TDD: replaced old flat-mapping tests with Mapping-B tests (Init=subgroup 0,
MFU-per-timestamp, fragmented-MFU shares subgroup, advance resets indexing,
MFU-first-group allowed, moof errors). moq-pub-mmtp 42 pass / 0, clippy clean
(changed files).

NOTE: main.rs does not yet call SubgroupsWriter::set_history_window, so the
publisher retains all subgroups (unbounded). Window wiring + its config source
is the remaining B-MIG-pub step.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…aining

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…completes B-MIG-pub

The Mapping B dispatch opens many concurrent subgroups per group (Init + one per
MFU); the moq-transport writer retains all subgroups unless a history window is
set, so an unbounded window would leak. Bound it from the catalog.

- moq-catalog: add optional `subgroupHistoryGroups` to MulticastConfig (global —
  a uniform retention/memory policy, not a per-rendition media property like
  groupDurationMs; covers the auto-created <name>/repair sibling too). Local
  deployment-policy extension; serde-optional so existing catalogs round-trip.
- moq-pub-mmtp build_state_map: read the window and call
  SubgroupsWriter::set_history_window on each source + repair writer.
  config-or-throw — error if absent for MMTP tracks (no silent unbounded
  default), and reject < 1. Placed after structural validation so missing-track /
  duplicate-packet-id errors still take precedence.
- m1-smoke.sh catalog: add subgroupHistoryGroups: 8.

TDD: build_state_map_errors_when_subgroup_history_window_absent (verified RED
without the check, GREEN with it). moq-pub-mmtp 43 pass, moq-catalog 24 pass,
clippy clean, workspace builds.

If per-track windows are ever needed (e.g. audio vs video group-rate disparity),
add an optional MulticastTrackRef override that falls back to this global value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…B dispatch

Stage 2 of the T1.7 staged smoke: a real FFmpeg moq_mmt multicast capture
(headers verbatim, payloads truncated; MPU groups 0-1) is replayed through
route() + dispatch() and asserted to produce the Mapping-B subgroup structure
(Init->subgroup 0, MFU subgroups contiguous 1..M keyed by per-sample timestamp,
fragmented MFUs sharing a subgroup). Validates dispatch against real muxer
output, not just synthetic vectors.

This replay surfaced a real muxer bug (see the paired FFmpeg fix): the resent
Init carried a stale mpu_seq=0, which looked like an MPU-sequence regression
after later groups. Fixture captured after that fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…it-mpu-seq fix

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
web-transport-quinn 0.11.4 (web-transport-proto 0.5.2) speaks the deprecated
WebTransport-over-HTTP/3 draft, which Chrome 146 has dropped: the relay closed
every browser session with an HTTP/3 capsule error the instant a stream opened
(diagnosed via Chrome netlog + relay trace on the T1.7 live E2E).

Bump web-transport 0.10.1->0.10.5, web-transport-proto 0.5.2->0.6.0,
web-transport-quinn 0.11.4->0.11.9 (no moq-native-ietf API churn). Chrome 146
now completes the full MoQ control handshake.

Verified: workspace builds; moq-transport 114, moq-pub-mmtp 44, moq-catalog 24.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y `.catalog`

Shaka MSF subscribes to the catalog track named `catalog` — the canonical,
REQUIRED name per draft-ietf-moq-msf-00 §5.2 ("The catalog track MUST have a
case-sensitive Track Name of `catalog`"). The moq-rs / moq.dev-hang ecosystem
(moq-pub, moq-sub, gst-moq-pub) instead uses `.catalog`; moq-transport §3.2.1
reserves the dot-prefix at the namespace level, so `.catalog` is non-idiomatic
for a media track name.

publish_catalog_track now posts the catalog JSON under both: `catalog`
(canonical, listed first) for MSF receivers and `.catalog` as a legacy
compatibility alias for non-MSF consumers (moq-pub, moq-sub, gst-moq-pub).
Drop `.catalog` once they migrate.

44 moq-pub-mmtp tests pass (both registrations pinned).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Orchestration (m4-t1.7-e2e.sh) + same-origin control/static server (serve.py) +
full-payload real moq_mmt capture (moq_mmt_capture_full.json, 251 pkts) for the
live single-pipe E2E:

  real moq_mmt capture -> moq-pub-mmtp (Mapping B) -> moq-relay-ietf
    -> Shaka MSF in headless Chrome over real WebTransport -> observe dump

Asserts Mapping B (Init->subgroup 0, MFUs->subgroup>=1 keyed by per-MFU
timestamp, objects in FI order) and exits non-zero on contract violation.
Fingerprint is computed locally from the cert PEM (no TLS connection). Pairs
with shaka-player demo/observe-mmtp.html. PASS: 5 Init + 59 MFU records.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fetches

m4-t1.7-karma.sh starts relay + publisher + control server, then runs the
shaka-player test runner filtered to the live MMTP integration spec (no --quick
so integration specs load). serve.py now sends Access-Control-Allow-Origin: *
on control responses so the Karma-origin spec can read /__fingerprint and
/__replay cross-origin (harmless for the same-origin standalone harness).

Verified: TOTAL 1 SUCCESS via the wrapper (exercises the live pipe — control
log shows fingerprint fetch + 251-packet replay); with no relay the spec goes
pending and a full msf run stays green (127 SUCCESS, exit 0).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ormance

draft-ietf-moq-catalogformat defines `packaging` (cmaf/loc) as THE container-
format field, and draft-ramadan-moq-mmt adds the value `mmtp`. Add Mmtp to
TrackPackaging so publishers emit the conformant `packaging: "mmtp"` that MSF
(Shaka) keys track selection on, instead of the non-conformant `container`
field. The `container` extension is retained for finer MMTP encapsulation
detail (mmtp/mfu/isobmff). Smoke catalogs updated to `packaging: "mmtp"`.

moq-catalog 24 / moq-pub-mmtp 44 pass; live E2E green without the catalog field
bridge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kkroo and others added 17 commits June 1, 2026 10:20
…cond package (#9)

* docs(planning): T1.6 scoping correction — second transport package

The scoping pass found only the ssm-transport collision; there are TWO existing
"transport" packages. Add @blockcast/mmt-transport (libmmt/packages/transport),
a high-level mode manager that does NOT own the WT connect dance — so a new
@blockcast/moq-transport is still the right call (the factory is a missing
primitive, not a dup). Adds the 3-layer naming story + ssm-transport hygiene note.

Mirrors the recommendation posted on BLO-8646.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(planning): qualify cross-repo transport table sources

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: kkroo <60861014+kkroo@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
…drail (#10)

Strengthen risk #2 to a dated DECISION (defer Opus; AAC mp4a.40.2 only) with
verified evidence (muxer AAC-only at :4952/:3316; Shaka already has Opus via
loc_parser/transmuxer.opus; catalog codec field generic → additive later). Adds
the load-bearing guardrail: C/A-V-sync must derive audio frame duration from the
codec, NOT hardcode AAC's 1024/sr — the one place AAC-first could bake in rework.

Mirrors the recommendation posted on BLO-8644.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…catalog FEC descriptor) (#12)

* docs(planning): resolve OD-1 (T1.5c) + MmtpHeaderExt API (T1.FEC), coupled

Both gates resolved via one shared "catalog FEC descriptor"
{ fecProtected, interleaveDepthMs, sourceSymbols(K), algorithm } owned by T1.FEC:
- OD-1 → (a) plumb FEC params (W = (K-1)·interleaveDepthMs) + targetLatency
  fallback for FEC-off; reject pure (c) — buffer never gated off.
- MmtpHeaderExt → write_to writes base only; packet assembler writes the SS_ID
  trailer; read_from two-phase, length-bounded. Trailer placement structural.

Recommendations posted on BLO-8645 (T1.FEC) + BLO-8647 (T1.5c-1.A).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(planning): T1.FEC MmtpHeaderExt API — libmmt owner sign-off (APPROVED)

Gate cleared: write_to base-only + source_fec_trailer_bytes accessor; packet
assembler writes the SS_ID trailer; two-phase length-bounded read_from. Approved
as recommended; recorded on BLO-8645.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(planning): address Ally #12 nits

- T1.5c: flag the interleaveDepth UNIT as an open sub-Q to pin before T1.FEC
  defines the descriptor (DEFAULT_FEC_DEPTH=30 is ~1s@30fps = 30 frames, NOT
  30ms; naive (K-1)·30ms over-counts ~31×). Don't ship a W formula until pinned.
- T1.FEC: add STATUS — API approved but NOT yet implemented in Rust; TS work in
  flight on libmmt codex/mmtp-fec-trailer-timeout; Rust+Shaka net-new.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BLO-8666: add FEC-on capture fixture and replay coverage while preserving existing FEC-off guard.
…lly #12 sub-Q) (#14)

* docs(planning): pin OD-1 interleaveDepth unit + W formula (vs moqtail/libmmt)

Resolves the Ally #12 sub-Q. Unit = ms (muxer emits depth_ms = D·1000/fps at
moqenc_mmt.c:2935/2989, no raw frames). W = computeFecTimeout (libmmt oti.ts:116)
= (K-1)·interleaveDepthMs·3, reuse don't reimplement. CLAMP to targetLatency on
FEC-on too — video defaults (K=32,depth=30→1000ms) give ~93s otherwise.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(planning): address Ally #14 nits on OD-1 pin

- Note cross-repo refs (moqenc_mmt.c:2935/2989, oti.ts:116) verified 2026-06-01;
  T1.FEC re-confirms at descriptor-definition time (sibling repos).
- Add the clamp's tradeoff: clamping W below computeFecTimeout defeats FEC
  recovery for blocks whose span exceeds targetLatency → clamp is a pathological-
  config safety net, not a normal mode; warn when it engages; fix with sane FEC
  params.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
T1.5c-1.B shipped; all 4 gates resolved/signed-off; T1.FEC + T1.5c-1.A
implemented by MulticastEngineer; T1.6 in review; B blocked on live E2E with
both infra gates (WT 0.6.0 relay, catalog framerate propagation) root-caused
and routed. Carries repo traps, paperclip fleet refs, standing rule, and next
targets.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The MMTP path now builds a playable stream (processMmtpTrack_), so the old
subgroup-dump assert parsed kind=/grp=/sg= lines lib/ no longer emits, and the
catalog lacked the framerate the path requires (CQ#1).

- Add "framerate": 30 to the catalog video track (mmtpFrameDuration_ requires
  numeric track.framerate; one field, no schema change).
- Replace the dead Mapping-B subgroup assert with a playable-stream-contract
  assert: >=1 SegmentReference, Init MPU stored, monotonic non-negative timing.

Wire-level Mapping B (Init->sg0, MFU->sg>=1, FI order) stays covered by the
moq-rs publisher tests; the live E2E now asserts what Shaka consumes.

Verified green on a real relay (devbox): status=done, 3 segments, init 769B,
durations=1/30. Pairs with shaka-player blo-4020-m4-t1-shaka-mmtp harness commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audio-bearing moq_mmt capture (packet_id=1 video + packet_id=2 AAC) that
unblocks BLO-8702 Sub-project C Work area 3 (receiver audio E2E). The existing
moq_mmt_capture_full.json is video-only.

Generated on devbox via the custom ffmpeg moq_mmt muxer (make_av_capture.sh);
436 pkts / ~4s, both tracks Init+MFU (avcC video, mp4a audio). Validated:
moq-pub-mmtp ingests cleanly with a 2-track catalog (track_count=2, no errors,
ANNOUNCE_OK); the video-only slice passes the full relay->Shaka harness
(29 segments). Pair with the 2-track catalog (channelConfig as a JSON string);
full audio->Shaka playback needs the WA1 receiver audio branch. See
capture-av-fixture.md for provenance + the audio-init-resend finding for B/8644.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ot a devbox home path

The moq_mmt capture muxer lives in the Blockcast/FFmpeg submodule (build-native);
system ffmpeg lacks it. Resolve it the way the pmg CI workflows do — prefer
$FFMPEG_PATH/build-native (set from the submodule), then the in-tree submodule
build — so make_av_capture.sh is portable to CI/agent contexts, not just devbox.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…merged

C (BLO-8702) done; shaka PR #5 squash-merged to blockcast/main (c631b0d56),
landing the whole MMTP stack (T1.2 video + T1.7 + C). Verified: build +@complete
0 errors, Mmtp 27/27. Append timestamped UPDATE block + refresh snapshot in the
live next-session prompt; mark C/B done and add SUPERSEDED banner in the older
session handoff. Opus deferred (8705/8713/8714); BLO-8704 now promotable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed, build-source gate greened

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the actionable findings from the multi-agent PR review of the
Mapping-B subgroup-per-MFU work. No functional defects were found; these
harden types, tests, and docs:

- subgroup.rs: set_history_window returns Result instead of assert!/panic
  (error-on-bad-init); debug_assert! monotonic group ids in create();
  multi-group prune-boundary test (window=3) + zero-window error test;
  soften "mirrors object-level" comment; note Duplicate-detection removal.
- header.rs: single has_source_fec_trailer() source of truth (fec_type) used
  by both write and split paths; debug_assert they agree; document that
  read_from leaves source_fec_payload_id None (SS_ID is a trailer).
- mmtp_parse.rs/publish.rs: surface the MPU aggregation flag and refuse
  aggregated MPUs in dispatch (mirrors the FT=Fragment refusal);
  first-fragment-loss survival test; u32 timestamp-wrap doc note.
- moq_mmt_capture.json: frozen group/subgroup oracle for the FEC-off replay
  (true regression gate vs the prior self-derived assertions).

cargo test --workspace green (moq-pub-mmtp 50, moq-transport 116).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure `cargo fmt` of files that were already fmt-dirty on the branch before
the review-fix work (formatting only, no behavior change). Kept in a separate
commit so the review-fix diff stays readable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: kkroo <60861014+kkroo@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Refresh vendored mmt-core to libmmt a5ea680 and preserve standalone reassembler feature.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
…ode) (#18)

Rebased onto blo-4020-m4-t1 (e145cf2) per the BLO-10088 operator ruling
(originally authored against blo-4020-m1 as PR #18 head 31a5d26).

Migration (original work, conflicts resolved against the Mapping-B m4-t1
shapes — TrackState::new constructor, Init/MFU subgroup dispatch):
- Delete `Container` enum + `Track.container`; add `TrackPackaging::FecRepair`
  and REQUIRED-per-§12.1 `MmtpMode` (mpu|mfu).
- validate(): accept catalog-declared fec-repair tracks (draft-ramadan-moq-fec
  §5.2), reject mmtp tracks lacking mmtpMode (MissingMmtpMode).
- build_state_map: skip packaging=fec-repair entries (publisher still derives
  its own `<name>/repair` siblings); drop dead priority_for_container
  (source tracks publish at priority 0; repair hardcodes 7).
- Citation sweep §7.2→§8.2/fec §6.1; smoke fixture emits packaging+mmtpMode.

Re-derived from unpushed agent commit 0a02292 (operator, BLO-10152
superseded):
- validate() enforces mmtpMode for packaging INHERITED from
  commonTrackFields (validate runs before expand_common_fields) — RED→GREEN
  tested (validate_rejects_inherited_mmtp_packaging_without_mmtp_mode).
- m1-smoke.sh: openssl self-signed localhost cert fallback when dev/cert is
  unavailable (locked-down agent containers; no sudo/mkcert -install).

Extended TrackPackaging round-trip test with fec-repair; kept the m4-t1-only
catalog-track-names + Mapping-B test surface intact.

cargo test -p moq-catalog -p moq-pub-mmtp: 28 + 50 passed. cargo fmt clean.

Co-authored-by: kkroo <60861014+kkroo@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

reviewed head: 8b05d83

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. The unavailable nested Codex CLI path was skipped per runtime rules; native-codex was applied directly to /tmp/pr.diff and exact changed files.

Critical Issues (0)

Important Issues (2)

  • [pr-review-toolkit/errors] moq-sub-raw/src/main.rs:87 — Subscriber drain failures are logged and then converted into a successful process exit. wait_tasks() returns () and swallows both Ok(Err(err)) from drain_track_to_writer() and join panics, so failures such as output-file creation errors, non-subgroup tracks, object read errors, or write failures only emit warn!; once all tasks finish the tokio::select! branch returns Ok(()). That makes smoke/E2E wrappers able to pass even when no valid raw capture was produced.

    • Return Result<()> from wait_tasks, propagate the first drain/join error, and have the select branch use res = wait_tasks(&mut tasks) => res? so CLI and CI callers observe failure.
  • [native-codex/types] moq-transport/src/serve/subgroup.rs:160 — The shared transport API now accepts duplicate (group_id, subgroup_id) entries. The removed ServeError::Duplicate guard means any caller outside the new MMTP publisher can accidentally create the same subgroup tuple twice; readers then receive two indistinguishable subgroups with the same ordering key, while downstream code is expected to reorder by (group, subgroup, object). This is a behavioral regression in moq-transport, not just an internal MMTP invariant.

    • Keep the deliver-all behavior, but preserve uniqueness by tracking/rejecting duplicate (group_id, subgroup_id) in retained state (or document and test a new explicit duplicate contract if the protocol layer truly allows it). The new MMTP publisher can still rely on monotonic subgroup allocation, but the library should not silently accept ambiguous duplicates from other callers.

Suggestions (0)

Strengths

  • The catalog packaging/mmtpMode validation covers inherited commonTrackFields and duplicate multicast packet IDs.
  • The Mapping-B dispatch tests include real capture replay and frozen subgroup/priority oracles, which is the right shape for this protocol work.
  • The subgroup delivery rewrite adds focused fanout, pruning, and multi-subgroup regression tests around the core transport change.

Recommended Action

  1. Fix Critical issues before merge: none found.
  2. Address Important issues this cycle before relying on the new raw subscriber and shared subgroup API in CI or downstream integrations.
  3. Consider Suggestions opportunistically: none.

kkroo and others added 2 commits June 13, 2026 03:35
… them

Ally review on #20: `wait_tasks` returned `()` and the `tokio::select!`
branch logged "all drain tasks finished" then fell through to `Ok(())`.
Drain errors (`Ok(Err(_))` from `drain_track_to_writer`, incl. output-file
creation failures), and join panics, were `warn!`-only — so a raw-capture
run could exit 0 even when every track failed to produce output, letting
smoke/E2E wrappers pass on an empty capture.

`wait_tasks` now returns `Result<()>`, propagating the first drain error or
join panic; the select branch threads it via `res?`. Adds unit tests for
both the error-propagation and all-succeed paths. Honors the repo's
no-silent-fallback rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ally review on #20: the deliver-all rewrite dropped the ServeError::Duplicate
guard and pushed uniqueness onto callers. That is a behavioral regression in
the shared moq-transport library, not just an MMTP-internal invariant — any
caller could create the same (group_id, subgroup_id) twice, leaving readers
with two subgroups sharing one ordering key.

Deliver-all and uniqueness are orthogonal: restore the guard (reject a repeated
tuple among RETAINED subgroups with ServeError::Duplicate) while keeping every
distinct subgroup. Pruned tuples are gone from delivery, so a monotonic
publisher (A2) never collides. Adds create_rejects_duplicate_group_subgroup;
the MMTP publisher's monotonic allocation (moq-pub-mmtp 50 tests) is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kkroo

kkroo commented Jun 13, 2026

Copy link
Copy Markdown
Author

Both Important findings addressed (additive commits, no force-push):

Green at 703b336: moq-transport 117 · moq-sub-raw 8 · moq-catalog 28 · moq-pub-mmtp 50 · cargo fmt clean. (No PR CI on this branch — local workspace tests are the gate.)

@kkroo
kkroo merged commit ad5c8ab into main Jun 13, 2026
kkroo added a commit that referenced this pull request Jun 13, 2026
… them

Ally review on #20: `wait_tasks` returned `()` and the `tokio::select!`
branch logged "all drain tasks finished" then fell through to `Ok(())`.
Drain errors (`Ok(Err(_))` from `drain_track_to_writer`, incl. output-file
creation failures), and join panics, were `warn!`-only — so a raw-capture
run could exit 0 even when every track failed to produce output, letting
smoke/E2E wrappers pass on an empty capture.

`wait_tasks` now returns `Result<()>`, propagating the first drain error or
join panic; the select branch threads it via `res?`. Adds unit tests for
both the error-propagation and all-succeed paths. Honors the repo's
no-silent-fallback rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant