From 5253e4e5b728405eecdffc8f1a115a39bf65026a Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Sun, 10 May 2026 16:01:20 -0700 Subject: [PATCH] labels: per-parser confidence + highest-confidence-wins registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces 'first-match-wins by array order' with 'highest-confidence- wins, array order tiebreaker'. Removes the arbitrariness when more than one parser can claim a disc (e.g. one with both bluray_project.bin and playlists.xml). New types in labels::mod: pub enum Confidence { Medium, High } pub struct ParseResult { labels: Vec, confidence } ParseResult::high(labels) / ::medium(labels) constructors Parser signature change: every parse() now returns Option instead of Option>. Updated all six parsers in lockstep: paramount: High (fully structured XML) criterion: High (fully structured XML) pixelogic: High by default, Medium when an unknown token component is encountered (the skip-unknown path now propagates the coverage gap to the caller instead of silently degrading) ctrm: High (structured key-value) dbp: High (anchor scan with vocab routing) deluxe: still returns None pending Phase D — signature aligned Registry behavior: extract() iterates all detect-positive parsers, picks highest Confidence with non-empty labels. Equal confidence falls to array order (deterministic). Same selection logic in analyze(). LabelAnalysis grew a confidence: Option field so the diagnostic surface (freemkv-tools labels-analyze) exposes which confidence tier the selected parser claimed. labels-analyze JSON and labels-corpus-check structural diff both gained the field. Precommit (cargo +1.86 fmt + clippy + test) green. --- src/labels/criterion.rs | 7 +- src/labels/ctrm.rs | 20 +++-- src/labels/dbp.rs | 8 +- src/labels/deluxe.rs | 4 +- src/labels/mod.rs | 178 +++++++++++++++++++++++++++++----------- src/labels/paramount.rs | 8 +- src/labels/pixelogic.rs | 50 +++++++---- 7 files changed, 195 insertions(+), 80 deletions(-) diff --git a/src/labels/criterion.rs b/src/labels/criterion.rs index b4a5a29..b1d52f1 100644 --- a/src/labels/criterion.rs +++ b/src/labels/criterion.rs @@ -3,7 +3,7 @@ //! Clean structured XML with Content/Qualifier per stream and //! stream number mapping via playbackconfig. -use super::{LabelPurpose, LabelQualifier, StreamLabel, StreamLabelType}; +use super::{LabelPurpose, LabelQualifier, ParseResult, StreamLabel, StreamLabelType}; use crate::sector::SectorReader; use crate::udf::UdfFs; use std::collections::HashMap; @@ -12,7 +12,7 @@ pub fn detect(udf: &UdfFs) -> bool { super::jar_file_exists(udf, "streamproperties.xml") } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { let sp_data = super::read_jar_file(reader, udf, "streamproperties.xml")?; let sp_text = std::str::from_utf8(&sp_data).ok()?; @@ -66,7 +66,8 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option bool { || super::jar_file_exists(udf, "language_streams.txt") } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { // Try language_streams.txt first (richer structured data) let ls_labels = parse_language_streams(reader, udf); @@ -22,12 +22,18 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option Some(merge(ls, mb)), - (Some(ls), None) => Some(ls), - (None, Some(mb)) => Some(mb), - (None, None) => None, + let labels = match (ls_labels, mb_labels) { + (Some(ls), Some(mb)) => merge(ls, mb), + (Some(ls), None) => ls, + (None, Some(mb)) => mb, + (None, None) => return None, + }; + if labels.is_empty() { + return None; } + // High confidence: both language_streams.txt and menu_base.prop + // are structured key-value formats with documented types. + Some(ParseResult::high(labels)) } fn merge(ls: Vec, mb: Vec) -> Vec { diff --git a/src/labels/dbp.rs b/src/labels/dbp.rs index 2bbef9d..ce20e62 100644 --- a/src/labels/dbp.rs +++ b/src/labels/dbp.rs @@ -34,7 +34,7 @@ //! Java-parser families share one source of truth. use super::class_reader::CpInfo; -use super::{StreamLabel, StreamLabelType, jar, vocab}; +use super::{ParseResult, StreamLabel, StreamLabelType, jar, vocab}; use crate::sector::SectorReader; use crate::udf::UdfFs; use std::collections::BTreeMap; @@ -49,7 +49,7 @@ pub fn detect(udf: &UdfFs) -> bool { jar::has_any_top_level_jar(udf) } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { jar::for_each_jar(reader, udf, |_entry_name, archive| { if !jar::has_path_prefix(archive, "com/dbp/") { return None; @@ -58,7 +58,9 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option bool { jar::has_any_top_level_jar(udf) } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { jar::for_each_jar(reader, udf, |entry_name, archive| { if !jar::has_path_prefix(archive, "com/bydeluxe/") { return None; diff --git a/src/labels/mod.rs b/src/labels/mod.rs index c59167e..caae301 100644 --- a/src/labels/mod.rs +++ b/src/labels/mod.rs @@ -73,28 +73,68 @@ pub enum LabelQualifier { // ── Parser registry ──────────────────────────────────────────────────────── // -// Each entry: (name, detect_fn, parse_fn) -// Order = priority. First match wins. Highest quality output first. +// Each entry: (name, detect_fn, parse_fn). Order = tiebreaker only — +// the registry picks the highest-confidence parse result, falling back +// to array order on confidence ties. type DetectFn = fn(&UdfFs) -> bool; -type ParseFn = fn(&mut dyn SectorReader, &UdfFs) -> Option>; +type ParseFn = fn(&mut dyn SectorReader, &UdfFs) -> Option; + +/// Per-parser claim of how reliable its output is. Used by the +/// registry to pick between parsers when more than one matches (e.g. +/// a disc that has both `bluray_project.bin` and `playlists.xml`). +/// +/// A parser SHOULD return `High` only when its full schema was +/// extracted with no fallback or guessing. `Medium` is for matched- +/// but-degraded outputs (some streams missing fields, fingerprint +/// matched but a sub-table couldn't be decoded, etc.). The registry +/// prefers `High` over `Medium`; ties fall to array order. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Confidence { + Medium, + High, +} + +/// Successful parser result. `None` from `parse()` still means "this +/// isn't my disc" (no labels at all); `Some(ParseResult { labels, .. })` +/// with `labels.is_empty()` is also a "no labels" case but reachable +/// via the analyzer (used by deluxe today to signal "I recognized the +/// framework but Phase D not yet implemented"). +#[derive(Debug, Clone)] +pub struct ParseResult { + pub labels: Vec, + pub confidence: Confidence, +} + +impl ParseResult { + /// Convenience for the common "I parsed N labels with full schema + /// coverage" case. + pub fn high(labels: Vec) -> Self { + ParseResult { + labels, + confidence: Confidence::High, + } + } + + /// Convenience for "I matched but had to fall back on some fields". + pub fn medium(labels: Vec) -> Self { + ParseResult { + labels, + confidence: Confidence::Medium, + } + } +} const PARSERS: &[(&str, DetectFn, ParseFn)] = &[ ("paramount", paramount::detect, paramount::parse), ("criterion", criterion::detect, criterion::parse), ("pixelogic", pixelogic::detect, pixelogic::parse), ("ctrm", ctrm::detect, ctrm::parse), - // dbp last: detects on any top-level .jar in /BDMV/JAR/ (every - // BD-J disc has one), so parse() does the real `com/dbp/` check - // and returns None on a mismatch. By placing dbp last, the - // earlier parsers' fast file-presence detects short-circuit and - // dbp only runs on discs that fell through everything else. // dbp and deluxe both detect on "any top-level .jar in /BDMV/JAR/" // (every BD-J disc trips that) and do the real vendor-prefix check - // in parse(). Order between them is somewhat arbitrary since either - // returns None on a mismatched jar, but dbp goes first because its - // parse path is cheaper (constant-pool iteration vs. deluxe's - // bytecode walking once Phase D lands). + // in parse(). Order between them is the tiebreaker on equal + // confidence; dbp goes first because its parse path is cheaper + // (constant-pool iteration vs. deluxe's bytecode walking). ("dbp", dbp::detect, dbp::parse), ("deluxe", deluxe::detect, deluxe::parse), ]; @@ -285,16 +325,41 @@ fn generate_audio_label( } fn extract(reader: &mut dyn SectorReader, udf: &UdfFs) -> Vec { + let mut best: Option<(&'static str, ParseResult)> = None; for (name, detect, parse) in PARSERS { - if detect(udf) { - tracing::info!(parser = name, "label parser matched"); - if let Some(labels) = parse(reader, udf) { - return labels; - } + if !detect(udf) { + continue; + } + tracing::info!(parser = name, "label parser detected"); + let Some(result) = parse(reader, udf) else { + continue; + }; + if result.labels.is_empty() { + continue; + } + // Pick highest confidence. Equal confidence → first wins + // (array order tiebreaker). + match &best { + None => best = Some((name, result)), + Some((_, b)) if result.confidence > b.confidence => best = Some((name, result)), + _ => {} + } + } + match best { + Some((name, r)) => { + tracing::info!( + parser = name, + confidence = ?r.confidence, + label_count = r.labels.len(), + "label parser selected", + ); + r.labels + } + None => { + tracing::info!("no label parser matched"); + Vec::new() } } - tracing::info!("no label parser matched"); - Vec::new() } /// Diagnostic introspection — returns the parser that matched, the @@ -302,43 +367,59 @@ fn extract(reader: &mut dyn SectorReader, udf: &UdfFs) -> Vec { /// that the discriminators looked at. Intended for `freemkv-tools /// labels-analyze` and corpus regression tooling, not production code /// paths. The matching/parsing logic is identical to [`extract`]; only -/// the return shape is richer. +/// the return shape is richer (includes confidence, all detected +/// parsers, and any parsers that produced empty results). #[doc(hidden)] pub fn analyze(reader: &mut dyn SectorReader, udf: &UdfFs) -> LabelAnalysis { let inventory = jar_inventory(udf); - // Record every parser whose discriminator matched — even if its - // parse step then returned None — so the analyzer can distinguish - // "no parser recognized this disc" from "parser recognized it but - // couldn't read the file" (e.g. content past a truncated capture) - // or "parser ran but produced no labels." let mut parsers_detected: Vec<&'static str> = Vec::new(); + let mut all_results: Vec<(&'static str, ParseResult)> = Vec::new(); + for (name, detect, parse) in PARSERS { - if detect(udf) { - tracing::info!(parser = name, "label parser matched"); - parsers_detected.push(name); - if let Some(labels) = parse(reader, udf) { - return LabelAnalysis { - parser: Some(name), - parsers_detected, - jar_inventory: inventory, - labels, - }; - } + if !detect(udf) { + continue; + } + tracing::info!(parser = name, "label parser detected"); + parsers_detected.push(name); + if let Some(r) = parse(reader, udf) { + all_results.push((name, r)); } } + + // Selection logic mirrors `extract`: highest confidence + non-empty, + // array order tiebreaker. + let chosen = all_results + .iter() + .filter(|(_, r)| !r.labels.is_empty()) + .max_by(|(_, a), (_, b)| { + // Cmp first by confidence (higher first), then position + // (earlier first). max_by yields the maximum, so we + // invert the index comparison. + a.confidence + .cmp(&b.confidence) + .then(std::cmp::Ordering::Equal) + }); + + let (parser, confidence, labels) = match chosen { + Some((name, r)) => (Some(*name), Some(r.confidence), r.labels.clone()), + None => (None, None, Vec::new()), + }; + if parsers_detected.is_empty() { tracing::info!("no label parser matched"); - } else { + } else if parser.is_none() { tracing::info!( detected = ?parsers_detected, "label parsers detected but produced no labels" ); } + LabelAnalysis { - parser: None, + parser, parsers_detected, + confidence, jar_inventory: inventory, - labels: Vec::new(), + labels, } } @@ -346,13 +427,16 @@ pub fn analyze(reader: &mut dyn SectorReader, udf: &UdfFs) -> LabelAnalysis { #[doc(hidden)] #[derive(Debug, Clone)] pub struct LabelAnalysis { - /// Which parser matched ("paramount" / "criterion" / "pixelogic" / - /// "ctrm") AND emitted labels. `None` means either no parser - /// recognized the disc, OR a parser recognized it but its parse - /// step returned None (file unreadable, no parseable tokens). Use - /// `parsers_detected` to disambiguate. + /// Which parser was SELECTED — the one whose `ParseResult` had + /// the highest confidence among non-empty results (array order + /// tiebreaker). `None` means either no parser recognized the + /// disc, OR every parser that recognized it returned no labels. + /// Use `parsers_detected` to disambiguate. pub parser: Option<&'static str>, - /// Every parser whose discriminator matched, in priority order. + /// Confidence of the selected parser, `None` if no parser was + /// selected. + pub confidence: Option, + /// Every parser whose discriminator matched, in registry order. /// Distinguishes "we recognized this disc but couldn't extract /// labels" from "we don't recognize this disc at all" — the /// former points at a parser bug or a truncated capture, the @@ -362,8 +446,8 @@ pub struct LabelAnalysis { /// and sorted. Helps spot unknown authoring formats when no /// parser detected. pub jar_inventory: Vec, - /// Raw labels emitted by the matched parser (empty if `parser` is - /// `None`). + /// Raw labels emitted by the selected parser (empty if `parser` + /// is `None`). pub labels: Vec, } diff --git a/src/labels/paramount.rs b/src/labels/paramount.rs index cbd9bc5..29bca97 100644 --- a/src/labels/paramount.rs +++ b/src/labels/paramount.rs @@ -12,7 +12,7 @@ //! sub_com1_idx="23,24,25" /> //! ``` -use super::{LabelPurpose, LabelQualifier, StreamLabel, StreamLabelType}; +use super::{LabelPurpose, LabelQualifier, ParseResult, StreamLabel, StreamLabelType}; use crate::sector::SectorReader; use crate::udf::UdfFs; @@ -20,7 +20,7 @@ pub fn detect(udf: &UdfFs) -> bool { super::jar_file_exists(udf, "playlists.xml") } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { let data = super::read_jar_file(reader, udf, "playlists.xml")?; let text = std::str::from_utf8(&data).ok()?; @@ -100,7 +100,9 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option bool { super::jar_file_exists(udf, "bluray_project.bin") } -pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option> { +pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { let data = super::read_jar_file(reader, udf, "bluray_project.bin")?; // min_len=4 matches the prior local extract_strings impl. The token // grammar is `{lang3}_{codec?}_{purpose?}_{region?}_` so the // shortest meaningful run is 4 chars (lang + underscore). let strings = text::extract_ascii_strings(&data, 4); + // Tracked across all parse_token calls in this run: did any stream + // hit an unrecognized token component (skip-unknown path)? If yes + // we downgrade confidence to Medium — the labels are still valid + // but the corpus surfaced something we don't catalogue. + let saw_unknown = AtomicBool::new(false); + let mut labels = Vec::new(); let mut in_feature = false; let mut audio_num: u16 = 0; @@ -53,7 +63,7 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option { audio_num += 1; @@ -76,10 +86,15 @@ pub fn parse(reader: &mut dyn SectorReader, udf: &UdfFs) -> Option Option { +fn parse_token_inner(s: &str, saw_unknown: Option<&AtomicBool>) -> Option { let clean = s.trim().trim_start_matches('\t').trim_end_matches('_'); let parts: Vec<&str> = clean.split('_').collect(); if parts.len() < 2 { @@ -135,8 +150,13 @@ fn parse_token(s: &str) -> Option { // behavior was `return None` here, which silently dropped // any stream containing a single uncatalogued token (e.g. // a new codec ID or framework variant). Better to surface - // what we know than discard a whole stream over one part. + // what we know than discard a whole stream over one part, + // but flag the parse as Medium-confidence so callers know + // some data was elided. tracing::debug!(part = %part, "pixelogic: unrecognized token component, skipping"); + if let Some(flag) = saw_unknown { + flag.store(true, Ordering::Relaxed); + } } } @@ -170,7 +190,7 @@ mod tests { #[test] fn parse_token_basic_audio() { - let l = parse_token("eng_MLP_").unwrap(); + let l = parse_token_inner("eng_MLP_", None).unwrap(); assert_eq!(l.stream_type, StreamLabelType::Audio); assert_eq!(l.language, "eng"); assert_eq!(l.codec_hint, "TrueHD"); @@ -179,7 +199,7 @@ mod tests { #[test] fn parse_token_basic_subtitle_sdh() { - let l = parse_token("eng_SDH_").unwrap(); + let l = parse_token_inner("eng_SDH_", None).unwrap(); assert_eq!(l.stream_type, StreamLabelType::Subtitle); assert_eq!(l.language, "eng"); assert_eq!(l.qualifier, LabelQualifier::Sdh); @@ -187,20 +207,20 @@ mod tests { #[test] fn parse_token_commentary() { - let l = parse_token("eng_MLP_ACOM_").unwrap(); + let l = parse_token_inner("eng_MLP_ACOM_", None).unwrap(); assert_eq!(l.stream_type, StreamLabelType::Audio); assert_eq!(l.purpose, LabelPurpose::Commentary); } #[test] fn parse_token_descriptive() { - let l = parse_token("eng_AC3_ADES_").unwrap(); + let l = parse_token_inner("eng_AC3_ADES_", None).unwrap(); assert_eq!(l.purpose, LabelPurpose::Descriptive); } #[test] fn parse_token_with_region() { - let l = parse_token("eng_MLP_US_").unwrap(); + let l = parse_token_inner("eng_MLP_US_", None).unwrap(); assert_eq!(l.language, "eng"); assert_eq!(l.variant, "US"); } @@ -210,7 +230,7 @@ mod tests { // Regression: pre-refactor, an unrecognized token part returned // None for the whole stream, silently dropping it. New // behavior: skip the unknown part, surface what we know. - let l = parse_token("eng_MLP_FUTUREFLAG_FOR_").unwrap(); + let l = parse_token_inner("eng_MLP_FUTUREFLAG_FOR_", None).unwrap(); assert_eq!(l.stream_type, StreamLabelType::Audio); assert_eq!(l.language, "eng"); assert_eq!(l.codec_hint, "TrueHD"); @@ -222,12 +242,12 @@ mod tests { // A token that has only a language and an unknown part with // no audio/subtitle classifier should still return None — // there's no way to file it as a stream. - assert!(parse_token("eng_UNKNOWN_").is_none()); + assert!(parse_token_inner("eng_UNKNOWN_", None).is_none()); } #[test] fn parse_token_rejects_non_lang_prefix() { - assert!(parse_token("XX_MLP_").is_none()); - assert!(parse_token("ENG_MLP_").is_none()); // uppercase not accepted as ISO 639-2 + assert!(parse_token_inner("XX_MLP_", None).is_none()); + assert!(parse_token_inner("ENG_MLP_", None).is_none()); // uppercase not accepted as ISO 639-2 } }