Merge codeaudit-1.7.4: chapter-span + hevc-guard + winner-hint tests, doc-drift, hoist MIN_FEATURE_SECS

docs/aacs-trace.md

@@ -12,3 +12,33 @@ unlock phase, then the key-resolution phase).
   carried verbatim so an app renderer never has to match an enum back to a
   name it already has. Only the OUTCOME / path enums are structured states
   the app maps to i18n English.
+
+## `KeyStep` — a source's contribution to the key phase
+
+Each source that ran contributes one `KeyStep`: `who` (its `label()`), `path`
+(the `Vec<KeyNode>` it walked), and `outcome` (`KeyOutcome`). Two fields
+de-conflate a keyless result — WHY a matched disc produced no usable key:
+
+- `matched_entry: Option<MatchedEntry>` — the shape of the source's MATCHED
+  entry when it matched this disc; `None` for a true miss or a source kind
+  with no such shape. An app MAY log it verbatim and render a
+  matched-but-no-key verdict.
+- `store_entries: Option<usize>` — per-disc entries loaded in the source's
+  store, when known, so a true-miss verdict can name the store size (`… not in
+  keydb (N entries loaded)`). `None` for a source that carries no such count.
+
+## `MatchedEntry` — booleans-and-lengths, never material
+
+A shape summary of a matched entry — safe to print at any log level, no key
+MATERIAL: `has_vuk`, `has_unit_keys`, `unit_keys_len`, `has_media_key`,
+`has_keydb_vid`, `enc_title_keys_len`, `vid_available`. It answers "the disc
+WAS found — so why no key?" (e.g. a Media Key present but no VID on this path).
+
+## `KeyNode::NoDerivableKey`
+
+A derivation-path node distinct from `NoEntry`: the source MATCHED this disc
+but could not derive any usable key from the matched entry (no derivation
+material for the path taken). `NoEntry` = the disc was not found at all;
+`NoDerivableKey` = found, but the key was not — the node that turns a flat
+`no entry` verdict into `matched disc > … > NO KEY`. See docs/keysource.md
+(`resolve_unit_keys`) for how a source populates it.

docs/hevc.md

@@ -111,6 +111,24 @@ streaming decode self-healing. Re-sending an identical param set is benign
 This strictly supersets the earlier change-only re-assert, so the
 param-set-revert fix is unaffected.
 
+## length-prefix self-check (issue #52)
+
+`length_prefix_tiles` walks the assembled `frame_data` ([u32-BE len][body]
+records) and confirms they EXACTLY tile it: every declared length fits, none is
+zero, and the last body ends precisely at the buffer end with no trailing
+bytes. `frame_data` is length-prefixed by construction (`push_length_prefixed`
+/ `handle_param_set` write a 4-byte BE prefix — `LENGTH_PREFIX_SIZE`, which MUST
+equal the hvcC `lengthSizeMinusOne + 1`), so a `false` means a framing desync a
+downstream demuxer would report as "Invalid NAL unit size (N>M)".
+
+`HevcParser::parse` runs this as defense in depth: if the self-check fails it
+logs `src` at WARN and DROPS the access unit (`return Vec::new()`) rather than
+emit a mis-framed frame. Because no real input can desync a
+built-by-construction buffer, the drop branch is exercised end-to-end via a
+test-only seam (`FORCE_FRAMING_DESYNC`) that appends a stray byte just before
+the guard; see `parse_drops_desynced_access_unit`. Disconnecting the guard from
+`parse` fails that test.
+
 ## `parse_mastering_display`
 
 Parses a Mastering Display Colour Volume SEI payload (Rec. ITU-T H.265

docs/keysource.md

@@ -99,6 +99,32 @@ on `disc_hash` and reads no samples; the online source submits the ctx's
 content samples (a base batch for `get_unit_keys`, an index-1 anchor batch
 for `get_fmts_indexes`) to the key service.
 
+## `resolve_unit_keys` — de-conflating "no entry" from "matched, no key"
+
+`get_unit_keys` returns a bare `Vec<UnitKey>`: an empty result cannot say
+WHETHER the source had no entry for this disc at all, or matched the disc and
+still derived nothing (the classic "matched but needs a VID this path can't
+supply"). `resolve_unit_keys` carries that distinction. It returns a
+`UnitKeyResolution`:
+
+* `keys: Vec<UnitKey>` — the terminal Unit Keys produced (empty = none).
+* `matched: bool` — the source matched this disc in its store (by hash / VID)
+  even if it then derived no key; `false` is a genuine miss.
+* `miss_path: Vec<KeyNode>` — when `matched` and `keys` is empty, the nodes
+  walked AFTER the implicit `MatchedDisc` node (e.g. `[NoVid]` or
+  `[NoDerivableKey]`); empty means the caller supplies a bare
+  `NoDerivableKey`. Ignored unless `matched` and `keys` is empty.
+* `matched_entry: Option<MatchedEntry>` — a booleans-and-lengths shape of the
+  matched entry (no key material) for the application to log.
+* `store_entries: Option<usize>` — per-disc entries loaded in this source's
+  store, so a true-miss verdict can name the store size.
+
+The trait's default is coarse: forward to `get_unit_keys` and report
+`matched = false`. A source that keys on a per-disc identity (a keydb)
+overrides it to set `matched` / `miss_path` / `matched_entry`, so the trace can
+render `matched disc > no VID > NO KEY` instead of a flat `no entry`. See
+docs/aacs-trace.md for the trace-node shapes.
+
 ## `resolve_and_apply_traced` — one-shot semantics and CPS-unit numbering
 
 One-shot per source: each source's `KeySource::get_unit_keys` is called

src/disc/mod.rs

@@ -4573,6 +4573,60 @@ mod tests {
         }
     }
 
+    /// A BD title with `n_chapters` marks clustered in the first `span_secs` of a
+    /// `dur`-second runtime (first at 0, last at `span_secs`) — lets a test drive
+    /// the CHAPTER_SPAN_MIN_FRAC boundary independently of chapter count.
+    fn bd_title_chaptered_span(
+        playlist: &str,
+        id: u16,
+        dur: f64,
+        size: u64,
+        clip_ids: &[&str],
+        n_chapters: usize,
+        span_secs: f64,
+    ) -> DiscTitle {
+        let chapters = (0..n_chapters)
+            .map(|i| Chapter {
+                time_secs: if n_chapters <= 1 {
+                    0.0
+                } else {
+                    i as f64 * span_secs / (n_chapters - 1) as f64
+                },
+                name: (i + 1).to_string(),
+            })
+            .collect();
+        DiscTitle {
+            chapters,
+            ..bd_title(playlist, id, dur, size, clip_ids)
+        }
+    }
+
+    // Issue #45 boundary: marks that cluster in the first ~20% of runtime span
+    // < CHAPTER_SPAN_MIN_FRAC of duration, so they are NOT a complete feature.
+    #[test]
+    fn is_complete_feature_presentation_false_when_marks_cluster_early() {
+        // 5 marks over the first 20% (span 0.2 < 0.5 of duration).
+        let clustered =
+            bd_title_chaptered_span("00001.mpls", 1, 1000.0, 1_000_000, &["00001"], 5, 200.0);
+        assert!(
+            !is_complete_feature_presentation(&clustered),
+            "marks spanning < 0.5 of runtime are not a complete feature"
+        );
+    }
+
+    // Issue #45 boundary: marks spanning exactly CHAPTER_SPAN_MIN_FRAC of the
+    // runtime (with >= MIN_FEATURE_CHAPTERS marks) DO look like a real feature.
+    #[test]
+    fn is_complete_feature_presentation_true_at_span_threshold() {
+        // 2 marks (== MIN_FEATURE_CHAPTERS) spanning exactly 0.5 of duration.
+        let at_threshold =
+            bd_title_chaptered_span("00001.mpls", 1, 1000.0, 1_000_000, &["00001"], 2, 500.0);
+        assert!(
+            is_complete_feature_presentation(&at_threshold),
+            "marks spanning >= 0.5 of runtime with enough marks are a complete feature"
+        );
+    }
+
     /// Same, but with an empty STN stream list — models a title whose FIRST
     /// PlayItem is a non-video bumper (`has_video()` is false; `streams` reflect
     /// PlayItem 0 only).

src/labels/fox.rs

@@ -157,8 +157,8 @@ pub(crate) fn labels_from_dcx(text: &str) -> Vec<StreamLabel> {
 
 // A feature runs at least this long (`durs` is seconds — durs=7628 is a 2h7m
 // feature). A stated sub-minute `name="feature"` is a decoy and is never
-// selected; the guard is inert when no `durs` is present. Mirrors paramount.
+// selected; the guard is inert when no `durs` is present. Shared with paramount.
-const MIN_FEATURE_SECS: u64 = 60;
+use super::MIN_FEATURE_SECS;
 
 // The `durs` (seconds) of a `<playlist>` element, digits only, when stated.
 fn playlist_duration_secs(element: &str) -> Option<u64> {

src/labels/mod.rs

@@ -293,6 +293,11 @@ const MIN_ANCHOR_STREAMS: usize = 2;
 // so 0 is unused; only stream_id-bearing labels may carry it.
 const NO_STN_SLOT: u16 = 0;
 
+// A stated feature playlist runs at least this long (seconds); a sub-minute
+// stated duration is a decoy (Fox name=feature, Paramount _Start_Angle), never
+// the feature. Inert when no duration is stated. Shared by fox and paramount.
+pub(crate) const MIN_FEATURE_SECS: u64 = 60;
+
 // Two ISO 639-2 codes that do NOT contradict: equal (case/padding
 // insensitive), or either side is empty ("unknown", never a contradiction).
 fn languages_compatible(a: &str, b: &str) -> bool {
@@ -3472,4 +3477,57 @@ mod feature_hint_pass_tests {
         assert_eq!(hint.playlist_id, Some(800));
         assert_eq!(hint.filename.as_deref(), Some("00800.mpls"));
     }
+
+    /// The winner_hint early-return: when the winning parser already carried a
+    /// non-empty hint, resolve_feature_hint returns it verbatim and never runs
+    /// the Tier-2 menu-walk. The disc's bdj_feature::resolve would yield 00800,
+    /// but the supplied winner_hint (00042) must win — proving the short-circuit.
+    #[test]
+    fn resolve_feature_hint_returns_winner_hint_over_menu_walk() {
+        let xml = br#"<playlists>
+            <playlist name="Feature" id="00800" aud="eng,fra,spa" duration="7000" />
+        </playlists>"#;
+        let jar = build_jar(&[("00000/playlists.xml", xml.to_vec())]);
+        let jar_dir = DirSpec {
+            name: "JAR".to_string(),
+            icb_lba: 30,
+            dir_data_lba: 31,
+            files: vec![file_with("00000.jar", 32, 4000, jar, true)],
+            subdirs: vec![],
+        };
+        let bdmv = DirSpec {
+            name: "BDMV".to_string(),
+            icb_lba: 20,
+            dir_data_lba: 21,
+            files: Vec::new(),
+            subdirs: vec![jar_dir],
+        };
+        let root = DirSpec {
+            name: String::new(),
+            icb_lba: 10,
+            dir_data_lba: 11,
+            files: Vec::new(),
+            subdirs: vec![bdmv],
+        };
+        let mut disc = MemDisc::new();
+        build_udf_skeleton(&mut disc, 10);
+        lay_dir(&mut disc, &root);
+        let udf = crate::udf::read_filesystem(&mut disc).expect("fs");
+
+        // Sanity: the Tier-2 menu-walk on this disc resolves to 00800.
+        let tier2 = bdj_feature::resolve(&mut disc, &udf);
+        assert_eq!(tier2.and_then(|h| h.playlist_id), Some(800));
+
+        // A DIFFERENT non-empty winner_hint must short-circuit before Tier-2.
+        let winner = FeaturePlaylistHint {
+            playlist_id: Some(42),
+            filename: Some("00042.mpls".to_string()),
+        };
+        let got = resolve_feature_hint(&mut disc, &udf, Some(winner.clone()));
+        assert_eq!(
+            got,
+            Some(winner),
+            "a non-empty winner_hint wins over the Tier-2 menu-walk result"
+        );
+    }
 }

src/labels/paramount.rs

@@ -223,8 +223,8 @@ pub(crate) fn feature_hint_from_xml(text: &str) -> Option<super::FeaturePlaylist
 
 // A playlist must run at least this long (seconds) to be the feature — kills the
 // `_Start_Angle` 2-second decoy (Sony SM3 UHD id 00243). Applied only when a
-// duration is actually stated; absent, it stays inert.
+// duration is actually stated; absent, it stays inert. Shared with fox.
-const MIN_FEATURE_SECS: u64 = 60;
+use super::MIN_FEATURE_SECS;
 
 // A `<playlist>` element's stated running time in seconds, if any. Read from the
 // first present of a set of duration-like attributes (the corpus is not a spec;

src/mux/codec/hevc.rs

@@ -186,6 +186,14 @@ thread_local! {
     static PARAM_REASSERT_REALLOCS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
 }
 
+// Test-only: forces a framing desync just before the parse() self-check guard
+// so its drop branch is exercised end-to-end (frame_data is length-prefixed by
+// construction, so no real input desyncs it). See parse_drops_desynced_access_unit.
+#[cfg(test)]
+thread_local! {
+    static FORCE_FRAMING_DESYNC: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+}
+
 // 33-bit 90kHz PTS wraps at 2^33 ticks (~26.5h). A backward step of ~2^33 is a
 // wraparound (unwrap, add 2^33) not a clip reset (arbitrary sub-2^33 backward
 // step); accepting steps within `PTS_WRAP_PERIOD`/2 of a full period separates the two cases.
@@ -599,6 +607,13 @@ impl CodecParser for HevcParser {
             }
         }
 
+        // Test-only seam: append a stray byte so the guard below sees a desync,
+        // exercising its drop branch end-to-end (no real input can desync here).
+        #[cfg(test)]
+        if FORCE_FRAMING_DESYNC.with(|c| c.get()) {
+            frame_data.push(0xFF);
+        }
+
         // Defense in depth (issue #52): frame_data is length-prefixed by
         // construction, but a desynced buffer would surface as "Invalid NAL unit
         // size". Drop it (log src) rather than emit a mis-framed access unit.
@@ -2827,6 +2842,47 @@ mod tests {
         );
     }
 
+    // The build a normal Annex-B AU (VPS + SPS + PPS + IDR) as a helper for the
+    // end-to-end guard test below.
+    #[cfg(test)]
+    fn annex_b_access_unit() -> Vec<u8> {
+        let sps = make_sps_with_chroma(1, 2, 2);
+        let mut data = Vec::new();
+        data.extend_from_slice(&[0x00, 0x00, 0x01]);
+        data.extend_from_slice(&hevc_nal_header(32)); // VPS
+        data.extend_from_slice(&[0xAA, 0xBB]);
+        data.extend_from_slice(&[0x00, 0x00, 0x01]);
+        data.extend_from_slice(&sps); // SPS
+        data.extend_from_slice(&[0x00, 0x00, 0x01]);
+        data.extend_from_slice(&hevc_nal_header(34)); // PPS
+        data.extend_from_slice(&[0xDD, 0xEE]);
+        data.extend_from_slice(&[0x00, 0x00, 0x01]);
+        data.extend_from_slice(&hevc_nal_header(19)); // IDR_W_RADL slice → keyframe
+        data.extend_from_slice(&[0x10, 0x20, 0x30]);
+        data
+    }
+
+    // Issue #52: the guard's WIRING in parse() — a desynced access unit is
+    // dropped (empty emit), a well-formed one is emitted. Disconnecting the guard
+    // from parse() would let the desynced case through and fail this test.
+    #[test]
+    fn parse_drops_desynced_access_unit() {
+        // Well-formed: emitted.
+        let mut parser = HevcParser::new();
+        let frames = parser.parse(&make_pes(annex_b_access_unit(), Some(0)));
+        assert_eq!(frames.len(), 1, "a well-formed access unit is emitted");
+
+        // Same AU, framing forced out of sync: the guard drops it.
+        FORCE_FRAMING_DESYNC.with(|c| c.set(true));
+        let mut parser2 = HevcParser::new();
+        let dropped = parser2.parse(&make_pes(annex_b_access_unit(), Some(0)));
+        FORCE_FRAMING_DESYNC.with(|c| c.set(false));
+        assert!(
+            dropped.is_empty(),
+            "a desynced access unit is dropped by parse(), not emitted"
+        );
+    }
+
     #[test]
     fn hvcc_handles_emulation_prevention_in_sps() {
         // Insert emulation-prevention bytes (00 00 03) after the parsed fields