From ea72e6df5f7e536a8088802d32631c4d0fd359f6 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:34:53 -0700 Subject: [PATCH] Give every audio and video codec its own registered Matroska CodecID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MkvTrack::audio's catch-all was `_ => CODEC_AC3`, and ebml.rs defined no A_AAC, A_MPEG/L2, A_MPEG/L3, A_FLAC or A_OPUS constant at all. A CodecID names the payload, so any of those codecs was written into the MKV declaring AC-3 while carrying something else — a player either refuses the track or decodes noise. Reachable through two ordinary paths, both verified: ifo.rs:577 maps DVD audio_coding_mode 3 to Codec::Mp2, so a DVD with MPEG audio muxed to mkv:// produced a track declaring A_AC3 over MP2 bytes; and mp4/read.rs:479 maps the `mp4a` sample entry to Codec::Aac for an mp4:// source. codec/mod.rs already has working parsers for MP2, MP3, AAC, FLAC and Opus, so the pipeline carried these codecs end to end and only the container label was wrong. MkvTrack::video had the same shape: `_ => CODEC_MPEG2` announced Codec::Mpeg1 and Codec::Av1 as MPEG-2 video. V_MPEG1 and V_AV1 added. Both catch-alls stay, because MkvTrack::audio/video return Self and have no error channel, but they are now reachable only by a non-audio/non-video or Unknown codec routed there in error. Two tests enumerate every real codec of each kind and cross-check each against Codec::kind(), so a codec added to the enum later cannot silently inherit another codec's ID. Both proven red first: Aac declared A_AC3 before the fix. --- src/mux/ebml.rs | 17 ++++++++ src/mux/mkv.rs | 109 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/mux/ebml.rs b/src/mux/ebml.rs index c3a899b..811c660 100644 --- a/src/mux/ebml.rs +++ b/src/mux/ebml.rs @@ -582,11 +582,28 @@ pub const CODEC_HEVC: &str = "V_MPEGH/ISO/HEVC"; pub const CODEC_H264: &str = "V_MPEG4/ISO/AVC"; pub const CODEC_VC1: &str = "V_MS/VFW/FOURCC"; pub const CODEC_MPEG2: &str = "V_MPEG2"; +/// MPEG-1 Video. Distinct from V_MPEG2: a decoder selects its bitstream +/// parser from this ID. +pub const CODEC_MPEG1: &str = "V_MPEG1"; +/// AV1. CodecPrivate carries the AV1CodecConfigurationRecord. +pub const CODEC_AV1: &str = "V_AV1"; pub const CODEC_AC3: &str = "A_AC3"; pub const CODEC_EAC3: &str = "A_EAC3"; pub const CODEC_TRUEHD: &str = "A_TRUEHD"; pub const CODEC_DTS: &str = "A_DTS"; pub const CODEC_PCM_BE: &str = "A_PCM/INT/BIG"; +/// AAC. The generic registered ID; the AudioSpecificConfig travels in +/// CodecPrivate, so no profile suffix is needed (and the `A_AAC/MPEG4/*` +/// suffixed forms are legacy). +pub const CODEC_AAC: &str = "A_AAC"; +/// MPEG-1/2 Audio Layer II — DVD audio_coding_mode 3. +pub const CODEC_MP2: &str = "A_MPEG/L2"; +/// MPEG-1/2 Audio Layer III. +pub const CODEC_MP3: &str = "A_MPEG/L3"; +/// FLAC. CodecPrivate carries the STREAMINFO metadata block. +pub const CODEC_FLAC: &str = "A_FLAC"; +/// Opus. CodecPrivate carries the OpusHead identification header. +pub const CODEC_OPUS: &str = "A_OPUS"; pub const CODEC_PGS: &str = "S_HDMV/PGS"; pub const CODEC_VOBSUB: &str = "S_VOBSUB"; diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index 367b032..0d1ecd7 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -368,6 +368,12 @@ impl MkvTrack { Codec::Hevc => ebml::CODEC_HEVC, Codec::Vc1 => ebml::CODEC_VC1, Codec::Mpeg2 => ebml::CODEC_MPEG2, + Codec::Mpeg1 => ebml::CODEC_MPEG1, + Codec::Av1 => ebml::CODEC_AV1, + // Every video codec this crate can produce is named above. The + // remaining arm is reached only by a non-video or Unknown codec + // routed here in error — see the audio counterpart for why this + // keeps a fallback rather than erroring. _ => ebml::CODEC_MPEG2, }; // An Unknown resolution has no real dimensions — emit (0, 0) so the @@ -492,6 +498,17 @@ impl MkvTrack { Codec::TrueHd => ebml::CODEC_TRUEHD, Codec::DtsHdMa | Codec::DtsHdHr | Codec::Dts => ebml::CODEC_DTS, Codec::Lpcm => ebml::CODEC_PCM_BE, + Codec::Aac => ebml::CODEC_AAC, + Codec::Mp2 => ebml::CODEC_MP2, + Codec::Mp3 => ebml::CODEC_MP3, + Codec::Flac => ebml::CODEC_FLAC, + Codec::Opus => ebml::CODEC_OPUS, + // Every audio codec this crate can produce is named above. The + // remaining arm is reached only by a non-audio or Unknown codec + // routed here in error; A_AC3 is the historical fallback and is + // wrong for such a track, but MkvTrack::audio has no error channel — + // `every_audio_codec_gets_its_own_registered_codec_id` pins the real + // codecs so a newly-added one cannot silently land here instead. _ => ebml::CODEC_AC3, }; // Unknown sample rate / channel layout: emit 0 so the serializer omits @@ -2380,6 +2397,98 @@ mod tests { ); } + #[test] + fn every_audio_codec_gets_its_own_registered_codec_id() { + // A CodecID names the payload. Falling through to A_AC3 for a codec the + // match had no arm for writes a track declaring AC-3 while carrying + // something else entirely — a player either refuses the track or decodes + // noise. Every audio codec this crate can produce must map to its own + // registered Matroska ID. + // + // Reachable: ifo.rs maps DVD audio_coding_mode 3 to Codec::Mp2, and + // mp4/read.rs maps the `mp4a` sample entry to Codec::Aac, so a DVD with + // MPEG audio and an mp4:// AAC source both hit this path. + for (codec, want) in [ + (Codec::Ac3, "A_AC3"), + (Codec::Ac3Plus, "A_EAC3"), + (Codec::TrueHd, "A_TRUEHD"), + (Codec::Dts, "A_DTS"), + (Codec::DtsHdHr, "A_DTS"), + (Codec::DtsHdMa, "A_DTS"), + (Codec::Lpcm, "A_PCM/INT/BIG"), + (Codec::Aac, "A_AAC"), + (Codec::Mp2, "A_MPEG/L2"), + (Codec::Mp3, "A_MPEG/L3"), + (Codec::Flac, "A_FLAC"), + (Codec::Opus, "A_OPUS"), + ] { + let got = MkvTrack::audio(&audio_stream(codec)).codec_id; + assert_eq!( + got, want, + "{codec:?} must declare {want}, got {got} — a wrong CodecID \ + mislabels the payload" + ); + } + // Every Codec of audio kind must be covered above: a new audio codec + // added to the enum must not silently inherit some other codec's ID. + let covered = [ + Codec::Ac3, + Codec::Ac3Plus, + Codec::TrueHd, + Codec::Dts, + Codec::DtsHdHr, + Codec::DtsHdMa, + Codec::Lpcm, + Codec::Aac, + Codec::Mp2, + Codec::Mp3, + Codec::Flac, + Codec::Opus, + ]; + for codec in covered { + assert_eq!( + codec.kind(), + crate::disc::CodecKind::Audio, + "{codec:?} is listed as audio here but kind() disagrees" + ); + } + } + + #[test] + fn every_video_codec_gets_its_own_registered_codec_id() { + // Same defect class as the audio counterpart: the catch-all declared + // V_MPEG2 for Codec::Mpeg1 and Codec::Av1, so an AV1 track was announced + // to the decoder as MPEG-2 video. + for (codec, want) in [ + (Codec::H264, "V_MPEG4/ISO/AVC"), + (Codec::Hevc, "V_MPEGH/ISO/HEVC"), + (Codec::Vc1, "V_MS/VFW/FOURCC"), + (Codec::Mpeg2, "V_MPEG2"), + (Codec::Mpeg1, "V_MPEG1"), + (Codec::Av1, "V_AV1"), + ] { + let v = VideoStream { + pid: 0xE0, + codec, + resolution: Resolution::R1080p, + frame_rate: crate::disc::FrameRate::F24, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt709, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + }; + let got = MkvTrack::video(&v).codec_id; + assert_eq!(got, want, "{codec:?} must declare {want}, got {got}"); + assert_eq!( + codec.kind(), + crate::disc::CodecKind::Video, + "{codec:?} is listed as video here but kind() disagrees" + ); + } + } + #[test] fn dolby_vision_config_profile7() { // dvcC for disc Profile 7 dual-layer: version 1.0, profile 7, all of