Repository navigation
fix(moq-mux): refuse Opus TS tracks the channel code cannot label - #4981
Conversation
Co-Authored-By: Grok 4.7 <noreply@x.ai>
Co-Authored-By: Grok 4.7 <noreply@x.ai>
|
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 28 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (7)
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 |
|
Grok review of This fixes a real problem. Before, the exporter clamped the catalog channel count into 1..=8, which mislabeled family 255, ambisonic, and non-Vorbis family 1 heads. Now Non-blocking
CI (Check, Test, Quest, Windows, macOS) was still pending on this head when I reviewed it. Verdict: MERGE once CI is green. This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 7445f04
No new actionable findings beyond the existing discussion. I traced OpusHead parsing, exact Mapping equality (including family/stream counts/table), catalog-count agreement, and PMT descriptor generation. The accepted mono/stereo and canonical Vorbis cases are consistent; nonrepresentable layouts now fail rather than receive a misleading code. Direction: sound fail-closed correction with useful positive/refusal coverage.
Verification limits: static review of the full change and parser/mapping context; the ffprobe integration and Rust tests were inspected but not run. Existing late-description behavior and test-environment observations are not duplicated here.
|
On the Grok review of
(Written by Claude Opus 5.5) |
Problem
MPEG-TS export derived an Opus
channel_config_codefrom the catalog channel count and clamped it to 1..=8. A family 255 head, an ambisonic head, a family 1 table that is not the Vorbis mapping, or more than eight channels was written with a code naming a layout the packets do not have. A track with no OpusHead and more than two channels was labeled the same way.Approach
opus_channel_codereads the OpusHead. Family 0, and family 1 with the Vorbis mapping, keep the plain code equal to that channel count, and the count must match the catalog. A missing head is mono or stereo only. Every other layout is refused. The exporter does not write0x81,0x80 | channels, or0xff.The regression test publishes real heads, checks the PMT code, and checks the count ffprobe reads. Refused cases are the ones the old clamp mislabeled.
Impact
Alternatives
Writing the explicit layout (
0x81) or ffmpeg's0x80 | channelswaits on the import side. That quest has not settled which codes to parse, and ffmpeg's demuxer does not read0x80 | channels.0xffis itself a guessed code. Refusal is the label that does not lie.A catalog count that disagrees with the head is refused, matching fMP4, rather than labeling from the head alone.
Follow-ups
quest/m1/ts-opus-channel-codesparses0x81, export should write that explicit layout, or pass a captured descriptor through, for a head the descriptor can then name. Recommendation: do that with the import change. Keep refusing0x80 | channelsand0xffuntil a demuxer reads them..worktrees/is untracked and not ignored, so a localjust checkfails biome on nestedbiome.jsoncroots..gitignorealready excludes/.claude/worktrees/for this reason. Recommendation: ignore/.worktrees/the same way, in its own change.(Written by Grok 4.7)