diff --git a/CHANGELOG.md b/CHANGELOG.md index ba5aa67..e7da885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,81 @@ # Changelog +## [1.0.0-rc.5.1] + +### Fixed + +- **CSS reads unlocked on enforcing drives.** CSS-protected DVDs on + drives that enforce CSS authentication previously produced an empty MKV + at exit 0, or hung indefinitely. The read path now issues the bus-auth + handshake (`css::auth::unlock_css_reads`) to unlock scrambled-sector + reads before attempting any data transfer, so the drive gates lift + correctly. +- **Keyless title-key recovery always runs.** The Stevenson known-plaintext + attack (`css::crack_key` / `src/css/stevenson.rs`) now recovers the + title key even when the bus-auth scan detects a CSS drive, removing a + code path that fell through to locked reads on certain disc/drive + combinations. A wrong key still fails cleanly (confirmed by a sector + descramble check) rather than producing silent garbage. +- **Early bail on undecryptable discs.** When CSS authentication succeeds + but no valid title key can be recovered, the mux path now terminates + with a clear error code instead of writing an empty (or zero-byte) + output file. +- **DVD audio channel count from AC-3 bitstream.** The audio channel count + is now parsed from the AC-3 elementary-stream bitfield rather than from + the IFO audio attributes, so the reported channel count always matches the + actual muxed audio even when the IFO attribute disagrees. Passthrough only + — no downmix is performed. (Selecting the correct audio sub-stream on discs + with non-standard ordering is a separate item — see Known issues.) +- **Interlaced MKV frame rate on Windows.** Interlaced content (576i/480i) + now emits a `DefaultDecodedFieldDuration` element in the MKV track + header, which Windows Media Foundation and Explorer use to derive the + display frame rate. Without it, players reported an incorrect or zero + frame rate on interlaced tracks. +- **Per-track `BPS` bitrate tags populated.** The `BPS` tag is written for + each track so players and shell extensions (Windows Explorer, MPC-HC, + etc.) can display the per-stream bitrate without reading the full file. +- **Interlaced field order corrected to TFF.** 576i tracks were written + with a bottom-field-first (BFF) container flag that disagreed with the + top-field-first order carried in the MPEG-2 stream; the MKV `FieldOrder` + element now matches the stream (TFF) so deinterlacers use the correct + field parity. +- **DVD first-play menu no longer prepended to the feature.** The title + VOBS base sector was read from the VTS menu-VOBS pointer (`vtsm_vobs`, + offset 0xC0) instead of the title-VOBS pointer (`vtstt_vobs`, 0xC4), so on + a disc that authors a per-title menu the entire menu VOB — e.g. a studio + first-play "the parental level has been set, press yes" prompt — was + prepended to the movie and every cell extent shifted back. The rip now + opens on the feature's first frame. + +### Changed + +- **AACS handshake skipped on DVDs.** The AACS authentication sequence is + no longer attempted on DVD discs (it never applied to CSS-encrypted + media); attempting it on a DVD drive was a no-op at best and surfaced + spurious errors at worst. + +### Added + +- **Structured disc diagnostics at `--log-level 3`.** A new diagnostic + pass emits structured log events at INFO level when the log level is 3 + or higher: DVD PGC/cell layout and IFO video/audio attributes; BD/UHD + playlist, clip, and AACS metadata. Provides a single-command snapshot + for diagnosing mux or authentication issues without instrumenting the + source. +- **Reduced per-operation log spam.** Mux-read and seek operations are + demoted to TRACE (were DEBUG); benign navigation-packet drops are + summarized as a single counter at the end of the title rather than + logged per-packet. + +### Known issues + +- **Wrong audio track on discs with non-standard substream ordering.** + Audio sub-stream ids are assigned by per-codec ordinal rather than read + from the IFO/PGC stream-number table, so a disc whose physical substream + order diverges from the convention may select the wrong audio track + (e.g. a 2.0 stream in place of 5.1). Diagnose with + `freemkv info disc://… --log-level 3`; fix tracked for the next release. + ## [1.0.0-rc.4.2] ### Fixed diff --git a/src/css/auth.rs b/src/css/auth.rs index 42c8776..e306814 100644 --- a/src/css/auth.rs +++ b/src/css/auth.rs @@ -1,12 +1,12 @@ //! CSS drive bus-authentication — read-unlock primitive. //! //! A CSS-enforcing DVD drive refuses to return scrambled sectors until a -//! CSS bus-auth handshake has run for the title. [`unlock_css_reads`] -//! issues that classic handshake (bus auth → disc-key REPORT KEY → bus -//! auth → title-key REPORT KEY) purely for its SCSI side effect of -//! unlocking scrambled-sector reads. The bytes the handshake returns are -//! NOT used as keys: the descramble title key is recovered keylessly by -//! the Stevenson known-plaintext attack (see [`super::crack_key`]). +//! CSS bus-auth handshake has set its Authentication Success Flag (ASF=1). +//! [`unlock_css_reads`] runs that bus-auth challenge-response (which is what +//! actually opens scrambled-sector reads), then a best-effort, non-fatal +//! disc-key REPORT KEY. The bytes are NOT used as keys: the descramble title +//! key is recovered keylessly by the Stevenson known-plaintext attack (see +//! [`super::crack_key`]). use crate::drive::Drive; use crate::error::{Error, Result}; @@ -119,11 +119,12 @@ const PERM_VARIANT: [[u8; 32]; 2] = [ /// CSS bus-auth **unlock** primitive. /// -/// Issues the full classic CSS handshake (bus auth → disc-key REPORT KEY → -/// bus auth → title-key REPORT KEY) purely to unlock the drive's -/// scrambled-sector read gating. The bytes returned by the handshake are -/// discarded — the descramble title key is recovered keylessly elsewhere -/// (the Stevenson known-plaintext attack in [`super::crack_key`]). +/// Runs the bus-auth challenge-response (which sets the drive's ASF=1 and is +/// what actually unlocks scrambled-sector reads), then a best-effort, +/// non-fatal disc-key REPORT KEY. The title-key REPORT KEY is NOT issued: it +/// is unnecessary (the descramble key is recovered keylessly by the Stevenson +/// attack in [`super::crack_key`]) and its hard failure on some USB bridges +/// used to abort the whole unlock (the 7014 bug). The bytes are discarded. pub fn unlock_css_reads(drive: &mut Drive, lba: u32) -> Result<()> { let t0 = std::time::Instant::now(); tracing::info!(target: "freemkv::css", phase = "unlock_css_reads", lba, "begin"); @@ -139,27 +140,24 @@ pub fn unlock_css_reads(drive: &mut Drive, lba: u32) -> Result<()> { r } -fn unlock_css_reads_inner(drive: &mut Drive, lba: u32) -> Result<()> { - tracing::debug!(target: "freemkv::css", lba, "css unlock: begin"); - // Session 1: bus auth → disc-key REPORT KEY (AGID consumed by - // READ_DVD_STRUCTURE). The block contents are unused; this is issued - // purely for the bus-auth unlock side effect. +fn unlock_css_reads_inner(drive: &mut Drive, _lba: u32) -> Result<()> { + tracing::debug!(target: "freemkv::css", "css unlock: begin"); + // The bus-auth challenge-response sets the drive's Authentication Success + // Flag (ASF=1), which is what opens scrambled-sector reads. This is the + // ONLY step required to unlock reads; a failure here is fatal — we + // genuinely cannot read scrambled sectors. let (agid, _bus_key) = bus_auth(drive).inspect_err(|e| { - tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: bus_auth(1) failed"); - })?; - tracing::debug!(target: "freemkv::css", agid, "css unlock: bus_auth(1) ok"); - read_disc_key(drive, agid).inspect_err(|e| { - tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: read_disc_key failed"); - })?; - tracing::debug!(target: "freemkv::css", "css unlock: disc-key REPORT KEY ok"); - - // Session 2: fresh bus auth → title-key REPORT KEY (needs separate AGID). - let (agid2, _bus_key2) = bus_auth(drive).inspect_err(|e| { - tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: bus_auth(2) failed"); - })?; - read_raw_title_key(drive, agid2, lba).inspect_err(|e| { - tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: read_raw_title_key failed"); + tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: bus_auth failed"); })?; + tracing::debug!(target: "freemkv::css", agid, "css unlock: bus_auth ok"); + // Disc-key REPORT KEY: issued BEST-EFFORT for any firmware that ties part + // of its read-unlock to it. The bytes are unused (the descramble key is + // recovered keylessly) and a failure is NON-FATAL — the gate is already + // open from bus-auth. This replaces the title-key REPORT KEY, whose hard + // failure used to abort the whole unlock (the 7014 bug on USB bridges). + if let Err(e) = read_disc_key(drive, agid) { + tracing::debug!(target: "freemkv::css", error_code = e.code(), "css unlock: disc-key REPORT KEY skipped (non-fatal)"); + } tracing::debug!(target: "freemkv::css", "css unlock: ok"); Ok(()) } @@ -311,34 +309,6 @@ fn read_disc_key(drive: &mut Drive, agid: u8) -> Result<()> { Ok(()) } -// ── Step 3: Title Key ───────────────────────────────────────────────────── - -/// Issue the title-key REPORT KEY (format 0x04) purely for the bus-auth -/// unlock side effect. The returned key bytes are not used. -fn read_raw_title_key(drive: &mut Drive, agid: u8, lba: u32) -> Result<()> { - let scsi = drive.scsi_mut(); - let mut cdb = [0u8; 12]; - cdb[0] = crate::scsi::SCSI_REPORT_KEY; - cdb[2] = (lba >> 24) as u8; - cdb[3] = (lba >> 16) as u8; - cdb[4] = (lba >> 8) as u8; - cdb[5] = lba as u8; - cdb[8] = 0x00; - cdb[9] = 0x0C; - cdb[10] = (agid << 6) | 0x04; - - let mut buf = [0u8; 12]; - let result = scsi.execute( - &cdb, - crate::scsi::DataDirection::FromDevice, - &mut buf, - 5_000, - ); - result.map_err(|_| Error::CssAuthFailed)?; - - Ok(()) -} - // ── CSSCryptKey ─────────────────────────────────────────────────────────── fn crypt_key(key_type: usize, variant: u8, challenge: &[u8; 10]) -> [u8; 5] { diff --git a/src/css/mod.rs b/src/css/mod.rs index 78aaf80..31f88ad 100644 --- a/src/css/mod.rs +++ b/src/css/mod.rs @@ -22,6 +22,13 @@ pub(crate) mod tables; use crate::disc::Extent; use crate::sector::SectorSource; +/// Consecutive CSS-locked (`05/6F/03`) reads before the crack scan early-bails. +/// The bus-auth read gate is global (all-or-nothing), so a run this long means +/// it is shut and nothing here is crackable — bail instead of grinding the full +/// 50_000-sector budget (which is what made rc5 appear to hang on a wedged USB +/// bridge). The counter resets to 0 on any readable batch. +const CSS_LOCKED_BAIL: u32 = 64; + /// CSS decryption state for a DVD title. #[derive(Debug, Clone)] pub struct CssState { @@ -105,7 +112,7 @@ pub fn crack_key_outcome( batch_sectors: u16, halt: Option<&crate::halt::Halt>, ) -> CrackOutcome { - crack_key_scan(reader, extents, batch_sectors, halt) + crack_key_scan(reader, extents, batch_sectors, halt, true) } /// [`crack_key`] with an optional cooperative-cancellation token. @@ -122,7 +129,7 @@ pub fn crack_key_halt( batch_sectors: u16, halt: Option<&crate::halt::Halt>, ) -> Option { - crack_key_scan(reader, extents, batch_sectors, halt).into_state() + crack_key_scan(reader, extents, batch_sectors, halt, false).into_state() } /// The crack scan, returning the full [`CrackOutcome`]. Tracks a @@ -134,6 +141,10 @@ fn crack_key_scan( extents: &[Extent], batch_sectors: u16, halt: Option<&crate::halt::Halt>, + // True only on the INITIAL scan: a fully CSS-locked (`05/6F/03`) result is a + // hard `ScrambledUncracked`. False on the per-VTS re-crack so a lapsed-AGID + // locked read returns None instead of killing a genuinely crackable title. + fail_on_locked: bool, ) -> CrackOutcome { // Batch the reads: a live optical drive at 1 sector/read is glacial, and the // crack only needs to FIND one scrambled sector whose 0x80 plaintext matches @@ -161,6 +172,14 @@ fn crack_key_scan( // NOT silently treat as unencrypted (which would mux scrambled MPEG as // plaintext → garbage at exit 0). See `CrackOutcome::ScrambledUncracked`. let mut saw_scrambled = false; + // A read rejected with sense `05/6F/03` ("scrambled sector without + // authentication") is positive proof of CSS encryption — never collapse it + // to "unencrypted". A run of consecutive locked reads means the bus-auth + // gate is shut (it is global, so reads are all-or-nothing), so the scan + // early-bails. `consecutive_locked` resets on any readable batch, so a + // crackable title (gate open) never trips it. + let mut saw_locked = false; + let mut consecutive_locked = 0u32; 'outer: for (extent_idx, ext) in extents.iter().enumerate() { let mut i = 0u32; @@ -189,6 +208,8 @@ fn crack_key_scan( let want = n as usize * 2048; match reader.read_sectors(ext.start_lba + i, n as u16, &mut buf[..want], true) { Ok(_) => { + // A readable batch: the gate is open — reset the locked run. + consecutive_locked = 0; for s in 0..n as usize { tried += 1; let sect = &buf[s * 2048..(s + 1) * 2048]; @@ -206,20 +227,36 @@ fn crack_key_scan( } } } - // A failed batch (bad sectors) still counts toward the budget so a - // damaged region can't loop forever; skip ahead by the batch. - Err(_) => tried += n, + // A failed batch still counts toward the budget so a damaged + // region can't loop forever. A CSS-locked failure (`05/6F/03`) + // proves encryption and, in a long enough run, means the read + // gate is shut — track it and early-bail rather than grind. + Err(e) => { + tried += n; + if e.scsi_sense().is_some_and(|s| s.is_css_locked()) { + saw_locked = true; + consecutive_locked += 1; + if consecutive_locked >= CSS_LOCKED_BAIL { + break 'outer; + } + } else { + consecutive_locked = 0; + } + } } i += n; } } - // Budget exhausted / extents walked with no key recovered. Distinguish the - // two indistinguishable-in-`Option` cases: if scrambled sectors were seen - // (case b: crack failed; case c: scrambled but the crackable region was - // unreadable), this is encrypted-but-uncracked — a hard failure. Only a - // scan that NEVER saw a scrambled sector is genuinely unencrypted (case a). - if saw_scrambled { + // Budget exhausted / extents walked / early-bailed with no key recovered. + // The disc is ENCRYPTED-but-uncracked (a hard failure on the initial scan) + // when EITHER a scrambled sector was actually seen, OR — on the initial scan + // only (`fail_on_locked`) — every read was CSS-locked (`05/6F/03`), itself + // proof of scrambling. A re-crack (`fail_on_locked` false) stays soft: a + // lapsed-AGID locked read yields None, not a hard fail, so a crackable title + // in another VTS isn't killed. Only a scan that saw neither a scrambled + // sector nor a CSS-lock is genuinely unencrypted. + if saw_scrambled || (saw_locked && fail_on_locked) { CrackOutcome::ScrambledUncracked } else { CrackOutcome::Unencrypted @@ -309,6 +346,9 @@ mod tests { reads: std::cell::RefCell>, flag_byte: u8, fail_all: bool, + /// Every read fails with CSS-locked sense `05/6F/03` (drive refusing + /// scrambled reads because the bus-auth gate isn't open). + lock_all: bool, } impl MockSource { @@ -317,6 +357,7 @@ mod tests { reads: std::cell::RefCell::new(Vec::new()), flag_byte, fail_all: false, + lock_all: false, } } } @@ -330,6 +371,17 @@ mod tests { _recovery: bool, ) -> Result { self.reads.borrow_mut().push(lba); + if self.lock_all { + return Err(Error::DiscRead { + sector: lba as u64, + status: Some(2), + sense: Some(crate::scsi::ScsiSense { + sense_key: 0x05, + asc: 0x6F, + ascq: 0x03, + }), + }); + } if self.fail_all { return Err(Error::DecryptFailed); } @@ -433,6 +485,60 @@ mod tests { ); } + /// Fix C (rc.5.1): on the INITIAL scan, a drive that refuses every read with + /// CSS-locked sense (`05/6F/03`) is encrypted-but-locked → + /// `ScrambledUncracked` (a hard failure), NOT `Unencrypted`. This is the + /// rc4.3 bug: every VOB read came back `6F/03`, so the scan saw no scrambled + /// sector and wrongly declared the disc unencrypted → 19 KB garbage. + #[test] + fn crack_outcome_css_locked_initial_is_scrambled_uncracked() { + let mut src = MockSource::new(0x30); + src.lock_all = true; // every read → 05/6F/03 + let extents = [Extent { + start_lba: 0, + sector_count: 100, + }]; + let outcome = crack_key_outcome(&mut src, &extents, 1, None); + assert!( + outcome.is_scrambled_uncracked(), + "every read 6F/03 on the initial scan → ScrambledUncracked, got {outcome:?}" + ); + } + + /// MISSING #1 guard: the re-crack path (the `Option`-returning `crack_key`, + /// `fail_on_locked == false`) must NOT hard-fail on a CSS-locked read — it + /// returns `None`. A lapsed-AGID re-crack of another VTS stays soft so a + /// genuinely crackable title isn't killed by a transient locked read. + #[test] + fn crack_key_recrack_locked_is_none_not_hard_fail() { + let mut src = MockSource::new(0x30); + src.lock_all = true; + let extents = [Extent { + start_lba: 0, + sector_count: 100, + }]; + assert!(crack_key(&mut src, &extents, 1).is_none()); + } + + /// Fix F: a fully CSS-locked scan early-bails near `CSS_LOCKED_BAIL` + /// consecutive locked reads instead of grinding the whole 50_000-sector + /// budget (the rc5 "stuck Scanning…" hang on a wedged bridge). + #[test] + fn crack_css_locked_scan_early_bails() { + let mut src = MockSource::new(0x30); + src.lock_all = true; + let extents = [Extent { + start_lba: 0, + sector_count: 10_000, + }]; + let _ = crack_key_outcome(&mut src, &extents, 1, None); + let n = src.reads.borrow().len(); + assert!( + n <= (CSS_LOCKED_BAIL as usize) + 1, + "locked scan early-bails near {CSS_LOCKED_BAIL}, not 10000; read {n}" + ); + } + /// The budget spans ALL extents, not per-extent: two extents summing past /// the cap must still stop at 50_000 total reads. /// diff --git a/src/diag.rs b/src/diag.rs new file mode 100644 index 0000000..30a1bd5 --- /dev/null +++ b/src/diag.rs @@ -0,0 +1,468 @@ +//! Structured scan diagnostics — the `--log-level 3` self-diagnosing dump. +//! +//! A bug report log must be self-diagnosing: everything needed to explain +//! *why* freemkv made the choices it did at scan must be in the log, in a +//! compact, machine-parseable form. This module emits one terse line per row +//! (title, cell, stream, decision) under the `tracing` target +//! `freemkv::diag`, which the CLI routes to `log.txt` when `--log-level 3` +//! (debug) is set. +//! +//! Format conventions (stable, greppable): +//! - Every line is prefixed by a `tag=` so a log scraper can filter +//! (`disc`, `title`, `dvd.cell`, `dvd.vattr`, `dvd.aattr`, `bd.clip`, +//! `bd.mark`, `aacs`, `stream`, `decision`). +//! - Raw bytes are shown as `0xNN` next to their decode so a wrong decode +//! is obvious against the raw value. +//! - This module only READS already-parsed scan state — it never re-reads +//! the disc and never mutates anything. +//! +//! The DVD per-cell table (with the raw cell-category byte) is emitted from +//! the IFO scan itself ([`dump_dvd_cells`]), because the per-cell +//! `ifo::DvdCell` detail is lowered away before the `Disc` is built. The +//! `Disc`-level dump ([`dump_disc`]) covers everything that survives +//! lowering: titles, streams, the picked main feature, and AACS state. + +use crate::disc::{ + AudioChannels, ColorSpace, Disc, DiscTitle, FrameRate, HdrFormat, Resolution, SampleRate, + Stream, +}; +use crate::ifo::{CellCategory, DvdTitle}; + +const DIAG: &str = "freemkv::diag"; + +// ── small format helpers (pure, unit-testable) ────────────────────────────── + +/// Compact name for a [`Resolution`] with the interlace marker preserved. +pub fn res_str(r: Resolution) -> &'static str { + match r { + Resolution::R480i => "480i", + Resolution::R480p => "480p", + Resolution::R576i => "576i", + Resolution::R576p => "576p", + Resolution::R720p => "720p", + Resolution::R1080i => "1080i", + Resolution::R1080p => "1080p", + Resolution::R2160p => "2160p", + Resolution::R4320p => "4320p", + Resolution::Unknown => "res?", + } +} + +/// Frames-per-second string for a [`FrameRate`]. +pub fn fps_str(f: FrameRate) -> &'static str { + match f { + FrameRate::F23_976 => "23.976", + FrameRate::F24 => "24", + FrameRate::F25 => "25", + FrameRate::F29_97 => "29.97", + FrameRate::F30 => "30", + FrameRate::F50 => "50", + FrameRate::F59_94 => "59.94", + FrameRate::F60 => "60", + FrameRate::Unknown => "fps?", + } +} + +/// PAL/NTSC field-rate family inferred from the frame rate (DVD has no +/// explicit field, so this is the colour/standard the muxer stamps). +pub fn tv_system_str(f: FrameRate) -> &'static str { + match f { + FrameRate::F25 | FrameRate::F50 => "PAL", + FrameRate::F23_976 | FrameRate::F29_97 | FrameRate::F59_94 => "NTSC", + _ => "—", + } +} + +/// CICP-ish short name for a [`ColorSpace`]. +pub fn color_str(c: ColorSpace) -> &'static str { + match c { + ColorSpace::Bt709 => "BT.709", + ColorSpace::Bt2020 => "BT.2020", + ColorSpace::Bt470bg => "BT.470BG", + ColorSpace::Smpte170m => "SMPTE-170M", + ColorSpace::Unknown => "color?", + } +} + +/// HDR format short name. +pub fn hdr_str(h: HdrFormat) -> &'static str { + match h { + HdrFormat::Sdr => "SDR", + HdrFormat::Hdr10 => "HDR10", + HdrFormat::Hdr10Plus => "HDR10+", + HdrFormat::DolbyVision => "DoVi", + HdrFormat::Hlg => "HLG", + } +} + +/// Channel count from an [`AudioChannels`] layout (what lands in the MKV +/// `Channels` element). +pub fn channel_count(ch: AudioChannels) -> u8 { + match ch { + AudioChannels::Mono => 1, + AudioChannels::Stereo => 2, + AudioChannels::Stereo21 => 3, + AudioChannels::Quad => 4, + AudioChannels::Surround50 => 5, + AudioChannels::Surround51 => 6, + AudioChannels::Surround61 => 7, + AudioChannels::Surround71 => 8, + AudioChannels::Unknown => 0, + } +} + +/// Sample-rate in Hz for a [`SampleRate`]. +pub fn sample_rate_hz(s: SampleRate) -> u32 { + match s { + SampleRate::S44_1 => 44100, + SampleRate::S48 => 48000, + SampleRate::S96 => 96000, + SampleRate::S192 => 192000, + SampleRate::S48_96 => 96000, + SampleRate::S48_192 => 192000, + SampleRate::Unknown => 0, + } +} + +// ── DVD cell-category dump (from the IFO scan, pre-lowering) ───────────────── + +/// One formatted cell row for the DVD per-PGC cell table. Returned as a +/// string so it can be unit-tested without a logger. +/// +/// Columns: `idx`, raw category (`cat=0xNN`) + decoded fields, first/last +/// sector, duration, and the keep/drop verdict from the bug-4 leading-cell +/// filter. +pub fn dvd_cell_row(idx: usize, cell: &crate::ifo::DvdCell, dropped: bool) -> String { + let c = CellCategory::decode(cell.category); + format!( + "tag=dvd.cell idx={idx} cat=0x{:02X} type={} block_mode={} block_type={} \ +seamless={} ilv={} plain={} first={} last={} dur={:.1}s {}", + cell.category, + c.cell_type, + c.block_mode, + c.block_type, + c.seamless_play as u8, + c.interleaved as u8, + c.is_plain_feature() as u8, + cell.first_sector, + cell.last_sector, + cell.duration_secs, + if dropped { "DROP(non-feature)" } else { "keep" }, + ) +} + +/// Emit the per-PGC cell table for one DVD title during the IFO scan. +/// +/// `vts`/`title` identify the row group; `title` is the `DvdTitle` whose +/// cells (and bug-4 leading-cell verdict) are dumped. Called from +/// `scan_dvd_titles` while the `DvdTitle` is still in scope (the per-cell +/// category byte is lowered away before the `Disc` exists). +pub fn dump_dvd_cells(vts: u8, title_num: u16, title: &DvdTitle) { + if !tracing::enabled!(target: DIAG, tracing::Level::DEBUG) { + return; + } + let feature_start = title.feature_start_cell(); + tracing::debug!( + target: DIAG, + "tag=dvd.pgc vts={vts} title={title_num} cells={} chapters={} \ + dur={:.1}s feature_start_cell={feature_start}", + title.cells.len(), + title.chapters, + title.duration_secs, + ); + for (i, cell) in title.cells.iter().enumerate() { + tracing::debug!(target: DIAG, "{}", dvd_cell_row(i, cell, i < feature_start)); + } + // Chapter/PTT map (program → cumulative start time). + for (i, &t) in title.chapter_times.iter().enumerate() { + tracing::debug!( + target: DIAG, + "tag=dvd.chap vts={vts} title={title_num} ch={} time={:.1}s", + i + 1, + t, + ); + } +} + +/// Emit the IFO `video_attr` / `audio_attr` decode for one DVD title set, +/// showing the raw bytes next to their decoded meaning. Called from the IFO +/// scan with the still-parsed `ifo::DvdTitleSet` view. +pub fn dump_dvd_attrs(ts: &crate::ifo::DvdTitleSet) { + if !tracing::enabled!(target: DIAG, tracing::Level::DEBUG) { + return; + } + tracing::debug!( + target: DIAG, + "tag=dvd.vobs vts={} vob_start_sector={}", + ts.vts_number, + ts.vob_start_sector, + ); + let v = &ts.video; + tracing::debug!( + target: DIAG, + "tag=dvd.vattr vts={} codec={:?} res={} aspect={:?} std={:?}", + ts.vts_number, + v.codec, + res_str(v.resolution), + v.aspect, + v.standard, + ); + for (i, a) in ts.audio_streams.iter().enumerate() { + tracing::debug!( + target: DIAG, + "tag=dvd.aattr vts={} idx={i} codec={:?} ch={} sr={}Hz lang={:?} sub_id={:?}", + ts.vts_number, + a.codec, + a.channels, + a.sample_rate, + a.language, + a.sub_stream_id.map(|x| format!("0x{x:02X}")), + ); + } + for (i, s) in ts.subtitle_streams.iter().enumerate() { + tracing::debug!( + target: DIAG, + "tag=dvd.sattr vts={} idx={i} lang={:?}", + ts.vts_number, + s.language, + ); + } +} + +// ── Disc-level dump (post-lowering: titles, streams, decisions, AACS) ──────── + +/// Emit the full scan diagnostic block for a built [`Disc`]. Terse, one line +/// per row, under target `freemkv::diag` at DEBUG. No-op unless that target +/// is enabled, so it costs nothing when `--log-level 3` is off. +pub fn dump_disc(disc: &Disc) { + if !tracing::enabled!(target: DIAG, tracing::Level::DEBUG) { + return; + } + + tracing::debug!( + target: DIAG, + "tag=disc vol={:?} format={:?} content={:?} cap_sectors={} layers={} titles={} encrypted={}", + disc.volume_id, + disc.format, + disc.content_format, + disc.capacity_sectors, + disc.layers, + disc.titles.len(), + disc.encrypted, + ); + + dump_aacs(disc); + + for (ti, title) in disc.titles.iter().enumerate() { + dump_title(ti, title); + } + + // freemkv's top-level DECISION: which title is the main feature. + if let Some(main) = disc.titles.first() { + tracing::debug!( + target: DIAG, + "tag=decision pick=main_feature title_idx=0 playlist={:?} dur={:.1}s \ + size={}B clips={} reason=canonical_title_order(fits-disc, fewest-clips, longest, richest-audio)", + main.playlist, + main.duration_secs, + main.size_bytes, + main.clips.len(), + ); + } +} + +fn dump_aacs(disc: &Disc) { + let Some(a) = disc.aacs.as_ref() else { + if disc.css.is_some() { + tracing::debug!(target: DIAG, "tag=aacs none crypto=CSS(DVD)"); + } else if disc.encrypted { + tracing::debug!(target: DIAG, "tag=aacs none crypto=encrypted-no-keys"); + } else { + tracing::debug!(target: DIAG, "tag=aacs none crypto=clear"); + } + return; + }; + // CPS-unit / unit-key counts: at scan `unit_keys` is empty (keys are + // resolved later); the unit-key count is the BE16 in the raw + // Unit_Key_RO.inf if captured. Report both: resolved count and raw len. + tracing::debug!( + target: DIAG, + "tag=aacs version={} bus_enc={} mkb_version={:?} disc_hash={} key_source={:?} \ + vuk={} unit_keys_resolved={} uk_ro_bytes={} mkb_bytes={}", + a.version, + a.bus_encryption, + a.mkb_version, + a.disc_hash, + a.key_source.name(), + a.vuk.is_some(), + a.unit_keys.len(), + a.uk_ro.len(), + a.mkb.len(), + ); +} + +fn dump_title(ti: usize, title: &DiscTitle) { + let (mut nv, mut na, mut ns) = (0u32, 0u32, 0u32); + for s in &title.streams { + match s { + Stream::Video(_) => nv += 1, + Stream::Audio(_) => na += 1, + Stream::Subtitle(_) => ns += 1, + } + } + tracing::debug!( + target: DIAG, + "tag=title idx={ti} playlist={:?} id={} dur={:.1}s size={}B clips={} \ + extents={} chapters={} v={nv} a={na} s={ns} fmt={:?}", + title.playlist, + title.playlist_id, + title.duration_secs, + title.size_bytes, + title.clips.len(), + title.extents.len(), + title.chapters.len(), + title.content_format, + ); + + // Per-clip rows (BD: PlayItem/CLPI; DVD has none). + for (ci, c) in title.clips.iter().enumerate() { + tracing::debug!( + target: DIAG, + "tag=clip title={ti} idx={ci} id={:?} in={} out={} dur={:.1}s src_packets={}", + c.clip_id, + c.in_time, + c.out_time, + c.duration_secs, + c.source_packets, + ); + } + + // Per-extent rows (the sectors freemkv will actually rip — the bug-4 + // decision is visible here: leading non-feature cells are already gone). + for (ei, e) in title.extents.iter().enumerate() { + tracing::debug!( + target: DIAG, + "tag=extent title={ti} idx={ei} start_lba={} sectors={}", + e.start_lba, + e.sector_count, + ); + } + + // freemkv's per-stream DECISIONS (what the muxer will write). + for (si, s) in title.streams.iter().enumerate() { + match s { + Stream::Video(v) => tracing::debug!( + target: DIAG, + "tag=stream title={ti} idx={si} kind=video pid=0x{:04X} codec={:?} \ + res={} interlaced={} fps={} std={} color={} hdr={} aspect={:?} secondary={}", + v.pid, + v.codec, + res_str(v.resolution), + v.resolution.is_interlaced(), + fps_str(v.frame_rate), + tv_system_str(v.frame_rate), + color_str(v.color_space), + hdr_str(v.hdr), + v.display_aspect, + v.secondary, + ), + Stream::Audio(a) => tracing::debug!( + target: DIAG, + "tag=stream title={ti} idx={si} kind=audio pid=0x{:04X} codec={:?} \ + channels={}({}) sr={}Hz lang={:?} secondary={}", + a.pid, + a.codec, + a.channels, + channel_count(a.channels), + sample_rate_hz(a.sample_rate), + a.language, + a.secondary, + ), + Stream::Subtitle(sub) => tracing::debug!( + target: DIAG, + "tag=stream title={ti} idx={si} kind=subtitle pid=0x{:04X} codec={:?} \ + lang={:?} forced={}", + sub.pid, + sub.codec, + sub.language, + sub.forced, + ), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn res_str_keeps_interlace_marker() { + assert_eq!(res_str(Resolution::R576i), "576i"); + assert_eq!(res_str(Resolution::R480i), "480i"); + assert_eq!(res_str(Resolution::R2160p), "2160p"); + } + + #[test] + fn fps_and_tv_system() { + assert_eq!(fps_str(FrameRate::F25), "25"); + assert_eq!(tv_system_str(FrameRate::F25), "PAL"); + assert_eq!(fps_str(FrameRate::F29_97), "29.97"); + assert_eq!(tv_system_str(FrameRate::F29_97), "NTSC"); + } + + #[test] + fn color_and_hdr() { + assert_eq!(color_str(ColorSpace::Bt470bg), "BT.470BG"); + assert_eq!(color_str(ColorSpace::Bt2020), "BT.2020"); + assert_eq!(hdr_str(HdrFormat::Hdr10), "HDR10"); + assert_eq!(hdr_str(HdrFormat::DolbyVision), "DoVi"); + assert_eq!(hdr_str(HdrFormat::Sdr), "SDR"); + } + + #[test] + fn channel_count_matches_layout() { + assert_eq!(channel_count(AudioChannels::Mono), 1); + assert_eq!(channel_count(AudioChannels::Stereo), 2); + assert_eq!(channel_count(AudioChannels::Surround51), 6); + assert_eq!(channel_count(AudioChannels::Surround71), 8); + } + + #[test] + fn sample_rate_hz_values() { + assert_eq!(sample_rate_hz(SampleRate::S48), 48000); + assert_eq!(sample_rate_hz(SampleRate::S96), 96000); + } + + /// The cell row shows the raw category byte (0xNN) beside the decode, and + /// the keep/drop verdict. A plain feature cell (0x00) is "keep"; a leading + /// secondary-block cell flagged dropped reads "DROP". + #[test] + fn cell_row_shows_raw_byte_and_verdict() { + let plain = crate::ifo::DvdCell { + first_sector: 100, + last_sector: 199, + category: 0x00, + duration_secs: 12.5, + }; + let row = dvd_cell_row(0, &plain, false); + assert!(row.contains("cat=0x00"), "{row}"); + assert!(row.contains("type=0"), "{row}"); + assert!(row.contains("first=100"), "{row}"); + assert!(row.contains("last=199"), "{row}"); + assert!(row.contains("dur=12.5s"), "{row}"); + assert!(row.contains("keep"), "{row}"); + assert!(!row.contains("DROP"), "{row}"); + + // 0x80 = middle-of-angle-block (cell_type=2), shown dropped. + let sec = crate::ifo::DvdCell { + first_sector: 0, + last_sector: 9, + category: 0x80, + duration_secs: 1.0, + }; + let row = dvd_cell_row(0, &sec, true); + assert!(row.contains("cat=0x80"), "{row}"); + assert!(row.contains("type=2"), "{row}"); + assert!(row.contains("DROP(non-feature)"), "{row}"); + } +} diff --git a/src/disc/dvd.rs b/src/disc/dvd.rs index 52b40cc..4b211cd 100644 --- a/src/disc/dvd.rs +++ b/src/disc/dvd.rs @@ -20,6 +20,10 @@ impl Disc { let mut title_number: u16 = 0; for ts in &dvd_info.title_sets { + // Diagnostic dump (--log-level 3): IFO video/audio attrs for this + // title set. No-op unless the freemkv::diag target is enabled. + crate::diag::dump_dvd_attrs(ts); + let video_stream = Stream::Video(VideoStream { pid: 0xE0, // DVD video PID (standard MPEG PS video stream) codec: ts.video.codec, @@ -82,9 +86,36 @@ impl Disc { for dvd_title in &ts.titles { title_number += 1; + // Diagnostic dump (--log-level 3): per-cell category table + + // chapter map for this title, BEFORE lowering drops the + // per-cell IFO detail. No-op unless freemkv::diag is enabled. + crate::diag::dump_dvd_cells(ts.vts_number, title_number, dvd_title); + + // Bug-4 leading-cell filter: drop any leading scene-index / + // interleaved-angle sub-block cells so the feature starts at the + // movie. Conservative — `feature_start_cell` only ever skips a + // prefix of secondary-block cells and never truncates a normal + // feature (category 0x00 on cell 0 → no-op). See + // `ifo::DvdTitle::feature_start_cell`. + let feature_start = dvd_title.feature_start_cell(); + let dropped_secs: f64 = dvd_title.cells[..feature_start] + .iter() + .map(|c| c.duration_secs) + .sum(); + if feature_start > 0 { + tracing::debug!( + target: "freemkv::scan", + vts = ts.vts_number, + title = title_number, + dropped_cells = feature_start, + dropped_secs, + "dvd: dropped leading non-feature cell(s)" + ); + } + // Build extents from cell sector ranges (absolute = vob_start + cell offset) let extents: Vec = dvd_title - .cells + .feature_cells() .iter() .map(|cell| { let start = ts.vob_start_sector.saturating_add(cell.first_sector); @@ -133,12 +164,16 @@ impl Disc { streams.extend(audio_streams.iter().cloned()); streams.extend(subtitle_streams); + // Chapter times are absolute from the PGC start. When leading + // cells are dropped the muxed video shifts earlier by exactly + // their total duration, so shift the chapter marks too (clamping + // any that fell inside the dropped head to 0). let chapters: Vec = dvd_title .chapter_times .iter() .enumerate() .map(|(i, &t)| Chapter { - time_secs: t, + time_secs: (t - dropped_secs).max(0.0), name: chapter_name(i), }) .collect(); @@ -342,16 +377,25 @@ mod tests { d } - /// Cell playback info entry (24 bytes): BCD time@4..8 (unused here), + /// Cell playback info entry (24 bytes): category byte@0, BCD time@4..8, /// first_sector(u32 BE)@8, last_sector(u32 BE)@20. fn write_cell(buf: &mut [u8], off: usize, first_sector: u32, last_sector: u32) { buf[off + 8..off + 12].copy_from_slice(&first_sector.to_be_bytes()); buf[off + 20..off + 24].copy_from_slice(&last_sector.to_be_bytes()); } + /// Like [`write_cell`] but also stamps the cell-category byte (`+0`) so a + /// test can build a leading scene-index / interleaved-angle sub-block cell. + fn write_cell_cat(buf: &mut [u8], off: usize, first: u32, last: u32, category: u8) { + write_cell(buf, off, first, last); + buf[off] = category; + } + /// Build a VTS_XX_0.IFO. Layout per ifo.rs: /// magic "DVDVIDEO-VTS"@0 - /// vob_start_sector(u32 BE)@0xC0 + /// vtstt_vobs (Title VOBS start sector, u32 BE)@0xC4 — the production + /// `vob_start_sector` the cell sectors are relative to. (0xC0 is + /// `vtsm_vobs`, the menu VOBS, which the scan must NOT use.) /// VTS_PGCIT sector ptr(u32 BE)@0xCC /// video attr byte@0x200 /// num_audio(u16 BE)@0x202, audio blocks (8B) @0x204 @@ -377,7 +421,7 @@ mod tests { let pgcit_sector = 2u32; let mut d = vec![0u8; 4 * 2048]; d[0..12].copy_from_slice(b"DVDVIDEO-VTS"); - d[0xC0..0xC4].copy_from_slice(&vob_start.to_be_bytes()); + d[0xC4..0xC8].copy_from_slice(&vob_start.to_be_bytes()); // vtstt_vobs (Title VOBS) d[0xCC..0xD0].copy_from_slice(&pgcit_sector.to_be_bytes()); d[0x200] = video_b0; d[0x202..0x204].copy_from_slice(&(audio.len() as u16).to_be_bytes()); @@ -493,6 +537,56 @@ mod tests { assert_eq!(t.content_format, ContentFormat::MpegPs); } + /// Regression (first-play menu prepended to the feature): `vob_start` must + /// come from the **Title** VOBS pointer `vtstt_vobs` (VTS_IFO 0xC4), NOT the + /// **menu** VOBS pointer `vtsm_vobs` (0xC0). On discs with a per-title menu + /// — e.g. the Universal "the parental level has been set, press yes" + /// first-play still — `vtsm_vobs` points at that menu VOB, which sits just + /// before the title VOB. Cell `first_sector` values are relative to + /// `vtstt_vobs`; reading 0xC0 prepended the menu and shifted every extent + /// back by `vtstt_vobs - vtsm_vobs`, so the rip opened on the parental + /// prompt instead of the movie (Greenland NTSC R1: vtsm=44, vtstt=3640). + /// + /// Here `build_vts` stamps `vtstt_vobs = 3640` (0xC4); we additionally stamp + /// a *different* `vtsm_vobs = 44` (0xC0). The extent must resolve from 3640. + #[test] + fn scan_dvd_titles_uses_title_vobs_not_menu_vobs() { + let mut disc = MemDisc::new(); + let vmg = build_vmg(&[(1, 1, 1)]); + // vtstt_vobs (title) = 3640; cell 0 first_sector = 0. + let mut vts = build_vts(3640, 0x00, &[], &[], &[(0, 99)], false); + // Stamp a bogus vtsm_vobs (menu) at 0xC0 — the wrong pointer the bug + // used. It must be ignored. + vts[0xC0..0xC4].copy_from_slice(&44u32.to_be_bytes()); + 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 titles = Disc::scan_dvd_titles(&mut disc, &udf); + assert_eq!(titles.len(), 1); + let t = &titles[0]; + assert_eq!(t.extents.len(), 1); + // Title VOBS (3640) + cell first_sector (0) = 3640 — NOT the menu 44. + assert_eq!( + t.extents[0].start_lba, 3640, + "extent must start at vtstt_vobs (0xC4), not vtsm_vobs (0xC0)" + ); + assert_ne!(t.extents[0].start_lba, 44, "must not use the menu VOBS"); + } + /// Multi-cell title: extents preserve cell order and each maps to its /// own (vob_start + first .. last) range. mux reads cells in order. #[test] @@ -825,4 +919,143 @@ mod tests { assert_eq!(t.chapters.len(), 1); assert_eq!(t.chapters[0].name, chapter_name(0)); } + + /// Build a VTS with explicit per-cell category bytes and an N-program map. + /// Returns the IFO bytes. Cells: `(first, last, category, dur_secs)`. + fn build_vts_cells( + vob_start: u32, + video_b0: u8, + cells: &[(u32, u32, u8, u8 /*BCD seconds*/)], + program_first_cells: &[u8], + ) -> Vec { + let pgcit_sector = 2u32; + let mut d = vec![0u8; 4 * 2048]; + d[0..12].copy_from_slice(b"DVDVIDEO-VTS"); + d[0xC4..0xC8].copy_from_slice(&vob_start.to_be_bytes()); // vtstt_vobs (Title VOBS) + d[0xCC..0xD0].copy_from_slice(&pgcit_sector.to_be_bytes()); + d[0x200] = video_b0; + // no audio / subs + let pg = pgcit_sector as usize * 2048; + d[pg..pg + 2].copy_from_slice(&1u16.to_be_bytes()); + let pgc_rel: u32 = 0x100; + d[pg + 8 + 4..pg + 8 + 8].copy_from_slice(&pgc_rel.to_be_bytes()); + let pgc = pg + pgc_rel as usize; + d[pgc + 0x02] = program_first_cells.len() as u8; // nr_of_programs + d[pgc + 0x03] = cells.len() as u8; // nr_of_cells + // Leave PGC-level BCD time zero → duration recomputed from cells. + let cell_tbl_rel: u16 = 0xF0; + let pgm_map_rel: u16 = 0xEC; + d[pgc + 0xE6..pgc + 0xE8].copy_from_slice(&pgm_map_rel.to_be_bytes()); + d[pgc + 0xE8..pgc + 0xEA].copy_from_slice(&cell_tbl_rel.to_be_bytes()); + for (i, &fc) in program_first_cells.iter().enumerate() { + d[pgc + pgm_map_rel as usize + i] = fc; + } + let cell_base = pgc + cell_tbl_rel as usize; + for (i, (first, last, cat, secs)) in cells.iter().enumerate() { + let off = cell_base + i * 24; + write_cell_cat(&mut d, off, *first, *last, *cat); + d[off + 6] = *secs; // BCD seconds in the cell time field + } + d + } + + /// End-to-end bug-4 fix: a feature PGC that opens with a leading + /// interleaved-angle sub-block cell (category 0x80 = middle-of-angle-block) + /// must have that cell DROPPED from the muxed extents, so the rip starts at + /// the real feature. Chapters shift earlier by the dropped duration. + #[test] + fn scan_dvd_titles_drops_leading_scene_index_cell() { + let mut disc = MemDisc::new(); + let vmg = build_vmg(&[(2, 1, 1)]); + // Cell 0: leading scene-index/angle sub-block (cat 0x80), 5s, sectors 0..9. + // Cell 1: feature start (cat 0x00), 59s, sectors 100..199. + // Cell 2: feature (cat 0x00), 59s, sectors 300..399. + // Programs: prog0 → cell 1 (feature start), prog1 → cell 3. + let vts = build_vts_cells( + 1000, + 0x00, + &[ + (0, 9, 0x80, 0x05), + (100, 199, 0x00, 0x59), + (300, 399, 0x00, 0x59), + ], + &[1, 3], + ); + 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]; + // The leading 0x80 cell is dropped: 2 feature extents, not 3. + assert_eq!(t.extents.len(), 2, "leading angle sub-block cell dropped"); + // First extent starts at the feature cell (vob 1000 + 100), not at 1000+0. + assert_eq!(t.extents[0].start_lba, 1000 + 100); + assert_eq!(t.extents[1].start_lba, 1000 + 300); + // Chapter times shift earlier by the dropped 5s. Program 0 was at the + // dropped head (clamped to 0); program 1 was at cell 3 = + // dur(cell0)+dur(cell1) = 5 + 59 = 64s, now 59s after the 5s shift. + assert_eq!(t.chapters.len(), 2); + assert!( + (t.chapters[0].time_secs - 0.0).abs() < 0.01, + "ch0 clamped to 0, got {}", + t.chapters[0].time_secs + ); + assert!( + (t.chapters[1].time_secs - 59.0).abs() < 0.01, + "ch1 shifted by dropped 5s → 59s, got {}", + t.chapters[1].time_secs + ); + } + + /// Conservative guard end-to-end: a normal feature (every cell category + /// 0x00) is muxed in full — the filter drops nothing and chapters are + /// unshifted. This is the "Silence of the Lambs" case. + #[test] + fn scan_dvd_titles_plain_feature_untouched() { + let mut disc = MemDisc::new(); + let vmg = build_vmg(&[(2, 1, 1)]); + let vts = build_vts_cells( + 1000, + crate::ifo::v_atr_byte(crate::ifo::VIDEO_FORMAT_PAL, crate::ifo::ASPECT_16X9), + &[(0, 99, 0x00, 0x30), (200, 299, 0x00, 0x30)], + &[1, 2], + ); + 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]; + // Nothing dropped: both cells become extents, starting at the very head. + assert_eq!(t.extents.len(), 2); + assert_eq!(t.extents[0].start_lba, 1000); // 1000 + 0, head intact + assert_eq!(t.extents[1].start_lba, 1200); + // Chapter 0 stays at 0.0 (no shift). + assert!((t.chapters[0].time_secs - 0.0).abs() < 0.01); + } } diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 32ee006..9836640 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1291,8 +1291,16 @@ impl Disc { // AACS handshake (Blu-ray/UHD). Acquires the Volume ID via the // cert-based mutual-auth handshake (the OEM route); drive unlock // itself runs separately behind the pluggable `Unlocker` seam. - tracing::info!(target: "freemkv::scan", "phase: AACS handshake"); - let (handshake, handshake_error) = Self::do_handshake(session, opts); + // AACS is Blu-ray/UHD only. A DVD uses CSS — skip the AACS handshake + // entirely (the drive already classified the disc as DVD at init), so a + // DVD never issues AACS OEM-VID / cert SCSI against the drive before the + // CSS bus-auth runs. + let (handshake, handshake_error) = if session.disc_is_dvd() { + (None, None) + } else { + tracing::info!(target: "freemkv::scan", "phase: AACS handshake"); + Self::do_handshake(session, opts) + }; tracing::info!(target: "freemkv::scan", handshake = handshake.is_some(), "phase: handshake done"); // Request max read speed — removes riplock on DVD @@ -1660,7 +1668,7 @@ impl Disc { elapsed_ms = scan_with_t0.elapsed().as_millis() as u64, "end" ); - Ok(Disc { + let disc = Disc { volume_id: udf_fs.volume_id.clone(), meta_title, format, @@ -1677,7 +1685,16 @@ impl Disc { // which set this when they observe scrambled-but-uncracked content. css_error: None, content_format, - }) + }; + + // Structured scan diagnostic block (--log-level 3). Emits the + // per-title / per-stream / decision / AACS rows under the + // `freemkv::diag` target; a no-op unless that target is enabled. + // (DVD per-cell category rows are emitted earlier from the IFO scan, + // before the per-cell detail is lowered away.) + crate::diag::dump_disc(&disc); + + Ok(disc) } // ── Internal helpers ──────────────────────────────────────────────────── diff --git a/src/drive/mod.rs b/src/drive/mod.rs index f0fd8d1..b30e4da 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -370,7 +370,7 @@ impl Drive { } /// True when the mounted disc is a DVD (profile family `0x0010..=0x001F`). - fn disc_is_dvd(&mut self) -> bool { + pub(crate) fn disc_is_dvd(&mut self) -> bool { matches!(self.current_profile(), Some(p) if (0x0010..=0x001F).contains(&p)) } diff --git a/src/ifo.rs b/src/ifo.rs index 0115f15..295f901 100644 --- a/src/ifo.rs +++ b/src/ifo.rs @@ -58,6 +58,122 @@ pub struct DvdTitle { pub struct DvdCell { pub first_sector: u32, pub last_sector: u32, + /// Raw cell-category byte at `cell_playback + 0` (DVD-Video spec). + /// Packs cell_type (bits 7-6), block_mode (bits 5-4), block_type + /// (bits 3-2), seamless_play (bit 1), interleaved (bit 0). Carried so + /// the extent builder can recognise non-feature leading cells + /// (scene-index / interleaved angle sub-blocks) and the diagnostic dump + /// can show why a cell was kept or dropped. + pub category: u8, + /// Per-cell playback duration in seconds (BCD time at `cell_playback + 4`). + /// Used by the diagnostic dump and the conservative leading-cell filter + /// (a short leading scene-index cell vs the multi-minute feature). + pub duration_secs: f64, +} + +/// Decoded view of a cell-category byte (`cell_playback + 0`), per the +/// DVD-Video spec `cell_playback_information` layout. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CellCategory { + /// bits 7-6: 0=normal, 1=first cell of angle block, 2=middle, 3=last. + pub cell_type: u8, + /// bits 5-4: 0=not in block, 1=first cell of block, 2=in block, 3=last. + pub block_mode: u8, + /// bits 3-2: 0=not part of a block, 1=angle block. + pub block_type: u8, + /// bit 1: seamless playback (STC continuous). + pub seamless_play: bool, + /// bit 0: interleaved (multi-angle / seamless-branch interleave). + pub interleaved: bool, +} + +impl CellCategory { + /// Decode the raw `cell_playback + 0` byte. + pub fn decode(raw: u8) -> Self { + CellCategory { + cell_type: (raw >> 6) & 0x03, + block_mode: (raw >> 4) & 0x03, + block_type: (raw >> 2) & 0x03, + seamless_play: (raw & 0x02) != 0, + interleaved: (raw & 0x01) != 0, + } + } + + /// A plain feature cell: not part of any angle/interleave block. Every + /// cell of a normal single-angle feature decodes to this (`category` + /// byte `0x00`, or `0x00` in every block field with only the + /// seamless/interleaved flags possibly set). Such a cell is NEVER + /// dropped by the leading-cell filter. + pub fn is_plain_feature(&self) -> bool { + self.cell_type == 0 && self.block_mode == 0 && self.block_type == 0 + } + + /// Marks a non-first piece of an angle / interleaved block: a "middle" or + /// "last" cell of an angle block (`cell_type ∈ {2,3}`), or an + /// in-block / last-of-block cell (`block_mode ∈ {2,3}`). Concatenating + /// these back-to-back with the first angle duplicates content at the head + /// of the feature. Conservative: the FIRST cell of a block + /// (`cell_type==1` / `block_mode==1`) is NOT flagged — it is the angle we + /// keep. + pub fn is_secondary_block_piece(&self) -> bool { + matches!(self.cell_type, 2 | 3) || matches!(self.block_mode, 2 | 3) + } +} + +impl DvdTitle { + /// Index of the first cell to include in the muxed feature. + /// + /// Bug-4 (scene-selection / logo at the head of the feature): the main + /// feature's PGC can open with leading cells that are NOT part of the + /// movie — a scene-index segment or an interleaved-angle sub-block. Those + /// are recognisable by their cell-category byte: a leading cell flagged as + /// a *secondary* piece of an angle/interleave block + /// ([`CellCategory::is_secondary_block_piece`]) is not feature content. + /// + /// This walks the leading run and returns the index of the first cell that + /// is a plain feature cell (category `0x00`-class). Cells before it that + /// are secondary block pieces are dropped from the feature extents. + /// + /// **Conservative by construction — it can NEVER truncate a normal + /// feature:** + /// - It only ever skips a *prefix*; the scan stops at the first + /// plain-feature cell and keeps everything from there on. + /// - A normal single-angle feature has category `0x00` on cell 0, so the + /// scan stops immediately at index 0 and drops nothing. + /// - It never drops on duration or any heuristic — only on the spec + /// category bits — and it never drops the FIRST cell of an angle block + /// (the angle we keep). + /// - As a final guard it never returns past the last cell, and never drops + /// when that would leave zero cells. + /// + /// For "The Silence of the Lambs" (every feature cell category `0x00`, + /// chapter 1 at 00:00:00) this returns 0 — a no-op — which is the correct + /// result: the disc's scene-index lives in a separate menu/title PGC, not + /// in leading cells of the feature PGC, so there is nothing to drop here. + pub fn feature_start_cell(&self) -> usize { + let n = self.cells.len(); + if n == 0 { + return 0; + } + let mut idx = 0; + while idx < n { + let cat = CellCategory::decode(self.cells[idx].category); + // Stop at the first cell that is genuine feature content. + if !cat.is_secondary_block_piece() { + break; + } + idx += 1; + } + // Never drop everything: if every leading cell looked like a secondary + // block piece (pathological/corrupt category bytes), fall back to + // keeping all cells rather than producing an empty feature. + if idx >= n { 0 } else { idx } + } + + /// The feature cells after the leading-cell filter ([`feature_start_cell`]). + pub fn feature_cells(&self) -> &[DvdCell] { + &self.cells[self.feature_start_cell()..] + } } /// DVD TV system, from VTS_V_ATR `video_format` (byte 0 bits 5-4). @@ -305,11 +421,25 @@ fn parse_vts( return Err(Error::IfoParse); } - // VTS_PGCIT sector pointer - let pgcit_sector = be_u32(&vts_data, 0xCC)?; + // VTSI_MAT (VTS_xx_0.IFO header) field offsets — fixed by the DVD-Video + // spec (libdvdread `vtsi_mat_t`). The offsets are constant; the sector + // values they point to are per-disc. + const VTSTT_VOBS_OFFSET: usize = 0xC4; // VTS title VOBS start sector (feature) + const VTS_PGCIT_OFFSET: usize = 0xCC; // VTS_PGCIT sector pointer - // First VOB sector - let vob_start_sector = be_u32(&vts_data, 0xC0)?; + // VTS_PGCIT sector pointer + let pgcit_sector = be_u32(&vts_data, VTS_PGCIT_OFFSET)?; + + // First sector of the VTS **Title** VOBS (`vtstt_vobs`, VTSTT_VOBS_OFFSET). + // The cell `first_sector` / `last_sector` values in the title PGCs are + // relative to this. Offset 0xC0 is `vtsm_vobs` — the VTS *menu* VOBS + // (VTS_xx_0.VOB), which on discs with a per-title menu (e.g. a Universal + // "the parental level has been set, press yes" first-play still) holds that + // interactive prompt. Reading the menu base instead prepended the menu VOB + // to the feature and shifted every cell extent back by + // `vtstt_vobs - vtsm_vobs` sectors, so the rip opened on the parental + // prompt instead of the movie. The title content lives at `vtstt_vobs`. + let vob_start_sector = be_u32(&vts_data, VTSTT_VOBS_OFFSET)?; // Video attributes at offset 0x200 (2 bytes) let video = parse_video_attr(&vts_data)?; @@ -617,11 +747,15 @@ fn parse_pgc(data: &[u8], pgc_offset: usize, chapters: u16) -> Result if co + 24 > data.len() { break; } + let category = byte_at(data, co)?; + let duration_secs = bcd_to_secs(&data[co + 4..co + 8]); let first_sector = be_u32(data, co + 8)?; let last_sector = be_u32(data, co + 20)?; cells.push(DvdCell { first_sector, last_sector, + category, + duration_secs, }); } } @@ -781,6 +915,8 @@ mod tests { let cell = DvdCell { first_sector: 100, last_sector: 200, + category: 0, + duration_secs: 0.0, }; assert_eq!(cell.first_sector, 100); assert_eq!(cell.last_sector, 200); @@ -1349,6 +1485,159 @@ mod tests { assert!(title.cells.is_empty()); } + // ───────────────────────────────────────────────────────────────────── + // Cell-category decode + bug-4 leading-cell filter. + // ───────────────────────────────────────────────────────────────────── + + fn cell(first: u32, last: u32, category: u8) -> DvdCell { + DvdCell { + first_sector: first, + last_sector: last, + category, + duration_secs: 0.0, + } + } + + /// CellCategory decodes the spec bitfields: cell_type (7-6), block_mode + /// (5-4), block_type (3-2), seamless (1), interleaved (0). + #[test] + fn cell_category_decode_bits() { + // 0x00 → plain feature, nothing set. + let c = CellCategory::decode(0x00); + assert_eq!(c.cell_type, 0); + assert_eq!(c.block_mode, 0); + assert_eq!(c.block_type, 0); + assert!(!c.seamless_play); + assert!(!c.interleaved); + assert!(c.is_plain_feature()); + assert!(!c.is_secondary_block_piece()); + + // cell_type=1 (first of angle block), block_mode=1 (first of block): + // 0b01_01_00_0_0 = 0x50. This is the angle we KEEP — not secondary. + let c = CellCategory::decode(0b01_01_00_00); + assert_eq!(c.cell_type, 1); + assert_eq!(c.block_mode, 1); + assert!(!c.is_plain_feature()); + assert!(!c.is_secondary_block_piece()); + + // cell_type=2 (middle of angle block): 0b10_00_00_00 = 0x80 → secondary. + assert!(CellCategory::decode(0b10_00_00_00).is_secondary_block_piece()); + // cell_type=3 (last of angle block) → secondary. + assert!(CellCategory::decode(0b11_00_00_00).is_secondary_block_piece()); + // block_mode=2 (in block) → secondary; block_mode=3 (last of block) → secondary. + assert!(CellCategory::decode(0b00_10_00_00).is_secondary_block_piece()); + assert!(CellCategory::decode(0b00_11_00_00).is_secondary_block_piece()); + + // seamless (bit1) + interleaved (bit0) on an otherwise-plain cell must + // NOT make it secondary — they don't mark non-feature content. + let c = CellCategory::decode(0b00_00_00_11); + assert!(c.seamless_play); + assert!(c.interleaved); + assert!(c.is_plain_feature()); + assert!(!c.is_secondary_block_piece()); + } + + /// A normal single-angle feature (every cell category 0x00) is never + /// filtered: feature_start_cell == 0, feature_cells == all cells. This is + /// the "Silence of the Lambs" case — the filter must be a no-op. + #[test] + fn feature_filter_noop_on_plain_feature() { + let t = DvdTitle { + chapters: 3, + duration_secs: 6780.0, + cells: vec![ + cell(0, 99, 0x00), + cell(100, 199, 0x00), + cell(200, 299, 0x00), + ], + chapter_times: vec![0.0, 100.0, 200.0], + palette: None, + }; + assert_eq!(t.feature_start_cell(), 0); + assert_eq!(t.feature_cells().len(), 3); + } + + /// A leading interleaved/angle-block sub-cell (category marks a secondary + /// block piece) is dropped; the scan stops at the first plain cell and + /// keeps the rest. + #[test] + fn feature_filter_drops_leading_secondary_block_cells() { + let t = DvdTitle { + chapters: 2, + duration_secs: 100.0, + cells: vec![ + cell(0, 9, 0b10_00_00_00), // middle of angle block → drop + cell(10, 19, 0b00_11_00_00), // last of block → drop + cell(20, 119, 0x00), // feature starts here + cell(120, 219, 0x00), + ], + chapter_times: vec![0.0, 50.0], + palette: None, + }; + assert_eq!(t.feature_start_cell(), 2); + let fc = t.feature_cells(); + assert_eq!(fc.len(), 2); + assert_eq!(fc[0].first_sector, 20); + } + + /// Conservative guard: if EVERY cell looks like a secondary block piece + /// (corrupt/pathological category bytes), the filter refuses to drop them + /// all — it returns 0 and keeps every cell rather than emit an empty + /// feature. + #[test] + fn feature_filter_never_empties_title() { + let t = DvdTitle { + chapters: 1, + duration_secs: 100.0, + cells: vec![cell(0, 9, 0b10_00_00_00), cell(10, 19, 0b11_00_00_00)], + chapter_times: vec![0.0], + palette: None, + }; + assert_eq!(t.feature_start_cell(), 0); + assert_eq!(t.feature_cells().len(), 2); + } + + /// An empty title (no cells) returns 0 and an empty slice — no panic. + #[test] + fn feature_filter_empty_cells() { + let t = DvdTitle { + chapters: 0, + duration_secs: 0.0, + cells: vec![], + chapter_times: vec![], + palette: None, + }; + assert_eq!(t.feature_start_cell(), 0); + assert!(t.feature_cells().is_empty()); + } + + /// parse_pgc populates the new `category` + `duration_secs` cell fields + /// from `cell_playback + 0` and the BCD time at `cell_playback + 4`. + #[test] + fn pgc_reads_cell_category_and_duration() { + let mut pgc = vec![0u8; 0xEA]; + pgc[0x02] = 1; + pgc[0x03] = 2; // 2 cells + pgc[0xE8] = 0x00; + pgc[0xE9] = 0xEA; + pgc.resize(0xEA + 48, 0); + // Cell 0: category byte = 0x80 (middle of angle block), 5s BCD. + pgc[0xEA] = 0x80; + pgc[0xEA + 6] = 0x05; + pgc[0xEA + 8..0xEA + 12].copy_from_slice(&10u32.to_be_bytes()); + // Cell 1: category 0x00 (plain feature), 7s BCD. + pgc[0xEA + 24] = 0x00; + pgc[0xEA + 24 + 6] = 0x07; + pgc[0xEA + 24 + 8..0xEA + 24 + 12].copy_from_slice(&20u32.to_be_bytes()); + let title = parse_pgc(&pgc, 0, 2).unwrap(); + assert_eq!(title.cells[0].category, 0x80); + assert!((title.cells[0].duration_secs - 5.0).abs() < 0.01); + assert_eq!(title.cells[1].category, 0x00); + assert!((title.cells[1].duration_secs - 7.0).abs() < 0.01); + // The leading secondary-block cell is filtered out of the feature. + assert_eq!(title.feature_start_cell(), 1); + } + /// Regression: a crafted IFO whose program-map byte names a first_cell /// index larger than the actual cell count must NOT panic. Before the fix, /// `cell_durations[..first_cell.saturating_sub(1)]` would panic with an diff --git a/src/io/pipeline.rs b/src/io/pipeline.rs index 72d4329..1c2c7af 100644 --- a/src/io/pipeline.rs +++ b/src/io/pipeline.rs @@ -290,6 +290,16 @@ impl Pipeline { let mut first_err: Option = None; let mut stopped = false; + // Rolling apply-throughput summary. The per-item "apply: OK" + // line was 99% of the mux log; collapse it into a periodic + // summary (count, avg ms, items/s) emitted ~every 5 s while + // debug tracing is on. The individual slow-apply ("took … s") + // STALL events below stay visible — those are signal, not noise. + let mut summary_count: u64 = 0; + let mut summary_nanos: u128 = 0; + let mut summary_since = Instant::now(); + const SUMMARY_INTERVAL: Duration = Duration::from_secs(5); + while let Ok(item) = rx.recv() { let debug = debug_enabled(); if debug { @@ -338,21 +348,50 @@ impl Pipeline { if let Some(start) = apply_start { let apply_elapsed = start.elapsed(); if apply_elapsed > Duration::from_millis(100) { + // STALL event — a single slow apply. Keep it visible: + // its presence is a signal, not per-frame noise. tracing::debug!( "Pipeline apply: took {:.2}s, item={}", apply_elapsed.as_secs_f64(), std::any::type_name::() ); - } else { + } + // Benign per-item OK: roll into the periodic summary + // rather than logging one line per frame. + summary_count += 1; + summary_nanos += apply_elapsed.as_nanos(); + if summary_since.elapsed() >= SUMMARY_INTERVAL && summary_count > 0 { + let secs = summary_since.elapsed().as_secs_f64(); + let avg_ms = (summary_nanos as f64 / summary_count as f64) / 1_000_000.0; tracing::debug!( - "Pipeline apply: OK in {:.3}ms, item={}", - apply_elapsed.as_micros(), + "Pipeline apply summary: {} items in {:.1}s, avg {:.3}ms, {:.0} items/s, type={}", + summary_count, + secs, + avg_ms, + summary_count as f64 / secs.max(1e-9), std::any::type_name::() ); + summary_count = 0; + summary_nanos = 0; + summary_since = Instant::now(); } } } + // Flush the residual apply-summary tail at end-of-stream so the + // last partial window's item count isn't silently dropped. + if summary_count > 0 && debug_enabled() { + let secs = summary_since.elapsed().as_secs_f64(); + let avg_ms = (summary_nanos as f64 / summary_count as f64) / 1_000_000.0; + tracing::debug!( + "Pipeline apply summary (final): {} items in {:.1}s, avg {:.3}ms, type={}", + summary_count, + secs, + avg_ms, + std::any::type_name::() + ); + } + // Final abandonment check: the common leak case is a // consumer wedged inside `apply` (a blocking write // syscall). When that syscall finally returns, the @@ -401,13 +440,16 @@ impl Pipeline { if let Some(start) = start { let elapsed = start.elapsed(); if elapsed > Duration::from_millis(10) { + // BLOCKED event — back-pressure stall. Keep visible. tracing::debug!( "Pipeline send: blocked {:.2}s, item={}", elapsed.as_secs_f64(), std::any::type_name::() ); } else { - tracing::debug!("Pipeline send: OK in {:.3}ms", elapsed.as_micros()); + // Benign per-item OK: trace-level (L4) only; the + // apply-side rolling summary carries throughput. + tracing::trace!("Pipeline send: OK in {:.3}ms", elapsed.as_micros()); } } Ok(()) diff --git a/src/io/writeback_file/mod.rs b/src/io/writeback_file/mod.rs index c7406dc..cee3718 100644 --- a/src/io/writeback_file/mod.rs +++ b/src/io/writeback_file/mod.rs @@ -101,6 +101,13 @@ pub(crate) struct WritebackFile { file: File, pipeline: WritebackPipeline, pos: u64, + /// Count of position-moving seeks (for the finalize summary). The MKV muxer + /// seeks back occasionally (cluster size patching, Cues, Segment header + /// backpatch); the per-seek DEBUG line is trace-level now, and this rolls + /// the total into one finalize summary. + seek_count: u64, + /// Sum of |delta| over all position-moving seeks, in bytes. + seek_bytes: u64, } impl WritebackFile { @@ -115,6 +122,8 @@ impl WritebackFile { file, pipeline, pos, + seek_count: 0, + seek_bytes: 0, }) } @@ -176,6 +185,14 @@ impl WritebackFile { /// then external commit/DB update) must not treat `Ok(())` as a /// durability barrier. pub(crate) fn sync_all(&mut self) -> io::Result<()> { + if self.seek_count > 0 { + tracing::debug!( + target: "mux", + "WritebackFile finalize: {} seeks, {} bytes seeked total", + self.seek_count, + self.seek_bytes + ); + } self.pipeline.finalize(); platform::durable_sync(&self.file) } @@ -219,10 +236,14 @@ impl Seek for WritebackFile { let from_pos = self.pos; let to_pos = p; let delta: i64 = (to_pos as i64).wrapping_sub(from_pos as i64); - tracing::debug!( + // Per-seek detail is trace-level (L4) — benign and high-frequency. + // The aggregate (count + total bytes) is logged once at finalize. + tracing::trace!( target: "mux", "WritebackFile seek from={from_pos} to={to_pos} delta={delta}" ); + self.seek_count += 1; + self.seek_bytes += delta.unsigned_abs(); self.pipeline.handle_seek(p); self.pos = p; } diff --git a/src/lib.rs b/src/lib.rs index c29fb1b..55ba4fb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,6 +89,7 @@ pub mod aacs; pub(crate) mod clpi; pub mod css; pub mod decrypt; +pub mod diag; pub mod disc; pub mod drive; pub mod error; diff --git a/src/mux/codec/ac3.rs b/src/mux/codec/ac3.rs index 28daaaa..fa88739 100644 --- a/src/mux/codec/ac3.rs +++ b/src/mux/codec/ac3.rs @@ -261,6 +261,81 @@ fn frame_duration_ns(data: &[u8], bsid: u8) -> u64 { (samples * 1_000_000_000 + rate / 2) / rate } +/// Base channel count per AC-3 `acmod` (A/52 Table 5.8), BEFORE the LFE. +/// Index is the 3-bit acmod value; add 1 when `lfeon` is set. +/// +/// ```text +/// 0 = 1+1 (Ch1, Ch2) -> 2 4 = 3/0 (L,C,R) -> 3 +/// 1 = 1/0 (C, mono) -> 1 5 = 2/1 (L,R,S) -> 3 +/// 2 = 2/0 (L, R) -> 2 6 = 3/1 (L,C,R,S) -> 4 +/// 3 = 3/0 (L,C,R) -> 3 7 = 3/2 (L,C,R,SL,SR) -> 5 +/// ``` +const ACMOD_CHANNELS: [u8; 8] = [2, 1, 2, 3, 3, 3, 4, 5]; + +/// Decode the channel count of an (E-)AC-3 frame from its bitstream `acmod` and +/// `lfeon`, starting at the 0x0B77 syncword. Returns `None` when the frame is +/// too short to carry the BSI bits. +/// +/// This is the AUTHORITATIVE channel count for the track header: the DVD IFO +/// `audio_attr_t.channels` nibble is a well-known unreliable/stale field, so +/// the muxer prefers this over the IFO-claimed count (mirrors MakeMKV / +/// HandBrake, which never trust the IFO audio nibble). LFE adds one channel +/// (e.g. acmod=7 + lfeon → 6 = 5.1). +/// +/// Bit layout from the syncword (A/52 §5.3.2 BSI): +/// +/// ```text +/// byte 5: bsid(5) | bsmod(3) +/// byte 6: acmod(3) | [cmixlev(2) if acmod has a centre and acmod!=1] +/// | [surmixlev(2) if acmod has surround] +/// | [dsurmod(2) if acmod==2] | lfeon(1) | ... +/// ``` +/// +/// `acmod` therefore always occupies byte-6 bits 7-5; `lfeon` follows a +/// variable number of optional 2-bit fields, so we track the bit cursor. +pub(crate) fn acmod_channels(data: &[u8]) -> Option { + // Need at least bytes 0..=6 to read acmod (byte 6) and its trailing + // optional fields + lfeon (which never spills past byte 7 for any acmod). + if data.len() < 8 { + return None; + } + let bsid = get_bsid(data); + // E-AC-3 (bsid >= 11, Annex E) uses a different BSI layout. DVD audio is + // always legacy AC-3 (bsid <= 8); for E-AC-3 we don't decode acmod here + // and let the caller fall back to the passed channel count. + if bsid >= 11 { + return None; + } + // Bit cursor over `data`, MSB-first, starting at byte 6 bit 7 (= bit 48). + let mut bit = 6 * 8; + let read = |n: usize, bit: &mut usize| -> u32 { + let mut v = 0u32; + for _ in 0..n { + let byte = data[*bit / 8]; + let shift = 7 - (*bit % 8); + v = (v << 1) | ((byte >> shift) & 1) as u32; + *bit += 1; + } + v + }; + let acmod = read(3, &mut bit) as usize; + // cmixlev: present when acmod has a centre channel AND is not the 1/0 + // (centre-only) mode — i.e. acmod & 0x1 != 0 && acmod != 0x1. + if (acmod & 0x1) != 0 && acmod != 0x1 { + let _cmixlev = read(2, &mut bit); + } + // surmixlev: present when acmod has a surround channel (acmod & 0x4). + if (acmod & 0x4) != 0 { + let _surmixlev = read(2, &mut bit); + } + // dsurmod: present only for the 2/0 (stereo) mode. + if acmod == 0x2 { + let _dsurmod = read(2, &mut bit); + } + let lfeon = read(1, &mut bit); + Some(ACMOD_CHANNELS[acmod] + lfeon as u8) +} + /// Find AC3/E-AC-3 syncword (0x0B77) in data. fn find_ac3_sync(data: &[u8]) -> Option { (0..data.len().saturating_sub(1)).find(|&i| data[i] == 0x0B && data[i + 1] == 0x77) @@ -968,6 +1043,112 @@ mod tests { assert!(parser.flush().is_empty()); } + // --- acmod_channels: channel count from the AC-3 BSI bitstream --- + + /// Build a minimal AC-3 BSI header (8 bytes) with a given acmod + lfeon. + /// byte5 = bsid<<3 (bsmod=0); byte6 carries acmod in bits 7-5 followed by + /// the optional mix-level fields and lfeon. We construct byte6/7 by writing + /// bits MSB-first in the exact order acmod_channels reads them. + fn make_bsi(acmod: u8, lfeon: bool) -> Vec { + // Collect the bit sequence after byte 6 bit 7: acmod(3), [cmixlev(2)], + // [surmixlev(2)], [dsurmod(2)], lfeon(1). Mix-level/dsurmod bits are + // arbitrary (0 here) — only their PRESENCE shifts lfeon's position. + let mut bits: Vec = Vec::new(); + for i in (0..3).rev() { + bits.push((acmod >> i) & 1); + } + if (acmod & 0x1) != 0 && acmod != 0x1 { + bits.push(0); + bits.push(0); // cmixlev + } + if (acmod & 0x4) != 0 { + bits.push(0); + bits.push(0); // surmixlev + } + if acmod == 0x2 { + bits.push(0); + bits.push(0); // dsurmod + } + bits.push(lfeon as u8); // lfeon + // Pack bits MSB-first starting at byte 6. + let mut frame = vec![0u8; 8]; + frame[0] = 0x0B; + frame[1] = 0x77; + frame[5] = 8 << 3; // bsid = 8 (legacy AC-3), bsmod = 0 + for (idx, &b) in bits.iter().enumerate() { + let bitpos = 6 * 8 + idx; + if b != 0 { + frame[bitpos / 8] |= 1 << (7 - (bitpos % 8)); + } + } + frame + } + + #[test] + fn acmod_channels_stereo_2_0_no_lfe() { + // acmod=2 (2/0 L,R), no LFE → 2 channels. Verifies the channel count is + // read from the AC-3 bitstream's acmod, independent of any IFO claim. + // (A disc whose IFO lists 5.1 but where the wrong physical substream is + // selected is a separate stream-SELECTION bug, not this label path — + // tracked for rc.5.2.) + assert_eq!(acmod_channels(&make_bsi(2, false)), Some(2)); + } + + #[test] + fn acmod_channels_5_1() { + // acmod=7 (3/2 L,C,R,SL,SR) + LFE → 6 channels (5.1). + assert_eq!(acmod_channels(&make_bsi(7, true)), Some(6)); + // 3/2 without LFE → 5 channels. + assert_eq!(acmod_channels(&make_bsi(7, false)), Some(5)); + } + + #[test] + fn acmod_channels_mono_and_dual_mono() { + // acmod=1 (1/0 centre/mono) → 1; with LFE → 2. + assert_eq!(acmod_channels(&make_bsi(1, false)), Some(1)); + assert_eq!(acmod_channels(&make_bsi(1, true)), Some(2)); + // acmod=0 (1+1 dual mono) → 2 base channels. + assert_eq!(acmod_channels(&make_bsi(0, false)), Some(2)); + } + + #[test] + fn acmod_channels_3_0_and_2_1() { + // acmod=4 (3/0 L,C,R) → 3 (exercises cmixlev present, surmixlev absent). + assert_eq!(acmod_channels(&make_bsi(4, false)), Some(3)); + // acmod=5 (2/1 L,R,S) → 3 (surmixlev present, no centre). + assert_eq!(acmod_channels(&make_bsi(5, false)), Some(3)); + // acmod=6 (3/1) + LFE → 5; lfeon position shifts after both + // cmixlev (centre) and surmixlev (surround) 2-bit fields. + assert_eq!(acmod_channels(&make_bsi(6, true)), Some(5)); + } + + #[test] + fn acmod_channels_short_frame_is_none() { + // Fewer than 8 bytes cannot carry the BSI bits → None (caller falls + // back to the IFO-claimed channel count). + assert_eq!(acmod_channels(&[0x0B, 0x77, 0, 0, 0, 8 << 3]), None); + assert_eq!(acmod_channels(&[]), None); + } + + #[test] + fn acmod_channels_eac3_is_none() { + // E-AC-3 (bsid >= 11) uses a different BSI layout; acmod_channels + // declines so the caller keeps the passed count. + let mut data = make_bsi(2, false); + data[5] = 16 << 3; // bsid = 16 (E-AC-3) + assert_eq!(acmod_channels(&data), None); + } + + #[test] + fn acmod_channels_parses_real_built_frame() { + // A frame built by make_ac3_frame (fscod/frmsizecod set, acmod bits 0) + // decodes acmod=0 → 2 channels (dual mono), confirming the cursor lands + // on the right bytes for a fully-formed frame, not just a stub header. + let frame = make_ac3_frame(0, 2); + // make_ac3_frame leaves byte 6 = 0 → acmod=0, lfeon=0 → 2 channels. + assert_eq!(acmod_channels(&frame), Some(2)); + } + // helper: PES with a generic pts for E-AC-3 tests fn make_eac3_pes(data: Vec) -> PesPacket { PesPacket { diff --git a/src/mux/disc.rs b/src/mux/disc.rs index 77a176c..dd491ef 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -161,6 +161,10 @@ pub struct DiscStream { halt: Option, event_fn: Option>, eof: bool, + /// Count of dropped DVD navigation packets (private_stream_2, 0xBF) — these + /// are expected on every disc; tallied and summarised once at EOF instead of + /// a per-packet WARN. + dropped_nav_packets: u64, // Cumulative bytes successfully read from the source. Drives // EventKind::BytesRead emission and autorip's per-device progress. @@ -284,6 +288,7 @@ impl DiscStream { halt: None, event_fn: None, eof: false, + dropped_nav_packets: 0, bytes_read_total: 0, bytes_total_extents, ts_demuxer, @@ -578,6 +583,13 @@ impl crate::pes::Stream for DiscStream { let t0 = self.profiling.then(std::time::Instant::now); if !self.fill_extents()? { self.eof = true; + if self.dropped_nav_packets > 0 { + tracing::debug!( + target: "mux", + "dropped {} DVD navigation packets (private_stream_2/0xBF) — expected, carry no elementary stream", + self.dropped_nav_packets + ); + } // Flush demuxer — last PES packet may still be in the assembler if let Some(ref mut demuxer) = self.ts_demuxer { for pes in &demuxer.flush() { @@ -603,12 +615,20 @@ impl crate::pes::Stream for DiscStream { // pipelined_stream.rs); the old (sub_id & 0x1F)+1 // heuristic mis-routed VobSub into the AC-3 parser. let Some(pid) = ps.dvd_pid() else { - tracing::warn!( - target: "mux", - "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", - ps.stream_id, - ps.sub_stream_id, - ); + if ps.is_nav() { + // Expected DVD navigation packet (PCI/DSI) — + // tally, no WARN. + self.dropped_nav_packets += 1; + } else { + // Unexpected unmappable stream_id (a + // possibly-dropped real stream). Keep the WARN. + tracing::warn!( + target: "mux", + "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", + ps.stream_id, + ps.sub_stream_id, + ); + } continue; }; let Some((_, track)) = @@ -714,12 +734,20 @@ impl crate::pes::Stream for DiscStream { // pipelined_stream.rs); the old (sub_id & 0x1F)+1 // heuristic mis-routed VobSub into the AC-3 parser. let Some(pid) = ps.dvd_pid() else { - tracing::warn!( - target: "mux", - "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", - ps.stream_id, - ps.sub_stream_id, - ); + if ps.is_nav() { + // Expected DVD navigation packet (PCI/DSI) — tally, + // no WARN. + self.dropped_nav_packets += 1; + } else { + // Unexpected unmappable stream_id (a possibly-dropped + // real stream). Keep the individual WARN. + tracing::warn!( + target: "mux", + "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", + ps.stream_id, + ps.sub_stream_id, + ); + } continue; }; let Some((_, track)) = diff --git a/src/mux/ebml.rs b/src/mux/ebml.rs index 51ca701..adc6446 100644 --- a/src/mux/ebml.rs +++ b/src/mux/ebml.rs @@ -419,6 +419,11 @@ pub const CODEC_ID: u32 = 0x86; pub const CODEC_PRIVATE: u32 = 0x63A2; pub const TRACK_NAME: u32 = 0x536E; pub const DEFAULT_DURATION: u32 = 0x23_E383; +/// DefaultDecodedFieldDuration — nanoseconds per FIELD (half a frame for +/// interlaced content). Emitting it on an interlaced track tells a reader the +/// field rate so it stops halving the frame rate (Windows shell shows 12.5 fps +/// for a 25 fps 576i stream without it). RFC 9559 / Matroska v4. +pub const DEFAULT_DECODED_FIELD_DURATION: u32 = 0x23_4E7A; // Video pub const VIDEO: u32 = 0xE0; @@ -434,6 +439,10 @@ pub const INTERLACED_PROGRESSIVE: u64 = 2; // NTSC DVD (480i) and HD (1080i) are top-field-first; PAL DVD (576i) is // bottom-field-first. 0xFF is our sentinel for "undetermined / omit". pub const FIELD_ORDER_TFF: u8 = 2; +// Bottom-field-first. Retained for completeness/round-trip tests; the muxer +// emits TFF for all DVD/HD interlaced content (DV is the only common BFF +// source and freemkv does not produce it). +#[allow(dead_code)] pub const FIELD_ORDER_BFF: u8 = 9; pub const FIELD_ORDER_UNDETERMINED: u8 = 0xFF; pub const DISPLAY_WIDTH: u32 = 0x54B0; @@ -472,6 +481,18 @@ pub const CUE_TRACK_POSITIONS: u32 = 0xB7; pub const CUE_TRACK: u32 = 0xF7; pub const CUE_CLUSTER_POSITION: u32 = 0xF1; +// Tags — per-track statistics tags. mkvmerge convention: a `BPS` SimpleTag +// per track carries the bits-per-second so readers (Windows Explorer's MKV +// property handler) that read the container tag rather than computing from +// stream size show a bitrate for every track, not just CBR audio. +pub const TAGS: u32 = 0x1254_C367; +pub const TAG: u32 = 0x7373; +pub const TARGETS: u32 = 0x63C0; +pub const TAG_TRACK_UID: u32 = 0x63C5; +pub const SIMPLE_TAG: u32 = 0x67C8; +pub const TAG_NAME: u32 = 0x45A3; +pub const TAG_STRING: u32 = 0x4487; + // Chapters pub const CHAPTERS: u32 = 0x1043_A770; pub const EDITION_ENTRY: u32 = 0x45B9; diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index f283020..60ac060 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -36,6 +36,9 @@ pub struct MkvTrack { // meaningful when interlaced; `FIELD_ORDER_UNDETERMINED` omits it. pub interlaced: bool, pub field_order: u8, + /// DefaultDecodedFieldDuration (ns per field) for interlaced video — half + /// the frame `default_duration_ns`. 0 = omit (progressive / unknown). + pub field_duration_ns: u64, // Audio-specific pub sample_rate: f64, pub channels: u8, @@ -131,17 +134,25 @@ impl MkvTrack { colour_primaries: primaries, colour_range: range, interlaced: v.resolution.is_interlaced(), - // PAL DVD (576i) is bottom-field-first; NTSC DVD (480i) is - // top-field-first. HD interlaced (1080i) is top-field-first. - // Progressive content leaves the field order undetermined. + // PAL DVD (576i), NTSC DVD (480i), and HD interlaced (1080i) are + // all top-field-first ("almost everything but DV is TFF"). MediaInfo + // reads "Top Field First" off the MPEG-2 picture coding extension, + // so the container element must agree — emitting BFF here for 576i + // (the pre-rc.5.1 value) was a wrong container value that disagreed + // with the stream. Progressive content leaves the order undetermined. field_order: if v.resolution.is_interlaced() { - match v.resolution { - Resolution::R576i => ebml::FIELD_ORDER_BFF, - _ => ebml::FIELD_ORDER_TFF, - } + ebml::FIELD_ORDER_TFF } else { ebml::FIELD_ORDER_UNDETERMINED }, + // One field is half a frame. For 576i 25 fps (40 ms frame) this is + // 20 ms; for 480i 29.97 fps (~33.4 ms frame) ~16.68 ms. Only set on + // interlaced tracks with a known frame duration. + field_duration_ns: if v.resolution.is_interlaced() && default_duration_ns > 0 { + default_duration_ns / 2 + } else { + 0 + }, sample_rate: 0.0, channels: 0, bit_depth: 0, @@ -214,6 +225,7 @@ impl MkvTrack { colour_range: 0, interlaced: false, field_order: ebml::FIELD_ORDER_UNDETERMINED, + field_duration_ns: 0, sample_rate: sr, channels: ch, bit_depth: 0, @@ -249,6 +261,7 @@ impl MkvTrack { colour_range: 0, interlaced: false, field_order: ebml::FIELD_ORDER_UNDETERMINED, + field_duration_ns: 0, sample_rate: 0.0, channels: 0, bit_depth: 0, @@ -305,6 +318,33 @@ pub struct MkvMuxer { info_offset: u64, tracks_offset: u64, chapters_offset: Option, + /// Total payload bytes muxed PER TRACK (index = track_idx). Used to emit a + /// per-track `BPS` statistics tag (bytes*8/duration) at finalize so Windows + /// shows a bitrate for every track, not just CBR audio. + track_bytes: Vec, + /// Track UIDs in track order (parallels `track_bytes`), for the BPS Targets. + track_uids: Vec, + /// Segment duration in seconds (from `Info`), for the BPS denominator. + duration_secs: f64, + /// Per-AC-3-audio-track channel-correction state. The DVD IFO audio nibble + /// is unreliable, so the channel count written in the track header is + /// corrected from the AC-3 bitstream `acmod` of the first frame on the + /// track. Each entry records the file offset of the 1-byte Channels value + /// (to patch in place) and the IFO-claimed count (to warn on disagreement); + /// `corrected` flips once patched so we only act on the first frame. + ac3_channel_fixups: std::collections::HashMap, +} + +/// Deferred AC-3 channel-count correction: the track header's `Channels` byte +/// is written up-front from the (unreliable) IFO count; on the first AC-3 frame +/// for the track the value is rewritten from the bitstream `acmod`. +struct Ac3ChannelFixup { + /// Absolute file offset of the 1-byte Channels value in the Tracks element. + value_offset: u64, + /// Channel count the IFO claimed (already written at `value_offset`). + claimed: u8, + /// True once the first frame has been parsed and the value finalised. + corrected: bool, } /// TimestampScale: nanoseconds per Matroska timestamp tick. 0.1 ms (100_000 ns). @@ -645,10 +685,15 @@ impl MkvMuxer { let tracks_start = writer.stream_position()?; let tracks_offset = tracks_start - segment_start; let tracks_pos = ebml::start_master(&mut writer, ebml::TRACKS)?; + let mut track_uids: Vec = Vec::with_capacity(tracks.len()); + let mut ac3_channel_fixups: std::collections::HashMap = + std::collections::HashMap::new(); for (i, track) in tracks.iter().enumerate() { + let track_uid = (i + 1) as u64 | 0x100_0000; + track_uids.push(track_uid); let entry_pos = ebml::start_master(&mut writer, ebml::TRACK_ENTRY)?; ebml::write_uint(&mut writer, ebml::TRACK_NUMBER, (i + 1) as u64)?; - ebml::write_uint(&mut writer, ebml::TRACK_UID, (i + 1) as u64 | 0x100_0000)?; + ebml::write_uint(&mut writer, ebml::TRACK_UID, track_uid)?; ebml::write_uint(&mut writer, ebml::TRACK_TYPE, track.track_type)?; ebml::write_uint(&mut writer, ebml::FLAG_LACING, 0)?; ebml::write_string(&mut writer, ebml::CODEC_ID, track.codec_id)?; @@ -682,6 +727,23 @@ impl MkvMuxer { )?; } + // DefaultDecodedFieldDuration (one FIELD = half a frame) on + // interlaced tracks. Per the Matroska schema it is a DIRECT child + // of TrackEntry (NOT inside Video). Without it an interlace-aware + // reader (Windows shell) assumes "block = one field" and reports + // half the frame rate (12.5 instead of 25 for 576i). DefaultDuration + // above stays the full-frame period (40 ms); this is 20 ms. + if track.track_type == ebml::TRACK_TYPE_VIDEO + && track.interlaced + && track.field_duration_ns > 0 + { + ebml::write_uint( + &mut writer, + ebml::DEFAULT_DECODED_FIELD_DURATION, + track.field_duration_ns, + )?; + } + // Video-specific if track.track_type == ebml::TRACK_TYPE_VIDEO && track.pixel_width > 0 { let vid_pos = ebml::start_master(&mut writer, ebml::VIDEO)?; @@ -748,7 +810,23 @@ impl MkvMuxer { // Omit Channels when unknown (0) — Matroska defaults it to 1 // rather than us fabricating a 6-channel count. if track.channels > 0 { + // Record the offset of the 1-byte Channels value so an AC-3 + // track can correct it from the bitstream acmod on its first + // frame (the IFO nibble is unreliable). write_uint emits + // ID(0x9F, 1B) + size(0x81, 1B) + value(1B) for 1..=255, so + // the value byte sits 2 bytes after the element start. + let chan_elem_pos = writer.stream_position()?; ebml::write_uint(&mut writer, ebml::CHANNELS, track.channels as u64)?; + if track.codec_id == ebml::CODEC_AC3 { + ac3_channel_fixups.insert( + i, + Ac3ChannelFixup { + value_offset: chan_elem_pos + 2, + claimed: track.channels, + corrected: false, + }, + ); + } } if track.bit_depth > 0 { ebml::write_uint(&mut writer, ebml::BIT_DEPTH, track.bit_depth as u64)?; @@ -803,6 +881,10 @@ impl MkvMuxer { info_offset, tracks_offset, chapters_offset, + track_bytes: vec![0u64; tracks.len()], + track_uids, + duration_secs, + ac3_channel_fixups, }) } @@ -969,6 +1051,42 @@ impl MkvMuxer { } self.frame_count += 1; + // Per-track byte total for the finalize-time BPS statistics tag. + if let Some(b) = self.track_bytes.get_mut(track_idx) { + *b += data.len() as u64; + } + + // Correct the AC-3 track's Channels element from the bitstream acmod on + // the FIRST frame of the track. The DVD IFO audio nibble is unreliable + // (it claims 5.1 on a 2.0 stream); the bitstream acmod is authoritative. + // Only the first frame triggers it; the byte width is unchanged so the + // patch is a single-byte in-place rewrite (then restore position). + if let Some(fixup) = self.ac3_channel_fixups.get_mut(&track_idx) { + if !fixup.corrected { + match super::codec::ac3::acmod_channels(data) { + Some(actual) if actual > 0 => { + if actual != fixup.claimed { + tracing::warn!( + target: "mux", + "AC-3 track {track_idx}: IFO claimed {} channels but bitstream acmod says {}; trusting the bitstream (possible wrong-stream selection)", + fixup.claimed, + actual, + ); + let here = self.writer.stream_position()?; + self.writer + .seek(std::io::SeekFrom::Start(fixup.value_offset))?; + self.writer.write_all(&[actual])?; + self.writer.seek(std::io::SeekFrom::Start(here))?; + } + fixup.corrected = true; + } + // Frame too short to carry the BSI bits — keep the passed + // (IFO) value and try again on the next frame. + _ => {} + } + } + } + Ok(()) } @@ -1015,6 +1133,12 @@ impl MkvMuxer { ebml::end_master(&mut self.writer, cues_pos)?; } + // Per-track BPS statistics tags (mkvmerge convention). A reader that + // reads the container `BPS` tag (Windows Explorer's MKV property + // handler) rather than computing bitrate from stream size shows a + // bitrate for EVERY track this way, not just CBR audio. + self.write_bps_tags()?; + // Back-patch SeekHead SeekPosition values now that all element offsets are known. for fixup in &self.seek_fixups { let offset = match fixup.target_id { @@ -1036,6 +1160,48 @@ impl MkvMuxer { Ok(()) } + /// Write a `Tags` master with a per-track `BPS` SimpleTag (bytes*8 / + /// duration_secs). Mirrors mkvmerge's per-track statistics tag so readers + /// that surface the container tag (Windows Explorer) show a bitrate for + /// every track. No-op when the duration is unknown (can't compute a rate) + /// or no track carried any bytes. + fn write_bps_tags(&mut self) -> io::Result<()> { + if self.duration_secs <= 0.0 { + return Ok(()); + } + if self.track_bytes.iter().all(|&b| b == 0) { + return Ok(()); + } + let tags_pos = ebml::start_master(&mut self.writer, ebml::TAGS)?; + // Snapshot to avoid borrowing self across the writer borrow. + let entries: Vec<(u64, u64)> = self + .track_uids + .iter() + .zip(self.track_bytes.iter()) + .map(|(&uid, &bytes)| (uid, bytes)) + .collect(); + for (uid, bytes) in entries { + if bytes == 0 { + continue; + } + // bits per second = bytes * 8 / duration_secs, rounded to nearest. + let bps = ((bytes as f64) * 8.0 / self.duration_secs).round() as u64; + let tag_pos = ebml::start_master(&mut self.writer, ebml::TAG)?; + // Targets → TagTrackUID (this tag applies to one track). + let targets_pos = ebml::start_master(&mut self.writer, ebml::TARGETS)?; + ebml::write_uint(&mut self.writer, ebml::TAG_TRACK_UID, uid)?; + ebml::end_master(&mut self.writer, targets_pos)?; + // SimpleTag(TagName="BPS", TagString=""). + let st_pos = ebml::start_master(&mut self.writer, ebml::SIMPLE_TAG)?; + ebml::write_string(&mut self.writer, ebml::TAG_NAME, "BPS")?; + ebml::write_string(&mut self.writer, ebml::TAG_STRING, &bps.to_string())?; + ebml::end_master(&mut self.writer, st_pos)?; + ebml::end_master(&mut self.writer, tag_pos)?; + } + ebml::end_master(&mut self.writer, tags_pos)?; + Ok(()) + } + fn start_cluster(&mut self, ts_ticks: i64) -> io::Result<()> { // Close previous cluster if open if self.cluster_open { @@ -1199,6 +1365,7 @@ mod tests { colour_range: 0, interlaced: false, field_order: ebml::FIELD_ORDER_UNDETERMINED, + field_duration_ns: 0, sample_rate: 0.0, channels: 0, bit_depth: 0, @@ -1226,6 +1393,7 @@ mod tests { colour_range: 0, interlaced: false, field_order: ebml::FIELD_ORDER_UNDETERMINED, + field_duration_ns: 0, sample_rate: 48000.0, channels: 6, bit_depth: 0, @@ -2986,12 +3154,12 @@ mod tests { #[test] fn video_emits_flag_interlaced_and_field_order() { - // An interlaced (576i PAL) track must emit FlagInterlaced=1 and - // FieldOrder=9 (bottom-field-first). A progressive track must emit - // FlagInterlaced=2 and NO FieldOrder. + // An interlaced track must emit FlagInterlaced=1 and its FieldOrder + // value. A progressive track must emit FlagInterlaced=2 and NO + // FieldOrder. let mut interlaced = make_video_track(); interlaced.interlaced = true; - interlaced.field_order = ebml::FIELD_ORDER_BFF; + interlaced.field_order = ebml::FIELD_ORDER_TFF; let muxer = MkvMuxer::new(Cursor::new(Vec::new()), &[interlaced], None, 0.0, &[]).unwrap(); let data = muxer.writer.into_inner(); let fi = find_id(&data, ebml::FLAG_INTERLACED).expect("FlagInterlaced present"); @@ -3004,8 +3172,8 @@ mod tests { let fo = find_id(&data, ebml::FIELD_ORDER).expect("FieldOrder present"); assert_eq!( data[fo + 2], - ebml::FIELD_ORDER_BFF, - "FieldOrder must be 9 (bottom-field-first) for PAL DVD" + ebml::FIELD_ORDER_TFF, + "FieldOrder value must round-trip through the writer" ); // Progressive track: FlagInterlaced=2, no FieldOrder. @@ -3030,6 +3198,161 @@ mod tests { ); } + #[test] + fn video_576i_defaults_to_top_field_first() { + // PAL 576i must default to TFF (2), not BFF — the container element must + // agree with the MPEG-2 stream (MediaInfo reads "Top Field First" off + // the picture coding extension). The pre-rc.5.1 BFF(9) was a wrong value. + let v = VideoStream { + pid: 0xE0, + codec: Codec::Mpeg2, + resolution: Resolution::R576i, + frame_rate: crate::disc::FrameRate::F25, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt470bg, + display_aspect: Some((16, 9)), + secondary: false, + label: String::new(), + }; + let t = MkvTrack::video(&v); + assert!(t.interlaced, "576i is interlaced"); + assert_eq!( + t.field_order, + ebml::FIELD_ORDER_TFF, + "576i must default to top-field-first" + ); + } + + #[test] + fn interlaced_576i_emits_default_decoded_field_duration() { + // 576i @ 25 fps: DefaultDuration = 40 ms (frame), and + // DefaultDecodedFieldDuration = 20 ms (field = half a frame). The field + // element stops interlace-aware readers (Windows) halving the frame rate. + let v = VideoStream { + pid: 0xE0, + codec: Codec::Mpeg2, + resolution: Resolution::R576i, + frame_rate: crate::disc::FrameRate::F25, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt470bg, + display_aspect: None, + secondary: false, + label: String::new(), + }; + let t = MkvTrack::video(&v); + assert_eq!(t.default_duration_ns, 40_000_000, "frame duration is 40 ms"); + assert_eq!(t.field_duration_ns, 20_000_000, "field duration is 20 ms"); + let muxer = MkvMuxer::new(Cursor::new(Vec::new()), &[t], None, 0.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + // DefaultDuration (frame) present and = 40 ms. + let dd = find_id(&data, ebml::DEFAULT_DURATION).expect("DefaultDuration present"); + // [id 3B][size 0x84][4-byte value] — 40_000_000 needs 4 bytes. + let frame_ns = u32::from_be_bytes([data[dd + 4], data[dd + 5], data[dd + 6], data[dd + 7]]); + assert_eq!(frame_ns, 40_000_000, "DefaultDuration is the full frame"); + // DefaultDecodedFieldDuration present and = 20 ms. + let fd = + find_id(&data, ebml::DEFAULT_DECODED_FIELD_DURATION).expect("field duration present"); + let field_ns = u32::from_be_bytes([data[fd + 4], data[fd + 5], data[fd + 6], data[fd + 7]]); + assert_eq!(field_ns, 20_000_000, "field duration is half the frame"); + } + + #[test] + fn progressive_video_omits_field_duration() { + // A progressive track must NOT carry DefaultDecodedFieldDuration. + let t = make_video_track(); // progressive + let muxer = MkvMuxer::new(Cursor::new(Vec::new()), &[t], None, 0.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + assert!( + find_id(&data, ebml::DEFAULT_DECODED_FIELD_DURATION).is_none(), + "no field duration for progressive content" + ); + } + + #[test] + fn finalize_emits_per_track_bps_tags() { + // At finalize a Tags master with a per-track BPS SimpleTag is written. + // BPS = bytes*8/duration_secs. With a 10 s duration and a video frame of + // 1000 bytes, video BPS = 1000*8/10 = 800. + let tracks = [make_video_track(), make_audio_track()]; + let shared = Arc::new(Mutex::new(Cursor::new(Vec::new()))); + let writer = SharedWriter(shared.clone()); + let mut muxer = MkvMuxer::new(writer, &tracks, None, 10.0, &[]).unwrap(); + // Video keyframe 1000 bytes; audio frame 500 bytes. + muxer + .write_frame(0, 0, true, &vec![0xABu8; 1000], None) + .unwrap(); + muxer + .write_frame(1, 0, false, &vec![0xCDu8; 500], None) + .unwrap(); + muxer.finish().unwrap(); + let data = shared.lock().unwrap().clone().into_inner(); + + // The Tags master must be present as a top-level Segment child. + let children = segment_children(&data); + assert!( + children.iter().any(|(id, _, _)| *id == ebml::TAGS), + "Tags element must be written at finalize" + ); + // The BPS values must appear as TagString text. Video: 800, Audio: 400. + let text = String::from_utf8_lossy(&data); + assert!(text.contains("BPS"), "BPS TagName must be present"); + assert!( + text.contains("800"), + "video BPS (1000*8/10) must be present" + ); + assert!(text.contains("400"), "audio BPS (500*8/10) must be present"); + } + + #[test] + fn no_bps_tags_when_duration_unknown() { + // With duration 0 (unknown) the BPS rate can't be computed; no Tags. + let tracks = [make_video_track()]; + let frames = vec![(0usize, 0i64, true, vec![0xABu8; 1000])]; + let (data, _) = mux_to_bytes(&tracks, &[], &frames); + let children = segment_children(&data); + assert!( + !children.iter().any(|(id, _, _)| *id == ebml::TAGS), + "no Tags element when duration is unknown" + ); + } + + #[test] + fn ac3_channels_corrected_from_bitstream_acmod() { + // The audio track header claims 6 channels (IFO 5.1), but the AC-3 + // bitstream's first frame has acmod=2 (2.0 stereo). The Channels element + // must be rewritten to 2 from the bitstream, not left at the IFO's 6. + let mut audio = make_audio_track(); // codec A_AC3, channels = 6 + audio.channels = 6; + let video = make_video_track(); + let shared = Arc::new(Mutex::new(Cursor::new(Vec::new()))); + let writer = SharedWriter(shared.clone()); + let mut muxer = MkvMuxer::new(writer, &[video, audio], None, 0.0, &[]).unwrap(); + // A minimal AC-3 BSI with acmod=2 (2/0 stereo), no LFE → 2 channels. + // byte5 = bsid 8 (legacy AC-3). byte6: acmod(010) | dsurmod(00) | + // lfeon(0) = 0b0100_0000 = 0x40. acmod_channels only needs >= 8 bytes. + let ac3 = vec![0x0B, 0x77, 0x00, 0x00, 0x00, 8 << 3, 0x40, 0x00]; + // Open a cluster with a video keyframe first (cluster invariant). + muxer.write_frame(0, 0, true, &[0x01, 0x02], None).unwrap(); + muxer.write_frame(1, 0, false, &ac3, None).unwrap(); + muxer.finish().unwrap(); + let data = shared.lock().unwrap().clone().into_inner(); + + // Locate the Channels element (0x9F) WITHIN the Tracks body (so a stray + // 0x9F in cluster/AC-3 payload can't be mistaken for the element) and + // assert the value byte is 2. + let (tracks_start, tracks_size) = segment_children(&data) + .into_iter() + .find_map(|(id, off, sz)| (id == ebml::TRACKS).then_some((off, sz as usize))) + .expect("Tracks element present"); + let tracks_body = &data[tracks_start..tracks_start + tracks_size]; + let ch = find_id(tracks_body, ebml::CHANNELS).expect("Channels element present"); + assert_eq!( + tracks_body[ch + 2], + 2, + "Channels must be corrected to 2 (bitstream acmod), not 6 (IFO)" + ); + } + #[test] fn dolby_vision_track_emits_block_addition_mapping() { // A DV track (dv_config set) must emit BlockAdditionMapping (0x41E4) diff --git a/src/mux/pipelined_stream.rs b/src/mux/pipelined_stream.rs index 9ace617..d7db3ed 100644 --- a/src/mux/pipelined_stream.rs +++ b/src/mux/pipelined_stream.rs @@ -64,6 +64,10 @@ pub struct PipelinedPesStream { /// decrypt failure instead of reporting a perfect rip. `None` for pipelines /// with no AACS decrypt step (e.g. the M2TS byte-stream path). decrypt_loss: Option>, + /// Count of dropped DVD navigation packets (private_stream_2, 0xBF). These + /// are expected on every disc; instead of a per-packet WARN they're tallied + /// and summarised once at EOF. + dropped_nav_packets: u64, } impl PipelinedPesStream { @@ -93,6 +97,7 @@ impl PipelinedPesStream { eof: false, skip_parse: std::env::var_os("FREEMKV_SKIP_PARSE").is_some(), decrypt_loss: None, + dropped_nav_packets: 0, } } @@ -174,12 +179,19 @@ impl PipelinedPesStream { // subtitle sub-id 0x20+j with audio track j+1, feeding // VobSub PES into the AC-3 parser. let Some(pid) = ps.dvd_pid() else { - tracing::warn!( - target: "mux", - "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", - ps.stream_id, - ps.sub_stream_id, - ); + if ps.is_nav() { + // Expected DVD navigation packet (PCI/DSI) — tally, no WARN. + self.dropped_nav_packets += 1; + } else { + // Unexpected unmappable stream_id — a possibly-dropped real + // stream. Keep the individual WARN: its repetition is signal. + tracing::warn!( + target: "mux", + "dropping unmappable PS packet (stream_id={:#04x}, sub_stream_id={:?})", + ps.stream_id, + ps.sub_stream_id, + ); + } continue; }; let Some((_, track)) = self.pid_to_track.iter().find(|(p, _)| *p == pid).copied() @@ -227,6 +239,13 @@ impl Stream for PipelinedPesStream { } false => { self.eof = true; + if self.dropped_nav_packets > 0 { + tracing::debug!( + target: "mux", + "dropped {} DVD navigation packets (private_stream_2/0xBF) — expected, carry no elementary stream", + self.dropped_nav_packets + ); + } // Drain any access unit a parser buffered past the last // PES (e.g. DTS-HD's final core+extension unit). let pid_to_track = &self.pid_to_track; diff --git a/src/mux/ps.rs b/src/mux/ps.rs index df7ef83..326c497 100644 --- a/src/mux/ps.rs +++ b/src/mux/ps.rs @@ -24,6 +24,9 @@ const PROGRAM_END_ID: u8 = 0xB9; /// Private stream 1 (AC3, DTS, LPCM, subtitles). const PRIVATE_STREAM_1: u8 = 0xBD; +/// Private stream 2 (0xBF) — DVD navigation (PCI/DSI). Carries no muxable +/// elementary stream; expected to be dropped on every disc. +const PRIVATE_STREAM_2: u8 = 0xBF; /// Hard cap on the demuxer's reassembly buffer. A length-0 (unbounded) video /// PES is delimited by the next PS-layer boundary; if a corrupt stream declares @@ -108,6 +111,16 @@ impl PsPacket { _ => None, } } + + /// Whether this is a DVD navigation packet (private_stream_2, 0xBF — + /// PCI/DSI). These carry no muxable elementary stream and are EXPECTED to + /// be dropped on every DVD, so a per-packet WARN is noise: the mux loops + /// count them and emit one finalize summary instead. A `dvd_pid()` of + /// `None` for any OTHER stream_id is unexpected (a possibly-dropped real + /// stream) and stays an individual WARN. + pub fn is_nav(&self) -> bool { + self.stream_id == PRIVATE_STREAM_2 + } } /// MPEG-2 Program Stream demuxer. diff --git a/src/scsi/mod.rs b/src/scsi/mod.rs index a7fb5af..d36531c 100644 --- a/src/scsi/mod.rs +++ b/src/scsi/mod.rs @@ -219,6 +219,16 @@ impl ScsiSense { self.sense_key == SENSE_KEY_ILLEGAL_REQUEST } + /// `true` for sense `05/6F/03` — MMC "READ OF SCRAMBLED SECTOR WITHOUT + /// AUTHENTICATION". The drive is enforcing CSS and the bus-auth read gate + /// is not (or no longer) open. Unlike a bare ILLEGAL REQUEST, this is + /// positive proof the sector is CSS-scrambled — the CSS crack scan keys on + /// it to distinguish "encrypted but locked" from "unreadable", and must + /// never collapse it to "unencrypted". + pub fn is_css_locked(&self) -> bool { + self.sense_key == SENSE_KEY_ILLEGAL_REQUEST && self.asc == 0x6F && self.ascq == 0x03 + } + /// `true` if `sense_key == ABORTED COMMAND (B)` — transient; one /// retry is usually safe. pub fn is_aborted_command(&self) -> bool {