From c94e9f4fb7452d285295b13689f7ff9169bb022c Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:55:08 -0700 Subject: [PATCH] End a label section where the next one's stream list begins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pixelogic walk finds the feature playlist's section by name and ends it at the next `SEG_`/`SF_`/`FPL_` marker. Those markers are section NAMES, and a project's trailing sections — the per-language notice, disclaimer and dub-credit cards — carry none. On 8 of the 11 affected-format discs in the corpus the feature playlist is the last NAMED section in the blob, so the terminator never fires and the walk consumes the whole tail of the file as more of the feature's stream list. The card names are `{lang3}_{card}`, which passes `is_stream_token`, so each one advances an STN counter, and a card whose name collides with a catalogued component emits a label outright. Measured on the worst disc: 95 entries past the end of a 9-audio/21-PG list, five phantom audio labels at STN 10-14 from `*_AC` notice cards (`AC` reads as the AC-3 codec), and 94 uncatalogued-component occurrences — which also took the parse from High to Medium confidence and fired the vocabulary-gap warning on four components that are deliberately not catalogued. A second disc fabricated one subtitle label from a token in a following playlist section named `FP_SingAlong`, which `FPL_` does not match. What every section has, named or not, is a stream list that opens with its video slots. So a `Video Stream N` entry repeating one this section already listed is the first entry of the NEXT section, and ends this one. Distinct video entries are kept, since a section may legitimately list a secondary video stream; the memo of them is bounded at the BD STN table's ceiling so disc bytes cannot grow it. Replaying all 11 blobs through `assign_labels` before and after: the two discs above lose exactly their phantom labels (11→6 and 5→4), the other nine are byte-identical. One residue is pinned rather than papered over: a card's name precedes its own section's video slot, so a forward-only walk can still count the FIRST card after the last real slot. It sits at the tail of a list nothing follows in, so it can renumber nothing — at worst it costs a parse its High confidence. No other parser in src/labels/ walks a flat entry sequence with a terminator set; the rest scope each stream to a structural range or read its number off the entry itself. paramount and criterion gain immunity pins for the boundary property specifically: a stream list cannot run into the next element's, and a missing element boundary shortens the list rather than extending it. --- CHANGELOG.md | 19 +++++ src/labels/criterion.rs | 42 +++++++++++ src/labels/paramount.rs | 51 +++++++++++++ src/labels/pixelogic.rs | 162 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 272 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c50044..679db36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,25 @@ (`pixelogic`, `paramount`, `mpls_universal`, `deluxe`); the rest are now pinned by tests proving they are immune. Three of the crate's own tests had been asserting the shifted numbering. +- **A feature's stream list ran on past its end and picked up menu clips as + streams.** The parser for one vendor's label blob finds the feature + playlist's section by name and ends it at the next named section — but on + most discs of that authoring style the feature playlist IS the last named + section, and the trailing per-language notice, disclaimer and dub-credit + cards carry no name marker at all. The walk therefore swallowed the whole + tail of the blob as more of the feature's own stream list. Those cards are + named per language, in the same shape as a stream token, so each one silently + advanced a stream-number counter, and the ones whose name collided with a + catalogued component were labelled as streams outright — on one disc, five + audio labels for slots 10 to 14 of a playlist that has nine. The same + collisions were being reported as vocabulary gaps and cost that disc's parse + its high-confidence rating. A section now also ends where the next one's + stream list begins, which is at a video slot the section has already listed. + Eight of eleven affected-format discs in the test corpus have no terminating + marker; two of them were producing labels for streams their feature playlist + does not contain. No other label parser walks a flat entry sequence this way + — the rest scope each stream to a structural range or read its number off the + entry itself, and two more now carry tests pinning that. - **A forced-narrative subtitle marker went uncatalogued.** The token marking the signs-and-on-screen-text pass that accompanies a dubbed presentation was not in the vocabulary, so that track lost its forced flag while every other diff --git a/src/labels/criterion.rs b/src/labels/criterion.rs index ffd351c..aafa4e4 100644 --- a/src/labels/criterion.rs +++ b/src/labels/criterion.rs @@ -276,6 +276,48 @@ mod tests { /// /// Mutation: skip elements with an empty `ID`/`LangInfoID` → the two /// real audio streams renumber to 1 and 2. + /// Immunity pin, section-boundary half. Each stream here is one closed XML + /// element, and every field is read out of `&text[start..end]` — the range + /// `xml::find_element` returned — so one element can never absorb the next + /// one's fields, however the document is malformed around it. Contrast the + /// flat-string walk in pixelogic, where a section whose end marker is + /// missing keeps consuming entries as STN slots. + /// + /// The missing-boundary case fails closed. An element with no close tag of + /// its own ends at the NEXT close tag, so it absorbs the element behind it + /// — the list comes back SHORTER. It cannot come back longer: nothing + /// outside a returned range is ever read as a stream, and `find_element` + /// yields `None` rather than a range running to EOF when no close tag + /// exists at all. A malformed document can cost this parser a slot; it can + /// never invent one. + /// + /// Mutation: read fields from the document rather than the element's + /// range, or let a close-less element run to EOF → the trailing elements + /// re-enter the list as extra streams. + #[test] + fn an_unterminated_stream_element_shortens_the_list_it_cannot_extend_it() { + let sp = concat!( + "a0ENG", + // No `` for this one. + "a1FRA", + "a2DEU", + ); + let infos = parse_stream_infos(sp); + assert_eq!( + infos.iter().map(|i| i.id.as_str()).collect::>(), + vec!["a0", "a1"], + "the close-less element absorbs the one behind it — two slots, not \ + three, and never four" + ); + assert_eq!(infos[1].language, "fra", "and keeps its own leading fields"); + + // With no close tag anywhere behind it, the element is not returned at + // all and the walk ends — the tail of the document never becomes a + // stream list. + let no_close = "a0ENG"; + assert!(parse_stream_infos(no_close).is_empty()); + } + #[test] fn unusable_stream_element_still_occupies_its_position() { let sp = r#" diff --git a/src/labels/paramount.rs b/src/labels/paramount.rs index f778609..1debe7b 100644 --- a/src/labels/paramount.rs +++ b/src/labels/paramount.rs @@ -192,6 +192,57 @@ fn find_feature_playlist(text: &str) -> Option { mod tests { use super::*; + /// Immunity pin, section-boundary half. The pixelogic parser walks a flat + /// string sequence and recognises its feature section's END by marker + /// alone, so a section with no marker behind it runs off into whatever + /// follows and counts it as more STN slots. Nothing here can do that: the + /// stream list is one attribute of one XML element, so its length is the + /// CSV's own cell count and its scope is the element's byte range that + /// `xml::find_element` returns. Text after the element — including the + /// next playlist's own `aud` — is not reachable from it. + /// + /// And when the boundary is MISSING the failure is closed, not open: + /// `xml::find_element` needs a matching close tag and yields `None` + /// without one, so an unterminated element ends the walk rather than + /// swallowing the rest of the document. + /// + /// Mutation: hand `labels_from_feature` the document instead of the + /// element, or let an unterminated element run to EOF → the bonus + /// playlist's languages join the feature's stream list. + #[test] + fn a_playlists_stream_list_cannot_run_into_the_next_playlist() { + let doc = r#" + + + "#; + let feature = find_feature_playlist(doc).expect("feature playlist found"); + let labels = labels_from_feature(&feature); + let got: Vec<(StreamLabelType, u16, &str)> = labels + .iter() + .map(|l| (l.stream_type, l.stream_number, l.language.as_str())) + .collect(); + assert_eq!( + got, + vec![ + (StreamLabelType::Audio, 1, "eng"), + (StreamLabelType::Audio, 2, "fra"), + (StreamLabelType::Subtitle, 1, "eng"), + (StreamLabelType::Subtitle, 2, "spa"), + ], + "the CSV's own cells are the whole stream list" + ); + + // Same document with the feature element left unterminated. + let unterminated = r#" + + + "#; + assert!( + find_feature_playlist(unterminated).is_none(), + "a missing element boundary truncates the walk, never extends it" + ); + } + /// `sub_com1_idx` is an unbounded index list parsed straight out of the /// disc's `playlists.xml` and was membership-tested with a linear /// `Vec::contains` once per subtitle stream — quadratic in the size of a diff --git a/src/labels/pixelogic.rs b/src/labels/pixelogic.rs index 374e023..30124e2 100644 --- a/src/labels/pixelogic.rs +++ b/src/labels/pixelogic.rs @@ -20,6 +20,13 @@ const AUDIO_CODECS: &[&str] = &["MLP", "AC3", "DTS", "DDL", "WAV", "AC"]; /// overflowing the u16 STN counters (panic in debug, wrap-to-0 in /// release, which would misnumber subsequent labels). const MAX_STREAMS_PER_TYPE: u16 = 512; +/// Sane upper bound on the number of DISTINCT video-slot entries one section +/// may list before the walk gives up on it. A section's stream list opens with +/// its video slots, and [`assign_labels`] remembers them to recognise where the +/// NEXT section starts (see the loop body). The BD STN table admits one primary +/// video plus at most 32 secondary ones, so a section claiming more than that is +/// not a stream list — and the memo must not grow without bound on disc bytes. +const MAX_VIDEO_SLOTS: usize = 33; /// Known region tokens const REGIONS: &[&str] = &[ "US", "UK", "CF", "PF", "CS", "LS", "BP", "PP", "SM", "TM", "CAN", "DUM", "FLE", @@ -152,6 +159,9 @@ fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec = Vec::new(); for s in strings { // Detect feature section start @@ -168,6 +178,7 @@ fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec Vec= MAX_VIDEO_SLOTS { + break; + } + video_slots.push(s); + continue; + } + // Stop accumulating once both counters reach the sane cap — a // crafted blob can't drive them to u16 overflow. if audio_num >= MAX_STREAMS_PER_TYPE && sub_num >= MAX_STREAMS_PER_TYPE { @@ -269,8 +309,10 @@ fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec Option { if s.starts_with("Audio Stream") { Some(StreamLabelType::Audio) @@ -914,6 +956,122 @@ mod tests { assert_eq!(labels[0].language, "eng"); } + /// Spec: the feature section also ends where the NEXT section's stream + /// list starts, which is the only boundary available when the feature + /// playlist is the last NAMED (`SEG_`/`SF_`/`FPL_`) section in the blob. + /// + /// Shape taken from a corpus disc whose feature playlist is the last named + /// section: the trailing per-language notice/disclaimer cards are emitted + /// as unnamed sections, each opening with its own `Video Stream 1` / + /// `AR_…` pair and titled with a plain clip name. Those clip names are + /// `{lang3}_{card}`, so they pass the stream-token gate and each one + /// advances an STN counter; a card whose name collides with a catalogued + /// component (`AC` reads as the AC-3 codec) even emits a label, for an STN + /// slot the feature playlist does not have. On that disc the walk ran 95 + /// entries past the end of the feature's own list, fabricated five audio + /// labels at STN 10-14 (the playlist has 9 audio slots), and reported 94 + /// uncatalogued components — which also downgraded the whole parse from + /// High to Medium confidence. + /// + /// Mutation: drop the repeated-video-slot boundary → `deu_Warning` and + /// `fra_ND` advance the subtitle counter and `eng_AC` emits a phantom + /// Dolby Digital label on an audio slot that does not exist. + #[test] + fn assign_labels_section_ends_at_the_next_sections_video_slot() { + let mut flag = UnknownParts::default(); + let tokens = strs(&[ + "FPL_MainFeature", + "Video Stream 1", + "AR_169", + // Audio list: 2 STN slots. + "Audio Stream 1", + "eng_ADES_", + // PG list: 2 STN slots. + "PG Stream 1", + "eng_SDH_", + // End of the feature's list. No named section follows — the next + // section is a notice card, announced only by its own video slot. + "deu_Warning", + "Video Stream 1", + "AR_169", + "fra_ND", + "Video Stream 1", + "AR_169", + "eng_AC", + ]); + let labels = assign_labels(&tokens, &mut flag); + + let got: Vec<(StreamLabelType, u16, &str)> = labels + .iter() + .map(|l| (l.stream_type, l.stream_number, l.language.as_str())) + .collect(); + assert_eq!( + got, + vec![ + (StreamLabelType::Audio, 2, "eng"), + (StreamLabelType::Subtitle, 2, "eng"), + ], + "only the feature playlist's own slots are labelled" + ); + // A card's name is emitted BEFORE its own section's video slot, so a + // forward-only walk meets the first one while still nominally inside + // the feature section and counts it. That residue is one entry, at the + // tail of a list nothing follows in — it cannot renumber any label, + // only cost the parse its High confidence. Every card behind it is + // past the boundary and never seen. Pinned rather than papered over. + assert_eq!( + flag.seen.iter().map(String::as_str).collect::>(), + vec!["WARNING"], + "only the card sitting between the last slot and the boundary leaks" + ); + } + + /// Companion to the above: the boundary is a video slot the section has + /// ALREADY listed, not any video slot. A section may legitimately list a + /// secondary video stream alongside the primary, and that must not cut its + /// audio and PG lists short. + /// Mutation: break on the first `Video Stream` entry seen after the + /// section start → the commentary at audio STN 2 disappears. + #[test] + fn assign_labels_keeps_a_sections_distinct_video_slots() { + let mut flag = UnknownParts::default(); + let tokens = strs(&[ + "FPL_MainFeature", + "Video Stream 1", + "Video Stream 2", + "AR_169", + "Audio Stream 1", + "eng_ACOM_", + ]); + let labels = assign_labels(&tokens, &mut flag); + let audio: Vec<_> = labels + .iter() + .filter(|l| l.stream_type == StreamLabelType::Audio) + .collect(); + assert_eq!(audio.len(), 1); + assert_eq!(audio[0].stream_number, 2, "audio list is not cut short"); + assert_eq!(audio[0].purpose, LabelPurpose::Commentary); + } + + /// The memo of a section's video slots is built from disc bytes, so it is + /// bounded: past [`MAX_VIDEO_SLOTS`] distinct entries the section is not a + /// stream list and the walk stops instead of retaining them all. + /// Mutation: drop the length guard → the memo grows with the blob. + #[test] + fn assign_labels_video_slot_memo_is_bounded() { + let mut flag = UnknownParts::default(); + let mut tokens = vec!["FPL_MainFeature".to_string()]; + for i in 1..=(MAX_VIDEO_SLOTS + 50) { + tokens.push(format!("Video Stream {i}")); + } + tokens.push("eng_ACOM_".to_string()); + let labels = assign_labels(&tokens, &mut flag); + assert!( + labels.is_empty(), + "the walk stops once the video-slot memo is full" + ); + } + /// Spec: the two per-type caps are independent — the loop only stops /// early once BOTH audio and subtitle counters have reached /// `MAX_STREAMS_PER_TYPE`. Reaching the audio cap alone must not cut