Repository navigation
refactor(mux)!: container formats name inputs - #6449
Conversation
Quest-Claim: 566f2385c06fd052
Delete ContainerFormat::Cmaf (and its moq-ffi, moq-c, and wrapper
mirrors) so a format names only what is parsed; "cmaf" parses as fmp4
again and "m2ts" is no longer a TS alias. Delete ContainerStream, a
copy of Container. Make ts::passthrough private, folding its errors into
moq_mux::Error. Container::cut and fmp4::Import::{cut, discontinuity}
return Result.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyiEy5FWz3z44DYZVo7vU
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EeyiEy5FWz3z44DYZVo7vU
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EeyiEy5FWz3z44DYZVo7vU
kixelated
changed the title
|
Outcome: quest finished with no open decisions, marked ready. (Written by Claude Opus 5.5) |
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
WalkthroughThe change removes the distinct CMAF container format and maps Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 51.35% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 12 files. (22 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely identifies the main refactor: container formats now name parsed inputs. Description check The pull request description is specific and directly relevant to the changeset. It explains the removal of ContainerFormat::Cmaf across moq-mux, moq-ffi, moq-c, and the Go and Dart wrappers, the remo… Full details: Description check
-
Explanation
The pull request description is specific and directly relevant to the changeset. It explains the removal of ContainerFormat::Cmaf across moq-mux, moq-ffi, moq-c, and the Go and Dart wrappers, the removal of the m2ts alias and ContainerStream, the privatization of ts::passthrough with its errors moved into moq_mux::Error, and the Result return-type changes for cut and discontinuity. It also gives migration steps and test results. The description matches the raw summary's files and the PR objectives.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 51.35% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 12 files. (22 skipped: 22 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e55967474
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Replaces main's new ContainerFormat::Cmaf release test with fmp4 passthrough, and adds the #6250 Fmp4 entry to the raw binding MIGRATE files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. 👍 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". |
# Conflicts: # quest/m1/api-audit/cli-flags.md
# Conflicts: # dart/moq_ffi/MIGRATE.md # go/ffi/MIGRATE.md # kt/moq-ffi/MIGRATE.md # quest/m1/api-audit/README.md # quest/m1/api-audit/container-format.md # swift/ffi/MIGRATE.md
kixelated
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ce0f0e239
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Main finished the CLI flags quest, so its deletion wins over this branch's edit to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # quest/m1/api-audit/README.md
# Conflicts: # quest/m1/api-audit/container-format.md
Finishes quest
quest/m1/api-audit/container-format.md:moq_mux::import::ContainerFormatnow names only the input it parses. Each format string means one thing.Changes
ContainerFormat::Cmafis gone from moq-mux, moq-ffi (MoqContainerFormat::Cmaf), moq-c (MOQ_CONTAINER_FORMAT_CMAF), the Go wrapper constant, and the regenerated Dart bindings. Nothing called it."cmaf"parses asFmp4again. CMAF passthrough is available only in Rust, throughfmp4::Import::with_passthroughandmoq_hls::import::Config::passthrough, which the CLI already uses."m2ts"no longer parses asTs. In the catalog,m2tsmeans the TS passthrough track, and on disk it means BDAV 192-byte packets, which the TS demuxer cannot read. No caller used the alias.ContainerFormat::Cmaf. It now buildsfmp4::Import::with_passthrough(true)directly, so the coverage stays.import::ContainerStreamis deleted. It was a line-for-line copy ofContainer.Container::newwith an emptyContainerInit::datadoes the same thing, and the split-stream test now covers that path.ts::passthroughmodule is private. Onlyts::Passthroughis exported. Itsdecodeandfinishreturnmoq_mux::Resultinstead ofanyhow. The four error structs are now themoq_mux::Error::{TsPacketSize, PcrRewind, PcrRestart, PcrGap}variants. The publicFALLBACK,GROUP_BYTES_MAX, andGROUP_OBJECTS_MAXconstants are crate-private, and the module docs moved ontoPassthrough.import::Container::cut,fmp4::Import::cut, andfmp4::Import::discontinuityreturnResult, likeimport::Track's. Callers in moq-hls, moq-c, and moq-ffi propagate it.doc/lib/{c,cpp,dart,go,kt,py,swift}. Migration notes are in the Public API section below rather thanMIGRATE.md, which PRs no longer write since moq-dev/moq#6610.Choices made without asking
ts::passthrough::{Import, Export}rename. The quest only said to consider it, and it would need the module to be public again, which goes against making it private.ts::Passthrough/ts::Releasestay.fmp4::Import::cutalso returnsResult. The quest named onlydiscontinuity.Container::cutdelegates to it, and the HLS importer calls both, so I made the two consistent.doc/lib/layers.mdis unchanged. It never mentionedContainerStream.Public API
Breaking:
ContainerFormat::Cmaf,import::ContainerStream,container::ts::passthrough::*(exceptts::Passthrough) are removed. The"m2ts"alias is gone.Passthrough::{decode, finish}returnmoq_mux::Result.Container::cutandfmp4::Import::{cut, discontinuity}returnResult. FourErrorvariants are added (the enum is#[non_exhaustive]).MoqContainerFormat::Cmafis removed.MOQ_CONTAINER_FORMAT_CMAFis removed.ContainerFormatvalue. Go's was the hand-writtenmedia.ContainerFormatCmaf. The others re-export the generated enum.The CMAF enum values were added in moq-dev/moq#6250 and never released. For a binding user, the net change since the last release is the corrected #6250 note below.
For the breaking cut: the
## UnreleasedfMP4 entries #6250 left onmaintell readers to pass the CMAF format value this PR deletes (rs/moq-ffi,rs/moq-c,py/moq-rs,swift,kt/moq,dart/moq, andgo/wrapperMIGRATE.md), andrs/moq-mux/MIGRATE.mdsays"cmaf"selectsContainerFormat::Cmaf. The notes below supersede those entries; collapse them at the cut.How to migrate:
Fmp4format (MoqContainerFormat::Fmp4/.fmp4/.FMP4/MoqContainerFormatFmp4,moq::ContainerFormat::kFmp4,MOQ_CONTAINER_FORMAT_FMP4,media.ContainerFormatFmp4) now publishes one Legacy frame per sample instead of one CMAF frame per fragment. A reader that follows the catalog's container needs no change. Republishing each fragment whole as CMAF is Rust-only, throughmoq_mux::container::fmp4::Import::with_passthrough. Drop any use of the unreleased CMAF format value.import::ContainerFormat::Fmp4(and"cmaf", still its alias) unwraps to Legacy;import::Containerhas no passthrough, so build anfmp4::Importwith.with_passthrough(true)to keep CMAF.ContainerStream: callimport::Container::newwithContainerInit::new(format, Bytes::new())instead; a container recovers its own framing, so it takes chunks split anywhere."m2ts": it no longer parses asContainerFormat::Ts; pass"ts","mpegts", or"mpeg2ts".ts::passthrough: the module is private;ts::Passthroughstays, and itsdecodeandfinishreturnmoq_mux::Result. Matchmoq_mux::Error::{TsPacketSize, PcrRewind, PcrRestart, PcrGap}where you downcast toPacketSizeError,PcrRewindError,PcrRestartError, orPcrGapError.passthrough::{FALLBACK, GROUP_BYTES_MAX, GROUP_OBJECTS_MAX}are gone; thets::Passthroughdocs describe the bounds.import::Container::cut,fmp4::Import::cut, andfmp4::Import::discontinuityreturnResult<()>, likeimport::Track's; propagate it with?.Wire: none. CLI: none, since
--passthroughis unchanged and no flag parses these strings.Checks
Re-merged main through 83df8615c6 after a third conflict drop (the api-audit quest file only: main edited it for moq-dev/moq#6610, and this branch's deletion wins).
just check(including the rs, py, js, cpp, obs, and dart checks) andjust test interop --all(172 checks) pass on the merged head.Re-merged main again through f206893047 after a second conflict drop: main finished the CLI flags quest (moq-dev/moq#6522), so its deletion wins over this branch's edit to
cli-flags.md.just check(including the rs, js, and py tests) andjust test interop --allpass on 636c8f3.Before that: re-merged main through 4ccf05aac6 after the queue dropped this PR on a conflict (
MIGRATE.mdand the api-audit quest README only; everyMIGRATE.mdtakes main's version and the notes moved to Public API above).just checkandjust test interop --all(172 checks) pass on the merged head.Earlier: After merging main:
just checkpassed (js, rs, quest, py, kt, swift, go, dart, plus the rs/js/py tests). The dart RSS leak testa non-null optional argument is releasedfailed twice under machine load and then passed, and it passed on main, so it is load noise, not this PR.quest check --base origin/mainpassed.just test interop --allpassed.Follow-ups
fmp4::Import::discontinuityonly resets decode times.Track::discontinuitypublishes hang's discontinuity marker. An HLSEXT-X-DISCONTINUITYprobably should publish the marker too, which would give the newResulta real failure path.ts::Import,ts::Export, andts::Releasestill report errors throughanyhow(Error::Other). Folding them into typedmoq_mux::Errorvariants is the same work this PR did for passthrough, at a larger scale.🤖 Generated with Claude Code
https://claude.ai/code/session_01EeyiEy5FWz3z44DYZVo7vU
(Written by Claude Opus 5.5)