diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a4da750..2951a3b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,10 +42,13 @@ jobs: # crates.io publish is an INDEPENDENT job: it serves EXTERNAL consumers only. # The freemkv binaries no longer depend on it (they git-tag-pin libfreemkv via # a committed [patch.crates-io]), so this publish runs in parallel with their - # release builds rather than gating them. It only `needs: verify` (the version - # check) — NOT `test` — so publish isn't serialized behind the test suite. + # release builds rather than gating them. It `needs: [verify, test]` so a + # failing test suite still blocks publication to crates.io — external + # consumers who `cargo add libfreemkv` must never receive a release whose + # tests were failing. (The two upstream jobs run in parallel, so this gate + # does not serialize publish behind test beyond their own completion.) publish: - needs: verify + needs: [verify, test] runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 diff --git a/src/aacs/keys.rs b/src/aacs/keys.rs index c9a1fb9..5a0dd04 100644 --- a/src/aacs/keys.rs +++ b/src/aacs/keys.rs @@ -2650,6 +2650,46 @@ mod tests { assert_eq!(mkb_version(&mkb), Some(0x0102_0304)); } + #[test] + fn mkb_type_category_c_20_is_uhd() { + // Type 0x10 record, BE24 length 0x0C (12). MKBType field (body + // offset 0 = pos+4) = MKB_20_CATEGORY_C (0x48141003). + let mkb = [ + 0x10, 0x00, 0x00, 0x0C, 0x48, 0x14, 0x10, 0x03, 0x00, 0x00, 0x00, 0x01, + ]; + assert_eq!(mkb_type_raw(&mkb), Some(MKB_20_CATEGORY_C)); + assert_eq!(mkb_type(&mkb), Some(MkbType::CategoryC20)); + assert_eq!(mkb_is_uhd(&mkb), Some(true)); + assert!(MkbType::CategoryC20.is_uhd()); + assert_eq!(MkbType::CategoryC20.generation(), AacsVersion::V20); + // Sanity on the 2.1 sibling. + assert_eq!(MkbType::from_raw(MKB_21_CATEGORY_C), MkbType::CategoryC21); + assert_eq!(MkbType::CategoryC21.generation(), AacsVersion::V21); + } + + #[test] + fn mkb_type_prerecorded_is_bluray_v10() { + // Type 0x10 record with MKB_TYPE_4_PRERECORDED (0x00041003) — a + // standard Blu-ray (AACS 1.0) block, not UHD. + let mkb = [ + 0x10, 0x00, 0x00, 0x0C, 0x00, 0x04, 0x10, 0x03, 0x00, 0x00, 0x00, 0x01, + ]; + assert_eq!(mkb_type(&mkb), Some(MkbType::Prerecorded)); + assert_eq!(mkb_is_uhd(&mkb), Some(false)); + assert!(!MkbType::Prerecorded.is_uhd()); + assert_eq!(MkbType::Prerecorded.generation(), AacsVersion::V10); + } + + #[test] + fn mkb_type_none_when_no_0x10_record() { + // A buffer whose only record is a 0x81 (verify-media-key) record and + // no 0x10 Type-and-Version record → mkb_type_raw returns None. + let mkb = [0x81, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00]; + assert_eq!(mkb_type_raw(&mkb), None); + assert_eq!(mkb_type(&mkb), None); + assert_eq!(mkb_is_uhd(&mkb), None); + } + #[test] fn mkb_find_mk_dv_skips_short_verify_record() { // A 0x81 record with rec_len < 20 carries no full mk_dv; the finder diff --git a/src/disc/dvd.rs b/src/disc/dvd.rs index 3ebcf7a..52b40cc 100644 --- a/src/disc/dvd.rs +++ b/src/disc/dvd.rs @@ -542,7 +542,7 @@ mod tests { let vmg = build_vmg(&[(1, 1, 1)]); let vts = build_vts( 0, - crate::ifo::v_atr_byte(crate::ifo::VIDEO_FORMAT_PAL, crate::ifo::ASPECT_4X3), + crate::ifo::v_atr_byte(crate::ifo::VIDEO_FORMAT_PAL, crate::ifo::ASPECT_16X9), &[], &[], &[(0, 9)], @@ -582,6 +582,11 @@ mod tests { ColorSpace::Bt470bg, "PAL DVD is SD BT.470BG, not BT.709" ); + assert_eq!( + v.display_aspect, + Some((16, 9)), + "ASPECT_16X9 IFO byte must map to a 16:9 display aspect" + ); } /// NTSC DVD video is SD SMPTE-170M colorimetry (not BT.709). Mirror of the @@ -631,6 +636,11 @@ mod tests { ColorSpace::Smpte170m, "NTSC DVD is SD SMPTE-170M, not BT.709" ); + assert_eq!( + v.display_aspect, + Some((4, 3)), + "ASPECT_4X3 IFO byte must map to a 4:3 display aspect" + ); } /// AC-3 audio gets sub_stream_id 0x80 → PID routed via dvd_audio_pid diff --git a/src/io/pipeline.rs b/src/io/pipeline.rs index c1697ba..72d4329 100644 --- a/src/io/pipeline.rs +++ b/src/io/pipeline.rs @@ -150,7 +150,11 @@ fn finish_with_grace( // observes it the moment its wedged syscall returns: it then skips // any further `apply` and skips `close()`, rather than running on to // finalise the abandoned output file. - abandoned.store(true, Ordering::Relaxed); + // `Release` here pairs with the `Acquire` loads in the consumer loop so + // the leaked consumer reliably observes the flag the moment its wedged + // syscall returns, even on weak memory models (ARM64/POWER) where + // `Relaxed` gives no cross-thread visibility guarantee. + abandoned.store(true, Ordering::Release); tracing::warn!( target: "freemkv::pipeline", phase = "finish_with_halt_grace_expired", @@ -299,7 +303,7 @@ impl Pipeline { // dead receiver, but we touch the output no further. The // final post-loop abandonment check returns the error // and skips `close()`. - if abandoned_consumer.load(Ordering::Relaxed) { + if abandoned_consumer.load(Ordering::Acquire) { continue; } @@ -359,7 +363,7 @@ impl Pipeline { // MKV Cues + patching the segment header) on a file the // caller already reported as failed is exactly the // write race we must not run. - if abandoned_consumer.load(Ordering::Relaxed) { + if abandoned_consumer.load(Ordering::Acquire) { return Err(Error::Halted); } diff --git a/src/keydb.rs b/src/keydb.rs index 54a82e4..36bbdb9 100644 --- a/src/keydb.rs +++ b/src/keydb.rs @@ -43,17 +43,6 @@ fn read_capped_to_string(reader: R) -> Result { String::from_utf8(buf).map_err(|_| Error::KeydbParse) } -/// Standard keydb storage path — the canonical location to write the keydb to. -/// -/// On Windows this is the idiomatic per-user roaming dir -/// `%APPDATA%\freemkv\keydb.cfg`, falling back to the legacy -/// `%USERPROFILE%\.config\freemkv\keydb.cfg` only if `APPDATA` is unset. On -/// Linux/macOS it stays the long-standing `$HOME/.config/freemkv/keydb.cfg`. -/// -/// The CLI's read-side search (first existing of several locations) lives in -/// `freemkv-keysources::keydb_search_paths`; this function is the single -/// *write* default used by `save`/`update`, kept in lock-step with that crate's -/// `default_keydb_path` for the same OS. /// Build the error returned when no home directory can be determined /// (`HOME`/`USERPROFILE` unset). This is an *environment* failure — the /// process has no home dir, which typically signals a stripped container @@ -66,6 +55,23 @@ fn no_home_dir() -> Error { } } +/// Standard keydb storage path — the canonical location to write the keydb to. +/// +/// On Windows this is the idiomatic per-user roaming dir +/// `%APPDATA%\freemkv\keydb.cfg`, falling back to the legacy +/// `%USERPROFILE%\.config\freemkv\keydb.cfg` only if `APPDATA` is unset. On +/// Linux/macOS it stays the long-standing `$HOME/.config/freemkv/keydb.cfg`. +/// +/// The CLI's read-side search (first existing of several locations) lives in +/// `freemkv-keysources::keydb_search_paths`; this function is the single +/// *write* default used by `save`/`update`. On Windows the two agree. On +/// Linux they can diverge: this write path always uses +/// `$HOME/.config/freemkv/keydb.cfg` and ignores `XDG_CONFIG_HOME`, whereas +/// the read-side search additionally checks `$XDG_CONFIG_HOME/freemkv` first. +/// A user who sets `XDG_CONFIG_HOME` to a non-`$HOME/.config` location will +/// therefore have `update-keys` write to the `$HOME` path while the read-side +/// search may prefer the `XDG_CONFIG_HOME` location — the `$HOME` path is still +/// in the search list, so the freshly-written keydb is found, just not first. pub fn default_path() -> Result { #[cfg(windows)] { diff --git a/src/mux/disc.rs b/src/mux/disc.rs index db5f5bf..77a176c 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -108,6 +108,11 @@ pub struct DiscStream { /// inline `decrypt::decrypt_sectors` step. `DecryptKeys::None` /// (raw / unencrypted disc) makes the decorator a pass-through. reader: DecryptingSectorSource>, + /// Shared decrypt-loss counter, cloned once at construction from + /// `reader.decrypt_loss()`. `lost_bytes()` loads it directly so the + /// per-frame hot path performs no per-call `Arc::clone` (matching the + /// `PipelinedPesStream` pattern). + decrypt_loss: std::sync::Arc, title: DiscTitle, /// Mirror of the keys handed in at construction. The decorator /// owns the cryptographic state; this field is kept for @@ -253,12 +258,17 @@ impl DiscStream { _ => 1, }; + // Wrap the input reader in DecryptingSectorSource so the internal + // fill_extents path sees plaintext bytes. For DecryptKeys::None + // (unencrypted / raw / test fixtures) the decorator is a pass-through. + let reader = DecryptingSectorSource::new(reader, decrypt_keys.clone()); + // Clone the shared loss counter once here so `lost_bytes()` never + // clones an Arc per frame on the mux hot path. + let decrypt_loss = reader.decrypt_loss(); + Self { - // Wrap the input reader in DecryptingSectorSource so the - // internal fill_extents path sees plaintext bytes. For - // DecryptKeys::None (unencrypted / raw / test fixtures) - // the decorator is a pass-through. - reader: DecryptingSectorSource::new(reader, decrypt_keys.clone()), + reader, + decrypt_loss, title, decrypt_keys, unit_align, @@ -802,11 +812,8 @@ impl crate::pes::Stream for DiscStream { // them). Both are real missing content the abort gate must see; without // the decrypt term a partial key failure reports lost_bytes=0 and a rip // missing segments passes even under abort_on_lost_secs=0. - self.lost_bytes.saturating_add( - self.reader - .decrypt_loss() - .load(std::sync::atomic::Ordering::Relaxed), - ) + self.lost_bytes + .saturating_add(self.decrypt_loss.load(std::sync::atomic::Ordering::Relaxed)) } } diff --git a/src/mux/resolve.rs b/src/mux/resolve.rs index 86a3b1b..39a90b5 100644 --- a/src/mux/resolve.rs +++ b/src/mux/resolve.rs @@ -249,6 +249,17 @@ fn css_key_missing(raw: bool, has_css: bool, keys: &crate::decrypt::DecryptKeys) !raw && has_css && matches!(keys, crate::decrypt::DecryptKeys::None) } +/// Scrambled-but-uncracked CSS guard (Fix 6). Returns `true` when decryption +/// is requested (`!raw`) and the scan recorded a hard CSS error +/// (`has_css_error` — `disc.css_error.is_some()`), meaning the content is +/// scrambled but no title key was recovered (so `disc.css` is `None`). Muxing +/// that case would pass scrambled MPEG through as plaintext, so the caller +/// fails fast with [`Error::CssKeyMissing`]. `--raw` is exempt (skips +/// decryption), so it always returns `false`. +fn css_error_aborts(raw: bool, has_css_error: bool) -> bool { + !raw && has_css_error +} + /// Open a PES input stream (produces PES frames). pub fn input(url: &str, opts: &InputOptions) -> io::Result> { let parsed = parse_url(url); @@ -290,7 +301,7 @@ pub fn input(url: &str, opts: &InputOptions) -> io::Result( mod tests { use super::StreamUrl; use super::aacs_key_missing; + use super::css_error_aborts; use super::css_key_missing; use super::parse_url; use super::validate_network_addr; @@ -779,6 +791,18 @@ mod tests { assert!(!css_key_missing(false, false, &DecryptKeys::None)); } + #[test] + fn css_error_field_aborts_unless_raw() { + // Fix 6: a scrambled-but-uncracked DVD records a hard error in + // `disc.css_error` (css is None). With decryption requested the + // input() guard must abort with CssKeyMissing. + assert!(css_error_aborts(false, true)); + // --raw skips decryption → never aborts on the css_error field. + assert!(!css_error_aborts(true, true)); + // No recorded css_error → the guard does not fire. + assert!(!css_error_aborts(false, false)); + } + #[test] fn raw_never_aborts() { // --raw skips decryption — must never hit the no-key abort, even on an