Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MPEG-TS names an Opus layout only by channel_config_code, so 3 to 8 channels produced no OpusHead and moq-audio refused the track. Synthesize the Vorbis family 1 mapping and let Config::encode emit mapping tables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mapping::new builds a channel mapping table from its parts and refuses family 0, a channel count the family forbids, or an inconsistent table. Parse and the Vorbis default go through it. Plan the TS export refusal, the TS extended channel codes, and GStreamer surround Opus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Four positional u8-ish arguments are easy to transpose; name them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mapping::new(mapping::Config { .. }) reads by role. opus::Mapping stays
re-exported at its published path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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: 9083694d4a
ℹ️ 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".
| /// Refuses family 0, a channel count the family does not allow, and a table | ||
| /// that names no streams, more coupled streams than streams, or a channel | ||
| /// past the decoded ones. | ||
| pub fn new(config: Config<'_>) -> Result<Self> { |
There was a problem hiding this comment.
At this commit, repo-wide callers of this constructor are limited to internal parsing, Mapping::vorbis, and this module's tests; the only external use described is a future GStreamer quest. Publishing mapping::Config and Mapping::new now unnecessarily freezes this shape into moq-mux's API even though the TS change can remain crate-private, so keep them private until that consumer lands.
AGENTS.md reference: AGENTS.md:L55-L60
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Keeping it public: this was a deliberate call recorded under Decisions in the PR description. opus::Config::mapping is already a public field and encode now emits any mapping, so without a public constructor an outside crate could only obtain a Mapping by parsing a head it already has. Mapping::new validates every field (the struct fields stay private), and the GStreamer surround quest in this PR (quest/m1/audio-codecs/gst-opus-surround.md) is the planned consumer. Mapping::vorbis stays crate-private since nothing outside moq-mux needs it yet.
(Written by Claude Opus 5.5)
|
Ready to land. CI is green (Check, Test, Android) on 9083694, the base is clean, and no fixups were needed. The one Codex finding (keep (Written by Claude Opus 5.5) |
Align the line's Quests list with #4283 and drop the completed ADTS refusals link. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Completes the
quest/m1/audio-codecs/ts-opus-surround.mdquest and deletes it. Targets the audio-codecs line (#4081).Summary
An MPEG-TS Opus stream names its layout only by
channel_config_code. For 3 to 8 channels the importer builtopus::Config::new(48_000, channels), whoseencoderefused more than two channels, so the catalog carried no OpusHead andmoq-audiorefused the track.opus::Mapping::new(mapping::Config { family, streams, coupled, table }), in a newopus::mappingmodule withopus::Mappingstill re-exported at its old path, is a public, validated constructor (Mapping's fields stay private, so every mapping is checked): it refuses family 0, a channel count the family forbids, and an inconsistent table.Config::parsenow builds its mapping through it.opus::Mapping::vorbis(channels)(crate-private) builds the family 1 mapping libopus and ffmpeg use for 1 to 8 channels (RFC 7845 §5.1.1.2) on top ofnew.opus::Config::encodeemits the mapping table of any family instead of refusing it, so a parsed head also re-encodes intact.opus_5_1.ts(ffmpeg libopus, center-only 440 Hz tone): a moq-mux test checks the imported head, and a newmoq-audiointegration test decodes it end to end to 5.1 with the tone on the center channel. Both fail without the importer change.Public API and wire impact
moq_mux::codec::opus::Config::encodenow succeeds for amappingwhose channel count matcheschannel_count, instead of returningError::UnsupportedMappingFamily. A mismatch isError::UnsupportedChannelCount.opus::mappingmodule holdingMapping(still reachable asopus::Mapping) andmapping::Config<'a>(public fields), plusMapping::new(mapping::Config) -> Result<Mapping>.Error::UnsupportedMappingFamilyis now what it returns for family 0 (message reworded), instead of anencodeerror.description.Decisions
Mapping::newis public and validated;Mapping::vorbisstays crate-private until a consumer outside moq-mux needs it.UnsupportedMappingFamilystays on main, now returned byMapping::newfor family 0.Test plan
just checkcargo test -p moq-audio --test importandcargo test -p moq-mux --lib import_opus_surroundfail with the importer change reverted and pass with it🤖 Generated with Claude Code
(Written by Opus 5.5)