From 835cc990ad958708487d8f3b0faab7e88d2da590 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:53:39 -0700 Subject: [PATCH] DVD vob_start absolute rebase + rc.5.3 audit fixes - ifo.rs: rebase VTS title VOBS to absolute disc LBA (file_start_lba + vtstt_vobs); fixes DVD rips opening on the menu region instead of the movie (e.g. SOTL). Adds absolute-placement regression test. - aacs/boil.rs: add mk_from_pk primitive (PK -> MK via MKB walk). - dvdnav/: nav-VM command decoder + start-cell resolver seam, parked behind USE_NAV_RESOLVER (kept compiled, never executed). - mux: FVI src.byte within-sector per spec; Unknown colour -> CICP unspecified (2,2,2,1); demux clear PCS -> NORMAL; ts.rs feed() base reset + boundary provenance fix. - Assorted audit fixes (doc/comment/test accuracy) across the crate. --- CHANGELOG.md | 17 +- src/aacs/boil.rs | 110 ++++++++++- src/aacs/mod.rs | 7 +- src/consts.rs | 5 +- src/css/stevenson.rs | 4 +- src/decrypt.rs | 11 +- src/disc/dvd.rs | 143 +++++++++++--- src/disc/extract.rs | 9 +- src/disc/patch.rs | 24 +-- src/drive/mod.rs | 6 +- src/dvdnav/mod.rs | 49 +++++ src/dvdnav/vmcmd.rs | 408 ++++++++++++++++++++++++++++++++++++++++ src/error.rs | 1 + src/ifo.rs | 14 +- src/keysource.rs | 6 + src/labels/mod.rs | 7 +- src/lib.rs | 1 + src/mux/codec/coding.rs | 15 +- src/mux/codec/h264.rs | 3 +- src/mux/codec/hevc.rs | 5 +- src/mux/demux_sink.rs | 6 +- src/mux/fvi_sink.rs | 16 +- src/mux/m2ts_mux/mod.rs | 5 +- src/mux/mkv.rs | 54 +++++- src/mux/mkvstream.rs | 92 ++++++--- src/mux/resolve.rs | 25 +++ src/mux/ts.rs | 95 +++++++++- src/mux/videomap.rs | 15 ++ src/pes.rs | 21 +-- src/unlock.rs | 80 +++++++- tests/fvi_pipeline.rs | 24 ++- 31 files changed, 1149 insertions(+), 129 deletions(-) create mode 100644 src/dvdnav/mod.rs create mode 100644 src/dvdnav/vmcmd.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index e54a7c6..daedc66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## [1.0.0-rc.6] — UNRELEASED +## [1.1.0-beta.1] — UNRELEASED ### Added @@ -13,6 +13,21 @@ ### Fixed +- **DVD rips now start on the movie, not the disc menu.** A VTS title VOB's + start sector was read from the IFO as a VTS-relative pointer but used as an + absolute disc address, so a DVD title's read extents began `ifo_lba` sectors + too early — the rip opened on the disc's menu / VMGI region and only drifted + into the feature minutes later (Silence of the Lambs, for example, showed + several minutes of the main menu before the movie). The title VOB is now + rebased to its absolute on-disc location, so the rip begins at the first frame + of the feature. Aspect ratio and chapter timing were already correct; only the + starting sector was wrong. (Covered by a new absolute-placement regression + test.) +- **Container metadata correctness.** Unknown colorimetry now emits the CICP + "unspecified" code point (2) consistently across the MKV track and the FVI + sidecar (previously 0); PGS subtitle wipes use the NORMAL composition state + rather than a full epoch reset; and FVI source-byte offsets are written + within-sector per the format spec. - **Multi-extent AACS alignment in `dir://` extraction.** The decrypted file-tree extractor anchored the AACS unit grid to the first extent only, so a file spanning multiple clip extents could mis-align and corrupt. Each extent diff --git a/src/aacs/boil.rs b/src/aacs/boil.rs index 27222fe..8aef997 100644 --- a/src/aacs/boil.rs +++ b/src/aacs/boil.rs @@ -15,11 +15,17 @@ //! //! ```text //! mk_from_dk(device_keys, mkb, vid) → MediaKey (Km) +//! mk_from_pk(processing_keys, mkb) → MediaKey (Km) //! vuk_from_mk(MediaKey, Vid) → Vuk (= AES-G(Km, VID)) //! uk_from_vuk(Vuk, enc_title_keys) → [UnitKey] (decrypt_unit_key each) //! ``` +//! +//! `mk_from_dk` and `mk_from_pk` are two entry points to the SAME Media Key: +//! the device-key path walks the MKB's Media-Key-Variant chain, the +//! processing-key path walks the MKB's Subset-Difference cvalue tables. Neither +//! needs a VID (the VID enters at `vuk_from_mk`). -use super::keys::{decrypt_unit_key, derive_vuk}; +use super::keys::{decrypt_unit_key, derive_media_key_from_pk, derive_vuk}; use super::types::DeviceKey; use super::variants::{KEY_CORRECTION_DATA_PLACEHOLDER, derive_media_key_variant, walk_mkb}; @@ -111,6 +117,31 @@ pub fn mk_from_dk( } } +/// Derive the Media Key (Km) from one or more Processing Keys and the disc MKB. +/// +/// Wraps [`derive_media_key_from_pk`] — the Subset-Difference PK→MK walk: each +/// processing key is validated (and tree-walked) against the MKB's cvalue tables +/// (records `0x04`/`0x05`) until one yields the Media Key whose verify record +/// (`0x81`/`0x86`) matches. Unlike [`mk_from_dk`] this path is reachable for +/// real discs — a leaked/precomputed AACS Processing Key in the keydb resolves +/// the Media Key directly. No VID is involved at this step; the VID enters at +/// [`vuk_from_mk`]. +/// +/// Returns [`Error::AacsMkUnavailable`] (E7018) when no processing key resolves +/// the MKB — the same terminal error as [`mk_from_dk`]; no numeric distinction +/// is load-bearing at this boundary. +/// +/// [`Error::AacsMkUnavailable`]: crate::error::Error::AacsMkUnavailable +pub fn mk_from_pk( + processing_keys: &[[u8; 16]], + mkb: &[u8], +) -> Result { + match derive_media_key_from_pk(mkb, processing_keys) { + Some(km) => Ok(MediaKey(km)), + None => Err(crate::error::Error::AacsMkUnavailable), + } +} + #[cfg(test)] mod tests { use super::*; @@ -197,4 +228,81 @@ mod tests { let e2 = mk_from_dk(&[dk], &mkb, Vid([0x09; 16])); assert!(matches!(e2, Err(crate::error::Error::AacsMkUnavailable))); } + + /// Build a 4-byte MKB record header (type + 3-byte big-endian total length, + /// header included) and append `body`. Mirrors the MKB record framing the + /// parser expects; no crypto. + fn mkb_record(rec_type: u8, body: &[u8]) -> Vec { + let total = 4 + body.len(); + let mut rec = Vec::with_capacity(total); + rec.push(rec_type); + rec.push(((total >> 16) & 0xFF) as u8); + rec.push(((total >> 8) & 0xFF) as u8); + rec.push((total & 0xFF) as u8); + rec.extend_from_slice(body); + rec + } + + /// `mk_from_pk` resolves a planted Processing Key against a synthetic MKB and + /// drives the FULL boil chain PK → MK → VUK → UK. The MKB is built with the + /// same (pk, cv, mk_dv, uv) construction the production SD walk validates, so + /// this proves a PK entry yields real Unit Keys — not just an `Ok`. + #[test] + fn mk_from_pk_drives_full_chain_to_uks() { + let pk: [u8; 16] = [ + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, + 0xFF, 0x00, + ]; + let mk: [u8; 16] = [ + 0xA0, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0xA8, 0xA9, 0xAA, 0xAB, 0xAC, 0xAD, + 0xAE, 0xAF, + ]; + let uv: [u8; 4] = [0x00, 0x00, 0x04, 0x00]; + + // cv = AES-E(pk, mk_raw), where mk_raw is mk with the last-4-bytes-uv XOR + // pre-undone, so the validate step XORs uv back in and recovers mk. + let mut mk_raw = mk; + for a in 0..4 { + mk_raw[12 + a] ^= uv[a]; + } + let cv = aes_ecb_encrypt(&pk, &mk_raw); + + // mk_dv = AES-E(mk, magic||pad): AES-D(mk, mk_dv) starts with the AACS + // verify sentinel. + let mut vd = [0x11u8; 16]; + vd[..8].copy_from_slice(&[0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF]); + let mk_dv = aes_ecb_encrypt(&mk, &vd); + + // Synthetic MKB: type/version (0x10), verify record (0x86 = mk_dv), + // one-entry SD index (0x04 = [u_mask_shift=0][uv]), one-entry cvalue + // table (0x05 = cv). + let mut sd = vec![0u8]; + sd.extend_from_slice(&uv); + let mut mkb = Vec::new(); + mkb.extend_from_slice(&mkb_record(0x10, &[0, 0, 0, 0x20, 0, 0, 0, 0x52])); + mkb.extend_from_slice(&mkb_record(0x86, &mk_dv)); + mkb.extend_from_slice(&mkb_record(0x04, &sd)); + mkb.extend_from_slice(&mkb_record(0x05, &cv)); + + // PK → MK. + let got_mk = mk_from_pk(std::slice::from_ref(&pk), &mkb).expect("planted PK resolves MK"); + assert_eq!(got_mk, MediaKey(mk), "mk_from_pk recovers the planted MK"); + + // MK → VUK → UK over an encrypted title key. + let vid = Vid([0x42u8; 16]); + let plain_uk = [0x7Eu8; 16]; + let vuk = vuk_from_mk(got_mk, vid); + let enc = aes_ecb_encrypt(&vuk.0, &plain_uk); + let uks = uk_from_vuk(vuk, std::slice::from_ref(&enc)); + assert_eq!(uks.len(), 1); + assert_eq!(uks[0].key, plain_uk, "PK chain recovers the title key"); + + // A corrupt PK resolves nothing. + let mut bad = pk; + bad[0] ^= 0xFF; + assert!(matches!( + mk_from_pk(std::slice::from_ref(&bad), &mkb), + Err(crate::error::Error::AacsMkUnavailable) + )); + } } diff --git a/src/aacs/mod.rs b/src/aacs/mod.rs index 289d172..e8711af 100644 --- a/src/aacs/mod.rs +++ b/src/aacs/mod.rs @@ -24,7 +24,7 @@ pub mod types; pub mod variants; // Boil-down derivation primitives (thin newtypes + wrappers over the crypto). -pub use boil::{MediaKey, UnitKey, Vid, Vuk, mk_from_dk, uk_from_vuk, vuk_from_mk}; +pub use boil::{MediaKey, UnitKey, Vid, Vuk, mk_from_dk, mk_from_pk, uk_from_vuk, vuk_from_mk}; // Structured, English-free resolution trace. pub use trace::{KeyNode, KeyOutcome, KeyStep, ResolutionTrace, UnlockOutcome, UnlockStep}; @@ -35,6 +35,10 @@ pub use decrypt::{ decrypt_unit_full, decrypt_unit_try_keys, is_aacs_scrambled, is_unit_aligned, ts_packet_total, ts_sync_count, unit_key_validates, }; +// `probe` is a reproduction-harness helper (see keys.rs), not part of the +// documented 1.0 surface; keep it reachable but off the rendered docs so we +// don't commit semver stability to test primitives. +#[doc(hidden)] pub use keys::probe; pub use keys::{ AacsVersion, ContentCert, MKB_20_CATEGORY_C, MKB_21_CATEGORY_C, MKB_TYPE_3_RECORDABLE, @@ -97,5 +101,6 @@ mod tests { let _ = mkb_content_len(&[]); let _ = is_variant_mkb(&walk_mkb(&[])); let _ = disc_hash_hex(&disc_hash(b"x")); + let _ = mk_from_pk(&[[0u8; 16]], &[]); } } diff --git a/src/consts.rs b/src/consts.rs index 29e06d9..7437cc2 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -48,8 +48,9 @@ pub const BD_SOURCE_PACKET_BYTES: usize = TS_PACKET_BYTES + BD_TIMESTAMP_PREFIX_ /// This is one registry used in two places that share the same value space: /// the MPEG-TS PMT `stream_type` (ISO/IEC 13818-1 Table 2-34) and the Blu-ray /// STN/CLPI `stream_coding_type` (BD-ROM Part 3). The standardized video codes -/// (`0x02`, `0x1B`, `0x24`, `0xEA`) are ISO assignments; the `0x80..=0xA2` -/// audio/graphics codes sit in the ISO user-private range and follow the +/// (`0x02`, `0x1B`, `0x24`) are ISO assignments (ISO/IEC 13818-1 Table 2-34); +/// `0xEA` (VC-1) is a BD-ROM convention in the ISO user-private range. The +/// `0x80..=0xA2` audio/graphics codes also sit in the user-private range and follow the /// Blu-ray Disc Association / ATSC A/52 convention. Because every consumer /// reads or writes this single byte, the family is unprefixed — the scope is /// "any elementary stream freemkv parses or muxes". diff --git a/src/css/stevenson.rs b/src/css/stevenson.rs index 84798a8..63236bf 100644 --- a/src/css/stevenson.rs +++ b/src/css/stevenson.rs @@ -402,7 +402,7 @@ mod tests { #[test] fn crack_unscrambled_returns_none() { - let sector = vec![0u8; 2048]; + let sector = vec![0u8; SECTOR_BYTES]; assert!(crack_title_key(§or).is_none()); } @@ -414,7 +414,7 @@ mod tests { #[test] fn recover_needs_min_plain() { - let sector = vec![0u8; 2048]; + let sector = vec![0u8; SECTOR_BYTES]; let short_plain = [0u8; 4]; assert!(recover_title_key(§or, &short_plain).is_none()); } diff --git a/src/decrypt.rs b/src/decrypt.rs index 71673ca..7273ce7 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -373,9 +373,16 @@ pub fn decrypt_sectors( continue; } let crib = css::stevenson::attack_crib(chunk); - let original: Option> = crib.as_ref().map(|_| chunk.to_vec()); + // Snapshot the ciphertext into a stack buffer (chunk is exactly + // 2048 here — guaranteed by the `< 2048` continue above) only + // when there's a crib to validate against, so the common + // cache-hit path costs no per-sector heap allocation. + let mut original = [0u8; 2048]; + if crib.is_some() { + original.copy_from_slice(chunk); + } css::lfsr::descramble_sector(title_key, chunk); - if let (Some(crib), Some(original)) = (crib, original) { + if let Some(crib) = crib { if chunk[0x80..0x80 + 10] != crib[..] { // Cached key is stale for this region — restore the // ciphertext and crack this sector's own key. diff --git a/src/disc/dvd.rs b/src/disc/dvd.rs index 6476d6c..45fb48c 100644 --- a/src/disc/dvd.rs +++ b/src/disc/dvd.rs @@ -106,7 +106,7 @@ impl Disc { }) .collect(); - for dvd_title in &ts.titles { + for (vts_title_idx, dvd_title) in ts.titles.iter().enumerate() { title_number += 1; // Diagnostic dump (--log-level 3): per-cell category table + @@ -114,13 +114,32 @@ impl Disc { // 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(); + // Feature start cell. Prefer the DVD nav-VM resolver, which + // PARKED (#40, menu-at-start playback). The "menu at the start" + // symptom (e.g. SOTL) was a sector-mapping fault — the absolute + // VOB rebase in `ifo::parse_vts` (`vob_start_sector = + // file_start_lba + vtstt_vobs`) — NOT a navigation problem, so + // feature-start resolution is unnecessary for correct rips. The + // nav resolver + verified VM decoder (`dvdnav`) are kept compiled + // but deliberately bypassed; flip `USE_NAV_RESOLVER` to re-enable + // once the nav executor is finished. The fallback is the + // structural leading-cell filter (`feature_start_cell`), which + // drops leading scene-index / interleaved-angle sub-block cells + // and is a no-op for a normal feature (category 0x00 on cell 0). + // See `dvdnav::resolve_feature_start`. + const USE_NAV_RESOLVER: bool = false; + let feature_start = if USE_NAV_RESOLVER { + crate::dvdnav::resolve_feature_start( + reader, + udf_fs, + ts.vts_number as u16, + (vts_title_idx + 1) as u16, + ) + .unwrap_or_else(|| dvd_title.feature_start_cell()) + } else { + dvd_title.feature_start_cell() + } + .min(dvd_title.cells.len()); let dropped_secs: f64 = dvd_title.cells[..feature_start] .iter() .map(|c| c.duration_secs) @@ -136,9 +155,9 @@ impl Disc { ); } - // Build extents from cell sector ranges (absolute = vob_start + cell offset) - let extents: Vec = dvd_title - .feature_cells() + // Build extents from cell sector ranges (absolute = vob_start + cell offset), + // starting at the resolved feature-start cell. + let extents: Vec = dvd_title.cells[feature_start..] .iter() .map(|cell| { let start = ts.vob_start_sector.saturating_add(cell.first_sector); @@ -553,8 +572,10 @@ mod tests { assert_eq!(titles.len(), 1); let t = &titles[0]; assert_eq!(t.extents.len(), 1); - // absolute start = vob_start(1000) + first_sector(10) = 1010. - assert_eq!(t.extents[0].start_lba, 1010); + // absolute start = ifo_lba + vtstt_vobs(1000) + first_sector(10). + // The IFO file sits at PART_START(3000) + data_lba(6000) = 9000, so + // 9000 + 1000 + 10 = 10010. + assert_eq!(t.extents[0].start_lba, 10010); // inclusive: 109 - 10 + 1 = 100 sectors. assert_eq!(t.extents[0].sector_count, 100); // DVD sector = 2048 bytes. @@ -607,12 +628,85 @@ mod tests { 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. + // ifo_lba(9000) + vtstt_vobs(3640) + first_sector(0) = 12640 — built + // from the Title VOBS (0xC4), NOT the menu VOBS (0xC0). The IFO file is + // at PART_START(3000) + data_lba(6000) = 9000. assert_eq!( + t.extents[0].start_lba, 12640, + "extent must start at ifo_lba + vtstt_vobs (0xC4), not vtsm_vobs (0xC0)" + ); + // Must not resolve from the menu VOBS (would be 9000 + 44 = 9044), nor + // use the raw IFO-relative vtstt_vobs (3640) without the absolute base. + assert_ne!(t.extents[0].start_lba, 9044, "must not use the menu VOBS"); + assert_ne!( t.extents[0].start_lba, 3640, - "extent must start at vtstt_vobs (0xC4), not vtsm_vobs (0xC0)" + "must add the IFO's absolute disc LBA, not use the raw relative value" + ); + } + + /// ABSOLUTE-REBASE regression (THESILENCEOFTHELAMBS / Greenland fix): + /// `ifo::parse_vts` now sets `vob_start_sector = file_start_lba(IFO) + + /// vtstt_vobs`, so an extent's `start_lba` must equal the sum of THREE + /// independent terms — the IFO file's absolute on-disc LBA, the + /// IFO-relative `vtstt_vobs` (0xC4), and the cell's `first_sector` — none of + /// which may be dropped. The earlier code used the bare relative + /// `vtstt_vobs`, placing every extent `ifo_lba` sectors too early (the rip + /// opened in the VMGI/menu region before drifting into the movie). The + /// other tests fold two of the three terms together (zero cell offset, or + /// a single combined expectation); this one keeps all three distinct and + /// non-overlapping so a regression to ANY two-term combination is caught. + #[test] + fn scan_dvd_titles_extent_is_absolute_three_term_sum() { + let mut disc = MemDisc::new(); + let vmg = build_vmg(&[(1, 1, 1)]); + // vtstt_vobs (Title VOBS, 0xC4) = 700; one cell first_sector = 33. + let vts = build_vts(700, 0x00, &[], &[], &[(33, 132)], false); + // IFO data at data_lba 6000 → absolute ifo_lba = PART_START(3000) + 6000. + let ifo_lba = PART_START + 6000; // 9000 + let vtstt_vobs = 700u32; + let first_sector = 33u32; + 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]; + assert_eq!(t.extents.len(), 1); + let got = t.extents[0].start_lba; + // The one correct answer: all three terms summed (9000 + 700 + 33). + assert_eq!( + got, + ifo_lba + vtstt_vobs + first_sector, + "extent start must be file_start_lba(IFO) + vtstt_vobs + cell.first_sector" + ); + // Each wrong two-term combination must be rejected: + assert_ne!( + got, + vtstt_vobs + first_sector, + "must not use the bare relative vtstt_vobs (missing the IFO's absolute LBA)" + ); + assert_ne!( + got, + ifo_lba + first_sector, + "must not drop vtstt_vobs (the Title VOBS pointer)" + ); + assert_ne!( + got, + ifo_lba + vtstt_vobs, + "must not drop the cell's first_sector offset" ); - 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 @@ -648,9 +742,9 @@ mod tests { ); let t = &Disc::scan_dvd_titles(&mut disc, &udf)[0]; assert_eq!(t.extents.len(), 2); - assert_eq!(t.extents[0].start_lba, 500); // 500 + 0 + assert_eq!(t.extents[0].start_lba, 9500); // ifo_lba(9000) + 500 + 0 assert_eq!(t.extents[0].sector_count, 100); - assert_eq!(t.extents[1].start_lba, 700); // 500 + 200 + assert_eq!(t.extents[1].start_lba, 9700); // ifo_lba(9000) + 500 + 200 assert_eq!(t.extents[1].sector_count, 100); assert_eq!(t.size_bytes, 200 * 2048); } @@ -1051,8 +1145,9 @@ mod tests { assert_eq!(titles[0].playlist, "VTS_01_1.VOB"); assert_eq!(titles[1].playlist, "VTS_02_2.VOB"); // Distinct vob_start → distinct extents. - assert_eq!(titles[0].extents[0].start_lba, 100); - assert_eq!(titles[1].extents[0].start_lba, 200); + // VTS_01 IFO @ PART_START(3000)+6000=9000; VTS_02 IFO @ 3000+7000=10000. + assert_eq!(titles[0].extents[0].start_lba, 9100); // 9000 + 100 + assert_eq!(titles[1].extents[0].start_lba, 10200); // 10000 + 200 } /// chapter_times from the IFO become Chapter entries with ordinal @@ -1168,8 +1263,8 @@ mod tests { // The leading 0x90 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); + assert_eq!(t.extents[0].start_lba, 9000 + 1000 + 100); // ifo_lba + vtstt + first + assert_eq!(t.extents[1].start_lba, 9000 + 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. @@ -1219,8 +1314,8 @@ mod tests { 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); + assert_eq!(t.extents[0].start_lba, 9000 + 1000); // ifo_lba + vtstt + 0, head intact + assert_eq!(t.extents[1].start_lba, 9000 + 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/extract.rs b/src/disc/extract.rs index 99a8516..9bddfea 100644 --- a/src/disc/extract.rs +++ b/src/disc/extract.rs @@ -234,9 +234,12 @@ impl Disc { // Acquire (rather than Relaxed) on these per-file delta loads: // `extract_tree` drives `dec` single-threaded so there is no race - // today, but Acquire costs nothing on x86 and gives a happens- - // before edge if file extraction is ever parallelised, so the - // delta can never read a torn/stale counter across iterations. + // today, and Acquire costs nothing on x86. Note this is only half + // the synchronisation: the paired counter store + // (sector/decrypting.rs `fetch_add`) is Relaxed, so an Acquire + // load alone does NOT yet establish a happens-before edge. Before + // file extraction is parallelised, upgrade that store to Release + // (or stronger) so the delta cannot read a stale counter. let before_loss = decrypt_loss.load(Ordering::Acquire); let (mut fr, halted) = extract_one_file(&mut dec, dest, pf, total_bytes, &mut done_bytes, opts)?; diff --git a/src/disc/patch.rs b/src/disc/patch.rs index 0d69cad..0f23858 100644 --- a/src/disc/patch.rs +++ b/src/disc/patch.rs @@ -825,12 +825,12 @@ pub(super) fn handle_read_success( state.stall_start = (state.now)(); state.bytes_good_last = bytes_good_now; } - if (state.now)().duration_since(state.stall_start) > std::time::Duration::from_secs(STALL_SECS) - { + let stall_elapsed = (state.now)().duration_since(state.stall_start); + if stall_elapsed > std::time::Duration::from_secs(STALL_SECS) { tracing::warn!( target: "freemkv::disc", phase = "patch_stall", - elapsed_secs = (state.now)().duration_since(state.stall_start).as_secs(), + elapsed_secs = stall_elapsed.as_secs(), bytes_good = bytes_good_now, bytes_good_start = state.bytes_good_start, "Patch stalled - no recovery for {}s, exiting pass", @@ -1107,13 +1107,12 @@ pub(super) fn handle_read_failure( state.stall_start = (state.now)(); state.bytes_good_last = bytes_good_now; } - if (state.now)().duration_since(state.stall_start) - > std::time::Duration::from_secs(STALL_SECS) - { + let stall_elapsed = (state.now)().duration_since(state.stall_start); + if stall_elapsed > std::time::Duration::from_secs(STALL_SECS) { tracing::warn!( target: "freemkv::disc", phase = "patch_stall", - elapsed_secs = (state.now)().duration_since(state.stall_start).as_secs(), + elapsed_secs = stall_elapsed.as_secs(), bytes_good = bytes_good_now, bytes_good_start = state.bytes_good_start, "Patch stalled (NOT_READY path) - no recovery for {}s, exiting pass", @@ -1193,12 +1192,12 @@ pub(super) fn handle_read_failure( state.stall_start = (state.now)(); state.bytes_good_last = bytes_good_now; } - if (state.now)().duration_since(state.stall_start) > std::time::Duration::from_secs(STALL_SECS) - { + let stall_elapsed = (state.now)().duration_since(state.stall_start); + if stall_elapsed > std::time::Duration::from_secs(STALL_SECS) { tracing::warn!( target: "freemkv::disc", phase = "patch_stall", - elapsed_secs = (state.now)().duration_since(state.stall_start).as_secs(), + elapsed_secs = stall_elapsed.as_secs(), consecutive_failures = state.consecutive_failures, bytes_good = bytes_good_now, bytes_good_start = state.bytes_good_start, @@ -1472,13 +1471,14 @@ pub(super) fn check_range_watchdog( state.range_bytes_good = bytes_good_now; state.range_start = (state.now)(); } - if (state.now)().duration_since(state.range_start).as_secs() >= frame.range_budget_secs { + let range_elapsed = (state.now)().duration_since(state.range_start); + if range_elapsed.as_secs() >= frame.range_budget_secs { tracing::warn!( target: "freemkv::disc", phase = "patch_range_stall", range_lba = frame.range_pos / 2048, range_sectors = frame.range_sectors, - elapsed_secs = (state.now)().duration_since(state.range_start).as_secs(), + elapsed_secs = range_elapsed.as_secs(), budget_secs = frame.range_budget_secs, bytes_recovered = state.range_bytes_good.saturating_sub(state.bytes_good_before), "Range stalled - moving to next range" diff --git a/src/drive/mod.rs b/src/drive/mod.rs index b460bb3..16fcf1c 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -433,8 +433,12 @@ impl Drive { } Ok(()) } - // No unlocker matched: not an error — fall through to OEM route. + // No unlocker matched, or one matched but only hit a capability + // failure (not firmware-unlockable / no OEM VID): not an error — + // fall through to the OEM host-cert route. Ok(None) => Ok(()), + // A genuine transport fault during unlock (UnlockError::Scsi) + // propagates here and aborts init — the bus is dead. Err(e) => Err(e), }; tracing::info!( diff --git a/src/dvdnav/mod.rs b/src/dvdnav/mod.rs new file mode 100644 index 0000000..84961e0 --- /dev/null +++ b/src/dvdnav/mod.rs @@ -0,0 +1,49 @@ +//! DVD-Video navigation — read-only resolver for the **main-feature start +//! point** (issue #40). Mirrors what a DVD player's nav VM resolves: First-Play +//! → menu "Play" → title dispatch → the first cell of the feature, so the rip +//! starts at the movie rather than at raw cell 0 (e.g. skipping a leading +//! logo/warning segment when the disc's own navigation does). +//! +//! Byte layout follows the DVD-Video specification (VMGI/VTSI headers, +//! PGC/cell tables, PCI/HLI button packets); the VM command decoder is +//! verified against libdvdnav's decoder. +//! +//! Current contents: [`vmcmd`] — the VM command decoder (proven against the +//! SOTL/Greenland test discs). The IFO/PCI parsing and the navigation executor +//! that resolves the start cell build on top of this. + +pub mod vmcmd; + +use crate::sector::SectorSource; + +/// Resolve the feature title's **true start cell** (0-based index into the +/// title PGC's cell list) by following the disc's own navigation — First-Play → +/// menu "Play" → title dispatch — the way a player reaches the movie. This is +/// what lets the rip begin at the feature instead of at raw cell 0 when the +/// disc's nav enters the title past a leading logo/warning segment (e.g. a +/// disc whose "Play" resolves to a later cell than cell 0). +/// +/// Returns `None` when navigation cannot be resolved, so the caller falls back +/// to the structural leading-cell filter (today's behaviour, ≈ cell 0 / 0:00). +/// +/// TODO(#40): the IFO/PCI parsing + nav executor (built on [`vmcmd`]) land +/// incrementally. Until the executor is complete this returns `None`, so wiring +/// it in is behaviour-neutral; improvements to the resolver take effect here +/// without touching the call site. +pub fn resolve_feature_start( + reader: &mut dyn SectorSource, + udf: &crate::udf::UdfFs, + vtsn: u16, + vts_ttn: u16, +) -> Option { + // `reader`/`udf` are the seam inputs the nav executor will consume to read + // VIDEO_TS.IFO + the VTS IFOs/menu VOBs. Reserved until that lands. + let _ = (reader, udf); + tracing::trace!( + target: "freemkv::dvdnav", + vtsn, + vts_ttn, + "nav start-cell resolver: unresolved — caller falls back to leading-cell filter" + ); + None +} diff --git a/src/dvdnav/vmcmd.rs b/src/dvdnav/vmcmd.rs new file mode 100644 index 0000000..713a15e --- /dev/null +++ b/src/dvdnav/vmcmd.rs @@ -0,0 +1,408 @@ +//! DVD-Video VM command decoder. +//! +//! An 8-byte navigation command as found in PGC command tables (pre/post/cell) +//! and PCI button info. Decoded per the DVD-Video VM instruction set and +//! verified against libdvdnav's command decoder. +//! +//! Bit model: the 8 bytes are a big-endian 64-bit word. `byte0` bits 7-5 are the +//! command **type**; for type 1, `byte0` bit 4 selects Link (0) vs Jump (1), and +//! `byte1` bits 3-0 are the sub-command. Compare predicates live in `byte1` +//! bits 6-4 with the operands in bytes 2-5. +//! +//! This module is pure decode + a register model — no I/O, no English (numeric +//! semantics only), matching libfreemkv conventions. The navigation *executor* +//! and IFO/PCI parsing build on top of this. + +/// A decoded navigation instruction. Only the variants freemkv's start-point +/// resolver needs are modelled explicitly; everything else is [`Instr::Other`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Instr { + Nop, + /// Stop executing the current command list (resume cell playback). + Break, + /// Goto command line within the same list (1-based). + Goto { + line: u8, + }, + /// Leave the current domain. + Exit, + /// Jump to a VMG title (1-based TT_SRPT index). + JumpTt { + ttn: u8, + }, + /// Jump to a title within the current VTS (1-based VTS title index). + JumpVtsTt { + ttn: u8, + }, + /// Jump to a part-of-title (chapter) within a VTS title. + JumpVtsPtt { + ttn: u8, + pttn: u16, + }, + /// Jump to the First-Play PGC. + JumpSsFp, + /// Jump to a Video-Manager menu (`menu` = menu id). + JumpSsVmgm { + menu: u8, + }, + /// Jump to a Video-Title-Set menu. + JumpSsVtsm { + vts: u8, + ttn: u8, + menu: u8, + }, + /// Jump to a specific VMGM menu PGC. + JumpSsVmgmPgc { + pgcn: u16, + }, + /// Call a sub-domain (raw retained; resume handled by the executor). + CallSs { + sub: u8, + }, + /// Link to a PGC number within the current domain. + LinkPgcn { + pgcn: u16, + }, + /// Link to a part-of-title within the current PGC's title. + LinkPttn { + pttn: u16, + }, + /// Link to a program number within the current PGC (1-based). + LinkPgn { + pgn: u8, + }, + /// Link to a cell number within the current PGC (1-based). + LinkCn { + cn: u8, + }, + /// A link "subset" op (LinkTopCell/NextPG/RSM/…); `sub` is the raw code. + LinkSub { + sub: u8, + }, + /// Set a GPRM. `op` is the set-op code (1=mov, 3=add, …); value is immediate + /// (`imm`) when `immediate`, else the contents of register `src`. + SetGprm { + reg: u8, + op: u8, + immediate: bool, + imm: u16, + src: u8, + }, + /// Set a system parameter / unmodelled set — executor may ignore. + SetSystem, + /// Anything not individually modelled (kept as raw bytes). + Other([u8; 8]), +} + +/// A compare predicate carried by a command (`byte1` bits 6-4). `None` = always. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Compare { + /// Compare op: 1=&,2===,3=!=,4=>=,5=>,6=<=,7=<. + pub op: u8, + /// Left register index (GPRM 0-15, SPRM 128+). + pub lhs_reg: u8, + /// Right side: immediate when `immediate`, else register `rhs_reg`. + pub immediate: bool, + pub imm: u16, + pub rhs_reg: u8, +} + +/// A fully decoded command: its predicate (if any) and the instruction. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Command { + pub compare: Option, + pub instr: Instr, +} + +// Command types — `byte0` bits 7-5. +const TYPE_SPECIAL: u8 = 0; +const TYPE_LINK_JUMP: u8 = 1; +const TYPE_SET_SYSTEM: u8 = 2; +const TYPE_SET_GPRM: u8 = 3; + +// Special (type 0) sub-commands — `byte1` bits 3-0. +const SP_GOTO: u8 = 1; +const SP_BREAK: u8 = 2; + +// Jump/Call (type 1, direct=1) sub-commands. +const JP_EXIT: u8 = 1; +const JP_JUMP_TT: u8 = 2; +const JP_JUMP_VTS_TT: u8 = 3; +const JP_JUMP_VTS_PTT: u8 = 5; +const JP_JUMP_SS: u8 = 6; +const JP_CALL_SS: u8 = 8; + +// Link (type 1, direct=0) sub-commands. NOTE: sub-op 0 is NOP/no-link and 1 is +// the LinkSub form (libdvdnav `decoder.c` `eval_link_instruction`). +const LK_SUB: u8 = 1; +const LK_PGCN: u8 = 4; +const LK_PTTN: u8 = 5; +const LK_PGN: u8 = 6; +const LK_CN: u8 = 7; + +// JumpSS sub-domain selector — `byte5` bits 7-6. +const SS_FP: u8 = 0; +const SS_VMGM_MENU: u8 = 1; +const SS_VTSM: u8 = 2; + +// Operand field widths (spec-defined bit counts). +const MASK_TTN: u8 = 0x7F; // 7-bit title number +const MASK_PGN: u8 = 0x7F; // 7-bit program number +const MASK_LINKOP: u8 = 0x1F; // 5-bit link sub-op +const MASK_REG: u8 = 0x0F; // 4-bit GPRM index +const MASK_MENU: u8 = 0x0F; // 4-bit menu id +const MASK_PTTN: u16 = 0x03FF; // 10-bit part-of-title +const MASK_PGCN: u16 = 0x7FFF; // 15-bit PGC number + +#[inline] +fn be16(b: &[u8; 8], o: usize) -> u16 { + ((b[o] as u16) << 8) | b[o + 1] as u16 +} + +// Compare-operand layouts ("if_version"s) per libdvdnav `decoder.c`. The op +// nibble is always `byte1` bits 6-4; the immediate flag is `byte1` bit 7. The +// operand *offsets* differ by command family. +// +// v1 (special + link): lhs reg = b[3]; rhs imm = bytes4-5 / rhs reg = b[4]. +// v2 (jump + system-set): lhs reg = b[6]; rhs reg = b[7] (registers only). +// v3 (set-GPRM): lhs reg = b[2]; rhs imm = bytes6-7 / rhs reg = b[6]. +fn if_v1(b: &[u8; 8]) -> Option { + let op = (b[1] >> 4) & 7; + (op != 0).then(|| Compare { + op, + lhs_reg: b[3], + immediate: b[1] >> 7 != 0, + imm: be16(b, 4), + rhs_reg: b[4], + }) +} +fn if_v2(b: &[u8; 8]) -> Option { + let op = (b[1] >> 4) & 7; + (op != 0).then(|| Compare { + op, + lhs_reg: b[6], + immediate: false, + imm: 0, + rhs_reg: b[7], + }) +} +fn if_v3(b: &[u8; 8]) -> Option { + let op = (b[1] >> 4) & 7; + (op != 0).then(|| Compare { + op, + lhs_reg: b[2], + immediate: b[1] >> 7 != 0, + imm: be16(b, 6), + rhs_reg: b[6], + }) +} + +/// Decode an 8-byte VM command. +pub fn decode(b: &[u8; 8]) -> Command { + let typ = b[0] >> 5; + let direct = (b[0] >> 4) & 1; + let setop = b[0] & 0x0F; + let cmd = b[1] & 0x0F; + + // Compare predicate, with the operand layout for this command family + // (libdvdnav `decoder.c` `vm_eval_command` type dispatch). + let compare = match (typ, direct) { + (TYPE_SPECIAL, _) => if_v1(b), + (TYPE_LINK_JUMP, 1) => if_v2(b), // jump + (TYPE_LINK_JUMP, 0) => if_v1(b), // link + (TYPE_SET_SYSTEM, _) => if_v2(b), + (TYPE_SET_GPRM, _) => if_v3(b), + _ => None, // 4/5/6 compound — not needed by the resolver + }; + + // JumpSS sub-domain selector lives in byte5 bits 7-6. + let ss_sel = b[5] >> 6; + + let instr = match typ { + TYPE_LINK_JUMP if direct == 1 => match cmd { + JP_EXIT => Instr::Exit, + JP_JUMP_TT => Instr::JumpTt { + ttn: b[5] & MASK_TTN, + }, + JP_JUMP_VTS_TT => Instr::JumpVtsTt { + ttn: b[5] & MASK_TTN, + }, + JP_JUMP_VTS_PTT => Instr::JumpVtsPtt { + ttn: b[5] & MASK_TTN, + pttn: be16(b, 2) & MASK_PTTN, + }, + JP_JUMP_SS => match ss_sel { + SS_FP => Instr::JumpSsFp, + SS_VMGM_MENU => Instr::JumpSsVmgm { + menu: b[5] & MASK_MENU, + }, + SS_VTSM => Instr::JumpSsVtsm { + vts: b[4], + ttn: b[3], + menu: b[5] & MASK_MENU, + }, + _ => Instr::JumpSsVmgmPgc { + pgcn: be16(b, 2) & MASK_PGCN, + }, + }, + JP_CALL_SS => Instr::CallSs { sub: ss_sel }, + _ => Instr::Nop, + }, + TYPE_LINK_JUMP => match cmd { + // direct == 0 (link). sub-op 0 = NOP/no-link. + LK_SUB => Instr::LinkSub { + sub: b[7] & MASK_LINKOP, + }, + LK_PGCN => Instr::LinkPgcn { + pgcn: be16(b, 6) & MASK_PGCN, + }, + LK_PTTN => Instr::LinkPttn { + pttn: be16(b, 6) & MASK_PTTN, + }, + LK_PGN => Instr::LinkPgn { + pgn: b[7] & MASK_PGN, + }, + LK_CN => Instr::LinkCn { cn: b[7] }, + _ => Instr::Nop, + }, + TYPE_SPECIAL => match cmd { + SP_GOTO => Instr::Goto { line: b[7] }, + SP_BREAK => Instr::Break, + _ => Instr::Nop, + }, + TYPE_SET_GPRM => Instr::SetGprm { + reg: b[3] & MASK_REG, + op: setop, + immediate: direct != 0, + imm: be16(b, 4), + src: b[5], + }, + TYPE_SET_SYSTEM => Instr::SetSystem, + _ => Instr::Other(*b), + }; + + Command { compare, instr } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn h(s: &str) -> [u8; 8] { + let v: Vec = (0..8) + .map(|i| u8::from_str_radix(&s[i * 2..i * 2 + 2], 16).unwrap()) + .collect(); + v.try_into().unwrap() + } + + // KATs taken from the real SOTL / Greenland discs (decoded in the PoC). + #[test] + fn greenland_first_play_is_jumptt_1() { + let c = decode(&h("3002000000010000")); + assert_eq!(c.instr, Instr::JumpTt { ttn: 1 }); + assert!(c.compare.is_none()); + } + + #[test] + fn sotl_first_play_is_jumpss_vtsm_root() { + // 30 06 ... byte5=0x83 -> sub 2 (VTSM), vts=byte4=1, menu=byte5&0xF=3 (root) + let c = decode(&h("3006000101830000")); + assert_eq!( + c.instr, + Instr::JumpSsVtsm { + vts: 1, + ttn: 1, + menu: 3 + } + ); + } + + #[test] + fn sotl_title_dispatch_is_conditional_linkpgn_2() { + // 20 a6 ... CmpLink: if GPRM0 == 2 -> LinkPGN 2 (cell 2 = the 5:02 start) + let c = decode(&h("20a6000000020002")); + assert_eq!(c.instr, Instr::LinkPgn { pgn: 2 }); + let cmp = c.compare.expect("conditional"); + assert_eq!(cmp.op, 2); // == + assert_eq!(cmp.lhs_reg, 0); // GPRM0 + assert!(cmp.immediate); + assert_eq!(cmp.imm, 2); + } + + #[test] + fn sotl_root_button_is_linkpgcn_37() { + assert_eq!( + decode(&h("2004000000000025")).instr, + Instr::LinkPgcn { pgcn: 37 } + ); + } + + #[test] + fn greenland_scene_button_is_linkpgn() { + assert_eq!( + decode(&h("2006000000001401")).instr, + Instr::LinkPgn { pgn: 1 } + ); + } + + #[test] + fn jumpvts_ptt_decodes_ttn_and_pttn() { + // synthetic: 30 05 | ptt(bytes2-3)=0x0002 | ttn(byte5)=1 + let c = decode(&h("3005000200010000")); + assert_eq!(c.instr, Instr::JumpVtsPtt { ttn: 1, pttn: 2 }); + } + + #[test] + fn setgprm_immediate_mov() { + // SOTL First-Play pre[0]: 71 00 | reg=byte3=6 | imm(bytes4-5)=0x03e8 -> g6 = 1000 + match decode(&h("7100000603e80000")).instr { + Instr::SetGprm { + reg, + op, + immediate, + imm, + .. + } => { + assert_eq!(reg, 6); + assert_eq!(op, 1); // mov + assert!(immediate); + assert_eq!(imm, 1000); + } + other => panic!("expected SetGprm, got {other:?}"), + } + } + + // Regression for the libdvdnav cross-check: link sub-op 0 = NOP, 1 = LinkSub. + #[test] + fn link_subop_zero_is_nop_one_is_linksub() { + assert_eq!(decode(&h("2000000000000000")).instr, Instr::Nop); + assert_eq!( + decode(&h("2001000000000010")).instr, + Instr::LinkSub { sub: 0x10 } + ); + } + + // if_version_1 register compare: rhs register is byte4 (not byte5). + #[test] + fn link_register_compare_rhs_is_byte4() { + // 20 26: link, cmp=EQ(2), dircmp=0(register) ; cmd=6 LinkPGN + let c = decode(&h("2026000304000002")); + assert_eq!(c.instr, Instr::LinkPgn { pgn: 2 }); + let cmp = c.compare.expect("conditional"); + assert!(!cmp.immediate); + assert_eq!(cmp.lhs_reg, 3); + assert_eq!(cmp.rhs_reg, 4); + } + + // if_version_2 jump compare: both operands are registers in byte6 / byte7. + #[test] + fn jump_compare_uses_bytes6_and_7() { + // 30 22: jump, cmp=EQ(2) ; cmd=2 JumpTT ttn=byte5=5 + let c = decode(&h("3022000000050607")); + assert_eq!(c.instr, Instr::JumpTt { ttn: 5 }); + let cmp = c.compare.expect("conditional"); + assert!(!cmp.immediate); + assert_eq!(cmp.lhs_reg, 6); + assert_eq!(cmp.rhs_reg, 7); + } +} diff --git a/src/error.rs b/src/error.rs index 002a4cb..a206a5b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1199,6 +1199,7 @@ mod tests { E_NO_DISC_KEY, E_CSS_KEY_MISSING, E_AACS_NO_HOST_CERT, + E_AACS_BUS_KEY_UNAVAILABLE, E_KEYDB_CONNECT, E_KEYDB_HTTP, E_KEYDB_INVALID, diff --git a/src/ifo.rs b/src/ifo.rs index 341d454..589ae4b 100644 --- a/src/ifo.rs +++ b/src/ifo.rs @@ -440,7 +440,19 @@ fn parse_vts( // 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)?; + // + // `vtstt_vobs` is a sector address **relative to the start of this VTS_xx_0.IFO + // file**, not an absolute disc LBA. The cell `first_sector`/`last_sector` + // values are in turn relative to `vtstt_vobs`. To turn them into the absolute + // disc LBAs the reader needs, add the IFO file's own on-disc location (from + // the UDF FS). Without this rebase every extent started `ifo_lba` sectors too + // early — for THESILENCEOFTHELAMBS the feature began at LBA 126 (the VMGI / + // VIDEO_TS.VOB main-menu region) instead of 132886 (VTS_03_1.VOB), so the + // first ~4.5 min of muxed video was the disc's main menu before the stream + // drifted into the movie. + let vtstt_vobs = be_u32(&vts_data, VTSTT_VOBS_OFFSET)?; + let ifo_lba = udf.file_start_lba(reader, &path)?; + let vob_start_sector = ifo_lba.saturating_add(vtstt_vobs); // Video attributes at offset 0x200 (2 bytes) let video = parse_video_attr(&vts_data)?; diff --git a/src/keysource.rs b/src/keysource.rs index 841ebd2..1378b74 100644 --- a/src/keysource.rs +++ b/src/keysource.rs @@ -100,6 +100,12 @@ pub struct DiscInputsCtx<'a> { impl<'a> DiscInputsCtx<'a> { /// Build a context over `inputs`, parsing the encrypted title keys at the /// stride for AACS major `version_u8` (1 = V10, else V20/V21). + /// + /// A present-but-malformed `unit_key_ro` (truncated / wrong magic / wrong + /// stride) parses to an empty key set, so a later [`Self::enc_title_keys`] + /// returns `Ok(&[])` indistinguishably from a disc that legitimately has no + /// title keys — the parse failure is swallowed here, not surfaced as an + /// error. pub fn new(inputs: &'a DiscInputs, version_u8: u8) -> Self { use crate::aacs::{AacsVersion, parse_unit_key_ro}; let enc_keys = if inputs.unit_key_ro.is_empty() { diff --git a/src/labels/mod.rs b/src/labels/mod.rs index f3ac45e..38e614e 100644 --- a/src/labels/mod.rs +++ b/src/labels/mod.rs @@ -1124,9 +1124,10 @@ mod registry_tests { // and as a marker for "these parsers exist." let _ = (name, detect, parse); } - // The loop above touches every registry entry; iterating a non-empty - // fixed-size array is the assertion (a `.is_empty()` check would be - // const-folded). The test fails to compile if the tuple shape changes. + // The loop above touches every registry entry. The non-empty + // invariant is covered separately by `parsers_registry_order_locked`, + // whose assert_eq! on the expected order fails if PARSERS is empty. + // This test fails to compile if the tuple shape changes. } } diff --git a/src/lib.rs b/src/lib.rs index d60067f..038c71b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -93,6 +93,7 @@ pub mod decrypt; pub mod diag; pub mod disc; pub mod drive; +pub mod dvdnav; pub mod error; pub mod event; pub mod halt; diff --git a/src/mux/codec/coding.rs b/src/mux/codec/coding.rs index 1600b91..9abc396 100644 --- a/src/mux/codec/coding.rs +++ b/src/mux/codec/coding.rs @@ -181,8 +181,13 @@ impl PictureInfo { match self.detail { CodingDetail::Mpeg2(m) => { if !m.frame_picture { - // A single field picture is inherently interlaced; the - // top_field_first bit names which field this picture is. + // A single field picture is inherently interlaced. Which + // field it actually codes is given by picture_structure + // (top/bottom), not by top_field_first — §6.3.10 constrains + // top_field_first to 0 for field pictures, so it is not the + // spec source here. picture_structure is not retained on + // this carrier, so top_field_first is used only as the lone + // field hint available (best-effort, not spec-derived). Some(if m.top_field_first { FieldOrder::Tff } else { @@ -203,8 +208,10 @@ impl PictureInfo { /// Number of field-display periods this picture occupies — the basis for /// soft-telecine (2:3 pulldown) timing. MPEG-2 (ISO/IEC 13818-2 §6.3.10, /// ffmpeg `nb_fields = repeat_pict + 2`): a field picture occupies 1 field, - /// a normal frame 2, a `repeat_first_field` frame 3 (or 4/6 in a progressive - /// sequence). Codecs without pulldown signalling report the normal 2 fields. + /// a normal frame 2, a `repeat_first_field` progressive-frame 3 (or 4/6 in a + /// progressive sequence); an rff bit on a non-progressive interlaced frame is + /// spec-forbidden (§6.3.10) and is treated as 2. Codecs without pulldown + /// signalling report the normal 2 fields. pub fn nb_fields(&self) -> u8 { match self.detail { CodingDetail::Mpeg2(m) => { diff --git a/src/mux/codec/h264.rs b/src/mux/codec/h264.rs index 8f7bb00..83d5763 100644 --- a/src/mux/codec/h264.rs +++ b/src/mux/codec/h264.rs @@ -325,7 +325,8 @@ impl CodecParser for H264Parser { } /// Parse `(chroma_format_idc, bit_depth_luma_minus8, bit_depth_chroma_minus8)` from -/// a High-Profile SPS NAL (profile_idc ∈ {100, 110, 122, 144}). +/// a High-Profile SPS NAL (profile_idc ∈ `HIGH_PROFILES` — the 14 chroma/bit-depth +/// extended profiles `codec_private` invokes this for). /// /// SPS RBSP layout (ITU-T H.264 §7.3.2.1.1) up to the fields we need: /// byte 0 NAL header (already known to be type 7) diff --git a/src/mux/codec/hevc.rs b/src/mux/codec/hevc.rs index a1c2a49..91ac64d 100644 --- a/src/mux/codec/hevc.rs +++ b/src/mux/codec/hevc.rs @@ -56,10 +56,7 @@ fn hevc_num_extra_slice_header_bits(pps_nal: &[u8]) -> Option { br.read_ue()?; // pps_pic_parameter_set_id br.read_ue()?; // pps_seq_parameter_set_id br.skip_bits(2)?; // dependent_slice_segments_enabled_flag, output_flag_present_flag - let mut n = 0u32; - for _ in 0..3 { - n = (n << 1) | br.read_bit()?; - } + let n = br.read_bits(3)?; Some(n) } diff --git a/src/mux/demux_sink.rs b/src/mux/demux_sink.rs index 91d28cf..42cb9e5 100644 --- a/src/mux/demux_sink.rs +++ b/src/mux/demux_sink.rs @@ -273,8 +273,8 @@ const SUP_HEADER_LEN: usize = SUP_MAGIC.len() + 4 + 4; const SEG_PCS: u8 = 0x16; /// PGS segment type: END of display set. const SEG_END: u8 = 0x80; -/// PCS `composition_state` value: Epoch Start (a fresh display). -const PCS_COMPOSITION_STATE_EPOCH_START: u8 = 0x80; +/// PCS `composition_state` value: Normal (an update to the current epoch). +const PCS_COMPOSITION_STATE_NORMAL: u8 = 0x00; /// PGS segment header on the wire (inside `frame.data`): type(1) + size(2 BE). const PGS_SEG_HEADER_LEN: usize = 3; /// Byte offset of `width`/`height` within a PCS segment (after type+size). @@ -346,7 +346,7 @@ impl PgsSupWriter { PCS_FRAME_RATE, 0x00, 0x00, // composition_number - PCS_COMPOSITION_STATE_EPOCH_START, + PCS_COMPOSITION_STATE_NORMAL, 0x00, // palette_update_flag 0x00, // palette_id PCS_NO_OBJECTS, diff --git a/src/mux/fvi_sink.rs b/src/mux/fvi_sink.rs index 1c1ed30..cca0b6b 100644 --- a/src/mux/fvi_sink.rs +++ b/src/mux/fvi_sink.rs @@ -82,8 +82,16 @@ fn write_fvi_record(w: &mut dyn Write, r: &PictureRecord) -> io::Result<()> { // `src` is REQUIRED by the record schema (Appendix A); when provenance is // absent the member is still emitted as null — a reader treats null as // "position unknown". + // + // Per `docs/FVI_FORMAT.md` §9, `src.byte` is the offset of the AU's first + // byte WITHIN its `sector` (not the absolute source offset). `SourcePos.byte` + // is the absolute offset, so reduce it modulo the sector size; `sector` + // already carries the whole-sector count. let src = match r.source { - Some(s) => serde_json::json!({ "sector": s.sector, "byte": s.byte }), + Some(s) => serde_json::json!({ + "sector": s.sector, + "byte": s.byte % u64::from(FVI_SECTOR_SIZE), + }), None => serde_json::Value::Null, }; @@ -355,8 +363,9 @@ mod tests { let dir = tempdir(); let path = dir.join("movie.fvi"); let mut sink = FviSink::create(&path, &mpeg2_title(), "iso://m.iso".into(), 1).unwrap(); - // Video frame on track 0 → indexed. - sink.write(&vframe(0, Some(i_pic()), Some(SourcePos::at_byte(2048)))) + // Video frame on track 0 → indexed. Offset 2148 = sector 1, byte 100 + // within that sector (exercises the within-sector `src.byte`, §9). + sink.write(&vframe(0, Some(i_pic()), Some(SourcePos::at_byte(2148)))) .unwrap(); // Audio frame on a non-video track → ignored. sink.write(&vframe(7, None, Some(SourcePos::at_byte(9999)))) @@ -387,6 +396,7 @@ mod tests { assert_eq!(rec["nb_fields"], 2); assert_eq!(rec["pts"], 0); assert_eq!(rec["src"]["sector"], 1); + assert_eq!(rec["src"]["byte"], 100); // 2148 % 2048 → within-sector (§9) assert!(rec.get("dts").is_none(), "no DTS on a frame → omitted"); assert!( rec.get("gop").is_none(), diff --git a/src/mux/m2ts_mux/mod.rs b/src/mux/m2ts_mux/mod.rs index d13a41d..72fe861 100644 --- a/src/mux/m2ts_mux/mod.rs +++ b/src/mux/m2ts_mux/mod.rs @@ -254,7 +254,10 @@ impl M2tsMux { // step — e.g. a leading audio frame ahead of the first video keyframe), // which still floors to 0 per the documented behavior. let delta = raw_90k.wrapping_sub(base) & 0x1_FFFF_FFFF; - if delta > (1 << 32) { 0 } else { delta } + // Signed 33-bit: the sign bit is bit 32 (value 2^32), so the entire + // upper half [2^32, 2^33) is negative (frame before base) and floors + // to 0. delta == 2^32 is the most-negative value (-2^32), hence `>=`. + if delta >= (1 << 32) { 0 } else { delta } } /// Emit one PES payload as a chain of TS packets on `pid`. If `pcr` diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index b27d41e..b14e070 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -28,6 +28,9 @@ const CICP_PRIMARIES_BT470BG: u8 = 5; const CICP_PRIMARIES_BT601_525: u8 = 6; /// ColourPrimaries = 9 (BT.2020 / BT.2100) — ITU-T H.273 Table 2. const CICP_PRIMARIES_BT2020: u8 = 9; +/// ColourPrimaries = 2 ("unspecified" — colorimetry unknown) — ITU-T H.273 +/// Table 2. +const CICP_PRIMARIES_UNSPECIFIED: u8 = 2; /// TransferCharacteristics = 1 (BT.709) — ITU-T H.273 Table 3. const CICP_TRANSFER_BT709: u8 = 1; @@ -41,6 +44,9 @@ const CICP_TRANSFER_PQ: u8 = 16; /// TransferCharacteristics = 18 (ARIB STD-B67 / Hybrid Log-Gamma) — ITU-T H.273 /// Table 3. const CICP_TRANSFER_HLG: u8 = 18; +/// TransferCharacteristics = 2 ("unspecified" — transfer unknown) — ITU-T H.273 +/// Table 3. +const CICP_TRANSFER_UNSPECIFIED: u8 = 2; /// MatrixCoefficients = 1 (BT.709) — ITU-T H.273 Table 4. const CICP_MATRIX_BT709: u8 = 1; @@ -50,6 +56,8 @@ const CICP_MATRIX_BT470BG: u8 = 5; const CICP_MATRIX_BT601_525: u8 = 6; /// MatrixCoefficients = 9 (BT.2020 non-constant luminance) — ITU-T H.273 Table 4. const CICP_MATRIX_BT2020NC: u8 = 9; +/// MatrixCoefficients = 2 ("unspecified" — matrix unknown) — ITU-T H.273 Table 4. +const CICP_MATRIX_UNSPECIFIED: u8 = 2; /// Matroska Colour/Range = 1 (broadcast / studio-swing "limited" range). RFC /// 9559 Range element. (0 = unspecified, 2 = full.) @@ -104,7 +112,16 @@ pub(crate) fn cicp_for_video(v: &VideoStream) -> (u8, u8, u8, u8) { CICP_PRIMARIES_BT601_525, COLOUR_RANGE_LIMITED, ), - ColorSpace::Unknown => (0, 0, 0, 0), + // Unknown colorimetry → CICP "unspecified" (code point 2) for matrix, + // transfer, and primaries, with limited range (the disc norm). Both the + // MKV sink and the FVI sidecar emit 2 so the two sinks of one title + // agree (matches `Colour::from_color_space`'s Unknown mapping). + ColorSpace::Unknown => ( + CICP_MATRIX_UNSPECIFIED, + CICP_TRANSFER_UNSPECIFIED, + CICP_PRIMARIES_UNSPECIFIED, + COLOUR_RANGE_LIMITED, + ), }; // Override the transfer for HDR signalled by the HdrFormat (the coarse enum // can't express PQ/HLG). Only applies on the enum fallback; a measured CICP @@ -1549,6 +1566,41 @@ mod tests { ); } + /// Unknown colorimetry with no measured CICP and no HDR must emit CICP + /// "unspecified" (code point 2) for matrix/transfer/primaries — never 0 — so + /// the MKV sink agrees with the FVI sidecar (`Colour::from_color_space`). + #[test] + fn unknown_color_space_emits_unspecified_cicp() { + let v = VideoStream { + pid: 0xE0, + codec: Codec::Hevc, + resolution: Resolution::R1080p, + frame_rate: crate::disc::FrameRate::F24, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Unknown, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + }; + let t = MkvTrack::video(&v); + assert_eq!( + ( + t.colour_matrix, + t.colour_transfer, + t.colour_primaries, + t.colour_range + ), + ( + CICP_MATRIX_UNSPECIFIED, + CICP_TRANSFER_UNSPECIFIED, + CICP_PRIMARIES_UNSPECIFIED, + COLOUR_RANGE_LIMITED + ), + "Unknown colorimetry must emit CICP 'unspecified' (2), not 0" + ); + } + /// Helper: search for a 4-byte big-endian EBML ID in a byte slice. fn find_id(data: &[u8], id: u32) -> Option { let bytes = id.to_be_bytes(); diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 9b57b92..971e84c 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -101,8 +101,12 @@ enum WriteMode { /// Header written; muxing live. Boxed (MkvMuxer is large) to keep the enum /// small (clippy::large_enum_variant). Active(Box>>), - /// Transient placeholder held only across the Pending → Active swap; never - /// observed by `read` / `write` / `finish`. + /// Sentinel held in `self.mode` while the muxer is being built (across the + /// Pending → Active swap). It is also the terminal state left behind after + /// `finish()` swaps the muxer out, and the degraded state left behind if + /// `activate()` fails partway (the first error still surfaces via `?`). In + /// that terminal state a subsequent `write()` no-ops (`Ok(())`) and `finish()` + /// does not re-finalize. Building, } @@ -188,7 +192,11 @@ impl MkvStream { /// available), then write the header and replay buffered frames. A no-op if /// not pending. The muxer only ever muxes the track it is given — this routes /// the parser's measured value onto that track first. - fn activate(&mut self, coding: Option) -> io::Result<()> { + fn activate( + &mut self, + coding: Option, + video_picture_seen: bool, + ) -> io::Result<()> { let mut pending = match std::mem::replace(&mut self.mode, Mode::Write(WriteMode::Building)) { Mode::Write(WriteMode::Pending(p)) => p, @@ -199,7 +207,7 @@ impl MkvStream { } }; if let Some(vt) = pending.video_track { - apply_coding_to_track(&mut pending.tracks[vt], coding); + apply_coding_to_track(&mut pending.tracks[vt], coding, video_picture_seen); } // --log-level 3: dump the FINAL TrackEntry metadata (field order set). for (i, track) in pending.tracks.iter().enumerate() { @@ -240,12 +248,20 @@ impl MkvStream { /// Set a video track's `FieldOrder` from the MEASURED coding of the first coded /// picture — the parser's value, the first time, never a guess. /// -/// A progressive track has no field order (left UNDETERMINED — expected). An -/// INTERLACED track that reaches here with no measured field order is a +/// A progressive track — or a progressive picture on an interlaced-flagged track +/// — has no field order (left UNDETERMINED — expected). An INTERLACED track that +/// reaches here WITH a video picture but no measured field order is a /// parser/source gap (MPEG-2 carries `top_field_first` on every interlaced /// picture, so it should never be missing): LOG it loudly so the source can be /// debugged, and leave UNDETERMINED — a muxer never fabricates a source fact. -fn apply_coding_to_track(track: &mut MkvTrack, coding: Option) { +/// `video_picture_seen == false` (an empty title finalized with no frames, or a +/// cap-triggered build that never saw the video frame) is NOT a defect — the +/// missing coding is expected there, so log it quietly. +fn apply_coding_to_track( + track: &mut MkvTrack, + coding: Option, + video_picture_seen: bool, +) { // HDR10 static metadata measured from the bitstream (HEVC SEI). Applied for // ANY track type that carries it (independent of interlace): the first coded // picture's PictureInfo holds it once both HDR10 SEI messages were seen. @@ -260,17 +276,32 @@ fn apply_coding_to_track(track: &mut MkvTrack, coding: Option track.field_order = ebml::FIELD_ORDER_TFF, Some(FieldOrder::Bff) => track.field_order = ebml::FIELD_ORDER_BFF, - other => { + // A progressive picture on an interlaced-flagged track carries no field + // order. Leave UNDETERMINED (not a guess) — there is no parser gap here. + Some(FieldOrder::Progressive) => { + track.field_order = ebml::FIELD_ORDER_UNDETERMINED; + } + None if video_picture_seen => { tracing::warn!( target: "mux", - "interlaced video track reached the muxer with NO measured field order \ - (field_order={:?}, coding_present={}); writing FieldOrder=UNDETERMINED \ - — NOT a guess. Debug why the source/parser did not set top_field_first.", - other, + "interlaced video track had a video picture but NO usable field order \ + (coding_present={}); writing FieldOrder=UNDETERMINED — NOT a guess. \ + Debug why the source/parser did not set top_field_first.", coding.is_some(), ); track.field_order = ebml::FIELD_ORDER_UNDETERMINED; } + None => { + // No video picture was ever measured (empty title finalized with no + // frames, or a cap-triggered build before the first video frame). + // Coding is legitimately absent, not a parser defect — log quietly. + tracing::debug!( + target: "mux", + "interlaced video track activated with no video picture \ + (empty/buffered-only title); writing FieldOrder=UNDETERMINED.", + ); + track.field_order = ebml::FIELD_ORDER_UNDETERMINED; + } } } @@ -427,7 +458,7 @@ impl crate::pes::Stream for MkvStream { // Pass the trigger frame's coding only when it IS the video frame; a // cap-triggered build never saw the video frame, so nothing measured // is passed (apply_coding_to_track then logs + leaves UNDETERMINED). - self.activate(if use_coding { frame.coding } else { None })?; + self.activate(if use_coding { frame.coding } else { None }, use_coding)?; if let Mode::Write(WriteMode::Active(m)) = &mut self.mode { return m.write_frame( frame.track, @@ -450,7 +481,10 @@ impl crate::pes::Stream for MkvStream { // A title that produced no frames (or only buffered ones) is still // finalized into a valid MKV: activate now with no measured coding. if matches!(self.mode, Mode::Write(WriteMode::Pending(_))) { - self.activate(None)?; + // No video picture was ever measured for this title (it produced no + // frames, or only buffered non-video ones): coding is legitimately + // absent, not a parser defect — `video_picture_seen=false`. + self.activate(None, false)?; } if let Mode::Write(WriteMode::Active(m)) = std::mem::replace(&mut self.mode, Mode::Write(WriteMode::Building)) @@ -923,7 +957,7 @@ mod tests { // MEASURED bottom-field-first → BFF (6). The red-flag fix. let mut t = interlaced_track(); - apply_coding_to_track(&mut t, Some(pic(false, false))); + apply_coding_to_track(&mut t, Some(pic(false, false)), true); assert_eq!( t.field_order, ebml::FIELD_ORDER_BFF, @@ -932,27 +966,37 @@ mod tests { // MEASURED top-field-first → TFF (1). let mut t = interlaced_track(); - apply_coding_to_track(&mut t, Some(pic(true, false))); + apply_coding_to_track(&mut t, Some(pic(true, false)), true); assert_eq!( t.field_order, ebml::FIELD_ORDER_TFF, "measured TFF → FieldOrder=1" ); - // Interlaced track, NO measured coding → UNDETERMINED (logged loudly, - // never faked). + // Interlaced track, a video picture but NO usable field order → + // UNDETERMINED (logged loudly, never faked). let mut t = interlaced_track(); - apply_coding_to_track(&mut t, None); + apply_coding_to_track(&mut t, None, true); assert_eq!( t.field_order, ebml::FIELD_ORDER_UNDETERMINED, "no measured value → UNDETERMINED, never a guess" ); + // Interlaced track activated with NO video picture (empty/buffered-only + // title) → UNDETERMINED, logged quietly (not a parser defect). + let mut t = interlaced_track(); + apply_coding_to_track(&mut t, None, false); + assert_eq!( + t.field_order, + ebml::FIELD_ORDER_UNDETERMINED, + "empty title → UNDETERMINED, never a guess" + ); + // Progressive picture on an interlaced-flagged track → UNDETERMINED (no // field order applies; not faked to TFF/BFF). let mut t = interlaced_track(); - apply_coding_to_track(&mut t, Some(pic(true, true))); + apply_coding_to_track(&mut t, Some(pic(true, true)), true); assert_eq!(t.field_order, ebml::FIELD_ORDER_UNDETERMINED); // A PROGRESSIVE track is never touched — field order stays UNDETERMINED. @@ -969,7 +1013,7 @@ mod tests { measured_cicp: None, }); assert!(!prog.interlaced); - apply_coding_to_track(&mut prog, Some(pic(false, false))); + apply_coding_to_track(&mut prog, Some(pic(false, false)), true); assert_eq!(prog.field_order, ebml::FIELD_ORDER_UNDETERMINED); } @@ -1011,18 +1055,18 @@ mod tests { let mut t = make(); assert!(t.hdr10.is_none(), "fresh track has no HDR10"); let pic = PictureInfo::coding_type_only(CodingType::I).with_hdr10(Some(h)); - apply_coding_to_track(&mut t, Some(pic)); + apply_coding_to_track(&mut t, Some(pic), true); assert_eq!(t.hdr10, Some(h), "measured HDR10 must reach the track"); // Picture without HDR10 → track stays None (never fabricated). let mut t = make(); let pic = PictureInfo::coding_type_only(CodingType::I); - apply_coding_to_track(&mut t, Some(pic)); + apply_coding_to_track(&mut t, Some(pic), true); assert!(t.hdr10.is_none(), "no measured HDR10 → track stays None"); // No coding at all → None. let mut t = make(); - apply_coding_to_track(&mut t, None); + apply_coding_to_track(&mut t, None, true); assert!(t.hdr10.is_none()); } diff --git a/src/mux/resolve.rs b/src/mux/resolve.rs index 2b76064..92695ea 100644 --- a/src/mux/resolve.rs +++ b/src/mux/resolve.rs @@ -12,6 +12,7 @@ //! | network:// | Yes (listen) | Yes (connect) | host:port (required) | //! | stdio:// | Yes (stdin) | Yes (stdout) | empty | //! | null:// | -- | Yes | empty | +//! | demux:// | -- | Yes | directory path (required) — per-track ES demux | //! | fvi:// | -- | Yes | file path (required) — per-picture video index | //! //! Bare paths without a scheme are rejected. @@ -920,6 +921,30 @@ mod tests { !parse_url("dir://x").is_disc_source(), "dir:// is a sink, never a disc source" ); + // fvi:// parses to Fvi with the raw remainder as the path, and is a + // sink (never a disc source) — parallel to the demux:// coverage above. + match parse_url("fvi://out/movie.fvi") { + StreamUrl::Fvi { path } => { + assert_eq!(path, PathBuf::from("out/movie.fvi")); + } + other => panic!("fvi:// must parse to Fvi, got {other:?}"), + } + assert_eq!(parse_url("fvi://x").scheme(), "fvi"); + assert_eq!(parse_url("fvi://x/y.fvi").path_str(), "x/y.fvi"); + assert!( + !parse_url("fvi://x").is_disc_source(), + "fvi:// is a sink, never a disc source" + ); + } + + /// `fvi://` is output-only: `input()` rejects it with StreamWriteOnly + /// (E9001 → Unsupported), mirroring `null://` / `demux://`. + #[test] + fn input_fvi_url_is_write_only() { + assert_eq!( + input_err_kind("fvi://out/movie.fvi"), + std::io::ErrorKind::Unsupported + ); } /// `dir://` is output-only: `input()` rejects it (StreamWriteOnly → diff --git a/src/mux/ts.rs b/src/mux/ts.rs index e0937fa..e0bbd1b 100644 --- a/src/mux/ts.rs +++ b/src/mux/ts.rs @@ -229,6 +229,12 @@ impl TsDemuxer { /// `data` in place. Zero-copy on the bulk path; one 192-byte copy /// on the boundary. pub fn feed(&mut self, data: &[u8]) -> Vec { + // A plain feed carries no provenance. Reset any base a prior + // `feed_at` left behind so mixing the two entry points is safe: + // after this call no `SourcePos` is stamped, and the stale running + // base can't leak a wrong offset into the boundary packet. + self.feed_base = 0; + self.has_base = false; self.feed_inner(data) } @@ -263,15 +269,20 @@ impl TsDemuxer { self.remainder.extend_from_slice(data); return completed; } + // Capture the remainder length before clearing — it's how many of + // the boundary packet's bytes lived in the PREVIOUS feed buffer, + // and `feed_base` currently points at the FIRST byte of THIS buffer. + let rem_len = self.remainder.len(); let mut boundary = [0u8; BD_SOURCE_PACKET_BYTES]; - boundary[..self.remainder.len()].copy_from_slice(&self.remainder); - boundary[self.remainder.len()..].copy_from_slice(&data[..need]); + boundary[..rem_len].copy_from_slice(&self.remainder); + boundary[rem_len..].copy_from_slice(&data[..need]); self.remainder.clear(); - // The boundary packet began in the PREVIOUS feed buffer; stamp it - // with the offset just before this buffer (its first bytes' base). - let src = self - .has_base - .then(|| crate::pes::SourcePos::at_byte(self.feed_base.saturating_sub(1))); + // The boundary packet's first byte sat `rem_len` bytes before the + // current feed_base (in the previous buffer). Stamp it there — not + // at `feed_base - 1`, which would be wrong by `rem_len - 1` bytes. + let src = self.has_base.then(|| { + crate::pes::SourcePos::at_byte(self.feed_base.saturating_sub(rem_len as u64)) + }); self.process_packet(&boundary, src, &mut completed); offset = need; } @@ -931,6 +942,76 @@ mod tests { assert!(result.is_empty()); } + /// A boundary packet (one split across two feeds) must be stamped with the + /// source offset of its FIRST byte, which sat `remainder.len()` bytes before + /// the current feed's base — not at `feed_base - 1`. We feed two 192-byte + /// packets via `feed_at`, splitting mid-second-packet so the second packet + /// is reassembled at the boundary, and assert its provenance lands exactly + /// on its first byte. + #[test] + fn boundary_packet_source_is_first_byte_not_base_minus_one() { + let pid = 0x1011; + let base: u64 = 20480; // sector-aligned (10 × 2048) + let mut demux = TsDemuxer::new(&[pid]); + + let pkt0 = ts_payload_packet(pid, true, 0, &pes_start(b"AAAA")); + let pkt1 = ts_payload_packet(pid, true, 1, &pes_start(b"BBBB")); + let mut full = pkt0; + full.extend_from_slice(&pkt1); + + // Split mid-pkt1 → pkt1 is reassembled from a 100-byte remainder + the + // next feed's head. pkt1's first byte is at absolute offset base + 192. + let split = BD_SOURCE_PACKET_BYTES + 100; + let out1 = demux.feed_at(base, &full[..split]); + assert!(out1.is_empty(), "pkt0's PES is still open"); + + // Second feed carries the rest; data[0] is at absolute base + split. + let out2 = demux.feed_at(base + split as u64, &full[split..]); + // pkt1 (PUSI) flushes pkt0's "AAAA" PES, stamped at pkt0's first byte. + assert_eq!(out2.len(), 1, "pkt0's PES completes when pkt1 starts"); + assert_eq!( + out2[0].source.map(|s| s.byte), + Some(base), + "AAAA PES provenance is pkt0's first byte" + ); + + // Flush emits pkt1's "BBBB" PES — its source is the boundary stamp. + let out3 = demux.flush(); + assert_eq!(out3.len(), 1, "pkt1's PES flushes out"); + assert_eq!( + out3[0].source.map(|s| s.byte), + Some(base + BD_SOURCE_PACKET_BYTES as u64), + "boundary packet provenance must be its first byte (base + 192), \ + not feed_base - 1" + ); + } + + /// Owner decision #7: a plain `feed()` must reset/ignore any base a prior + /// `feed_at()` left behind, so mixing the two is safe. After a `feed_at` + /// primes a base, the next plain `feed` must stamp `None` on PES packets it + /// begins. + #[test] + fn plain_feed_resets_prior_feed_at_base() { + let pid = 0x1011; + let mut demux = TsDemuxer::new(&[pid]); + + // Prime a base via feed_at; pkt0's PES stays open. + let out1 = demux.feed_at(20480, &ts_payload_packet(pid, true, 0, &pes_start(b"AAAA"))); + assert!(out1.is_empty()); + + // Plain feed must clear the base. pkt1 (PUSI) flushes "AAAA" (which was + // stamped during feed_at) and starts "BBBB" with NO provenance. + let out2 = demux.feed(&ts_payload_packet(pid, true, 1, &pes_start(b"BBBB"))); + assert_eq!(out2.len(), 1, "AAAA completes"); + + let out3 = demux.flush(); + assert_eq!(out3.len(), 1, "BBBB flushes"); + assert_eq!( + out3[0].source, None, + "PES begun by a plain feed must carry no source after a prior feed_at" + ); + } + // ── scan_streams PMT parsing ────────────────────────────────────────── /// Wrap a 188-byte TS packet body in a 192-byte BD-TS packet diff --git a/src/mux/videomap.rs b/src/mux/videomap.rs index e121e98..733ccf0 100644 --- a/src/mux/videomap.rs +++ b/src/mux/videomap.rs @@ -551,6 +551,21 @@ mod tests { }, "measured CICP must override the coarse color_space enum" ); + + // Unknown colorimetry, SDR, no measured CICP → all code points map to + // "unspecified" (2), matching `from_color_space(Unknown)`. Both sinks of + // one title must emit 2, never 0. + let c = Colour::from_video(&mk(HdrFormat::Sdr, ColorSpace::Unknown, None)); + assert_eq!( + c, + Colour { + primaries: 2, + transfer: 2, + matrix: 2, + full_range: false, + }, + "Unknown colorimetry must emit CICP 'unspecified' (2), not 0" + ); } #[test] diff --git a/src/pes.rs b/src/pes.rs index a502f86..102658e 100644 --- a/src/pes.rs +++ b/src/pes.rs @@ -107,20 +107,17 @@ impl PesFrame { /// truncated `.pes` data would be accepted as a graceful end. pub fn deserialize(r: &mut dyn std::io::Read) -> std::io::Result> { // Probe one byte first to distinguish clean EOF from a truncated - // header. + // header. Loop on EINTR so back-to-back signals don't fail a + // recoverable read — symmetric with read_exact's internal retry + // on the rest of the header and the data below. let mut first = [0u8; 1]; - match r.read(&mut first) { - Ok(0) => return Ok(None), // clean EOF, no frame started - Ok(_) => {} - Err(e) if e.kind() == std::io::ErrorKind::Interrupted => { - // Retry-once on EINTR before committing to the header read. - match r.read(&mut first) { - Ok(0) => return Ok(None), - Ok(_) => {} - Err(e) => return Err(e), - } + loop { + match r.read(&mut first) { + Ok(0) => return Ok(None), // clean EOF, no frame started + Ok(_) => break, + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => return Err(e), } - Err(e) => return Err(e), } let mut header = [0u8; 22]; // 1 + 8 + 1 + 8 + 4 diff --git a/src/unlock.rs b/src/unlock.rs index 0114d1d..8a7174c 100644 --- a/src/unlock.rs +++ b/src/unlock.rs @@ -104,11 +104,17 @@ pub fn register_unlocker(u: Box) { /// * `Ok(Some((name, vid)))` — a registered unlocker matched, put the drive /// into extended mode, and returned the OEM Volume ID. The caller stashes /// the VID for the handshake phase and need not run the cert handshake. -/// * `Ok(None)` — no unlocker matched, OR the matching unlocker failed -/// ([`UnlockError`], logged). Either way the drive is usable in stock mode -/// and the caller falls through to the in-tree cert handshake. Folding an -/// unlock failure into `Ok(None)` keeps drive `init()` infallible — a drive -/// that simply isn't firmware-unlockable must not fail init. +/// * `Ok(None)` — no unlocker matched, OR the matching unlocker reported a +/// *capability* failure ([`UnlockError::FirmwareNotUnlockable`], +/// [`UnlockError::VidUnavailable`], or a cert-auth outcome — all logged). +/// Either way the drive is usable in stock mode and the caller falls +/// through to the in-tree cert handshake. Folding a capability failure into +/// `Ok(None)` keeps drive `init()` infallible — a drive that simply isn't +/// firmware-unlockable must not fail init. +/// * `Err(_)` — the matching unlocker hit a genuine SCSI/transport fault +/// ([`UnlockError::Scsi`]). The bus is broken, not merely unsupported, so +/// this propagates and aborts init rather than silently falling through to +/// a cert handshake that would also fail. pub(crate) fn route_unlock( scsi: &mut dyn ScsiTransport, id: &DriveId, @@ -124,10 +130,29 @@ pub(crate) fn route_unlock( let name = u.name().to_string(); match u.unlock(scsi, id) { Ok(vid) => return Ok(Some((name, vid))), + // A genuine SCSI/transport fault is not "this drive can't be + // unlocked" — the bus is broken. Propagate so init() aborts + // instead of falling through to a cert handshake that will + // also fail on the same dead transport. The numeric code from + // the originating error is logged; the returned error is the + // canonical transport-error variant. + Err(UnlockError::Scsi(code)) => { + tracing::error!( + target: "freemkv::unlock", + unlocker = %name, + code, + "unlocker hit a transport fault during unlock; aborting init" + ); + return Err(crate::error::Error::ScsiError { + opcode: 0, + status: 0, + sense: None, + }); + } + // A firmware unlocker that can't unlock / has no OEM VID: + // fall through to the cert handshake. Debug-only structured + // log (variant identifiers, no English prose). Err(e) => { - // A firmware unlocker that can't unlock / has no OEM VID: - // fall through to the cert handshake. Debug-only structured - // log (variant identifiers, no English prose). tracing::warn!( target: "freemkv::unlock", unlocker = %name, @@ -228,6 +253,10 @@ mod tests { /// `None` → `unlock` yields `Err(UnlockError::VidUnavailable)` so /// `route_unlock` falls through to the cert handshake. vid: Option<[u8; 16]>, + /// When `Some(code)`, `unlock` yields `Err(UnlockError::Scsi(code))` + /// (a transport fault) instead of consulting `vid`, so `route_unlock` + /// propagates an error and aborts init. + scsi_err: Option, /// Records whether set_max_read_speed was invoked. speed_ran: Arc, } @@ -238,6 +267,7 @@ mod tests { ran, // Default: a successful unlock returning an all-zero VID. vid: Some([0u8; 16]), + scsi_err: None, speed_ran: Arc::new(AtomicBool::new(false)), } } @@ -245,6 +275,10 @@ mod tests { self.vid = vid; self } + fn with_scsi_err(mut self, code: u16) -> Self { + self.scsi_err = Some(code); + self + } fn with_speed(mut self, speed_ran: Arc) -> Self { self.speed_ran = speed_ran; self @@ -263,6 +297,9 @@ mod tests { _id: &DriveId, ) -> std::result::Result { self.ran.store(true, Ordering::SeqCst); + if let Some(code) = self.scsi_err { + return Err(UnlockError::Scsi(code)); + } match self.vid { Some(v) => Ok(Vid(v)), None => Err(UnlockError::VidUnavailable), @@ -359,6 +396,33 @@ mod tests { assert!(got.is_none(), "no match → cert fallback"); } + /// A matching unlocker that hits a genuine transport fault + /// (`UnlockError::Scsi`) makes `route_unlock` PROPAGATE an `Err` rather + /// than fold to `Ok(None)`: a dead bus must abort init, not silently fall + /// through to a cert handshake that would also fail. Capability failures + /// (`VidUnavailable` etc.) still fold to `Ok(None)` — proven by the sibling + /// routing tests; this one pins the transport-fault exception. + #[test] + fn route_unlock_propagates_scsi_transport_fault() { + let mut scsi = NoopTransport; + + register_unlocker(Box::new( + FakeUnlocker::new("SCSIVNDR", Arc::new(AtomicBool::new(false))) + .with_scsi_err(crate::error::E_SCSI_ERROR), + )); + + let got = route_unlock(&mut scsi, &fake_id("SCSIVNDR")); + assert!( + got.is_err(), + "a transport fault during unlock aborts init (propagates Err)" + ); + assert_eq!( + got.unwrap_err().code(), + crate::error::E_SCSI_ERROR, + "propagated error is the canonical transport-error code" + ); + } + /// `unlocker_set_max_read_speed` consults the FIRST matching unlocker's /// `set_max_read_speed`. A matching unlocker is invoked; a non-match is a /// safe no-op (nothing invoked, `Ok(())`). diff --git a/tests/fvi_pipeline.rs b/tests/fvi_pipeline.rs index c6f3c31..6039176 100644 --- a/tests/fvi_pipeline.rs +++ b/tests/fvi_pipeline.rs @@ -160,7 +160,7 @@ fn two_gop_image() -> Vec { data[..a.len()].copy_from_slice(&a); let mut b = ps_pack_header(); - b.extend_from_slice(&video_pes(&gop_es(), 3003)); // ~0.1s later + b.extend_from_slice(&video_pes(&gop_es(), 3003)); // one frame (~33 ms) later at 29.97 fps (3003/90000 s) let off = 3 * 2048; data[off..off + b.len()].copy_from_slice(&b); @@ -276,13 +276,21 @@ fn fvi_sink_indexes_real_mpeg2_pipeline_output() { vec![0, 3], "stamped src sectors must reach the .fvi in arrival order; got {src_sectors:?}" ); - // For each stamped record, sector == byte / 2048 (SourcePos::at_byte). The - // byte offset is exact (here 14 into each region, just past the pack - // header), so it is NOT sector-aligned — provenance is byte-exact. + // Per FVI_FORMAT.md §9, `src.byte` is the offset of the AU's first byte + // WITHIN its 2048-byte `src.sector`, so it is always < 2048. The two + // stamped sources sit 14 bytes into their sector (just past the pack + // header), so the within-sector byte is exact (14) — provenance is + // byte-exact, carried unchanged from demux. for r in &records { - if let (Some(sector), Some(byte)) = (r["src"]["sector"].as_u64(), r["src"]["byte"].as_u64()) - { - assert_eq!(sector, byte / 2048, "src.sector must equal src.byte / 2048"); + if let Some(byte) = r["src"]["byte"].as_u64() { + assert!( + byte < 2048, + "src.byte is a within-sector offset (§9), must be < 2048; got {byte}" + ); + assert_eq!( + byte, 14, + "stamped src.byte is 14 (just past the pack header)" + ); } } @@ -338,7 +346,7 @@ fn fvi_sink_indexes_non_mpeg2_frames_codec_agnostically() { assert_eq!(recs[0]["type"], "I"); assert_eq!(recs[0]["pts"], 1234); assert_eq!(recs[0]["src"]["sector"], 4); // 8192 / 2048 - assert_eq!(recs[0]["src"]["byte"], 8192); + assert_eq!(recs[0]["src"]["byte"], 0); // 8192 is sector-aligned → within-sector offset 0 (§9) assert!( recs[0].get("field_order").is_none() && recs[0].get("nb_fields").is_none(), "coding-absent record omits field_order/nb_fields"