refactor(moq-mux)!: split the fMP4 init segment out of Fragment - #2692
Conversation
`Fragment` described two different emits with one struct. The init segment carried `init: true`, `independent: false` and `duration: 0.0`, while every media fragment carried `init: false` and meant the other two. Three of its four fields were sentinels on one of the two shapes it had to cover. `Export::next_chunk` now returns a `Chunk`, whose `Init` variant holds the bytes and nothing else, and whose `Fragment` variant holds a `Fragment` with no dead field. A consumer that only wants bytes stays on `next`, which is unchanged. `Chunk` is left exhaustive rather than `#[non_exhaustive]`: the set is closed by the container format, which emits an init segment and media segments and nothing else, so a wildcard arm would be permanent noise on every match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…it/fragment accessors Fragment::duration was f64 seconds; the computation already ran in std::time::Duration and flattened at the end, so carry the Duration through and let a playlist writer call as_secs_f64 itself. Chunk::init and Chunk::fragment replace the panicking test helpers, so a consumer expecting one variant writes chunk.fragment().expect(..). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Reviewed the final head after syncing with dev and adding the typed-time guidance. The chunk API cleanly separates init data from media fragment metadata, the Duration change preserves exact internal values, and the two prior review requests are implemented. just check and all 482 moq-mux nextest tests passed. No blocking findings.
(Written by GPT-5)
kixelated
left a comment
There was a problem hiding this comment.
Reviewed the final head after merging the latest dev. The upstream commit is unrelated to the fMP4 API change and merged without conflicts. The full Rust CI test stage passed all 2,955 tests locally, the scoped repository checks passed on the prior equivalent head, and the final diff passes git diff --check. No blocking findings.\n\n(Written by GPT-5)
Summary
fmp4::Fragmentdescribed two different emits with one struct. The init segment carriedinit: true,independent: false, andduration: 0.0; every media fragment carriedinit: falseand meant the other two. Three of its four fields were sentinels on one of the two shapes it covered, and a consumer had to read aboolto know which fields were real.Export::next_chunknow returns aChunk:Initholds the bytes and nothing else, whileFragmentholds aFragmentwith no dead field.Export::nextis unchanged for consumers that only want bytes.Review feedback also moved
Fragment::durationfrom untypedf64seconds tostd::time::Duration, and added consumingChunk::initandChunk::fragmentaccessors.rs/CLAUDE.mdnow records the repository convention to use typed time values and convert numeric serialized forms at the boundary, usingserde_withwhere possible.Chunkis deliberately not#[non_exhaustive]. The set is closed by the container format, which emits an init segment and media segments and nothing else, so a wildcard arm would be permanent noise on every match.Public API changes
fmp4::ChunkInit(Bytes),Fragment(Fragment)Chunk::data,Chunk::into_dataChunk::init,Chunk::fragmentFragment::initFragment::durationf64seconds tostd::time::DurationExport::next_fragmentnext_chunk, now returnsOption<Chunk>Export::poll_next_fragmentpoll_next_chunk, now returnsOption<Chunk>Export::next/poll_nextThis targets
devbecause it breaks publishedmoq-muxAPIs.Test plan
Run on the final head after merging the latest
origin/dev:nix develop --command just fixnix develop --command just checknix develop --command just rs test -p moq-mux: 482 passed, 0 failedjust test smoke-fullwas not run because this changes no wire format,moq-ffiAPI, or gateway behavior.Cross-Package Sync
No table rows apply.
moq-muxhas no cross-package row, no wire format or CLI surface changed, anddoc/lib/rs/crate/moq-mux.mddescribes the crate's scope rather than this API.(Written by GPT-5)