From 01d4a1ba00d3841b4d72ba76f1b9f562e02f9126 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:25:04 -0700 Subject: [PATCH] Catalogue the DUB forced-narrative marker and report vocabulary gaps once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of the same subtitle-labelling bug class. The numbering half landed already: an entry the parser could not parse still occupies an STN slot, so skipping it shifted every later label onto the wrong stream. This is the other half — an entry the parser counts correctly but cannot INTERPRET. `DUB` names the forced-narrative subtitle track authored to accompany a language's dubbed audio presentation: the signs and on-screen-text pass a viewer still needs once the dialogue itself is dubbed. It is the same editorial class as `*_TXT_FOR_`, spelled differently by some authoring runs. Uncatalogued, it matched no component arm, so the token signalled neither audio nor subtitle and the whole stream record was dropped at the domain guard — a genuine forced track left with no forced qualifier even after the numbering was right. Evidence, from two independent discs in the corpus: the token appears only inside the PG list, embedded in an otherwise contiguous run of `{lang}[_{region}]_TXT_FOR_` siblings — one forced-narrative slot per localized language — and takes exactly the slot where that language's forced entry belongs. Both discs also carry that language's FULL subtitle track as a separate, separately-labelled slot, so the DUB entry is not it. The stream it lands on is a sparse PG track, the signature of a forced pass rather than full dialogue. Deliberately not added to vocab::qualifier: that maps free-form English label text, where a bare "dub" means dubbed AUDIO. The forced-subtitle reading is specific to this token grammar. The corpus sweep that found it also produced four components that are deliberately NOT catalogued. They are per-language notice and disclaimer clip names that merely collide with the `{lang3}_{component}` token shape; each occurrence sits in a one-video-stream section next to the disclaimer entry it names. They carry no editorial meaning, and mapping them would attach a qualifier to a stream on the strength of a filename. Which is also why an unmapped component now reports once per parse rather than once per occurrence. It was a debug line nobody reads, and that is how this gap survived to a user complaint; but a per-occurrence warn would bury the signal under dozens of routine collisions on an ordinary disc. One bounded, deduplicated line names the distinct components and says plainly that any forced/SDH/commentary meaning they carry went unapplied. The backing set is capped and truncates by chars, not bytes — the components come from untrusted disc bytes, and a byte-offset slice can split a multi-byte sequence and panic. Two existing tests encoded the wrong behaviour and are corrected: the STN numbering test expected the forced run to skip the DUB slot, and the unclassifiable-slot test used DUB as its example of a token with no meaning. The latter now uses a genuinely uncatalogued component. --- src/labels/pixelogic.rs | 335 ++++++++++++++++++++++++++++++++++------ 1 file changed, 287 insertions(+), 48 deletions(-) diff --git a/src/labels/pixelogic.rs b/src/labels/pixelogic.rs index 9885d34..374e023 100644 --- a/src/labels/pixelogic.rs +++ b/src/labels/pixelogic.rs @@ -24,6 +24,74 @@ const MAX_STREAMS_PER_TYPE: u16 = 512; const REGIONS: &[&str] = &[ "US", "UK", "CF", "PF", "CS", "LS", "BP", "PP", "SM", "TM", "CAN", "DUM", "FLE", ]; +/// How many DISTINCT uncatalogued token components one parse will retain for +/// the end-of-parse report. Disc bytes are untrusted, so the set that backs +/// the report is capped: a crafted blob carrying thousands of distinct +/// components must not grow it without bound. Past the cap the components are +/// still counted (and still logged individually at debug), just not retained +/// by name — the report says so. +const MAX_REPORTED_UNKNOWN: usize = 16; +/// Longest retained form of a single uncatalogued component. Components come +/// from disc bytes and can be arbitrarily long; truncation is by CHARS, not +/// bytes, so a multi-byte sequence can never be split (which would panic). +const MAX_UNKNOWN_LEN: usize = 32; + +/// Collects the uncatalogued token components one parse ran into, so the run +/// can report them ONCE at the end instead of either staying silent or +/// emitting a line per occurrence. +/// +/// Why aggregate: an unmapped vendor component is how a forced/SDH/commentary +/// qualifier goes missing, and a per-occurrence `debug!` is invisible in +/// practice — the gap only surfaces when a user complains about a mislabelled +/// track. But a per-occurrence `warn!` is unusable in the other direction: a +/// disc can carry dozens of per-language segment names that merely COLLIDE +/// with the `{lang3}_{component}` token shape (localized notice/disclaimer +/// clip names, for instance), and warning on each would bury real signal under +/// routine noise. One bounded, deduplicated line per parse is loud enough to +/// notice and quiet enough to live with. +#[derive(Debug, Default)] +struct UnknownParts { + /// Distinct components, deduplicated and ordered for a stable log line. + /// Bounded by [`MAX_REPORTED_UNKNOWN`]. + seen: std::collections::BTreeSet, + /// Total occurrences, including ones past the retention cap. + total: usize, +} + +impl UnknownParts { + fn record(&mut self, part: &str) { + self.total = self.total.saturating_add(1); + if self.seen.len() >= MAX_REPORTED_UNKNOWN { + return; + } + // Char-wise truncation: `part` is uppercased disc text, not + // guaranteed ASCII, and slicing by byte offset could split a + // multi-byte char and panic. + self.seen + .insert(part.chars().take(MAX_UNKNOWN_LEN).collect()); + } + + fn is_empty(&self) -> bool { + self.total == 0 + } + + /// Emit the single end-of-parse report, if this parse hit anything. + fn report(&self) { + if self.is_empty() { + return; + } + let names: Vec<&str> = self.seen.iter().map(String::as_str).collect(); + tracing::warn!( + components = ?names, + distinct = self.seen.len(), + occurrences = self.total, + truncated = self.seen.len() >= MAX_REPORTED_UNKNOWN, + "pixelogic: uncatalogued token components in this disc's label blob; \ + any editorial meaning they carry (forced / SDH / commentary / dub) \ + was NOT applied to the affected streams" + ); + } +} pub fn detect(_reader: &mut dyn SectorSource, udf: &UdfFs) -> bool { super::jar_file_exists(udf, "bluray_project.bin") @@ -36,22 +104,27 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option // 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. Parsing is - // single-threaded and sequential, so a plain bool suffices. - let mut saw_unknown = false; + // Tracked across all parse_token calls in this run: which uncatalogued + // token components did the blob contain? If any, we downgrade confidence + // to Medium — the labels are still valid but the disc surfaced something + // we don't catalogue — and report them once (see `UnknownParts`). Parsing + // is single-threaded and sequential, so a plain owned collector suffices. + let mut unknown = UnknownParts::default(); - let labels = assign_labels(&strings, &mut saw_unknown); + let labels = assign_labels(&strings, &mut unknown); + + // Reported even when the parse yields nothing: "we recognized the format, + // couldn't classify its components, and produced no labels" is precisely + // the case worth surfacing. + unknown.report(); if labels.is_empty() { return None; } - let confidence = if saw_unknown { - Confidence::Medium - } else { + let confidence = if unknown.is_empty() { Confidence::High + } else { + Confidence::Medium }; Some(ParseResult { labels, confidence }) } @@ -60,7 +133,7 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option /// `StreamLabel` per editorial token, numbered in STN order. Split out /// from `parse` so the section/numbering logic is unit-testable without /// a `SectorSource`/`UdfFs`. -fn assign_labels(strings: &[String], saw_unknown: &mut bool) -> Vec { +fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec { // The authoritative per-feature stream list lives in the `FPL_` // (FeaturePLaylist) section, in STN order. `SEG_*` entries are menu // segments (intros, logos, disclaimers, previews) that can also carry @@ -150,7 +223,7 @@ fn assign_labels(strings: &[String], saw_unknown: &mut bool) -> Vec continue; } - if let Some(label) = parse_token_inner(s, Some(&mut *saw_unknown)) { + if let Some(label) = parse_token_inner(s, Some(&mut *unknown)) { domain = label.stream_type; match label.stream_type { StreamLabelType::Audio => { @@ -225,7 +298,7 @@ fn is_stream_token(s: &str) -> bool { lang.len() == 3 && lang.chars().all(|c| c.is_ascii_lowercase()) } -fn parse_token_inner(s: &str, mut saw_unknown: Option<&mut bool>) -> Option { +fn parse_token_inner(s: &str, mut unknown: Option<&mut UnknownParts>) -> Option { let clean = s.trim().trim_start_matches('\t').trim_end_matches('_'); let parts: Vec<&str> = clean.split('_').collect(); if parts.len() < 2 { @@ -285,6 +358,32 @@ fn parse_token_inner(s: &str, mut saw_unknown: Option<&mut bool>) -> Option) -> Option = labels @@ -710,29 +814,36 @@ mod tests { assert_eq!(l.codec_hint, "Dolby Digital"); } - /// Spec: an unknown component sets saw_unknown flag. - /// Mutation: remove the flag-setting → Medium confidence never triggered. + /// Spec: an unknown component is recorded by name, not merely flagged. + /// Mutation: drop the `record` call → Medium confidence never triggered + /// and the end-of-parse report never names the gap. #[test] - fn parse_token_unknown_sets_saw_unknown_flag() { - let mut flag = false; - let _ = parse_token_inner("eng_MLP_FUTURETOKEN_", Some(&mut flag)); - assert!(flag, "unknown component must set saw_unknown flag"); + fn parse_token_unknown_is_recorded_by_name() { + let mut acc = UnknownParts::default(); + let _ = parse_token_inner("eng_MLP_FUTURETOKEN_", Some(&mut acc)); + assert!(!acc.is_empty(), "unknown component must be recorded"); + assert!( + acc.seen.contains("FUTURETOKEN"), + "the report must name the component, got {:?}", + acc.seen + ); } - /// Spec: a known-only token leaves saw_unknown=false. - /// Mutation: always set the flag → all parses downgrade to Medium. + /// Spec: a known-only token records nothing. + /// Mutation: always record → all parses downgrade to Medium and every + /// normal disc emits the warn. #[test] - fn parse_token_all_known_leaves_flag_false() { - let mut flag = false; - let _ = parse_token_inner("eng_MLP_ACOM_US_", Some(&mut flag)); - assert!(!flag, "all-known token must NOT set saw_unknown flag"); + fn parse_token_all_known_records_nothing() { + let mut acc = UnknownParts::default(); + let _ = parse_token_inner("eng_MLP_ACOM_US_", Some(&mut acc)); + assert!(acc.is_empty(), "all-known token must record nothing"); } /// Spec: `Audio Stream N` placeholder advances audio_num but emits no label. /// Mutation: also emit a label for placeholder → audio#N+1 shifts to N+2. #[test] fn assign_labels_audio_placeholder_advances_counter_no_label() { - let mut flag = false; + let mut flag = UnknownParts::default(); let tokens = strs(&["FPL_MainFeature", "Audio Stream 1", "eng_MLP_"]); let labels = assign_labels(&tokens, &mut flag); let a: Vec<_> = labels @@ -747,7 +858,7 @@ mod tests { /// Mutation: don't end on SEG_ → tokens from a following segment are parsed. #[test] fn assign_labels_fpl_section_ends_on_seg_boundary() { - let mut flag = false; + let mut flag = UnknownParts::default(); let tokens = strs(&[ "FPL_MainFeature", "eng_MLP_", @@ -763,7 +874,7 @@ mod tests { /// Mutation: remove the cap check → counter wraps past 512. #[test] fn assign_labels_max_streams_cap_prevents_overflow() { - let mut flag = false; + let mut flag = UnknownParts::default(); // Build 520 Audio Stream placeholders inside FPL, then an editorial token. let mut tokens = vec!["FPL_MainFeature".to_string()]; for i in 1..=520 { @@ -791,7 +902,7 @@ mod tests { /// single token), so the section would never end on `SF_` alone. #[test] fn assign_labels_fpl_section_ends_on_sf_boundary() { - let mut flag = false; + let mut flag = UnknownParts::default(); let tokens = strs(&[ "FPL_MainFeature", "eng_MLP_", @@ -812,7 +923,7 @@ mod tests { /// a legitimate subtitle stream that comes after audio saturates. #[test] fn assign_labels_audio_cap_alone_does_not_stop_subtitle_processing() { - let mut flag = false; + let mut flag = UnknownParts::default(); let mut tokens = vec!["FPL_MainFeature".to_string()]; for i in 1..=(MAX_STREAMS_PER_TYPE as usize) { tokens.push(format!("Audio Stream {}", i)); @@ -842,7 +953,7 @@ mod tests { /// this scenario — dropping the trailing audio token. #[test] fn assign_labels_subtitle_cap_alone_does_not_stop_audio_processing() { - let mut flag = false; + let mut flag = UnknownParts::default(); let mut tokens = vec!["FPL_MainFeature".to_string()]; for _ in 1..=(MAX_STREAMS_PER_TYPE as usize) { tokens.push("eng_SDH_".to_string()); @@ -869,7 +980,7 @@ mod tests { /// by the number of unlabelled PG slots ahead of it. #[test] fn assign_labels_pg_placeholder_advances_sub_counter_only() { - let mut flag = false; + let mut flag = UnknownParts::default(); let tokens = strs(&[ "FPL_MainFeature", "Audio Stream 1", @@ -896,17 +1007,18 @@ mod tests { } /// Spec: a token-shaped entry the grammar cannot classify (`fra_CF_` — - /// REGION only; `jpn_DUB_` — uncatalogued component) still occupies an STN - /// slot in the list currently being enumerated. + /// REGION only, so it signals neither audio nor subtitle; `jpn_ZZQ_` — + /// an uncatalogued component) still occupies an STN slot in the list + /// currently being enumerated. /// Mutation: skip unclassifiable tokens → later labels shift down. #[test] fn assign_labels_unclassifiable_token_still_occupies_a_slot() { - let mut flag = false; + let mut flag = UnknownParts::default(); let tokens = strs(&[ "FPL_MainFeature", "PG Stream 1", // switches the domain to PG, slot 1 "fra_CF_", // region-only: no label, but PG slot 2 - "jpn_DUB_", // uncatalogued: no label, but PG slot 3 + "jpn_ZZQ_", // uncatalogued: no label, but PG slot 3 "eng_SDH_", // PG slot 4 ]); let labels = assign_labels(&tokens, &mut flag); @@ -918,6 +1030,133 @@ mod tests { assert_eq!(subs, vec![4]); } + /// Spec: `DUB` is a subtitle-domain forced-narrative marker — the same + /// editorial class as `TXT_FOR`, spelled differently. A token whose ONLY + /// non-language component is `DUB` must survive the + /// `!is_audio && !is_subtitle` guard and come back flagged Forced. + /// Mutation: drop the `DUB` arm → the token falls through to the unknown + /// branch, classifies as neither domain, and the whole stream is dropped + /// (no label at all, so no forced flag on a genuine forced track). + #[test] + fn parse_token_dub_is_a_forced_narrative_subtitle() { + let l = parse_token_inner("jpn_DUB_", None).expect("DUB must classify as a subtitle"); + assert_eq!(l.stream_type, StreamLabelType::Subtitle); + assert_eq!(l.language, "jpn"); + assert_eq!(l.qualifier, LabelQualifier::Forced); + assert_eq!(l.purpose, LabelPurpose::Normal); + // Case-insensitive like every other component. + let l = parse_token_inner("jpn_dub_", None).expect("lowercase DUB must classify too"); + assert_eq!(l.qualifier, LabelQualifier::Forced); + } + + /// Spec: now that `DUB` is catalogued it must NOT be reported as an + /// uncatalogued component, so a disc that uses it keeps High confidence + /// and emits no warn. + /// Mutation: leave `DUB` in the unknown branch → the disc is downgraded to + /// Medium and warns on every parse. + #[test] + fn parse_token_dub_is_not_reported_as_uncatalogued() { + let mut acc = UnknownParts::default(); + let _ = parse_token_inner("jpn_DUB_", Some(&mut acc)); + assert!(acc.is_empty(), "DUB is catalogued, got {:?}", acc.seen); + } + + /// Spec: a `DUB` slot sitting inside a run of `*_TXT_FOR_` siblings — the + /// shape both corpus discs show — yields a forced label on ITS OWN slot, + /// contiguous with its neighbours. + /// Mutation: classify DUB as audio → the PG run gains a hole at that slot + /// and the audio list gains a spurious entry. + #[test] + fn assign_labels_dub_slot_is_forced_in_a_forced_run() { + let mut flag = UnknownParts::default(); + let tokens = strs(&[ + "FPL_MainFeature", + "PG Stream 1", + "eng_TXT_FOR_", // PG slot 2 + "fra_CF_TXT_FOR_", + "jpn_DUB_", // PG slot 4 — same class, different spelling + "spa_TXT_FOR_", + ]); + let labels = assign_labels(&tokens, &mut flag); + let forced: Vec<_> = labels + .iter() + .filter(|l| l.qualifier == LabelQualifier::Forced) + .map(|l| (l.stream_type, l.stream_number, l.language.as_str())) + .collect(); + assert_eq!( + forced, + vec![ + (StreamLabelType::Subtitle, 2, "eng"), + (StreamLabelType::Subtitle, 3, "fra"), + (StreamLabelType::Subtitle, 4, "jpn"), + (StreamLabelType::Subtitle, 5, "spa"), + ] + ); + assert!(flag.is_empty(), "no uncatalogued components in this run"); + } + + /// Spec: `UnknownParts` deduplicates, so a disc carrying the SAME + /// uncatalogued component on dozens of entries reports it once. This is + /// what keeps the warn usable on discs whose per-language segment names + /// collide with the token shape. + /// Mutation: use a Vec instead of a set → `distinct` grows with occurrences. + #[test] + fn unknown_parts_dedups_but_counts_every_occurrence() { + let mut acc = UnknownParts::default(); + for _ in 0..50 { + acc.record("ZZQ"); + } + acc.record("QQZ"); + assert_eq!(acc.seen.len(), 2, "two distinct components"); + assert_eq!(acc.total, 51, "every occurrence counted"); + } + + /// Spec: the retained set is bounded — a crafted blob with thousands of + /// distinct components must not grow it without bound, and must not panic. + /// Mutation: drop the cap check → unbounded memory from disc bytes. + #[test] + fn unknown_parts_retention_is_bounded() { + let mut acc = UnknownParts::default(); + for i in 0..10_000 { + acc.record(&format!("PART{}", i)); + } + assert_eq!(acc.seen.len(), MAX_REPORTED_UNKNOWN); + assert_eq!(acc.total, 10_000, "occurrences still counted past the cap"); + } + + /// Spec: an over-long component is truncated by CHARS, so a multi-byte + /// sequence is never split. Byte-offset truncation would panic here. + /// Mutation: `part[..MAX_UNKNOWN_LEN].to_string()` → panics mid-char. + #[test] + fn unknown_parts_truncates_on_char_boundaries() { + let mut acc = UnknownParts::default(); + let long: String = "é".repeat(500); // 2 bytes per char + acc.record(&long); + let stored = acc.seen.iter().next().expect("recorded"); + assert_eq!(stored.chars().count(), MAX_UNKNOWN_LEN); + // Round-trips as valid UTF-8 — no split code point. + assert!(stored.chars().all(|c| c == 'é')); + } + + /// Spec: the per-language notice/disclaimer clip names some discs carry + /// (`{lang}_ND`, `{lang}_Warning`, …) merely COLLIDE with the token shape. + /// They are not stream tokens and carry no editorial meaning, so they must + /// stay uncatalogued — mapping them would attach a qualifier to a stream + /// on the strength of a filename. What they must do is collapse into ONE + /// report rather than one line each. + /// Mutation: warn per occurrence → dozens of lines on an ordinary disc. + #[test] + fn unknown_parts_collapses_a_wall_of_segment_name_collisions() { + let mut acc = UnknownParts::default(); + for lang in ["ara", "bul", "ces", "dan", "deu", "ell"] { + let _ = lang; + acc.record("ND"); + acc.record("WARNING"); + } + assert_eq!(acc.seen.len(), 2, "one entry per distinct component"); + assert_eq!(acc.total, 12); + } + /// Spec: `is_stream_token` accepts exactly what `parse_token_inner`'s own /// entry gate accepts — `{lang3}_{component}…` — so the section's /// `Video Stream 1` / `AR_169` entries never consume a stream slot.