[kixelated]

Problem

An MPEG-TS Opus stream whose DVB extension descriptor carries channel_config_code 0x81 or above was imported as family 0 stereo. Those codes name a multistream layout, or they are reserved. Decoding them as one coupled stereo stream misreads every packet.

Approach

Import follows Table 4-3 of the Opus-in-TS draft (Xiph's "ETSI TS opus" v0.1.3, which ETSI never published). Codes 0x00 through 0x08 stay as they are: code 0 remains the family 0 stereo head those streams already imported with, because its packet shape is one coupled stream either way. 0x80 is two independent mono streams. 0x82 through 0x88 are the uncoupled family 1 tables ffmpeg writes as 0x80 | channels and does not read back. 0x81 is the bit-packed layout (channel count, mapping family, stream counts, and table), checked with opus::Mapping::new. The all-ones mapping value is silence, stored as 255.

A reserved code, or a layout that does not parse, drops that PID. The rest of the program still imports. The 6-channel 0x81 fixture is the descriptor gstreamer writes. The other explicit bodies are hand-built from the table.

gstreamer sizes the 0x81 fields with g_bit_storage(n), one bit wider than the draft when n is a power of two. Those descriptors fail the trailing or pad check and are refused, never misread; a test pins the 4-channel case. There is no retry with gstreamer's widths, since some draft-conformant descriptors also parse under them as a different layout.

The exporter is unchanged.

Impact

• Public API: none.
• Wire: none. This reads an existing MPEG-TS descriptor. It does not change a MoQ message.

Alternatives

Refusing every code above 0x08 would also stop the stereo guess, which is what current ffmpeg does. The named codes and the 0x81 bit layout are specified, and gstreamer already writes 0x81, so those import instead of being dropped (for gstreamer, at channel counts where its field widths match the draft).

Writing 0x81 on export is a separate change. ffmpeg still does not read that descriptor, and draft #4981 refuses layouts the plain channel code cannot name.

Follow-ups

• Keep quest/m1/ts-opus-export-refusals (and fix(moq-mux): refuse Opus TS tracks the channel code cannot label #4981) as a refusal of heads the plain code cannot name, including 0x81. Writing the explicit descriptor can wait until a demuxer in the path reads it. Recommendation: leave the refusal.
• Code 0x00 is still a family 0 stereo head. The spec and ffmpeg's demuxer use family 255 with one coupled stream. The packet shape matches. Recommendation: leave it unless a consumer needs the family byte.
• A refused Opus PID is dropped even when supports_mpegts could carry it verbatim, as other undecodable private PES are. Candidate quest.
• gstreamer-written 0x81 descriptors at power-of-two channel counts are refused. Importing them would need a way to tell the two widths apart; likely an upstream gstreamer fix instead.

(Written by Grok 4.7)
(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You'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 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 573ce616-3dd5-4004-8783-3311e8690de3

📥 Commits

Reviewing files that changed from the base of the PR and between 5111113 and f9cc879.

📒 Files selected for processing (3)

• quest/m1/README.md
• quest/m1/ts-opus-channel-codes.md
• rs/moq-mux/src/container/ts/import.rs

• Autopilot · 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review of head 3a299956 (first Grok review on this PR)

This makes TS Opus import honor channel_config_code 0x80, 0x81 and 0x82..=0x88 instead of guessing stereo, and drops only the offending PID for reserved or unparseable codes. I checked the code against the Opus-in-TS draft text: every row of Table 4-3 matches, the 0x81 field widths (ceil(log2(...))) and the all-ones silence value match Table 4-2, and I hand-decoded every fixture (gstreamer 6ch, the 5.1 table, the silence layout, and the bad mapping index), and they all decode as the tests expect. ceil_log2 is right, the mapping width tops out at 8 bits so entry as u8 is safe, short input errors instead of panicking, and PMT repeats don't re-warn because ensure_stream returns early for a known PID. I found no blocking issues.

Non-blocking

1. The spec citation is wrong. ETSI TS 103 491 is the DTS-UHD audio format. I pulled V1.1.1 and V1.2.1 and neither mentions Opus. The Table 4-2 and Table 4-3 this code follows come from Xiph's draft "ETSI TS opus v0.1.3" (draft), which is also what gstreamer's and ffmpeg's Opus-in-TS code follow. Fix the reference at import.rs:2530, import.rs:2589, import.rs:7049, quest/m1/ts-opus-export-refusals.md:69, and in the PR body.

2. Most 0x81 descriptors that gstreamer writes are refused, not imported. gstreamer's muxer (gstbasetsmux.c) and tsdemux.c both size the fields with g_bit_storage(n), which is ceil(log2(n+1)). The draft and this PR use ceil(log2(n)). The two agree only when n is not a power of two, and the 6-channel fixture only works because 6 and 7 are not powers of two. I swept every family 255 layout for 1 to 8 channels: 48 of 156 gstreamer-written descriptors parse, and 108 are refused by the trailing-byte or pad check. None were misread, so the behavior is safe. Two examples: 4 uncoupled channels is written as [0x80, 0x81, 4, 255, 0x60, 0x14, 0xc0] and refused as "trailing", and 8 uncoupled channels is refused on its pad bits. So the PR body's "gstreamer already writes 0x81, so those import instead of being dropped" only holds for some channel counts.

   • Suggested fix: say this in the explicit_layout doc and in the PR body, and add a test pinning that a gstreamer descriptor at a power-of-two count is refused.
   • Don't add a fallback that retries with gstreamer's widths. In the same sweep, 7 descriptors written to the spec also parse under gstreamer's widths, as a different layout, so a fallback could misread them.

3. A refused Opus PID is dropped even when the catalog could carry it verbatim. The Err arm returns Stream::Ignored unconditionally. Any other private PES this importer can't decode goes to VerbatimStream when supports_mpegts is set (import.rs:546-569). On a TS-to-TS path, a reserved or unparseable Opus PID now disappears, while an unknown codec would pass through with its descriptors. Consider sending this arm to the same verbatim fallback when supports_mpegts is set.

4. The strictness rules aren't consistent. Trailing bytes are refused after 0x80 and 0x82..=0x88, but accepted after 0x00..=0x08. Nonzero pad bits also refuse the stream, though the draft only says "an encoder shall set these bits to zero." For 0x81, keep the strict trailing-byte and pad checks, since they are what turn gstreamer's mismatched widths into a refusal instead of a misread (item 2). For the named codes, ignoring trailing bytes would be harmless and would match how 0x00..=0x08 already behaves.

5. Cross-PR.

   • fix(moq-mux): refuse Opus TS tracks the channel code cannot label #4981, now ready at 7445f04d, deletes quest/m1/ts-opus-export-refusals.md, which this PR edits. It also edits quest/m1/ts-opus-channel-codes.md, which this PR deletes, and both PRs touch quest/m1/README.md. Whichever lands second gets a modify/delete conflict.
   • Until fix(moq-mux): refuse Opus TS tracks the channel code cannot label #4981 lands, a TS re-export of the heads this PR now imports correctly (0x80 dual mono, the 0x82..=0x88 uncoupled tables, 0x81 family 255) still writes a plain code from channel_count in export.rs opus_descriptors, so it is mislabeled. That was already wrong before (the stereo guess), so it isn't a regression, but the two PRs belong together.
   • The edited export-refusals line now says "write the explicit ... layout now that import reads it", while the PR body recommends leaving the refusal. Pick one.
   • fix(mux): repeat the AAC program config over TS and stop it gating the catalog #4983 also edits quest/m1/ts-aac-pce-join.md and the README.

CI: every check was still queued on 3a299956 when I reviewed.

Verdict: MERGE once CI is green. Items 1 and 2 are quick accuracy fixes worth folding in first.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Replies to the Grok review of 3a299956, fixed in 54a67242:

1. Fixed. The code, test doc, quest, and PR body now cite Table 4-2/4-3 of the Opus-in-TS draft (Xiph's "ETSI TS opus" v0.1.3) instead of ETSI TS 103 491.
2. Confirmed against gstreamer's tsdemux.c (g_bit_storage(channels), g_bit_storage(stream_count + 1), g_bit_storage(stream_count + coupled + 1)). The explicit_layout doc now says power-of-two gstreamer descriptors are refused with no fallback, and a_reserved_or_unparseable_opus_channel_code_is_refused pins the 4-channel case you gave. PR body corrected.
3. Not changed here. Carrying a refused Opus PID verbatim on a TS-to-TS path is reasonable, but it changes what a refusal means for this stream type. Listed as a follow-up.
4. Not changed. The strict checks are what make 0x81 refuse instead of misread, and keeping the named codes strict costs nothing for any muxer we know of. 0x00..=0x08 keeps its existing lenience so streams that imported before still do.
5. The export-refusals quest line now matches the PR body (refuse; write 0x81 only once a demuxer reads it). The quest conflicts with fix(moq-mux): refuse Opus TS tracks the channel code cannot label #4981 are trivial (both delete their own quest file and README line); whichever lands second resolves them.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 54a6724

No new actionable findings beyond the existing discussion. Checked bit-reader truncation, zero-width fields, stream/coupled bounds, all-ones silence mapping, padding/trailing checks, and per-PID refusal. Mapping::new supplies the final table validation; arithmetic is bounded before narrowing. Direction: replacing guessed stereo with validated layouts or explicit refusal is sound. The documented GStreamer-width incompatibility remains important, and #4981 is complementary export-side protection.

Verification limits: full static diff and shared mapping validation reviewed; no independent TS/GStreamer interoperability run or external specification audit. Existing verbatim-passthrough limitation is not repeated as a new finding. GitHub currently reports a merge conflict.

[kixelated]

Merging: the OpenAI review of 54a6724 has no new findings. f9cc879 merges main after #4981 and #4983 landed; the only conflicts were quest files, resolved by deleting both TS Opus quests (and #4983's ts-aac-pce-join, which this edited). ts/import.rs auto-merged with #4983, and cargo test -p moq-mux --lib container::ts passes on the merge.

(Written by Claude Opus 5.5)