diff --git a/src/aacs/decrypt.rs b/src/aacs/decrypt.rs index f5eb86b..40b290f 100644 --- a/src/aacs/decrypt.rs +++ b/src/aacs/decrypt.rs @@ -29,8 +29,15 @@ pub const ALIGNED_UNIT_SECTORS: u32 = (ALIGNED_UNIT_LEN / SECTOR_LEN) as u32; /// disc whose clip `start_lba` is not itself 3-aligned would otherwise mis-gate /// (reject readable units, then report "Decryption failed") on exactly the /// titles whose clips land off a 3-boundary. +/// +/// `lba` is always `>= unit_base` by contract (a read never begins before the +/// extent base it is measured against). `saturating_sub` makes the `lba < +/// unit_base` case well-defined anyway — it clamps the offset to 0, which is a +/// unit boundary — rather than the latent `wrapping_sub` trap where an +/// underflow wraps to ~2^32 and, because `2^32 ≡ 1 (mod 3)`, mis-reports the +/// alignment (e.g. `lba == unit_base - 1` would falsely read as aligned). pub fn is_unit_aligned(lba: u32, unit_base: u32) -> bool { - lba.wrapping_sub(unit_base) % ALIGNED_UNIT_SECTORS == 0 + lba.saturating_sub(unit_base) % ALIGNED_UNIT_SECTORS == 0 } /// Size of one sector. @@ -331,6 +338,43 @@ mod tests { assert_eq!(dec, plain); } + #[test] + fn is_unit_aligned_relative_to_base() { + // Aligned at the base and every 3 sectors above it; misaligned between. + assert!(is_unit_aligned(100, 100), "base itself is aligned"); + assert!(is_unit_aligned(103, 100), "one unit past base is aligned"); + assert!(is_unit_aligned(106, 100)); + assert!(!is_unit_aligned(101, 100)); + assert!(!is_unit_aligned(102, 100)); + // Non-3-aligned base: alignment is RELATIVE to the base, not absolute. + assert!(is_unit_aligned(101, 101), "non-3-aligned base is aligned"); + assert!(is_unit_aligned(104, 101)); + assert!(!is_unit_aligned(102, 101)); + } + + #[test] + fn is_unit_aligned_lba_below_base_is_well_defined() { + // Latent-trap contract (rc.5.2 audit #5): a read never starts before its + // extent base, but if `lba < unit_base` the result must be well-defined, + // NOT the `wrapping_sub` underflow that — because 2^32 ≡ 1 (mod 3) — + // would falsely report alignment. `saturating_sub` clamps to offset 0, + // which is a unit boundary, so any `lba <= unit_base` reads as aligned. + assert!( + is_unit_aligned(99, 100), + "lba just below base must not wrap" + ); + assert!(is_unit_aligned(98, 100)); + assert!(is_unit_aligned(0, 100)); + // The specific wrapping_sub trap value: unit_base - 1. With wrapping_sub + // this is 0xFFFF_FFFF % 3 == 0 → falsely "aligned" by underflow; with + // saturating_sub it is genuinely 0 → aligned, for the right reason. + assert!(is_unit_aligned(u32::MAX, u32::MAX)); // base == lba, trivially aligned + assert!( + is_unit_aligned(0, u32::MAX), + "max base, lba 0 must saturate to 0" + ); + } + #[test] fn test_decrypt_unit_unencrypted() { // A clear unit (TS syncs intact) is not scrambled → passes through. diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 8527c45..696fc08 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -442,6 +442,13 @@ pub(crate) fn correct_truehd_channels(reader: &mut dyn SectorSource, title: &mut return; } let mut buf = vec![0u8; n as usize * 2048]; + // Anchor the AACS unit-alignment gate to the title's encrypted-region start + // before probing. Without this a `DecryptingSectorSource` falls back to an + // absolute `start_lba % 3` gate; a non-3-aligned `ext.start_lba` then trips + // DecryptFailed on the very first probe read, so the TrueHD channel count is + // never corrected and Atmos / 7.1 is silently understated as 5.1. No-op for + // CSS / unencrypted sources (set_unit_base default is a no-op). + reader.set_unit_base(ext.start_lba); if reader .read_sectors(ext.start_lba, n, &mut buf, true) .is_err() diff --git a/src/mux/codec/hevc.rs b/src/mux/codec/hevc.rs index a06ab74..6da0543 100644 --- a/src/mux/codec/hevc.rs +++ b/src/mux/codec/hevc.rs @@ -78,11 +78,19 @@ pub struct HevcParser { // so the rewrite branch is never reached and output is byte-identical to a // parser without this field. pending_clip_boundary: bool, - // Highest PES PTS (90 kHz ticks) seen on this video stream so far. Used to - // AUTO-DETECT a non-seamless clip boundary from the bitstream when the - // caller never plumbs one in (the common case — see `BACKSTEP_TICKS`). - // `None` until the first AU with a PTS. + // Highest PES PTS seen on this video stream so far, on a MONOTONIC 64-bit + // timeline (raw 33-bit PTS unwrapped across 2^33 wraparounds — see + // `pts_wrap_offset`). Used to AUTO-DETECT a non-seamless clip boundary from + // the bitstream when the caller never plumbs one in (the common case — see + // `BACKSTEP_TICKS`). `None` until the first AU with a PTS. high_pts: Option, + // Accumulated 2^33-tick offset applied to raw PES PTS values to unwrap them + // onto the monotonic timeline `high_pts` lives on. The 33-bit 90 kHz PTS + // wraps every ~26.5 h; a BD clip can start at a high base and cross the wrap + // mid-title. Without unwrapping, the 2^33→0 step looks like a backward clip + // reset and false-arms the CRA→BLA rewrite (corrupting a legitimate in-clip + // CRA and dropping valid RASL pictures). Each detected wrap adds 2^33 here. + pts_wrap_offset: i64, } // A backward PES-PTS step larger than this (90 kHz ticks) marks a non-seamless @@ -97,6 +105,18 @@ pub struct HevcParser { // concatenated multi-clip title otherwise produces. const BACKSTEP_TICKS: i64 = 270_000; +// The 33-bit 90 kHz PES PTS counter wraps at 2^33 ticks (~26.5 h). When the raw +// PTS steps backward by approximately a full period — i.e. it landed just past +// the wrap — it is a counter wraparound, NOT a clip reset: unwrap it (add 2^33) +// instead of arming the CRA→BLA rewrite. A genuine non-seamless clip join resets +// the PTS to a fresh small base, a backward step of arbitrary (sub-2^33) size; a +// wrap is specifically a step of ~2^33. We accept any backward step within one +// `PTS_WRAP_PERIOD`/2 of a full period as a wrap (the new value is below the old +// high-water but within a reorder window of the wrap point), which cleanly +// separates the two cases since a clip reset to a small base is nowhere near 2^33 +// below the high-water unless the title is itself ~26 h long (impossible on BD). +const PTS_WRAP_PERIOD: i64 = 1 << 33; + impl Default for HevcParser { fn default() -> Self { Self::new() @@ -115,6 +135,7 @@ impl HevcParser { cur_pps: None, pending_clip_boundary: false, high_pts: None, + pts_wrap_offset: 0, } } @@ -263,17 +284,31 @@ impl CodecParser for HevcParser { // clip then consumes. Without this, the splice CRA's RASL leading // pictures reference pre-join frames gone after concatenation and a // linear decoder floods "Could not find ref with POC N" (the Top Gun - // UHD defect). Uses the RAW 90 kHz PES PTS (not the rebased mux - // timeline) and tracks the high-water mark so a single in-clip B-frame - // dip never arms it. DTS-only AUs (no PTS) leave the watermark untouched. + // UHD defect). Uses the 90 kHz PES PTS (not the rebased mux timeline) + // UNWRAPPED onto a monotonic 64-bit timeline first — the raw 33-bit PTS + // wraps every ~26.5 h, and a single-clip title that crosses 2^33→0 would + // otherwise false-arm the rewrite (corrupting a legitimate in-clip CRA). + // Tracks the high-water mark so a single in-clip B-frame dip never arms + // it. DTS-only AUs (no PTS) leave the watermark untouched. if let Some(raw_pts) = pes.pts { - match self.high_pts { - Some(high) if raw_pts < high - BACKSTEP_TICKS => { - self.pending_clip_boundary = true; - self.high_pts = Some(raw_pts); + // Unwrap onto the monotonic timeline. If the offset-adjusted value + // dropped to roughly a full period (2^33) below the high-water, the + // 33-bit counter wrapped: add another period and re-check, rather + // than treat the wrap as a backward clip reset. + let mut unwrapped = raw_pts + self.pts_wrap_offset; + if let Some(high) = self.high_pts { + if high - unwrapped > PTS_WRAP_PERIOD / 2 { + self.pts_wrap_offset += PTS_WRAP_PERIOD; + unwrapped += PTS_WRAP_PERIOD; } - Some(high) => self.high_pts = Some(high.max(raw_pts)), - None => self.high_pts = Some(raw_pts), + } + match self.high_pts { + Some(high) if unwrapped < high - BACKSTEP_TICKS => { + self.pending_clip_boundary = true; + self.high_pts = Some(unwrapped); + } + Some(high) => self.high_pts = Some(high.max(unwrapped)), + None => self.high_pts = Some(unwrapped), } } @@ -1261,6 +1296,40 @@ mod tests { ); } + /// Regression for the 33-bit PTS wraparound false-trigger (rc.5.2 audit #1): + /// a SINGLE clip whose raw 90 kHz PES PTS crosses the 2^33 counter wrap + /// (~26.5 h) must NOT be mistaken for a non-seamless clip join. Before the + /// fix the raw 2^33→0 backward step armed `pending_clip_boundary` and the + /// next in-clip CRA was wrongly rewritten CRA→BLA_W_LP (dropping valid RASL + /// pictures — visible corruption). After unwrapping onto a monotonic + /// timeline the wrap is absorbed and the CRA stays CRA. + #[test] + fn cra_after_33bit_pts_wrap_not_rewritten() { + let mut parser = HevcParser::new(); + let period = 1i64 << 33; + // Single clip, PTS climbing toward the 33-bit wrap. Start just below 2^33. + let near_wrap = period - 90_000; // ~1 s before the wrap point + parser.parse(&make_pes(cra_au(&[0x01]), Some(near_wrap))); + parser.parse(&make_pes(cra_au(&[0x02]), Some(near_wrap + 3750))); + // The counter wraps: raw PTS resets to a small value, but this is the + // SAME continuous clip, one frame later. A naive raw comparison sees a + // ~2^33 backward step and false-arms the boundary. + let wrapped = parser.parse(&make_pes(cra_au(&[0x03]), Some(7500))); + assert_eq!( + nal_type_of(&nals_of(&wrapped[0].data)[0]), + NAL_CRA_NUT, + "a CRA whose PTS merely wrapped 2^33->0 must stay CRA, not become BLA" + ); + // Continue past the wrap: PTS keeps climbing from the new low base; still + // one continuous clip, the CRA after must remain CRA. + let after = parser.parse(&make_pes(cra_au(&[0x04]), Some(11250))); + assert_eq!( + nal_type_of(&nals_of(&after[0].data)[0]), + NAL_CRA_NUT, + "post-wrap in-clip CRA must stay CRA" + ); + } + /// Test 3: non-CRA NALs are never rewritten even when a boundary IS marked. /// IDR (19), RASL (8/9), VPS/SPS/PPS, and a trailing slice all pass through /// unmodified; the IDR clears the pending boundary so no later CRA is wrongly diff --git a/src/mux/disc.rs b/src/mux/disc.rs index a62ef15..9bb9562 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -501,6 +501,27 @@ impl DiscStream { break; } + // Recovery read also failed. A transport failure here (status + // 0xFF: USB-bridge crash / disconnect) is NOT a skippable bad + // unit — same as the original 10s read above. The line-442 + // short-circuit only inspected `res`; the 60s recovery read + // (`rec`) can wedge the bridge on its own, and falling into the + // `skip_errors` branch below would zero-fill + advance, treating + // a dead bridge as a skippable unit and marching the whole disc + // at one bridge-recovery per probe (hard rule #2, "runs forever, + // no MKV"). Re-check `rec` and abort, mirroring line 442. + if let Some(e) = rec.as_ref().err() { + if e.is_scsi_transport_failure() { + let (status, sense) = extract_scsi_context(e); + return Err(crate::error::Error::DiscRead { + sector: lba as u64, + status: Some(status), + sense, + } + .into()); + } + } + // Recovery read also failed. Skip the WHOLE failed unit or bail. // Zero-filling and advancing by the full unit keeps // current_offset unit-aligned, so the next read still begins on a @@ -1084,6 +1105,222 @@ mod tests { } } + /// `SectorSource` that mirrors a marginal sector recoverable only with the + /// drive's full ECC budget: every read covering `bad_sector` FAILS while + /// `recovery=false` (the fast 10s pass) and SUCCEEDS (zeroed bytes) once + /// `recovery=true` (the 60s ECC pass). Drives the single-pass bottom-out + /// "last-chance recovery read" success branch in `fill_extents`, which the + /// other test sources (ignoring the flag) never exercise. + struct RecoverableReader { + capacity: u32, + bad_sector: u32, + /// `(lba, count, recovery)` for every issued read. + log: std::sync::Arc>>, + } + + impl crate::sector::SectorSource for RecoverableReader { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + recovery: bool, + ) -> crate::error::Result { + self.log.lock().unwrap().push((lba, count, recovery)); + let end = lba + count as u32; + let covers_bad = self.bad_sector >= lba && self.bad_sector < end; + // Fail on the fast (non-recovery) pass; the 60s ECC recovery read + // succeeds. Distinct non-0x02 sense byte so a transport-failure + // re-check (status 0xFF) is provably NOT triggered here. + if covers_bad && !recovery { + return Err(crate::error::Error::DiscRead { + sector: self.bad_sector as u64, + status: Some(0x02), + sense: None, + }); + } + let bytes = count as usize * 2048; + buf[..bytes].fill(0); + Ok(bytes) + } + + fn capacity_sectors(&self) -> u32 { + self.capacity + } + } + + /// Coverage for the single-pass bottom-out RECOVERY-READ SUCCESS branch + /// (rc.5.2 audit #3): a sector that fails the fast 10s read but reads clean + /// on the 60s ECC recovery read must have its RECOVERED data muxed — the + /// cursor advances over the whole unit, byte counters move, and NO skip is + /// counted. Pre-fix the test sources ignored `recovery`, so this branch was + /// untested. Uses `unit_align=1` (None) so the bottom-out unit is a single + /// sector, exercising the `(sectors as u32) <= align` path precisely. + #[test] + fn recovery_read_success_muxes_recovered_data_no_skip() { + const COUNT: u32 = 10; + let bad = 4u32; + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let reader = RecoverableReader { + capacity: COUNT, + bad_sector: bad, + log: log.clone(), + }; + let mut stream = DiscStream::new( + Box::new(reader), + synthetic_title(COUNT), + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::BdTs, + ); + // skip_errors=false: if the recovery read did NOT succeed, fill_extents + // would return Err — so reaching EOF cleanly proves recovery worked. + stream.skip_errors = false; + + let mut guard = 0; + loop { + match stream.fill_extents() { + Ok(true) => {} + Ok(false) => break, + Err(e) => panic!("recovery read should have succeeded, got: {e}"), + } + guard += 1; + assert!(guard < 1000, "fill_extents did not reach EOF"); + } + + // No skip counted: the recovered unit was muxed, not zero-filled. + assert_eq!( + stream.errors, 0, + "a successful recovery read must not count as a skipped sector" + ); + assert_eq!( + stream.lost_bytes, 0, + "a successful recovery read loses no bytes" + ); + // All COUNT sectors' worth of bytes were read through to the cursor end. + assert_eq!( + stream.bytes_read_total, + COUNT as u64 * 2048, + "every sector (including the recovered one) must be counted as read" + ); + + // The bad sector was retried with recovery=true and that read SUCCEEDED. + let reads = log.lock().unwrap(); + assert!( + reads + .iter() + .any(|&(lba, count, rec)| rec && lba == bad && count == 1), + "expected a recovery=true single-sector read at the bad sector; got {reads:?}" + ); + // And the fast pass at the bad sector did happen with recovery=false. + assert!( + reads.iter().any(|&(lba, _c, rec)| !rec && lba == bad), + "expected a non-recovery read to have first failed at the bad sector" + ); + } + + /// `SectorSource` that fails the fast (non-recovery) read covering + /// `bad_sector` with an ordinary bad-sector error (status 0x02), then fails + /// the 60s ECC recovery read with a TRANSPORT failure (status 0xFF). Models + /// a bridge that wedges precisely during the last-chance recovery read. + struct RecoveryTransportFailReader { + capacity: u32, + bad_sector: u32, + log: std::sync::Arc>>, + } + + impl crate::sector::SectorSource for RecoveryTransportFailReader { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + recovery: bool, + ) -> crate::error::Result { + self.log.lock().unwrap().push((lba, count, recovery)); + let end = lba + count as u32; + if self.bad_sector >= lba && self.bad_sector < end { + let status = if recovery { + crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE + } else { + 0x02 + }; + return Err(crate::error::Error::DiscRead { + sector: self.bad_sector as u64, + status: Some(status), + sense: None, + }); + } + let bytes = count as usize * 2048; + buf[..bytes].fill(0); + Ok(bytes) + } + + fn capacity_sectors(&self) -> u32 { + self.capacity + } + } + + /// Regression (rc.5.2 audit #2): a transport failure on the 60s ECC + /// RECOVERY read (not just the initial 10s read) must ABORT, even under + /// `skip_errors=true`. The line-442 short-circuit only inspected the + /// original `res`; without a re-check the wedged-bridge recovery failure + /// fell into the skip branch — zero-fill + advance — marching the disc at + /// one bridge-recovery per unit ("runs forever, no MKV", hard rule #2). The + /// fix re-checks the recovery error for `is_scsi_transport_failure()` before + /// the skip block and returns `Error::DiscRead`. + #[test] + fn transport_failure_on_recovery_read_aborts_even_with_skip_errors() { + const COUNT: u32 = 10; + let bad = 4u32; + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let reader = RecoveryTransportFailReader { + capacity: COUNT, + bad_sector: bad, + log: log.clone(), + }; + let mut stream = DiscStream::new( + Box::new(reader), + synthetic_title(COUNT), + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::BdTs, + ); + stream.skip_errors = true; + + // Drive fill_extents across batches: the good leading sectors mux fine, + // and the batch covering the bad sector shrinks to size 1, fails the + // fast read (0x02), then the bottom-out recovery read returns the + // transport failure (0xFF) — which must abort. + let mut res = Ok(true); + for _ in 0..1000 { + res = stream.fill_extents(); + if !matches!(res, Ok(true)) { + break; + } + } + assert!( + res.is_err(), + "a transport failure on the recovery read must abort fill_extents, got {res:?}" + ); + assert_eq!( + stream.errors, 0, + "a recovery-read transport-failure abort must NOT count as a skip" + ); + assert_eq!( + stream.lost_bytes, 0, + "a transport-failure abort zero-fills nothing" + ); + // Prove the bottom-out recovery read was actually reached and aborted on. + let reads = log.lock().unwrap(); + assert!( + reads + .iter() + .any(|&(lba, count, rec)| rec && lba == bad && count == 1), + "expected a recovery=true read at the bad sector to have been attempted; got {reads:?}" + ); + } + /// Regression: a USB-bridge transport crash (status=0xFF) during a direct /// single-pass `disc://→mkv://` rip must ABORT immediately, even under /// `skip_errors=true`. The pre-fix behavior treated it as a skippable bad