audit: module-by-module code audit fixes and test coverage

Lands the code-audit branches for aacs, css, keys, labels, udf, ifo and dvd, bdnav/mpls/clpi, disc/bluray, extract, io, scsi, the mkv, mp4, network and mpg muxers, the audio and video codecs, and CI. Fixes include MKB and FMTS validation, UDF bounds, IFO fallback to the BUP, DTS Express framing, MPEG-2 field pairing and mpg re-basing; the rest pins existing behaviour with tests.
2026-10-07 18:13:10 +00:00 · 2026-09-29 20:45:05 -07:00
parent 1d78420366
commit 4031513574
196 changed files with 14014 additions and 4397 deletions
@@ -239,32 +239,6 @@ fn aacs_decrypt_unit_key_roundtrip() {
    );
 }

-/// Test: aacs_vuk_derivation
-///
-/// Verify derive_vuk: VUK = AES-ECB-DECRYPT(media_key, volume_id) XOR volume_id
-#[test]
-fn aacs_vuk_derivation_roundtrip() {
-    let media_key = [
-        0x25u8, 0x2F, 0xB6, 0x36, 0xE8, 0x83, 0x52, 0x9E, 0x11, 0x9A, 0xB7, 0x15, 0xF4, 0xEB, 0x16,
-        0x40,
-    ];
-    let volume_id = [
-        0xA1u8, 0x3C, 0xBE, 0x2C, 0xE4, 0x05, 0x65, 0xD1, 0x04, 0xB5, 0x3E, 0x76, 0x8C, 0x70, 0x0E,
-        0x30,
-    ];
-
-    let vuk = aacs::derive::derive_vuk(&media_key, &volume_id);
-
-    // VUK should be non-zero and different from both inputs
-    assert_ne!(vuk, [0u8; 16], "VUK should not be all zeros");
-    assert_ne!(vuk, media_key, "VUK should differ from media_key");
-    assert_ne!(vuk, volume_id, "VUK should differ from volume_id");
-
-    // Verify determinism
-    let vuk2 = aacs::derive::derive_vuk(&media_key, &volume_id);
-    assert_eq!(vuk, vuk2, "derive_vuk not deterministic");
-}
-
 /// Test: `is_clean` distinguishes clean vs scrambled units via the TS proof floor.
 #[test]
 fn aacs_is_clean_detection() {
@@ -449,7 +423,8 @@ fn aacs_cross_validation_alternate_key() {
        plaintext[off] = 0x47;
        off += 192;
    }
-    // No flag set: the CBC-encrypted body scrambles the packet syncs.
+    // Byte 0 stays 0xFF (CPI bits set); decrypt_unit ignores the flag. The CBC-encrypted
+    // body scrambles the packet syncs.
    let expected = plaintext.clone();

    let mut header = [0u8; 16];
@@ -553,164 +528,6 @@ fn css_roundtrip_multiple_keys() {

 // ── CSS keyless recovery tests ─────────────────────────────────────────────

-// Attempt keyless recovery on synthetically scrambled sectors.
-#[test]
-fn css_keyless_recovery_validates_cracked_key() {
-    let candidates: &[([u8; 5], [u8; 5])] = &[
-        (
-            [0x42, 0x13, 0x37, 0xBE, 0xEF],
-            [0x11, 0x22, 0x33, 0x44, 0x55],
-        ),
-        (
-            [0x01, 0x02, 0x03, 0x04, 0x05],
-            [0xAA, 0xBB, 0xCC, 0xDD, 0xEE],
-        ),
-        (
-            [0x10, 0x20, 0x30, 0x40, 0x50],
-            [0x05, 0x06, 0x07, 0x08, 0x09],
-        ),
-        (
-            [0xAB, 0xCD, 0xEF, 0x01, 0x23],
-            [0x12, 0x34, 0x56, 0x78, 0x9A],
-        ),
-        (
-            [0x55, 0xAA, 0x55, 0xAA, 0x55],
-            [0x00, 0x00, 0x00, 0x00, 0x00],
-        ),
-    ];
-
-    let mut any_cracked = false;
-
-    for (key, seed) in candidates {
-        let mut sector = vec![0x00u8; 2048];
-        sector[0x14] = 0x30;
-        sector[0x54..0x59].copy_from_slice(seed);
-        sector[0x80] = 0x00;
-        sector[0x81] = 0x00;
-        sector[0x82] = 0x01;
-        sector[0x83] = 0xE0;
-        sector[0x84] = 0x00;
-        sector[0x85] = 0x00;
-        sector[0x86] = 0x80;
-        sector[0x87] = 0x80;
-        sector[0x88] = 0x05;
-        sector[0x89] = 0x21;
-
-        let original = sector.clone();
-
-        // "Encrypt" by descrambling plaintext (XOR keystream)
-        css::lfsr::descramble_sector(key, &mut sector);
-        sector[0x14] = 0x30;
-
-        let cracked = css::keyless::crack_title_key(&sector);
-
-        if let Some(cracked_key) = cracked {
-            let mut test = sector.clone();
-            css::lfsr::descramble_sector(&cracked_key, &mut test);
-
-            assert_eq!(test[0x80], 0x00, "PES byte 0 mismatch");
-            assert_eq!(test[0x81], 0x00, "PES byte 1 mismatch");
-            assert_eq!(test[0x82], 0x01, "PES byte 2 mismatch");
-            assert_eq!(test[0x83], 0xE0, "PES byte 3 mismatch");
-            assert_eq!(
-                &test[0x80..2048],
-                &original[0x80..2048],
-                "cracked key did not recover original plaintext"
-            );
-
-            any_cracked = true;
-            eprintln!(
-                "keyless recovery succeeded: key={:02X?} seed={:02X?} cracked={:02X?}",
-                key, seed, cracked_key
-            );
-        }
-    }
-
-    if !any_cracked {
-        eprintln!(
-            "keyless recovery did not converge on any synthetic key/seed pair. \
-             This is expected: synthetic sectors lack the TAB1 output encoding \
-             present in real CSS-encrypted DVD sectors."
-        );
-        // Don't pass vacuously: if the attack can't converge on synthetic data,
-        // still assert descramble_sector is deterministic and non-trivial
-        // (transforms the payload) so a real regression is still caught.
-        for (key, seed) in candidates {
-            let mut base = vec![0x00u8; 2048];
-            base[0x14] = 0x30;
-            base[0x54..0x59].copy_from_slice(seed);
-            base[0x80..0x8A]
-                .copy_from_slice(&[0x00, 0x00, 0x01, 0xE0, 0x00, 0x00, 0x80, 0x80, 0x05, 0x21]);
-            let mut a = base.clone();
-            let mut b = base.clone();
-            css::lfsr::descramble_sector(key, &mut a);
-            css::lfsr::descramble_sector(key, &mut b);
-            assert_eq!(a, b, "descramble must be deterministic for key={key:02X?}");
-            assert_ne!(
-                &a[0x80..2048],
-                &base[0x80..2048],
-                "descramble must transform the payload for key={key:02X?}"
-            );
-        }
-    }
-}
-
-/// Verify that `recover_title_key` works when given exact known plaintext,
-/// even for combinations where `crack_title_key` (which guesses the pattern)
-/// might not converge.
-#[test]
-fn css_recover_title_key_with_exact_plaintext() {
-    let title_key: [u8; 5] = [0x42, 0x13, 0x37, 0xBE, 0xEF];
-    let seed: [u8; 5] = [0x11, 0x22, 0x33, 0x44, 0x55];
-
-    let mut sector = vec![0x00u8; 2048];
-    sector[0x14] = 0x30;
-    sector[0x54..0x59].copy_from_slice(&seed);
-    let pes_header: [u8; 10] = [0x00, 0x00, 0x01, 0xE0, 0x00, 0x00, 0x80, 0x80, 0x05, 0x21];
-    sector[0x80..0x8A].copy_from_slice(&pes_header);
-    #[allow(clippy::needless_range_loop)]
-    for i in 0x8A..2048 {
-        sector[i] = ((i * 13 + 7) & 0xFF) as u8;
-    }
-    let original = sector.clone();
-
-    // Scramble
-    css::lfsr::descramble_sector(&title_key, &mut sector);
-    sector[0x14] = 0x30;
-
-    // Recover with exact known plaintext
-    let recovered = css::keyless::recover_title_key(&sector, &pes_header);
-
-    if let Some(rkey) = recovered {
-        let mut test = sector.clone();
-        css::lfsr::descramble_sector(&rkey, &mut test);
-        assert_eq!(
-            &test[0x80..2048],
-            &original[0x80..2048],
-            "recovered key did not produce correct plaintext"
-        );
-        eprintln!("recover_title_key succeeded: {:02X?}", rkey);
-    } else {
-        eprintln!(
-            "recover_title_key returned None for key={:02X?} seed={:02X?}. \
-             The LFSR0 recovery phase may not converge for this combination.",
-            title_key, seed
-        );
-        // Don't pass vacuously: if recovery didn't converge, still assert
-        // descramble_sector is deterministic and non-trivial (transforms payload).
-        let mut a = original.clone();
-        let mut b = original.clone();
-        css::lfsr::descramble_sector(&title_key, &mut a);
-        css::lfsr::descramble_sector(&title_key, &mut b);
-        assert_eq!(a, b, "descramble must be deterministic");
-        assert_ne!(
-            &a[0x80..2048],
-            &original[0x80..2048],
-            "descramble must transform the payload"
-        );
-    }
-}
-
 /// Test: aacs_parse_unit_key_ro with minimal valid data
 #[test]
 fn aacs_parse_unit_key_ro_minimal() {
@@ -50,8 +50,8 @@ fn scan_image_empty_reader() {
    let opts = ScanOptions::default();
    let result = Disc::scan_image(&mut reader, 0, &opts);
    assert!(
-        result.is_err(),
-        "scan_image should fail with empty reader (no AVDP)"
+        matches!(result, Err(libfreemkv::error::Error::UdfNotFilesystem)),
+        "empty reader (no AVDP) must surface the not-UDF verdict"
    );
 }

@@ -15,9 +15,9 @@ use std::path::{Path, PathBuf};

 /// §2.2, the libfreemkv row of the structural guards.
 const BANNED: &[&str] = &[
-    ".get_unit_keys(",
-    ".get_fmts_indexes(",
-    ".resolve_unit_keys(",
+    "get_unit_keys(",
+    "get_fmts_indexes(",
+    "resolve_unit_keys(",
    "decrypt_with(",
    "AacsKeyMap::from_ranges",
    "with_key_map(",
@@ -154,6 +154,17 @@ fn hits(rel: &str, src: &str) -> Vec<(usize, &'static str)> {
            if code[k..k + t.len()] != t[..] || inside(k) {
                continue;
            }
+            // The three trait ops are banned as calls (method or UFCS), not as `fn` items.
+            let method = matches!(
+                tok,
+                "get_unit_keys(" | "get_fmts_indexes(" | "resolve_unit_keys("
+            );
+            let before = code[..k].last().copied().unwrap_or(' ');
+            let is_item =
+                code[..k].ends_with(&['f', 'n', ' ']) || before.is_alphanumeric() || before == '_';
+            if method && is_item {
+                continue;
+            }
            let via_helper = k >= helper.len() && code[k - helper.len()..k] == helper[..];
            if tok == "decrypt_unit(" && via_helper {
                continue;
@@ -223,6 +234,10 @@ fn the_structural_guard_skips_comments_and_keeps_its_allow_path() {
    );
    let url = "let u = \"http://x\"; AacsKeyMap::from_ranges_phased(r);\n";
    assert_eq!(hits("src/x.rs", url), [(1, "AacsKeyMap::from_ranges")]);
+    let ufcs = "fn f() { KeySource::get_unit_keys(&s, c); }\n";
+    assert_eq!(hits("src/mux/x.rs", ufcs), [(1, "get_unit_keys(")], "UFCS");
+    let item = "impl K for S { fn get_unit_keys(&self) {} fn my_resolve_unit_keys(&self) {} }\n";
+    assert!(hits("src/x.rs", item).is_empty(), "fn items are not calls");
    let raw = "let s = r#\"x.get_fmts_indexes( // \"#; s.resolve_unit_keys(c);\n";
    assert_eq!(
        hits("src/x.rs", raw).len(),
@@ -443,6 +443,29 @@ fn no_raw_execute_in_drive_or_identity() {
    check("raw transport execute outside Drive::exec", hits, ALLOW);
 }

+/// The lowercased receiver token before the atomic op at `i`. Trailing whitespace is
+/// trimmed first, so a rustfmt-wrapped `self.cancel\n    .load(..)` still names `cancel`.
+fn atomic_receiver(code: &str, i: usize) -> String {
+    code[..i]
+        .trim_end()
+        .rsplit(|c: char| c.is_whitespace() || "(&*!,{;=".contains(c))
+        .next()
+        .unwrap_or("")
+        .to_ascii_lowercase()
+}
+
+#[test]
+fn atomic_receiver_sees_through_wrapped_chains() {
+    for code in [
+        "if self.cancel.load(x)",
+        "if self.cancel\n        .load(x)",
+        "if self.Cancel  \n\t.load(x)",
+    ] {
+        let recv = atomic_receiver(code, code.find(".load(").unwrap());
+        assert_eq!(recv, "self.cancel", "{code:?}");
+    }
+}
+
 /// §5.8 "A raw-halt-load grep test. Its allow-list: the `from_arc` bridges and the patch
 /// latch." A `Halt`'s flag is read via `check` / `is_cancelled`, never as a raw atomic.
 /// The patch latch is the engine's (§4.2 `EngineHalt`); libfreemkv has none.
@@ -489,11 +512,7 @@ fn no_raw_halt_loads_in_production() {
        }
        for pat in ATOMIC_OPS {
            for i in s.hits(pat) {
-                let recv = s.code[..i]
-                    .rsplit(|c: char| c.is_whitespace() || "(&*!,{;=".contains(c))
-                    .next()
-                    .unwrap_or("")
-                    .to_ascii_lowercase();
+                let recv = atomic_receiver(&s.code, i);
                if recv.contains("halt") || recv.contains("cancel") {
                    hits.push((s, i));
                }
@@ -535,13 +554,36 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
        r#"cfg_attr(not(target_os="linux"),allow(unused_variables))"#,
        1,
    ),
-    ("src/io/sink/mod.rs", r#"allow(dead_code)"#, 1),
    (
        "src/keys/arrival.rs",
        r#"allow(clippy::too_many_arguments)"#,
        1,
    ),
-    ("src/labels/class_reader.rs", r#"allow(dead_code)"#, 2),
+    (
+        "src/labels/class_reader.rs",
+        r#"expect(dead_code,reason="everyJVMS4.4tagisdecoded;labelparsersreadonlysomepayloads")"#,
+        1,
+    ),
+    (
+        "src/labels/class_reader.rs",
+        r#"expect(dead_code,reason="parsedinfullperJVMS4.1;labelparsersreadthepoolandmethods")"#,
+        1,
+    ),
+    (
+        "src/labels/class_reader.rs",
+        r#"expect(dead_code,reason="parsedinfullperJVMS4.5/4.6")"#,
+        1,
+    ),
+    (
+        "src/labels/class_reader.rs",
+        r#"expect(dead_code,reason="parsedperJVMS4.7.3;thedecodertracksonlythestack")"#,
+        1,
+    ),
+    (
+        "src/labels/class_reader.rs",
+        r#"expect(dead_code,reason="payloadsarefaultdetailforDebug;callersmatchthevariant")"#,
+        1,
+    ),
    (
        "src/labels/ctrm.rs",
        r#"allow(clippy::items_after_test_module)"#,
@@ -564,11 +606,9 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
    (
        "src/platform/fs_type/linux.rs",
        r#"allow(clippy::unnecessary_cast)"#,
-        3,
+        2,
    ),
-    ("src/platform/fs_type/mod.rs", r#"allow(dead_code)"#, 1),
    ("src/scsi/linux.rs", r#"allow(non_camel_case_types)"#, 1),
-    ("src/scsi/mod.rs", r#"allow(dead_code)"#, 1),
    (
        "src/scsi/mod.rs",
        r#"cfg_attr(not(any(feature="rip",target_os="windows")),allow(dead_code))"#,
@@ -592,7 +632,7 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
    (
        "src/scsi/mod.rs",
        r#"cfg_attr(not(target_os="windows"),allow(dead_code))"#,
-        2,
+        3,
    ),
    (
        "src/scsi/mod.rs",
@@ -600,7 +640,6 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
        1,
    ),
    ("src/scsi/windows.rs", r#"allow(non_snake_case)"#, 3),
-    ("src/udf.rs", r#"allow(clippy::only_used_in_recursion)"#, 1),
    ("src/udf.rs", r#"allow(clippy::too_many_arguments)"#, 1),
 ];

@@ -275,8 +275,13 @@ fn rescan_returns_stable_disc_identity() {
            .map(|a| (a.disc_hash.clone(), a.mkb_version, a.volume_id))
    };

+    assert_eq!(
+        first_snapshot.is_some(),
+        second_snapshot.is_some(),
+        "AACS state appeared or vanished across rescans"
+    );
    let (Some(a), Some(b)) = (first_snapshot, second_snapshot) else {
-        return;
+        return; // clear/DVD disc: no AACS state to compare
    };
    assert_eq!(a.0, b.0, "disc_hash drifted across rescans");
    assert_eq!(a.1, b.1, "mkb_version drifted across rescans");
@@ -1,7 +1,7 @@
 //! macOS platform-integration tests that do NOT require an optical drive.
 //!
-//! These exercise the Mac-only code paths under `platform/fs_type/macos.rs`,
-//! `io/platform_macos.rs`, and `io/writeback_file` end-to-end against the real
+//! These exercise the Mac-only code paths under `io/platform_macos.rs`
+//! and `io/writeback_file` end-to-end against the real
 //! kernel. They run under `cargo test` without `--ignored`, so CI on macOS
 //! (and the local `cargo test` a developer would run before pushing) covers
 //! them automatically.
@@ -76,6 +76,13 @@ fn writeback_file_create_with_size_hint_preallocates() {
        4096,
        "reported length should equal bytes written"
    );
+    // F_PREALLOCATE reserves the hint's extents (st_blocks is in 512-byte units); the
+    // call is best-effort in production, so only this check observes it.
+    let allocated = std::os::unix::fs::MetadataExt::blocks(&meta) * 512;
+    assert!(
+        allocated >= hint,
+        "hint not preallocated: {allocated} bytes"
+    );
 }

 #[test]
@@ -1,8 +1,7 @@
-//! Regression tests for Pass N (patch) fix — decrypt key inversion bug.
+//! Decrypt-seam checks behind the Pass N (patch) key inversion fix.
 //!
-//! Tests that decrypt_sectors is invoked correctly when opts.decrypt=true.
-//! The 2026-05-03 bug at `libfreemkv/src/disc/mod.rs:1938-1942` inverted
-//! the decrypt key arms, causing patch to pass DecryptKeys::None on encrypted discs.
+//! The patch pass itself now lives in freemkv-engine, so this file cannot guard the arm
+//! selection; it pins the libfreemkv decrypt seam that pass relies on.

 use libfreemkv::{aacs, decrypt::DecryptKeys};

@@ -188,8 +188,11 @@ fn scan_iso_propagates_scan_error() {
        .suffix(".iso")
        .tempfile()
        .expect("tempfile create");
-    tmp.write_all(&vec![0u8; 8 * SECTOR_SIZE]).expect("write");
+    tmp.write_all(&vec![0u8; 300 * SECTOR_SIZE]).expect("write");
    tmp.flush().expect("flush");
    let result = libfreemkv::scan_iso(tmp.path(), ScanOptions::default());
-    assert!(result.is_err(), "non-UDF image must error");
+    assert!(
+        matches!(result, Err(libfreemkv::error::Error::UdfNotFilesystem)),
+        "non-UDF image must surface the not-UDF verdict"
+    );
 }
@@ -1,6 +1,8 @@
 //! Integration tests for the SCSI error-decoding contract.
 //!
-//! Pins the contract every backend must satisfy via a mock `ScsiTransport`:
+//! Pins how the error type classifies pre-built outcomes handed back by a mock
+//! `ScsiTransport`; no backend or sense parser runs here (see `scsi::parse_sense` tests).
+//! The contract a backend must satisfy:
 //! healthy result → `Ok(ScsiResult { .. })`; transport-level failure →
 //! `Error::ScsiError { status: SCSI_STATUS_TRANSPORT_FAILURE, sense: None }`
 //! ([`Error::is_scsi_transport_failure`] true); SCSI-level failure (CHECK
@@ -282,30 +284,6 @@ fn test_asc_ascq_round_trip_through_error() {
    assert_eq!(sense.ascq, 0x05);
 }

-// ── 12. Healthy short transfer (resid > 0) ────────────────────────────────
-
-#[test]
-fn test_healthy_short_transfer_reports_partial_bytes() {
-    // Transport reported back fewer bytes than requested (resid = 16).
-    // For a 96-byte buffer that means bytes_transferred = 80.
-    let payload = vec![0u8; 96];
-    let mut transport = MockTransport::new(vec![MockOutcome::Ok {
-        data: payload,
-        resid: 16,
-    }]);
-
-    let cdb = [libfreemkv::scsi::SCSI_INQUIRY, 0, 0, 0, 0x60, 0];
-    let mut buf = [0u8; 96];
-    let r = transport
-        .execute(&cdb, DataDirection::FromDevice, &mut buf, 1_000)
-        .expect("ok");
-    assert_eq!(r.status, 0);
-    assert_eq!(
-        r.bytes_transferred, 80,
-        "bytes_transferred must equal data.len() - resid"
-    );
-}
-
 // ── 13. Error::Display does not leak English in the SCSI variant ──────────
 // Library rule: no English in error display, only "E{code}" + structured data.
 // Guards the SCSI variant since it's the most-emitted error in the rip path.
@@ -15,7 +15,8 @@ fn test_drive_read_per_cdb_timeout_bounds_call() {
    let path = Path::new(&device);

    let mut drive = libfreemkv::Drive::open(path).expect("open drive");
-    let timeout_ms: u32 = 5_000;
+    // Mirrors the crate-private scsi::READ_TIMEOUT_MS used by Drive::read.
+    let timeout_ms: u32 = 10_000;

    let start = std::time::Instant::now();
    let _ = drive.read(0, 1, &mut [0u8; 2048], false);
@@ -126,11 +126,7 @@ fn open_input_bare_path_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("not a valid stream URL") || msg.contains("E9002"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9002"), "got: {}", msg);
 }

 #[test]
@@ -142,11 +138,7 @@ fn open_output_bare_path_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("not a valid stream URL") || msg.contains("E9002"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9002"), "got: {}", msg);
 }

 #[test]
@@ -157,11 +149,7 @@ fn open_input_m2ts_empty_path_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("requires a file path") || msg.contains("E9003"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9003"), "got: {}", msg);
 }

 #[test]
@@ -172,11 +160,7 @@ fn open_output_null_input_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("write-only") || msg.contains("E9001"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9001"), "got: {}", msg);
 }

 #[test]
@@ -188,11 +172,7 @@ fn open_output_disc_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("read-only") || msg.contains("E9000"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9000"), "got: {}", msg);
 }

 #[test]
@@ -203,11 +183,7 @@ fn open_input_network_no_port_errors() {
        Err(e) => e.to_string(),
        Ok(_) => panic!("expected error"),
    };
-    assert!(
-        msg.contains("PES pipeline") || msg.contains("missing port") || msg.contains("E9004"),
-        "got: {}",
-        msg
-    );
+    assert!(msg.contains("E9004"), "got: {}", msg);
 }

 #[test]
@@ -379,6 +355,20 @@ fn disc_title_empty() {
    assert!(dt.playlist.is_empty());
 }

+// Every named codec must round-trip through its serialized id (FMKV meta headers).
+#[test]
+fn codec_id_roundtrips_for_every_named_codec() {
+    use Codec::*;
+    let all = [
+        Hevc, H264, Vc1, Mpeg2, Mpeg1, Av1, TrueHd, DtsHdMa, DtsHdHr, Dts, Ac3, Ac3Plus, Lpcm, Aac,
+        Mp2, Mp3, Flac, Opus, Pgs, DvdSub, Srt, Ssa,
+    ];
+    for c in all {
+        assert_ne!(c.id(), "unknown", "{c:?} missing from the codec table");
+        assert_eq!(c.id().parse::<Codec>(), Ok(c), "{c:?} id must parse back");
+    }
+}
+
 // ── Meta codec roundtrip ─────────────────────────────────────

 #[test]
@@ -1,5 +1,6 @@
 //! UDF parser tests using a MockSectorReader.

+use libfreemkv::error::Error;
 use libfreemkv::error::Result;
 use libfreemkv::{SectorSource, read_filesystem};
 use std::collections::HashMap;
@@ -208,59 +209,15 @@ fn make_file_icb(data_lba: u32, data_len: u32, file_size: u64) -> Vec<u8> {

 // ── Tests ──────────────────────────────────────────────────────────────────

-#[test]
-fn mock_sector_reader_roundtrip() {
-    let mut reader = MockSectorReader::new();
-
-    // Write a recognizable pattern to sector 100
-    let mut data = vec![0u8; SECTOR_SIZE];
-    data[0] = 0xAB;
-    data[1] = 0xCD;
-    data[2047] = 0xFF;
-    reader.set_sector(100, data.clone());
-
-    // Read it back
-    let mut buf = vec![0u8; SECTOR_SIZE];
-    let n = reader.read_sectors(100, 1, &mut buf, true).unwrap();
-    assert_eq!(n, SECTOR_SIZE);
-    assert_eq!(buf[0], 0xAB);
-    assert_eq!(buf[1], 0xCD);
-    assert_eq!(buf[2047], 0xFF);
-
-    // Reading an unmapped sector returns zeros
-    let mut buf2 = vec![0xFFu8; SECTOR_SIZE];
-    let n2 = reader.read_sectors(999, 1, &mut buf2, true).unwrap();
-    assert_eq!(n2, SECTOR_SIZE);
-    assert_eq!(buf2[0], 0);
-    assert_eq!(buf2[2047], 0);
-}
-
-#[test]
-fn mock_sector_reader_multi_sector() {
-    let mut reader = MockSectorReader::new();
-
-    let mut s10 = vec![0u8; SECTOR_SIZE];
-    s10[0] = 10;
-    reader.set_sector(10, s10);
-
-    let mut s11 = vec![0u8; SECTOR_SIZE];
-    s11[0] = 11;
-    reader.set_sector(11, s11);
-
-    // Read 2 consecutive sectors
-    let mut buf = vec![0u8; SECTOR_SIZE * 2];
-    let n = reader.read_sectors(10, 2, &mut buf, true).unwrap();
-    assert_eq!(n, SECTOR_SIZE * 2);
-    assert_eq!(buf[0], 10);
-    assert_eq!(buf[SECTOR_SIZE], 11);
-}
-
 #[test]
 fn read_filesystem_no_avdp() {
    // Empty reader — sector 256 is all zeros, tag_id=0 != 2
    let mut reader = MockSectorReader::new();
    let result = read_filesystem(&mut reader);
-    assert!(result.is_err(), "should fail when no AVDP at sector 256");
+    assert!(
+        matches!(result, Err(Error::UdfNotFilesystem)),
+        "no AVDP at sector 256 is the deterministic not-UDF verdict"
+    );
 }

 #[test]
@@ -272,7 +229,10 @@ fn read_filesystem_bad_avdp_tag() {
    reader.set_sector(256, bad);

    let result = read_filesystem(&mut reader);
-    assert!(result.is_err(), "should fail when AVDP tag_id is not 2");
+    assert!(
+        matches!(result, Err(Error::UdfNotFilesystem)),
+        "AVDP tag_id != 2 is the not-UDF verdict"
+    );
 }

 #[test]
@@ -285,8 +245,8 @@ fn read_filesystem_no_partition_descriptor() {

    let result = read_filesystem(&mut reader);
    assert!(
-        result.is_err(),
-        "should fail when no partition descriptor in VDS"
+        matches!(result, Err(Error::UdfNotFilesystem)),
+        "no partition descriptor is the not-UDF verdict"
    );
 }

@@ -305,7 +265,10 @@ fn read_filesystem_bad_fsd_tag() {
    // FSD should be at sector partition_start but we leave it as zeros (tag_id=0 != 256).

    let result = read_filesystem(&mut reader);
-    assert!(result.is_err(), "should fail when FSD tag_id is not 256");
+    assert!(
+        matches!(result, Err(Error::UdfNotFilesystem)),
+        "FSD tag_id != 256 is the not-UDF verdict"
+    );
 }

 #[test]
@@ -455,16 +418,3 @@ fn find_dir_case_insensitive() {
    assert!(fs.find_dir("BDMV/STREAM").is_none());
    assert!(fs.find_dir("NONEXISTENT").is_none());
 }
-
-#[test]
-fn sector_reader_is_object_safe() {
-    // Verify SectorSource can be used as a trait object
-    let mut reader = MockSectorReader::new();
-    reader.set_sector(0, vec![42u8; SECTOR_SIZE]);
-
-    let dyn_reader: &mut dyn SectorSource = &mut reader;
-    let mut buf = vec![0u8; SECTOR_SIZE];
-    let n = dyn_reader.read_sectors(0, 1, &mut buf, true).unwrap();
-    assert_eq!(n, SECTOR_SIZE);
-    assert_eq!(buf[0], 42);
-}