Give every audio and video codec its own registered Matroska CodecID
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.
This commit is contained in:
Matthew Jackson
@@ -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";
|
||||
|
||||
|
||||
+109
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user