From e96528ad5b9750b52ed79c3606aa89e84e4a6276 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Tue, 23 Jun 2026 15:38:49 -0700 Subject: [PATCH] DVD: correct PAL/NTSC, anamorphic aspect, and SD colour Fix three DVD video-attribute bugs surfaced by a PAL disc detected as NTSC: - PAL/NTSC: parse video_format from VTS_V_ATR bits 5-4, not bits 1-0 (the old mask read permitted_df, so PAL 576i/25fps was mis-detected as NTSC 480i/29.97). Named consts replace the magic bit positions. - Anamorphic aspect: write MKV DisplayWidth/Height from the disc's display_aspect (16:9 720x576 -> 1024x576) instead of square pixels, so 16:9 DVDs no longer render as 4:3. - Colour: stamp SD colorimetry (PAL=BT.470BG, NTSC=SMPTE-170M) instead of BT.709 (HD). Adds VideoStream.display_aspect (threaded through every muxer) plus TvSystem/DvdAspect/ColorSpace plumbing, with regression tests. Removes the deprecated Disc mux set_halt bridge (use with_halt). --- src/disc/bluray.rs | 4 ++ src/disc/dvd.rs | 85 ++++++++++++++++++++++-- src/disc/mod.rs | 19 ++++++ src/ifo.rs | 128 +++++++++++++++++++++++++----------- src/keysource.rs | 2 +- src/labels/mod.rs | 1 + src/mux/disc.rs | 45 ++++--------- src/mux/m2ts.rs | 1 + src/mux/meta.rs | 3 + src/mux/mkv.rs | 55 +++++++++++++++- src/mux/mkvstream.rs | 3 + src/mux/network.rs | 1 + src/mux/pipelined_stream.rs | 1 + src/mux/stdio.rs | 1 + src/mux/ts.rs | 2 + tests/disc_tests.rs | 1 + tests/streams.rs | 5 ++ 17 files changed, 279 insertions(+), 78 deletions(-) diff --git a/src/disc/bluray.rs b/src/disc/bluray.rs index 6dbb928..0d43612 100644 --- a/src/disc/bluray.rs +++ b/src/disc/bluray.rs @@ -137,6 +137,10 @@ impl Disc { 2 => ColorSpace::Bt2020, _ => ColorSpace::Unknown, }, + // Blu-ray HD/UHD video is square-pixel; display aspect + // equals the pixel grid (16:9). Anamorphic SD-on-BD is + // not special-cased here. + display_aspect: None, secondary: s.secondary, // No user-facing English in the library (numeric-code // rule): the Dolby Vision enhancement layer is signalled diff --git a/src/disc/dvd.rs b/src/disc/dvd.rs index c183a4f..3ebcf7a 100644 --- a/src/disc/dvd.rs +++ b/src/disc/dvd.rs @@ -24,12 +24,25 @@ impl Disc { pid: 0xE0, // DVD video PID (standard MPEG PS video stream) codec: ts.video.codec, resolution: ts.video.resolution, - frame_rate: match ts.video.standard.as_str() { - "PAL" => FrameRate::F25, - _ => FrameRate::F29_97, + frame_rate: match ts.video.standard { + crate::ifo::TvSystem::Pal => FrameRate::F25, + crate::ifo::TvSystem::Ntsc => FrameRate::F29_97, }, hdr: HdrFormat::Sdr, - color_space: ColorSpace::Bt709, + // DVD is SD, not HD: PAL is BT.470BG, NTSC is SMPTE-170M. + // Stamping BT.709 (HD) mis-tags the colour primaries/transfer. + color_space: match ts.video.standard { + crate::ifo::TvSystem::Pal => ColorSpace::Bt470bg, + crate::ifo::TvSystem::Ntsc => ColorSpace::Smpte170m, + }, + // DVD pixels are anamorphic 720x480/576; the real display shape + // is the IFO aspect flag, not the pixel grid. Carry it so the + // MKV muxer writes a correct 16:9 / 4:3 DisplayWidth/Height + // instead of the square-pixel 3:2 / 5:4 it would otherwise emit. + display_aspect: Some(match ts.video.aspect { + crate::ifo::DvdAspect::R16x9 => (16, 9), + crate::ifo::DvdAspect::R4x3 => (4, 3), + }), secondary: false, label: String::new(), }); @@ -527,8 +540,14 @@ mod tests { fn scan_dvd_titles_pal_frame_rate_and_video_pid() { let mut disc = MemDisc::new(); let vmg = build_vmg(&[(1, 1, 1)]); - // video b0 low 2 bits = 1 → PAL. - let vts = build_vts(0, 0x01, &[], &[], &[(0, 9)], false); + let vts = build_vts( + 0, + crate::ifo::v_atr_byte(crate::ifo::VIDEO_FORMAT_PAL, crate::ifo::ASPECT_4X3), + &[], + &[], + &[(0, 9)], + false, + ); let udf = build_video_ts_fs( &mut disc, &[ @@ -558,6 +577,60 @@ mod tests { assert_eq!(v.pid, 0xE0, "DVD video PID is fixed 0xE0"); assert_eq!(v.frame_rate, FrameRate::F25, "PAL → 25 fps"); assert_eq!(v.resolution, Resolution::R576i, "PAL → 576i"); + assert_eq!( + v.color_space, + ColorSpace::Bt470bg, + "PAL DVD is SD BT.470BG, not BT.709" + ); + } + + /// NTSC DVD video is SD SMPTE-170M colorimetry (not BT.709). Mirror of the + /// PAL test with `VIDEO_FORMAT_NTSC` → 480i / 29.97 / SMPTE-170M. + #[test] + fn scan_dvd_titles_ntsc_color_is_smpte170m() { + let mut disc = MemDisc::new(); + let vmg = build_vmg(&[(1, 1, 1)]); + let vts = build_vts( + 0, + crate::ifo::v_atr_byte(crate::ifo::VIDEO_FORMAT_NTSC, crate::ifo::ASPECT_4X3), + &[], + &[], + &[(0, 9)], + false, + ); + let udf = build_video_ts_fs( + &mut disc, + &[ + FileSpec { + name: "VIDEO_TS.IFO".into(), + icb_lba: 60, + data_lba: 5000, + contents: vmg, + }, + FileSpec { + name: "VTS_01_0.IFO".into(), + icb_lba: 62, + data_lba: 6000, + contents: vts, + }, + ], + ); + let t = &Disc::scan_dvd_titles(&mut disc, &udf)[0]; + let v = t + .streams + .iter() + .find_map(|s| match s { + Stream::Video(v) => Some(v), + _ => None, + }) + .expect("video stream"); + assert_eq!(v.frame_rate, FrameRate::F29_97, "NTSC → 29.97 fps"); + assert_eq!(v.resolution, Resolution::R480i, "NTSC → 480i"); + assert_eq!( + v.color_space, + ColorSpace::Smpte170m, + "NTSC DVD is SD SMPTE-170M, not BT.709" + ); } /// AC-3 audio gets sub_stream_id 0x80 → PID routed via dvd_audio_pid diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 98d66c9..6467f76 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -182,6 +182,13 @@ pub struct VideoStream { pub hdr: HdrFormat, /// Color space pub color_space: ColorSpace, + /// Intended display aspect ratio as `(num, den)` when the coded pixels are + /// **anamorphic** (display shape ≠ pixel grid) — e.g. DVD 720x576 shown as + /// 16:9 → `Some((16, 9))`. `None` means square pixels: the display aspect + /// equals the pixel dimensions (HD/UHD, BD). Consumed by the MKV muxer to + /// write DisplayWidth/DisplayHeight; passthrough muxers (TS/M2TS) ignore it + /// because the aspect already lives in the elementary stream. + pub display_aspect: Option<(u32, u32)>, /// Whether this is a secondary stream (PiP, Dolby Vision EL) pub secondary: bool, /// Extra label (e.g. "Dolby Vision EL") @@ -366,6 +373,13 @@ pub enum HdrFormat { pub enum ColorSpace { Bt709, Bt2020, + /// SD PAL/576-line colorimetry (ITU-R BT.470 System B/G — primaries 5, + /// transfer 5, matrix 5). DVDs are SD, not HD: stamping BT.709 mis-tags + /// their colour. + Bt470bg, + /// SD NTSC/480-line colorimetry (SMPTE 170M / BT.601-525 — primaries 6, + /// transfer 6, matrix 6). + Smpte170m, Unknown, } @@ -843,6 +857,8 @@ impl ColorSpace { match self { ColorSpace::Bt709 => "BT.709", ColorSpace::Bt2020 => "BT.2020", + ColorSpace::Bt470bg => "BT.470BG", + ColorSpace::Smpte170m => "SMPTE 170M", ColorSpace::Unknown => "", } } @@ -850,6 +866,8 @@ impl ColorSpace { const ALL_CS: &[(&'static str, ColorSpace)] = &[ ("bt709", ColorSpace::Bt709), ("bt2020", ColorSpace::Bt2020), + ("bt470bg", ColorSpace::Bt470bg), + ("smpte170m", ColorSpace::Smpte170m), ("unknown", ColorSpace::Unknown), ]; @@ -3500,6 +3518,7 @@ mod tests { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: String::new(), })], diff --git a/src/ifo.rs b/src/ifo.rs index fe668c9..0d55c89 100644 --- a/src/ifo.rs +++ b/src/ifo.rs @@ -60,14 +60,30 @@ pub struct DvdCell { pub last_sector: u32, } +/// DVD TV system, from VTS_V_ATR `video_format` (byte 0 bits 5-4). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TvSystem { + Ntsc, + Pal, +} + +/// DVD display aspect ratio, from VTS_V_ATR `display_aspect_ratio` +/// (byte 0 bits 3-2). The pixels are anamorphic 720x480/576 either way; +/// this is the intended *display* shape. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DvdAspect { + R4x3, + R16x9, +} + /// DVD video stream attributes. #[derive(Debug, Clone)] #[allow(dead_code)] pub struct DvdVideoAttr { pub codec: Codec, pub resolution: Resolution, - pub aspect: String, - pub standard: String, + pub aspect: DvdAspect, + pub standard: TvSystem, } /// DVD audio stream attributes. @@ -350,33 +366,59 @@ fn parse_vts( // ── Attribute parsers ─────────────────────────────────────────────────────── +// ── VTS_V_ATR byte 0 bitfield layout (DVD-Video spec, MSB first) ────────── +// bits 7-6 mpeg_version | bits 5-4 video_format | bits 3-2 display_aspect +// | bits 1-0 permitted_df +// Naming the positions is the guard against the original bug: video_format is +// bits 5-4, NOT bits 1-0 (those are the pan&scan/letterbox permission). Reading +// the low two bits mis-detected every PAL disc as NTSC → 720x480 not 720x576. +const V_ATR_VIDEO_FORMAT_SHIFT: u8 = 4; +const V_ATR_ASPECT_SHIFT: u8 = 2; +const V_ATR_FIELD_MASK: u8 = 0x03; +// video_format field values (2/3 are reserved → parsed as NTSC). +pub(crate) const VIDEO_FORMAT_NTSC: u8 = 0; +pub(crate) const VIDEO_FORMAT_PAL: u8 = 1; +// display_aspect_ratio field values (1/2 are reserved → parsed as 4:3). +pub(crate) const ASPECT_4X3: u8 = 0; +pub(crate) const ASPECT_16X9: u8 = 3; + +/// Compose a VTS_V_ATR byte 0 from its `video_format` / `display_aspect` +/// fields, mirroring the layout [`parse_video_attr`] reads. Test-only — keeps +/// fixtures self-documenting (`v_atr_byte(VIDEO_FORMAT_PAL, ASPECT_16X9)`) +/// instead of opaque packed hex. +#[cfg(test)] +pub(crate) fn v_atr_byte(video_format: u8, display_aspect: u8) -> u8 { + (video_format << V_ATR_VIDEO_FORMAT_SHIFT) | (display_aspect << V_ATR_ASPECT_SHIFT) +} + /// Parse video attributes from VTS header offset 0x200. fn parse_video_attr(data: &[u8]) -> Result { let b0 = byte_at(data, 0x200)?; - let standard = match b0 & 0x03 { - 0 => "NTSC", - 1 => "PAL", - _ => "NTSC", + // video_format (bits 5-4): NTSC / PAL; reserved values (2/3) → NTSC. + let standard = match (b0 >> V_ATR_VIDEO_FORMAT_SHIFT) & V_ATR_FIELD_MASK { + VIDEO_FORMAT_PAL => TvSystem::Pal, + VIDEO_FORMAT_NTSC => TvSystem::Ntsc, + _ => TvSystem::Ntsc, }; - let aspect = match (b0 >> 2) & 0x03 { - 0 => "4:3", - 3 => "16:9", - _ => "4:3", + // display_aspect_ratio (bits 3-2): 4:3 / 16:9; reserved values (1/2) → 4:3. + let aspect = match (b0 >> V_ATR_ASPECT_SHIFT) & V_ATR_FIELD_MASK { + ASPECT_16X9 => DvdAspect::R16x9, + ASPECT_4X3 => DvdAspect::R4x3, + _ => DvdAspect::R4x3, }; - let resolution = if standard == "PAL" { - Resolution::R576i - } else { - Resolution::R480i + let resolution = match standard { + TvSystem::Pal => Resolution::R576i, + TvSystem::Ntsc => Resolution::R480i, }; Ok(DvdVideoAttr { codec: Codec::Mpeg2, resolution, - aspect: aspect.to_string(), - standard: standard.to_string(), + aspect, + standard, }) } @@ -757,8 +799,8 @@ mod tests { let video = DvdVideoAttr { codec: Codec::Mpeg2, resolution: Resolution::R480i, - aspect: "16:9".to_string(), - standard: "NTSC".to_string(), + aspect: DvdAspect::R16x9, + standard: TvSystem::Ntsc, }; assert_eq!(video.codec, Codec::Mpeg2); @@ -844,14 +886,11 @@ mod tests { #[test] fn video_attr_parsing() { - // Build minimal data with video attrs at 0x200 let mut data = vec![0u8; 0x204]; - // NTSC, 16:9, 720x480: standard=0b00, aspect=0b11, resolution=0b00 - // b0 = 0b00_00_11_00 = 0x0C - data[0x200] = 0x0C; + data[0x200] = v_atr_byte(VIDEO_FORMAT_NTSC, ASPECT_16X9); let attr = parse_video_attr(&data).unwrap(); - assert_eq!(attr.standard, "NTSC"); - assert_eq!(attr.aspect, "16:9"); + assert_eq!(attr.standard, TvSystem::Ntsc); + assert_eq!(attr.aspect, DvdAspect::R16x9); assert_eq!(attr.resolution, Resolution::R480i); assert_eq!(attr.codec, Codec::Mpeg2); } @@ -859,12 +898,25 @@ mod tests { #[test] fn video_attr_pal() { let mut data = vec![0u8; 0x204]; - // PAL, 4:3, 720x576: standard=0b01, aspect=0b00, resolution=0b00 - // b0 = 0b00_00_00_01 = 0x01 - data[0x200] = 0x01; + data[0x200] = v_atr_byte(VIDEO_FORMAT_PAL, ASPECT_4X3); let attr = parse_video_attr(&data).unwrap(); - assert_eq!(attr.standard, "PAL"); - assert_eq!(attr.aspect, "4:3"); + assert_eq!(attr.standard, TvSystem::Pal); + assert_eq!(attr.aspect, DvdAspect::R4x3); + assert_eq!(attr.resolution, Resolution::R576i); + } + + /// Real-world regression: a PAL 16:9 anamorphic disc (the Silence of the + /// Lambs UK SKU). Must parse as PAL / 16:9 / 576i. The old code read the TV + /// system from bits 1-0 (permitted_df, here 0) and reported NTSC/480i — the + /// case that shipped broken because only NTSC discs (where the wrong bits + /// coincide on 0) were ever tested. + #[test] + fn video_attr_pal_16x9_anamorphic() { + let mut data = vec![0u8; 0x204]; + data[0x200] = v_atr_byte(VIDEO_FORMAT_PAL, ASPECT_16X9); + let attr = parse_video_attr(&data).unwrap(); + assert_eq!(attr.standard, TvSystem::Pal); + assert_eq!(attr.aspect, DvdAspect::R16x9); assert_eq!(attr.resolution, Resolution::R576i); } @@ -1021,25 +1073,25 @@ mod tests { assert!(byte_at(&data, 2).is_err()); } - /// Video attr standard bits (b0 & 0x03): 0=NTSC, 1=PAL, else NTSC. - /// Value 2 and 3 fall into the NTSC default. Verify the catch-all. + /// A reserved video_format value (2/3) falls into the NTSC default. #[test] fn video_attr_reserved_standard_defaults_ntsc() { let mut data = vec![0u8; 0x204]; - data[0x200] = 0x02; // standard bits = 0b10 → default NTSC + // A reserved value is anything past PAL (2 or 3). + data[0x200] = v_atr_byte(VIDEO_FORMAT_PAL + 1, ASPECT_4X3); let attr = parse_video_attr(&data).unwrap(); - assert_eq!(attr.standard, "NTSC"); + assert_eq!(attr.standard, TvSystem::Ntsc); assert_eq!(attr.resolution, Resolution::R480i); } - /// Video aspect bits ((b0>>2)&0x03): 0=4:3, 3=16:9, else 4:3. - /// Value 1/2 fall into the 4:3 default (catch-all arm). + /// A reserved display_aspect value (1/2) falls into the 4:3 default. #[test] fn video_attr_reserved_aspect_defaults_4_3() { let mut data = vec![0u8; 0x204]; - data[0x200] = 0b00_01_00_00; // aspect bits = 0b01 → default 4:3 + // A reserved aspect value is between 4:3 (0) and 16:9 (3). + data[0x200] = v_atr_byte(VIDEO_FORMAT_NTSC, ASPECT_4X3 + 1); let attr = parse_video_attr(&data).unwrap(); - assert_eq!(attr.aspect, "4:3"); + assert_eq!(attr.aspect, DvdAspect::R4x3); } /// Audio coding_mode (b0>>5 & 0x07): 0=AC3, 2=MPEG1, 3=MP2, 4=LPCM, @@ -1079,7 +1131,7 @@ mod tests { #[test] fn audio_attr_reserved_rate_defaults_48k() { let mut data = vec![0u8; 8]; - data[0] = 0b000_10_000; // rate flag = 0b10 + data[0] = 0b0001_0000; // sample-rate flag (bits 4-3) = 0b10 let attr = parse_audio_attr(&data, 0).unwrap(); assert_eq!(attr.sample_rate, 48000); } diff --git a/src/keysource.rs b/src/keysource.rs index 2d6dda9..03078d7 100644 --- a/src/keysource.rs +++ b/src/keysource.rs @@ -143,7 +143,7 @@ mod tests { None } } - let mut s = MinimalSource; + let s = MinimalSource; assert!(!s.needs_samples(), "needs_samples must default to false"); } diff --git a/src/labels/mod.rs b/src/labels/mod.rs index 29e61a8..cb3478f 100644 --- a/src/labels/mod.rs +++ b/src/labels/mod.rs @@ -1296,6 +1296,7 @@ mod apply_tests { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt2020, + display_aspect: None, secondary: false, label: String::new(), }) diff --git a/src/mux/disc.rs b/src/mux/disc.rs index 8c66bd6..db5f5bf 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -11,7 +11,9 @@ use crate::event::{BatchSizeReason, Event, EventKind}; use crate::halt::Halt; use crate::sector::{DecryptingSectorSource, SectorSource}; use std::io; +#[cfg(test)] use std::sync::Arc; +#[cfg(test)] use std::sync::atomic::AtomicBool; /// Ramp back up to the preferred batch size after this many sectors @@ -149,9 +151,8 @@ pub struct DiscStream { /// When set and the token is cancelled, fill_extents returns Err(Halted) /// at the next retry boundary. Unlike skip_errors, this propagates the /// error up so the rip terminates cleanly. Construct with - /// [`DiscStream::with_halt`] (preferred) or set post-hoc via the - /// deprecated [`DiscStream::set_halt`] bridge — both populate this same - /// field and either entry point yields one source of truth. + /// [`DiscStream::with_halt`], passing the same `Halt` clone handed to + /// sweep / patch / mux so every phase observes one Stop signal. halt: Option, event_fn: Option>, eof: bool, @@ -296,29 +297,13 @@ impl DiscStream { /// during dense bad-sector regions (where the outer PES read() loop /// can spend minutes inside fill_extents before emitting a frame). /// - /// Preferred over the post-hoc [`DiscStream::set_halt`] bridge — - /// pass the same `Halt` clone you hand to sweep / patch / mux so - /// every phase observes a single Stop signal. + /// Pass the same `Halt` clone you hand to sweep / patch / mux so every + /// phase observes a single Stop signal. pub fn with_halt(mut self, halt: Halt) -> Self { self.halt = Some(halt); self } - /// Bridge for callers that haven't migrated to the - /// [`DiscStream::with_halt`] constructor-time path yet. Wraps the - /// supplied `Arc` as a [`Halt`] (`Halt::from_arc`) and - /// stores it in the same internal slot, so a halt installed via - /// either entry point goes through one halt-check inside - /// `fill_extents`. Calling `set_halt` after `with_halt` (or vice - /// versa) replaces the previous token with the new one. - #[deprecated( - since = "1.0.0", - note = "use `DiscStream::with_halt(Halt)` at construction instead" - )] - pub fn set_halt(&mut self, flag: Arc) { - self.halt = Some(Halt::from_arc(flag)); - } - fn is_halted(&self) -> bool { self.halt .as_ref() @@ -918,11 +903,9 @@ mod tests { assert_eq!(frames, 0); } - /// `is_halted()` must observe a cancellation signal regardless of - /// which entry point installed the token. The deprecated - /// `set_halt(Arc)` and the new `with_halt(Halt)` are - /// two views over one slot — flipping either bit must cause the - /// next `fill_extents` retry boundary to bail. + /// `is_halted()` must observe a cancellation signal installed via + /// `with_halt(Halt)` — flipping the token must cause the next + /// `fill_extents` retry boundary to bail. #[test] fn halt_via_with_halt_observed_by_is_halted() { let halt = Halt::new(); @@ -1212,21 +1195,21 @@ mod tests { } #[test] - fn halt_via_set_halt_bridge_observed_by_is_halted() { + fn halt_via_with_halt_from_arc_observed_by_is_halted() { let arc = Arc::new(AtomicBool::new(false)); - let mut stream = DiscStream::new( + let stream = DiscStream::new( Box::new(ZeroReader { capacity: 8 }), synthetic_title(8), crate::decrypt::DecryptKeys::None, 8, crate::disc::ContentFormat::BdTs, - ); - stream.set_halt(arc.clone()); + ) + .with_halt(Halt::from_arc(arc.clone())); assert!(!stream.is_halted()); arc.store(true, std::sync::atomic::Ordering::Relaxed); assert!( stream.is_halted(), - "set_halt(Arc) bridge must observe Arc-side flips" + "with_halt(Halt::from_arc) must observe Arc-side flips" ); } } diff --git a/src/mux/m2ts.rs b/src/mux/m2ts.rs index a8e11d1..997a6e4 100644 --- a/src/mux/m2ts.rs +++ b/src/mux/m2ts.rs @@ -113,6 +113,7 @@ mod tests { frame_rate: FrameRate::F24, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: String::new(), })], diff --git a/src/mux/meta.rs b/src/mux/meta.rs index 4bc9e4f..a9e4aa6 100644 --- a/src/mux/meta.rs +++ b/src/mux/meta.rs @@ -209,6 +209,7 @@ impl M2tsMeta { .unwrap_or(crate::disc::FrameRate::Unknown), hdr: hdr_fmt, color_space: cs, + display_aspect: None, secondary: *secondary, label: label.clone(), }) @@ -403,6 +404,7 @@ mod tests { frame_rate: FrameRate::F23_976, hdr, color_space: cs, + display_aspect: None, secondary: false, label: String::new(), })); @@ -651,6 +653,7 @@ mod tests { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt2020, + display_aspect: None, secondary: false, label: "x".into(), })); diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index 5d56b9f..7ba2df2 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -83,9 +83,12 @@ impl MkvTrack { } else { 0 }; + // (matrix, transfer, primaries, range) — ITU-T H.273 / CICP codes. let (matrix, transfer, primaries, range) = match v.color_space { ColorSpace::Bt2020 => (9, 16, 9, 1), // bt2020nc, PQ, bt2020, limited ColorSpace::Bt709 => (1, 1, 1, 1), // bt709 + ColorSpace::Bt470bg => (5, 5, 5, 1), // PAL SD: BT.470BG matrix/transfer/primaries + ColorSpace::Smpte170m => (6, 6, 6, 1), // NTSC SD: SMPTE 170M / BT.601-525 ColorSpace::Unknown => (0, 0, 0, 0), }; // Override transfer for non-PQ HDR @@ -94,6 +97,17 @@ impl MkvTrack { HdrFormat::Hlg => 18, _ => transfer, }; + // Display dimensions. For square-pixel video (HD/UHD/BD) the display + // aspect equals the pixel grid, so display == pixel. For anamorphic + // content (DVD: 720x480/576 pixels shown as 16:9 or 4:3) the coded + // pixels are NOT square — keep the coded height and derive the width so + // DisplayWidth:DisplayHeight carries the intended DAR (e.g. 720x576 + // 16:9 → 1024x576). Without this, players use the square-pixel ratio + // and show the disc as 5:4 / 3:2 instead of 16:9. + let (display_width, display_height) = match v.display_aspect { + Some((an, ad)) if an > 0 && ad > 0 && h > 0 => ((h * an + ad / 2) / ad, h), + _ => (w, h), + }; Self { track_type: ebml::TRACK_TYPE_VIDEO, codec_id, @@ -105,8 +119,8 @@ impl MkvTrack { pixel_width: w, pixel_height: h, default_duration_ns, - display_width: w, - display_height: h, + display_width, + display_height, colour_matrix: matrix, colour_transfer: transfer, colour_primaries: primaries, @@ -1075,6 +1089,43 @@ mod tests { use super::*; use std::io::Cursor; + /// Anamorphic DVD: a 720x576 (R576i) PAL stream flagged 16:9 must write a + /// DisplayWidth/Height carrying the 16:9 DAR (1024x576), NOT the square-pixel + /// 720x576 (which players show as ~5:4). Square-pixel video + /// (`display_aspect == None`) keeps display == pixel. + #[test] + fn video_track_anamorphic_display_aspect() { + let base = VideoStream { + pid: 0xE0, + codec: Codec::Mpeg2, + resolution: Resolution::R576i, + frame_rate: crate::disc::FrameRate::F25, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt709, + display_aspect: Some((16, 9)), + secondary: false, + label: String::new(), + }; + let t = MkvTrack::video(&base); + assert_eq!((t.pixel_width, t.pixel_height), (720, 576)); + assert_eq!( + (t.display_width, t.display_height), + (1024, 576), + "16:9 anamorphic must emit a 16:9 DAR, not square-pixel 720x576" + ); + + let square = VideoStream { + display_aspect: None, + ..base + }; + let t2 = MkvTrack::video(&square); + assert_eq!( + (t2.display_width, t2.display_height), + (720, 576), + "square pixels: display == pixel" + ); + } + /// Helper: search for a 4-byte big-endian EBML ID in a byte slice. fn find_id(data: &[u8], id: u32) -> Option { let bytes = id.to_be_bytes(); diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 058caba..f80b1f1 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -582,6 +582,9 @@ fn parse_track( frame_rate: FrameRate::Unknown, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + // Remux input: the source MKV's DisplayWidth/Height is preserved + // by the writer separately; nothing anamorphic to reconstruct here. + display_aspect: None, secondary: is_secondary, label: name, })) diff --git a/src/mux/network.rs b/src/mux/network.rs index d43b12d..fd12aea 100644 --- a/src/mux/network.rs +++ b/src/mux/network.rs @@ -352,6 +352,7 @@ mod tests { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt2020, + display_aspect: None, secondary: false, label: "Main".into(), }), diff --git a/src/mux/pipelined_stream.rs b/src/mux/pipelined_stream.rs index 639ff43..9ace617 100644 --- a/src/mux/pipelined_stream.rs +++ b/src/mux/pipelined_stream.rs @@ -618,6 +618,7 @@ mod tests { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt2020, + display_aspect: None, secondary, label: String::new(), })); diff --git a/src/mux/stdio.rs b/src/mux/stdio.rs index 26b2349..1db0189 100644 --- a/src/mux/stdio.rs +++ b/src/mux/stdio.rs @@ -156,6 +156,7 @@ mod tests { frame_rate: crate::disc::FrameRate::F23_976, hdr: crate::disc::HdrFormat::Hdr10, color_space: crate::disc::ColorSpace::Bt2020, + display_aspect: None, secondary: false, label: String::new(), })); diff --git a/src/mux/ts.rs b/src/mux/ts.rs index 552e5f6..eec60e0 100644 --- a/src/mux/ts.rs +++ b/src/mux/ts.rs @@ -690,6 +690,8 @@ pub fn scan_streams(data: &[u8]) -> Option> { frame_rate: FrameRate::Unknown, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + // TS is a passthrough container — aspect stays in the ES. + display_aspect: None, secondary: false, label: String::new(), })) diff --git a/tests/disc_tests.rs b/tests/disc_tests.rs index f786c72..3222c31 100644 --- a/tests/disc_tests.rs +++ b/tests/disc_tests.rs @@ -166,6 +166,7 @@ fn title_with_video( frame_rate: FrameRate::F23_976, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: String::new(), })], diff --git a/tests/streams.rs b/tests/streams.rs index c0ca526..9af490c 100644 --- a/tests/streams.rs +++ b/tests/streams.rs @@ -20,6 +20,7 @@ fn sample_disc_title() -> DiscTitle { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: "Main".into(), }), @@ -397,6 +398,7 @@ fn meta_codec_roundtrip() { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: String::new(), })); @@ -498,6 +500,7 @@ fn meta_all_stream_types() { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Hdr10, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: "Primary".into(), }), @@ -682,6 +685,7 @@ fn mkvstream_meta_preserves_all_streams() { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: "Main Video".into(), }), @@ -780,6 +784,7 @@ fn mkvstream_e2e_h264_produces_valid_mkv() { frame_rate: FrameRate::F23_976, hdr: HdrFormat::Sdr, color_space: ColorSpace::Bt709, + display_aspect: None, secondary: false, label: "Main".into(), })],