Repository navigation
feat(ffi)!: fold the container stream importer into ContainerProducer - #5919
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A container recovers its own framing, so MoqMediaContainerProducer already takes chunks split anywhere. MoqMediaContainerStreamProducer and MoqContainerStreamInit were the same importer minus cut/seek and the leading bytes; delete them from moq-ffi and every wrapper. Completes the ffi-shape epic, whose last child settled this name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Quest outcome (background agent, so the open decisions have not been put to the maintainer yet):
(Written by Claude Opus 5.5) |
|
Maintainer decision: delete I re-checked the (Written by Claude Opus 5.5) |
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. |
|
Warning Review limit reachedYou'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 40 minutes. View limit details
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 |
![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: ad2e223324
ℹ️ 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".
|
Merging. Summary:
Follow-up: delete the now-unused (Written by Claude Opus 5.5) |
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
Problem
MoqMediaContainerStreamProducer(media.ContainerStreamProducerin the wrappers) is a 5-word generated name, the shape the maintainer called terrible on #5356. The quest asked whether to rename it by role.Looking at what it does, it does not need a name at all. A container recovers its own framing, so
moq_mux::import::ContainerandContainerStreamboth use the sameContainerImpl, and moq-mux says a whole chunk and a stream chunk decode the same way. The stream importer isMoqMediaContainerProducerwith empty leading bytes and withoutcut/seek.Approach
Delete
MoqMediaContainerStreamProducerandMoqContainerStreamInitfrom moq-ffi and every wrapper. To feed a raw byte stream, constructmedia.ContainerProducerwith emptydataand write chunks split anywhere. Its docs now say that.This was the last child of the
ffi-shapeepic, and the epic's own work (layer namespaces, the layers docs page, thers/moq-ffi/AGENTS.mdpattern) has landed. So this PR deletes the epic and its references. That unblocksm1/c/package,m1/auth/bindings, andm1/auth/token-in-band.Decisions
Settled by the maintainer: delete the stream importer and use
media.ContainerProducerwith emptydata.ContainerProduceris a superset because it also hascutandseek.media.TrackStreamProducerstays because it really is different: a codec stream infers frame boundaries, whileTrackProducertakes timestamped frames.media.StreamProducer, or keepingmedia.ContainerStreamProducer.Also approved: delete the
ffi-shapeepic now that its last child lands.Public API impact
Breaking binding API. None of it has shipped:
releasestill hasBroadcastProducer.publish_container_stream. No wire change.rs/moq-ffi: removedMoqMediaContainerStreamProducerandMoqContainerStreamInit.MoqMediaContainerProducerdocs say chunks may split anywhere.py/moq-rs: removedmoq.media.ContainerStreamProducerandContainerStreamInit.go/wrapper: removedmedia.NewContainerStreamProducer,ContainerStreamProducer, andContainerStreamInit.swift: removedMedia.ContainerStreamProducerandMedia.ContainerStreamInit.kt,dart: removed theContainerStreamProducerandContainerStreamInitaliases.dart/moq_ffiis regenerated.cpp: removedmoq::MediaContainerStreamProducerandmoq::ContainerStreamInit.cpp/obsnever used them.rs/moq-c,doc/lib/c,doc/bin/obs.md: unchanged. moq-c never had a stream importer; its container takes whole chunks.rs/moq-mux: unchanged.import::ContainerStreamis now unused in the repo (see follow-ups).MIGRATE.md(moq-ffi, py, go, swift, kt, dart, cpp): the unreleased "byte-stream importers take an init record" entry is replaced by "byte streams use the container producer", with an example.doc/lib/{py,go,swift,kt,dart,cpp},doc/lib/layers.md,py/moq-rs/README.md,py/moq-rs/docs/index.md,go/wrapper/README.md.Validation
just checkpasses on the branch.just test interop --all: all checks passed.ContainerProducers. moq-mux'sa_split_stream_publishes_its_tracksalready shows that a container decodes from chunks split mid-header.Follow-ups
moq_mux::import::ContainerStream. It has no users left, andContainerwith emptyContainerInit::datadoes the same thing. It is a breaking change to moq-mux, so it is not in this PR.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code