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