Skip to content

feat(mmtp): M.1b complete — B1 fragmentation contract, B2 SBN repair grouping, B3+B4 docs (BLO-8047) - #28

Merged
kkroo merged 4 commits into
blo-4020-m1from
blo-4020-m1b-frag
Jun 22, 2026
Merged

feat(mmtp): M.1b complete — B1 fragmentation contract, B2 SBN repair grouping, B3+B4 docs (BLO-8047)#28
kkroo merged 4 commits into
blo-4020-m1from
blo-4020-m1b-frag

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Jun 21, 2026

Copy link
Copy Markdown

Summary

Lands all four M.1b follow-up items (deferred from M.1 ADR) onto blo-4020-m1. This is the consolidation PR that closes BLO-8047.

Item Branch Commits Status
B1 — raw-passthrough fragmentation contract blo-4020-m1b-frag ec8e4b7 ✅ code + test
B2 — SBN-keyed AL-FEC repair subgroup grouping blo-4020-m1b-sbn daea950 ✅ code + 43 tests (landed via #27)
B3 — object_id_delta correctness investigation blo-4020-m1b-obj-id-delta 47ca965 ✅ docs only
B4 — G6 byte-diff vs libmoq investigation blo-4020-m1b-g6-bytediff d972e3c ✅ docs only

B1 — Raw-passthrough fragmentation contract (ec8e4b7)

Pins the ADR reversal: publisher must not error on fragmentation_indicator != 0. I-frame fragmentation is mandatory (e.g. 4K H.265 I-frame = 220–1100 fragments over AMT/UDP). Receiver reassembles via mmt-core::MfuReassembler. Test covers FI ∈ {1,2,3}.

B2 — SBN-keyed repair subgroup grouping (daea950, via #27)

Repair subgroups are now keyed by FEC Source Block Number (SBN = SS_ID / K, K=32) per draft-ramadan-moq-fec §6.1. Parser reads SourceFecPayloadId from the MMTP header (cursor-safe); RepairSink tracks current_sbn; 43 unit tests pass including an off-by-one guard at the SBN=0→1 boundary (ss_id=31 stays in SBN=0).

B3 — object_id_delta docs (47ca965)

Investigation result: object_id_delta encoding is wrong in the upstream Cloudflare moq-rs. Results documented in .planning/moq-rs-m1b-obj-id-delta-results.md; upstream bug filed separately.

B4 — G6 byte-diff docs (d972e3c)

Investigation result: moq-pub-mmtp and libmoq wire formats diverge at the SUBGROUP/OBJECT frame level by design (different draft versions). Documented in .planning/moq-rs-m1b-g6-bytediff-results.md; tracked for IETF draft-15+ alignment.

Test coverage

All 43 unit tests pass:

  • B1: accepts_fragmented_mfu_packets_at_fi_1_2_3 (regression guard for FI passthrough)
  • B2: repair_group_keyed_by_sbn_not_mpu_group, repair_group_advances_with_source_block, repair_before_source_fec_block_errors, parses_fec_source_packet_populates_source_fec_payload_id

Closes BLO-8047.

🤖 Generated with Claude Code

kkroo and others added 4 commits May 28, 2026 20:59
…8047 §B1)

Locks the publisher's MMTP fragmentation behavior as raw-passthrough:
each MMTP packet — Init *and* every MFU fragment with
fragmentation_indicator ∈ {0, 1, 2, 3} — is emitted as a separate MoQ
object in the (packet_id, mpu_sequence) subgroup. The publisher does
NOT interpret FI. Receivers reassemble using mmt-core::MfuReassembler.

ARCHITECTURAL PUSHBACK on the ADR's original M.1b plan ("if FI != 0,
publisher errors. Defer reassembly to M.1b") — same shape as the T8
pushback during M.1. Dimensional math (AMT MTU floor ≈ 1416 B after
IP/UDP/AMT/MMTP/MPU overhead; 4K I-frames need 220-1100 fragments;
8K needs 750-2900) shows an error-on-FI-non-zero rule would reject
every video stream above 1080p audio. Both real receivers (moqtail's
@moq/hang and Shaka via WASM) already consume raw MMTP packets and
reassemble themselves; publisher-side reassembly would force them to
undo it before re-CMAF for MSE.

Pinned end-to-end without changing dispatch logic:

* moq-pub-mmtp/src/mmtp_parse.rs — adds
  accepts_fragmented_mfu_packets_at_fi_1_2_3: route() handles
  FI=1/2/3 packets cleanly (all prior parser tests use FI=0 only).

* moq-pub-mmtp/src/publish.rs — adds
  fragmented_mfu_packets_share_one_subgroup_raw_passthrough: 4
  packets (Init + 3 MFU fragments) at the same mpu_seq land in
  exactly one subgroup, payloads preserved verbatim.

* moq-pub-mmtp/examples/synth_mmtp.rs — adds --fragment N flag and
  build_fragmented_mpu_sequence helper. Default 0 keeps the existing
  M.1 Init-only behavior; N >= 1 emits Init + N MFU fragments per
  MPU with FI sequence [0, 1, 2..., 2, 3] and consistent fragment
  counter. Helper unit-tested via cargo test --example synth_mmtp.

* .planning/m1-smoke.sh — adds FRAGMENT=N env knob threading
  through to synth_mmtp --fragment. Default 0 preserves M.1 smoke
  exactly; FRAGMENT=3 exercises the raw-passthrough fragmentation
  path end-to-end. Per-track sha256 matches at both FRAGMENT=0
  (hashes identical to .planning/moq-rs-m1-results.md) and
  FRAGMENT=3 (4x larger files, same byte-equality guarantee).

* .planning/moq-rs-m1-adr.md — replaces line 288 ("if FI != 0,
  publisher errors") with the raw-passthrough contract. Updates the
  CODEX summary to mark the fragmentation TODO closed by contract
  (not by code) and point B2/B3 at BLO-8047.

* .planning/moq-rs-m1b-frag-results.md — full smoke verdict: PASS at
  both FRAGMENT=0 (regression check) and FRAGMENT=3 (B1=C contract).
  Per-track sha256 table, mlog growth tracking, DoD coverage matrix.

Net: ~300 lines added, ~20 lines edited, 0 lines deleted. No changes
to dispatch logic or any runtime production path — B1=C closed the
contract by tests + documentation + a smoke-time toggle.

Tracks: BLO-8047 (M.1b umbrella) under BLO-4020 (Cloudflare moq-rs
migration). Lands as a follow-up PR off blockcast/blo-4020-m1.
* feat(mmtp): B2 SBN-keyed repair subgroup grouping (BLO-8047)

AL-FEC repair subgroups are now keyed by FEC Source Block Number (SBN)
instead of source MPU group_id, so the receiver can correlate repair
symbols to exactly the K=32 source symbols they protect per
draft-ramadan-moq-fec §6.1.

Changes:
- mmtp_parse: add `source_fec_payload_id: Option<SourceFecPayloadId>` to
  `PacketRouting`; parse SourceFecPayloadId from bytes after MMTP header
  when fec_type=1 (WithSourcePayloadId), using cursor-based Mpu parsing so
  the FEC payload ID bytes are consumed before MpuHeader reads begin.
- publish: add `FEC_K: u32 = 32` constant; add `current_sbn: Option<u32>`
  to `TrackState`; replace `RepairSink::current_group_id` with
  `current_sbn: Option<u32>`; update Mpu dispatch to track current_sbn from
  source packets with fec_type=1; update Repair dispatch to use current_sbn
  as repair group_id (errors when no FEC block seen yet).
- main: initialise new `current_sbn` fields in `build_state_map`.

Tests: rename `repair_group_advances_with_source_mpu` →
`repair_group_advances_with_source_block` (SBN boundary crosses MPU
boundary); add `repair_group_keyed_by_sbn_not_mpu_group` (two MPUs in same
FEC block → one repair subgroup; MPU in new block → new repair subgroup).
All 42 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(mmtp/b2): address Ally review — parse test, anchor cast, dedupe update

- mmtp_parse: add `parses_fec_source_packet_populates_source_fec_payload_id`
  test that builds a raw fec_type=1 byte buffer and calls route() directly,
  verifying cursor advances past SourceFecPayloadId before MpuHeader (Ally
  Important issue: route() FEC path was uncovered).
- mmtp_parse: add compile-time anchor `const _: u8 = FecType::WithSourcePayloadId as u8`
  so a discriminant change silently breaking parse is caught at compile time
  (Ally S2).
- publish: remove redundant inner current_sbn update inside new-group branch;
  the outer update after the if-else covers both new-group and continuation
  packets; clarify via comment (Ally S1).

43 unit tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(mmtp): address Ally round-2 review findings (BLO-8047)

- I1: replace 'const _: u8' with 'const _: () = assert!()' so the
  compile-time anchor actually fails the build if FecType::WithSourcePayloadId
  drifts from 0x01 (ISO/IEC 23008-1 §3.1)
- I2: add ss_id=31 dispatch + SBN=0 assertion to
  repair_group_advances_with_source_block; exercises the off-by-one
  boundary (31/32=0) that the 0→32 jump silently skipped
- S1: mpu_with_fec helper now uses FecType::WithSourcePayloadId as u8
  instead of the bare literal 1; consistent with production code
- S2: drop stale 'and 15' from repair_group_advances_with_source_block
  comment (ss_id=15 was never dispatched)
- S3: narrow repair_before_source_fec_block_errors assertion to match
  only 'before any source FEC block' (drop the redundant || branch)

---------

Co-authored-by: kkroo <60861014+kkroo@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…upstream

Codex #6 was right. Forensic analysis of the M.1b §B1 FRAGMENT=3 smoke
mlog dumps confirms the publisher hardcodes object_id_delta=0 for every
object on the wire. Root cause: moq-transport/src/session/subscribed.rs
line 281 has a literal comment ("before delta logic") and ships a
hardcoded 0.

Evidence (mlog grep):
- Publisher → relay direction: 40 subgroup_object_parsed events, ALL
  with object_id=0. 10 subgroups × 4 objects/subgroup matches the
  FRAGMENT=3 emission shape (5 MPUs × 2 tracks × Init+3 MFU fragments).
- Relay → subscriber direction: 40 subgroup_object_created events with
  object_id ∈ {0,1,2,3} — the relay's egress SubgroupWriter::next_object_id
  auto-increments and MASKS the publisher's bug for any subscriber that
  traverses moq-relay-ietf.

Impact:
- M.1 smoke passes byte-equality because moq-sub-raw doesn't use object_id
  AND the relay re-sequences on egress.
- Direct publisher → subscriber topology (no relay) would receive
  object_id=0 for every object. Any subscriber that uses object_id
  for ordering, dedup, or skip-ahead breaks.
- Multi-hop relay chains: only the first hop is wire-broken; relays
  re-sequence on each egress.

No moq-pub-mmtp change needed. moq-pub-mmtp consumes
SubgroupWriter::write correctly; the bug is one layer below, in how
SubgroupWriter serializes to the wire via the serve_subgroup task in
subscribed.rs.

Recommended fix is upstream (Cloudflare moq-rs). Tentative patch shape
captured in the results doc. Pending confirmation before filing the
upstream issue at cloudflare/moq-rs.

Tracks: BLO-8047 §B3 under BLO-4020.
…esign

Static code comparison between cast's moq-lite stack and moq-pub-mmtp's
IETF moq-transport draft-14 stack. Runtime side-by-side capture skipped
(cost > benefit; cast needs Traffic Ops + FFmpeg + libmoq mlog wiring
per CLAUDE.md). Static analysis covers the structural divergences a
runtime byte-diff would prove.

Key findings:

1. WIRE FORMATS DIVERGE AT THE SUBGROUP/OBJECT LAYER BY DESIGN.
   - moq-lite (hang-mmt-fec): single DataType::Group, header = 2 varints
     (subscribe_id + sequence), payload = stream tail. One MMTP packet =
     one Group stream. No subgroup_id, no object_id_delta concept.
   - IETF moq-transport draft-14 (moq-rs): SubgroupHeader with multiple
     stream-type variants (0x04-0x05 + extension flags), header =
     track_alias + group_id + subgroup_id + priority, then N objects
     with object_id_delta + extension_headers + payload_length + status.
     Multi-object subgroups by design.

2. CAST USES `moq_lite` DIRECTLY (NOT moq-lite::ietf).
   packages/cast/src/bridge_moq.rs: `use moq_lite::{...}`. The
   hang-mmt-fec/ietf module is a parallel implementation that's never
   imported by cast.

3. hang-mmt-fec/ietf EXPLICITLY DOES NOT SUPPORT MULTI-OBJECT SUBGROUPS.
   Comment at ietf/group.rs:64-66: "Since we don't support subgroups or
   object ID > 0, this is trivial to support." moq-pub-mmtp's
   FRAGMENT=3 smoke (4 objects per subgroup) would be rejected at
   decode time if pointed at a hang-mmt-fec/ietf receiver.

4. B3 OBJECT_ID_DELTA BUG IS A NO-OP FOR THE CAST PATH because cast
   uses moq-lite, not IETF moq-transport. B3 only matters for the
   M.4 (receiver migration) target.

5. B2 PER-FEC-BLOCK GROUPING IS A NO-OP FOR THE CAST PATH for the
   same reason — moq-lite has no subgroup/SBN concept.

Net implication for M.4: receiver migration is a major rewrite, not a
config switch. Receivers must learn:
- Multi-object SubgroupHeader decode + object_id_delta computation.
- IETF-style track_alias resolution (vs moq-lite's direct subscribe_id).
- Per-object framing state machine.

Full structural diff table + IETF draft-15+ tracking notes (overhead
measurement at high I-frame fragment counts, multi-object subgroup
parser coverage, first-object delta semantics) in the results doc.

Tracks: BLO-8047 §B4 under BLO-4020.
@allyblockcast

allyblockcast Bot commented Jun 21, 2026

Copy link
Copy Markdown
Author

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
reviewed head: d972e3c

All four findings from the #27 review (I1 route() untested, S1 redundant current_sbn update, S2 FecType as u8 no anchor, S3 no continuation-SBN test) are cleanly addressed here. B1 and B2 implementation looks correct; two new Important-level issues surfaced.


Important Issues (2)

  • [code] moq-pub-mmtp/src/publish.rsPacketType::Repair dispatch arm — Repair group assignment uses state.current_sbn (last seen from a source MPU packet) rather than the SBN carried in the repair packet's own RepairFecPayloadId. If a repair packet for block N+1 arrives before any source packet has advanced current_sbn to N+1 — which is valid at block boundaries when the encoder emits repair before the first source of the next block — the repair object lands in block N's group instead of N+1's. route() currently ignores RepairFecPayloadId entirely (only SourceFecPayloadId on fec_type=1 is parsed). Recommendation: parse RepairFecPayloadId from repair packets in route() and use routing.repair_fec_payload_id.sbn(FEC_K) directly in the Repair dispatch arm, falling back to state.current_sbn only when the repair packet carries no SBN.

  • [code] moq-pub-mmtp/examples/synth_mmtp.rs:636-639build_fragmented_mpu_sequence match arm (0, 1) => 0 emits FI=0 ("complete", per ISO/IEC 23008-1) for fragment_count=1, not FI=1. A caller using --fragment 1 silently gets a non-fragmented MFU packet, defeating the purpose of exercising the FI != 0 path. The CLI docstring says "values >= 2 exercise the raw-passthrough fragmentation path" but does not warn that --fragment 1 falls through to FI=0. The assert only rejects fragment_count == 0. Recommendation: add a CLI-level validator (value_parser = |s: &str| -> Result<u8, _> { ... assert >= 2 ... }) or change the assert to fragment_count >= 2 with a descriptive message, since fragment_count=1 as defined today is useless for B1 coverage.


Suggestions (2)

  • [tests] moq-pub-mmtp/src/mmtp_parse.rs:822synth_fec_source_packet sets mpu.payload_length = 0 but appends [0xAA, 0xBB] (2 bytes) after the MPU header. route() doesn't consume payload bytes so the test passes, but any future caller that feeds this helper's output to a full MMTP decoder will see a payload_length-vs-actual-bytes mismatch and either stop reading early or misalign the cursor. Set mpu.payload_length = 2 to match.

  • [code] moq-pub-mmtp/src/mmtp_parse.rs:762SourceFecPayloadId is parsed based solely on hdr.fec_type before hdr.packet_type is checked. A malformed repair packet with fec_type=1 would consume 4 bytes as SourceFecPayloadId and return a PacketRouting with source_fec_payload_id = Some(...), violating the field's documented invariant ("Absent on repair packets"). In dispatch() the current_sbn update is safely inside the Mpu arm so practical impact is nil on conformant streams. For structural correctness, gate the FEC parse with hdr.packet_type == PacketType::Mpu && hdr.fec_type == ....


Strengths

  • const _: () = assert!(FecType::WithSourcePayloadId as u8 == 1) is an elegant build-time anchor that eliminates the enum-vs-wire-value drift risk from feat(mmtp): B2 SBN-keyed AL-FEC repair subgroup grouping (BLO-8047) #27 S2.
  • SBN off-by-one boundary test (ss_id=31 → SBN=0, ss_id=32 → SBN=1) is exactly right for integer-division semantics and closes feat(mmtp): B2 SBN-keyed AL-FEC repair subgroup grouping (BLO-8047) #27 S3.
  • repair_group_keyed_by_sbn_not_mpu_group provides crisp RED→GREEN coverage for the core B2 invariant (two MPUs in same block share one repair group; third MPU in new block opens a new one).
  • B3 investigation (moq-rs-m1b-obj-id-delta-results.md) is unusually thorough: mlog grep evidence, root-cause line citation in upstream codebase, relay-masking explanation, and a concrete fix sketch with the right caveat about first-object-delta spec ambiguity.
  • fragmented_mfu_packets_share_one_subgroup_raw_passthrough in publish.rs covers the full dispatch contract at the right layer (one create_group call, four verbatim writes).

Recommended Action

  1. Evaluate the repair SBN ordering guarantee for the Blockcast MMTP encoder: if the encoder always emits all source packets for block N before any repair for block N, the Important BLO-4020 M.1: MMTP publisher on IETF moq-transport (draft-14+) #1 is effectively spec-impossible and can be noted as a known limitation. If not, fix before merge.
  2. Add --fragment CLI validation to reject fragment_count < 2 (Important BLO-8047 §B1: raw-passthrough fragmentation contract (stacked on #1) #2) before the smoke docs go stale.
  3. Fix Suggestions opportunistically.

Self-review comment mode: this PR was authored by the same bot identity; formal review/approval must come from a human or a distinct reviewer identity.

@kkroo
kkroo merged commit 0312307 into blo-4020-m1 Jun 22, 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.

1 participant