diff --git a/src/labels/criterion.rs b/src/labels/criterion.rs index 6345201..ffd351c 100644 --- a/src/labels/criterion.rs +++ b/src/labels/criterion.rs @@ -263,6 +263,41 @@ mod tests { assert_eq!(nums, vec![1, 2, 1]); } + /// Immunity pin. `parse_stream_infos` emits one `StreamInfo` per + /// `*StreamInfos` element unconditionally — no filter, no `continue` — so + /// an element whose fields are missing or unrecognized still occupies its + /// position, and `assign_stream_numbers` still spends a number on it. + /// + /// That is the property that keeps this parser out of the failure mode + /// where a skipped entry pulls every later label one stream forward. It + /// is load-bearing for the fallback path specifically: with no + /// `playbackconfig.xml` the numbers come purely from position in this + /// list, so dropping an element there would shift the rest. + /// + /// Mutation: skip elements with an empty `ID`/`LangInfoID` → the two + /// real audio streams renumber to 1 and 2. + #[test] + fn unusable_stream_element_still_occupies_its_position() { + let sp = r#" + a0ENG_US + + a2FRACOMMENTARY + s0WAT + s1ENGSDH + "#; + let infos = parse_stream_infos(sp); + assert_eq!(infos.len(), 5, "every element yields a StreamInfo"); + let nums = + assign_stream_numbers(&infos, &HashMap::new()).expect("numbering space not exhausted"); + assert_eq!( + nums, + vec![1, 2, 3, 1, 2], + "the blank element owns audio slot 2, so the commentary is slot 3" + ); + assert_eq!(infos[2].purpose, LabelPurpose::Commentary); + assert_eq!(infos[4].qualifier, LabelQualifier::Sdh); + } + #[test] fn fallback_does_not_collide_with_partial_map() { // Map claims audio "a1" -> 1. The unmapped audio "a0" must NOT diff --git a/src/labels/ctrm.rs b/src/labels/ctrm.rs index 60961c7..6465eb8 100644 --- a/src/labels/ctrm.rs +++ b/src/labels/ctrm.rs @@ -557,6 +557,72 @@ mod tests { assert!(labels.is_empty()); } + /// Immunity pin. `language_streams.txt` states each stream's number in + /// field 3, so a row the parser cannot use is simply dropped — it can + /// never renumber the rows behind it. This is the property that keeps + /// this parser out of the STN-slot-shifting failure mode that bites + /// parsers which count positionally: there, a skipped entry silently + /// pulls every later label one stream forward. + /// + /// Mutation: replace `parts[2]` with a running per-type counter → the + /// three unusable rows here collapse the survivors onto 1/2 and 1. + #[test] + fn ls_stream_numbers_come_from_the_row_not_a_counter() { + let labels = parse_language_streams_text( + "id,audio_production,4,eng\n\ + id,audio_bonus_extended,5,eng\n\ + id,audio_production,0,fra\n\ + id,audio_production,7,fra\n\ + id,subtitle_production\n\ + id,subtitle_narrative,9,deu\n", + ); + let nums: Vec<(StreamLabelType, u16)> = labels + .iter() + .map(|l| (l.stream_type, l.stream_number)) + .collect(); + assert_eq!( + nums, + vec![ + (StreamLabelType::Audio, 4), + (StreamLabelType::Audio, 7), + (StreamLabelType::Subtitle, 9), + ], + "an unusable row drops out without shifting the numbering" + ); + assert_eq!(labels[2].qualifier, LabelQualifier::Forced); + } + + /// Immunity pin, `menu_base.prop` side: the number comes from the + /// entry's own `streamNumber` property, so a skipped entry (commented + /// out, `streamNumber=0`, neither audio nor subtitle) leaves the + /// surviving entries on their authored slots. + /// + /// Mutation: number by iteration order → the survivors collapse to 1/2. + #[test] + fn menu_base_stream_numbers_come_from_the_entry_not_a_counter() { + let labels = parse_props( + "#audio_0.class=AudioButton\n\ + #audio_0.streamNumber=1\n\ + audio_1.class=AudioButton\n\ + audio_1.streamNumber=0\n\ + audio_2.class=AudioButton\n\ + audio_2.streamNumber=6\n\ + other_1.class=SomeOtherButton\n\ + other_1.streamNumber=2\n\ + subtitle_1.class=SubtitleButton\n\ + subtitle_1.streamNumber=11\n", + ); + let nums: Vec<(StreamLabelType, u16)> = labels + .iter() + .map(|l| (l.stream_type, l.stream_number)) + .collect(); + assert_eq!( + nums, + vec![(StreamLabelType::Audio, 6), (StreamLabelType::Subtitle, 11),], + "skipped entries must not renumber the ones that survive" + ); + } + /// Spec: `eda` variant → `Descriptive` purpose. /// Mutation: miss the `eda` branch → purpose stays Normal. #[test] diff --git a/src/labels/dbp.rs b/src/labels/dbp.rs index 41ae6aa..3d84542 100644 --- a/src/labels/dbp.rs +++ b/src/labels/dbp.rs @@ -272,6 +272,44 @@ mod tests { assert_eq!(sub.qualifier, LabelQualifier::Sdh); } + /// Immunity pin. Every dbp label states its own slot in the `AudioN` / + /// `SubtitleN` token, so the numbering survives gaps and skipped entries + /// intact. Nothing here counts positionally, which is what keeps this + /// parser out of the failure mode where a skipped entry pulls every later + /// label one stream forward. + /// + /// Mutation: number by iteration order → `Audio4` becomes 2 and + /// `Subtitle3` becomes 1, silently rebinding both to other streams. + #[test] + fn stream_numbers_come_from_the_token_not_iteration_order() { + let class_bytes = build_class(&[ + "LTextField,Audio1,English Dolby Atmos,Fontstrip_Composite,296,763", + // Slots 2 and 3 have no menu TextField authored. + "LTextField,Audio4,French 5.1 Dolby Digital,Fontstrip_Composite,296,803", + // Not a stream: the disable-subtitles button. + "ATextField,Subtitle0,None,Fontstrip_Composite,1312,843", + // Unparseable slot token — dropped, and must shift nothing. + "HTextField,SubtitleX,German,Fontstrip_Composite,1312,883", + "HTextField,Subtitle3,English SDH,Fontstrip_Composite,1312,763", + ]); + let mut archive = build_jar(&[("com/dbp/Menu.class", class_bytes)]); + + let labels = scan_jar(&mut archive); + let nums: Vec<(StreamLabelType, u16)> = labels + .iter() + .map(|l| (l.stream_type, l.stream_number)) + .collect(); + assert_eq!( + nums, + vec![ + (StreamLabelType::Audio, 1), + (StreamLabelType::Audio, 4), + (StreamLabelType::Subtitle, 3), + ], + "unlabelled and unusable slots leave the authored numbers alone" + ); + } + /// A `CONSTANT_Utf8_info` carries a `u16` length (JVMS §4.4.7), so one /// crafted constant contributes up to 65535 bytes and the `u16` stream /// keyspace admits 65536 slots per type — ~4 GiB of retained `String` per diff --git a/src/labels/deluxe.rs b/src/labels/deluxe.rs index 42072c7..b6d4066 100644 --- a/src/labels/deluxe.rs +++ b/src/labels/deluxe.rs @@ -951,12 +951,16 @@ impl MasterEnumTable { /// on Deluxe don't carry a CodingType; their codec is implicit /// PGS via the BD spec). /// - Construction has Language but no CodingType → subtitle stream. -/// - No Language → not a stream (skip). +/// - Neither, and its binding type never yielded a stream → not a +/// stream (skip). See [`slot_kind`] for why the binding type is +/// consulted rather than the language alone. fn interpret_streams(constructions: &[Construction], master: &MasterEnumTable) -> Vec { let mut audio_idx: u16 = 0; let mut sub_idx: u16 = 0; let mut out = Vec::new(); + let slot_kinds = slot_kinds(constructions); + for c in constructions { let mut lang_ord: Option = None; let mut purpose_ord: Option = None; @@ -979,7 +983,26 @@ fn interpret_streams(constructions: &[Construction], master: &MasterEnumTable) - } } - let Some(lang_ord) = lang_ord else { continue }; + let Some(lang_ord) = lang_ord else { + // No language resolved. If the construction is still recognisably + // a stream binding it OCCUPIES its STN slot and must advance the + // counter — there is just nothing to label. Numbering only the + // slots that resolve renumbers the rest 1..N and lands every + // surviving label on the wrong stream. + // + // `saturating_add` is safe here where it would not be on the + // emitting path below: no label is produced, so parking the + // counter at `u16::MAX` binds nothing. The next slot that DOES + // resolve hits the `checked_add` guard and stops emission. + match slot_kind(c, coding_type.is_some(), &slot_kinds) { + Some(StreamLabelType::Audio) => audio_idx = audio_idx.saturating_add(1), + Some(StreamLabelType::Subtitle) => sub_idx = sub_idx.saturating_add(1), + // Not a stream binding (`new StringBuilder` and friends in the + // same ``): no slot, no counter. + None => {} + } + continue; + }; // Audio when a CodingType is present (audio binding type // always references org.bluray.ti.CodingType); subtitle @@ -1057,6 +1080,69 @@ fn interpret_streams(constructions: &[Construction], master: &MasterEnumTable) - out } +/// Which stream list each binding type enumerates, learned from the +/// constructions that DID resolve a language. +/// +/// A `` walk emits a [`Construction`] for every `new X; … ; +/// invokespecial X.` it sees, so the list mixes real stream bindings +/// with whatever else the class initializer builds. `binding_type` is the +/// constructed class name, which is how the two are told apart: the stream +/// bindings all share one class (Deluxe splits audio and subtitle across two), +/// and that class is identifiable from the slots that resolved. +/// +/// A binding type that resolved as both kinds is left out — with no consistent +/// answer, guessing a list to advance would be worse than not advancing. +fn slot_kinds(constructions: &[Construction]) -> HashMap<&str, Option> { + let mut kinds: HashMap<&str, Option> = HashMap::new(); + for c in constructions { + let mut has_lang = false; + let mut has_coding = false; + for arg in &c.args { + match arg { + StackVal::EnumRef { + kind: "Language", .. + } => has_lang = true, + StackVal::CodingType(_) => has_coding = true, + _ => {} + } + } + if !has_lang { + continue; + } + let kind = if has_coding { + StreamLabelType::Audio + } else { + StreamLabelType::Subtitle + }; + kinds + .entry(c.binding_type.as_str()) + .and_modify(|e| { + if *e != Some(kind) { + *e = None; + } + }) + .or_insert(Some(kind)); + } + kinds +} + +/// The stream list an unresolved construction occupies a slot in, or `None` +/// when it is not a stream binding. +/// +/// A `org.bluray.ti.CodingType` argument is decisive on its own: nothing but +/// an audio stream binding is handed one. Otherwise fall back to what the +/// binding type's resolved siblings showed (see [`slot_kinds`]). +fn slot_kind( + c: &Construction, + has_coding_type: bool, + slot_kinds: &HashMap<&str, Option>, +) -> Option { + if has_coding_type { + return Some(StreamLabelType::Audio); + } + slot_kinds.get(c.binding_type.as_str()).copied().flatten() +} + /// Map a `org.bluray.ti.CodingType` field name (as observed in /// getstatic operands on Deluxe binding classes) to a human-readable /// codec hint string. @@ -2716,6 +2802,87 @@ mod tests { assert_eq!(out[0].language, "eng"); } + /// Each stream binding in a binding class's `` is one STN slot, + /// in STN order — that is the whole basis for numbering them positionally + /// here. Whether the abstract interpreter managed to RESOLVE a slot's + /// language does not change how many slots the disc has: a `getstatic` + /// whose owning class was not fingerprinted as a master enum, or whose + /// field is missing from the resolved ordinal map, arrives as + /// `StackVal::Unknown`. + /// + /// A slot that resolved nothing has no label to emit, but it must still + /// consume its number. Skipping it renumbers every slot behind it and + /// binds their labels — language, commentary, descriptive-audio — one + /// stream early. + #[test] + fn interpret_streams_unresolved_slot_still_consumes_its_number() { + let audio_slot = |lang: Option| Construction { + binding_type: "AudioSlot".into(), + args: vec![ + match lang { + Some(ordinal) => StackVal::EnumRef { + kind: "Language", + ordinal, + }, + // Language `getstatic` the decoder could not resolve. + None => StackVal::Unknown, + }, + StackVal::CodingType("DOLBY_AC3_AUDIO".into()), + ], + }; + let sub_slot = |lang: Option| Construction { + binding_type: "SubtitleSlot".into(), + args: vec![match lang { + Some(ordinal) => StackVal::EnumRef { + kind: "Language", + ordinal, + }, + None => StackVal::Unknown, + }], + }; + + let constructions = vec![ + audio_slot(Some(0)), // audio STN 1 — English + audio_slot(None), // audio STN 2 — unresolved + audio_slot(Some(1)), // audio STN 3 — French + sub_slot(Some(0)), // PG STN 1 — English + sub_slot(None), // PG STN 2 — unresolved + sub_slot(Some(2)), // PG STN 3 — Spanish + // Not a stream binding at all: no language, no CodingType, and a + // binding type that never yielded a stream. Must not take a slot. + Construction { + binding_type: "java/lang/StringBuilder".into(), + args: Vec::new(), + }, + sub_slot(Some(1)), // PG STN 4 — French + ]; + + let out = interpret_streams(&constructions, &lang_enum_master()); + + let audio: Vec<_> = out + .iter() + .filter(|l| l.stream_type == StreamLabelType::Audio) + .map(|l| (l.language.as_str(), l.stream_number)) + .collect(); + assert_eq!( + audio, + vec![("eng", 1), ("fra", 3)], + "the unresolved audio slot owns STN 2" + ); + + let sub: Vec<_> = out + .iter() + .filter(|l| l.stream_type == StreamLabelType::Subtitle) + .map(|l| (l.language.as_str(), l.stream_number)) + .collect(); + assert_eq!( + sub, + vec![("eng", 1), ("spa", 3), ("fra", 4)], + "the unresolved PG slot owns STN 2; the non-stream construction \ + owns nothing" + ); + } + #[test] fn interpret_streams_purpose_routed_through_deluxe_enum() { let constructions = vec![Construction { diff --git a/src/labels/mpls_universal.rs b/src/labels/mpls_universal.rs index 6b293b9..ab09d0a 100644 --- a/src/labels/mpls_universal.rs +++ b/src/labels/mpls_universal.rs @@ -110,14 +110,8 @@ fn build_labels(playlists: &[crate::mpls::Playlist]) -> Vec { for playlist in playlists { for entry in &playlist.streams { - let label_type = match entry.stream_type { - 2 | 5 => StreamLabelType::Audio, // primary + secondary audio - 3 => StreamLabelType::Subtitle, // PG subtitle - // 1 = primary video, 6 = secondary video, 7 = DV EL - // → no StreamLabelType variant for video, skip. - // 4 = IG (interactive graphics) — not a user-facing - // stream, skip. - _ => continue, + let Some(label_type) = label_type_for(entry) else { + continue; }; let language = normalize_language(&entry.language); @@ -156,6 +150,37 @@ fn build_labels(playlists: &[crate::mpls::Playlist]) -> Vec { labels } +/// Which per-type numbering list an STN entry belongs to, or `None` when it +/// is not a labellable stream at all. +/// +/// This MUST agree with the stream list `disc::bluray` builds from the same +/// entries, because that list is what `labels::apply_labels` counts against +/// when it binds `stream_number`. The two counters run over the same STN +/// entries in the same order, so any entry one side keeps and the other drops +/// — or files under a different type — shifts every later label of that type +/// onto the wrong stream. Three rules, all mirroring `disc::bluray`: +/// +/// * `coding_type == 0` is the STN table's empty/padding slot. Not a +/// stream on either side. +/// * a PG coding_type in an audio STN slot is a subtitle, not audio. +/// `mpls::parse_stream_entry` has a dedicated arm for this layout, so it +/// is an authored shape rather than a corruption. +/// * video (1 / 6 / 7 = primary, secondary, Dolby Vision EL) and IG (4) +/// have no `StreamLabelType`; they are numbered in their own STN lists +/// and never interleave with the audio or PG lists. +fn label_type_for(entry: &crate::mpls::StreamEntry) -> Option { + use crate::consts::coding_type as c; + if entry.coding_type == 0 { + return None; + } + match entry.stream_type { + 2 | 5 if entry.coding_type == c::PG => Some(StreamLabelType::Subtitle), + 2 | 5 => Some(StreamLabelType::Audio), + 3 => Some(StreamLabelType::Subtitle), + _ => None, + } +} + fn has_mpls_extension(name: &str) -> bool { // Case-insensitive ".mpls" suffix. Some discs use uppercase, // some lowercase; UDF filenames preserve case but we don't. @@ -412,6 +437,72 @@ mod tests { assert_eq!(labels[2].language, "fra"); } + /// `stream_number` is bound by `labels::apply_labels` against the title's + /// own stream list, which `disc::bluray` builds from these same STN + /// entries. That builder DROPS an entry whose `coding_type` is 0 — the + /// STN table's empty/padding slot — so it must not be counted here + /// either. Counting it advances the audio counter past a stream that + /// never materializes, and every label behind it binds one stream late. + #[test] + fn padding_stn_entry_does_not_consume_a_label_slot() { + let pl = playlist_with(vec![ + audio_entry(0x1100, 0x83, 12, 1, "eng"), + // coding_type 0: STN padding. Not a stream. + audio_entry(0x1101, 0x00, 0, 0, ""), + audio_entry(0x1102, 0x81, 6, 1, "fra"), + ]); + let labels = labels_from_playlists(&[pl]); + assert_eq!(labels.len(), 2, "the padding slot yields no label"); + assert_eq!(labels[0].language, "eng"); + assert_eq!(labels[0].stream_number, 1); + assert_eq!(labels[1].language, "fra"); + assert_eq!( + labels[1].stream_number, 2, + "padding is absent from the title's stream list, so `fra` is \ + audio stream 2" + ); + } + + /// A PG coding_type sitting in an audio STN slot is a real, documented + /// shape — `mpls::parse_stream_entry` has an explicit arm for it, and + /// `disc::bluray` builds it as a Subtitle stream, not an Audio one. This + /// module must classify it the same way, or the audio counter runs one + /// ahead and the subtitle counter one behind for every later stream. + #[test] + fn pg_coding_type_in_an_audio_slot_counts_as_a_subtitle() { + let mut misplaced = audio_entry(0x1200, 0x90, 0, 0, "spa"); + misplaced.stream_type = 2; + let pl = playlist_with(vec![ + audio_entry(0x1100, 0x83, 12, 1, "eng"), + misplaced, + audio_entry(0x1101, 0x81, 6, 1, "fra"), + pg_entry(0x1201, "deu"), + ]); + let labels = labels_from_playlists(&[pl]); + + let audio: Vec<_> = labels + .iter() + .filter(|l| l.stream_type == StreamLabelType::Audio) + .map(|l| (l.language.as_str(), l.stream_number)) + .collect(); + assert_eq!( + audio, + vec![("eng", 1), ("fra", 2)], + "the PG entry is not an audio stream and must not number one" + ); + + let sub: Vec<_> = labels + .iter() + .filter(|l| l.stream_type == StreamLabelType::Subtitle) + .map(|l| (l.language.as_str(), l.stream_number)) + .collect(); + assert_eq!( + sub, + vec![("spa", 1), ("deu", 2)], + "it is subtitle stream 1, ahead of the PG-slot entry" + ); + } + #[test] fn dedup_streams_across_playlists() { // Two playlists, same English TrueHD 7.1 PID 0x1100 in both. diff --git a/src/labels/paramount.rs b/src/labels/paramount.rs index 0d028a6..f778609 100644 --- a/src/labels/paramount.rs +++ b/src/labels/paramount.rs @@ -59,13 +59,26 @@ fn labels_from_feature(feature: &str) -> Vec { .map(|s| s.split(',').filter_map(|i| i.trim().parse().ok()).collect()) .unwrap_or_default(); - // stream_number must match apply_labels' monotonic 1-based - // per-type counter, which increments once per *real* stream — so - // it counts only non-empty slots, not the raw CSV index. The - // commentary index comparison stays on the raw CSV index `i`, - // since aud_com1_idx is positional against the original CSV. - let mut audio_num: u16 = 0; + // The CSV *is* the STN list: one cell per stream, in stream order, + // and `aud_com1_idx` is a 0-based index into those same cells. So + // `stream_number` is the cell's own 1-based position — NOT a counter + // that only advances on cells carrying a language. + // + // A cell with an empty language still occupies its STN slot; it just + // has nothing to label. Renumbering the surviving cells 1..N shifts + // every label behind an empty cell one slot forward, which is how a + // marker authored for one stream ends up written onto the stream in + // front of it (see the subtitle side, where the marker is `forced`). + // + // `u16::try_from` rather than `saturating_add`: past the 1-based u16 + // numbering space every cell would collapse onto `u16::MAX`, binding + // several streams to one label. Stop emitting instead. Unreachable on + // real media — the BD STN_table admits at most 32 primary audio + // streams per playlist. for (i, lang) in aud.split(',').enumerate() { + let Ok(stream_number) = u16::try_from(i + 1) else { + break; + }; let lang = lang.trim(); if lang.is_empty() { continue; @@ -75,9 +88,8 @@ fn labels_from_feature(feature: &str) -> Vec { } else { LabelPurpose::Normal }; - audio_num = audio_num.saturating_add(1); labels.push(StreamLabel { - stream_number: audio_num, + stream_number, stream_type: StreamLabelType::Audio, language: lang.to_string(), name: String::new(), @@ -101,10 +113,15 @@ fn labels_from_feature(feature: &str) -> Vec { .map(|s| s.split(',').filter_map(|i| i.trim().parse().ok()).collect()) .unwrap_or_default(); - // As with audio: count only non-empty slots for stream_number, - // but keep com/forced lookups on the raw CSV index `i`. - let mut sub_num: u16 = 0; + // As with audio: the cell position IS the STN slot. `forced_sub` and + // `sub_com1_idx` are indexed against those same cells, so an empty + // cell must not renumber the cells behind it — a forced marker + // authored for one PG slot would otherwise be written onto an + // earlier, full-dialogue subtitle track. for (i, lang) in sub.split(',').enumerate() { + let Ok(stream_number) = u16::try_from(i + 1) else { + break; + }; let lang = lang.trim(); if lang.is_empty() { continue; @@ -122,9 +139,8 @@ fn labels_from_feature(feature: &str) -> Vec { LabelQualifier::None }; - sub_num = sub_num.saturating_add(1); labels.push(StreamLabel { - stream_number: sub_num, + stream_number, stream_type: StreamLabelType::Subtitle, language: lang.to_string(), name: String::new(), @@ -182,18 +198,26 @@ mod tests { /// single attacker-supplied file. /// /// Proof is by deadline rather than micro-benchmark. With the linear scan - /// this fixture (200 000 streams x 1 000 001 indices) measures 31 s in a - /// release build and far longer in debug; with a set it measures 0.03 s - /// release / 0.56 s debug. A 10 s deadline sits ~18x above the slowest - /// passing measurement and ~3x below the fastest failing one, and makes a - /// regression fail fast instead of hanging CI. + /// the original fixture (200 000 streams x 1 000 001 indices) measured + /// 31 s in a release build and far longer in debug; with a set it measured + /// 0.03 s release / 0.56 s debug. A 10 s deadline sits ~18x above the + /// slowest passing measurement and ~3x below the fastest failing one, and + /// makes a regression fail fast instead of hanging CI. + /// + /// The CSV now stops at the end of the 1-based `u16` stream-numbering + /// space, so only the first 65 535 cells are scanned. `INDICES` is raised + /// to keep the linear-scan work product (`cells x indices`) at or above + /// the original fixture's, preserving that deadline margin. /// /// Correctness is pinned on fixture-derived literals: indices 0, 2 and 4 /// are the commentary tracks, 1 and 3 are not. #[test] fn commentary_index_lookup_is_not_quadratic() { + /// Cells offered. Everything past `u16::MAX` is unnumberable and the + /// parser stops there, so the scanned prefix is 65 535 cells. const STREAMS: usize = 200_000; - const INDICES: usize = 1_000_000; + const SCANNED: usize = u16::MAX as usize; + const INDICES: usize = 3_100_000; let (tx, rx) = std::sync::mpsc::channel(); let worker = std::thread::spawn(move || { let mut feature = String::from(r#" { worker.join().expect("worker panicked"); - assert_eq!(labels.len(), STREAMS); + assert_eq!(labels.len(), SCANNED); assert_eq!(labels[0].purpose, LabelPurpose::Commentary); assert_eq!(labels[1].purpose, LabelPurpose::Normal); assert_eq!(labels[2].purpose, LabelPurpose::Commentary); @@ -259,11 +283,56 @@ mod tests { .collect() } + /// The `aud` / `sub` CSVs are the vendor's STN-ordered stream lists: one + /// slot per stream, and `aud_com1_idx` / `forced_sub` are indexed against + /// those same slot positions. A slot whose language cell is empty carries + /// nothing to label but still OCCUPIES its slot, so it must not renumber + /// the slots behind it. + /// + /// Numbering only the slots that carry a language collapsed every later + /// label one position forward per empty cell, which is how a forced + /// marker authored for one STN slot lands on the full-subtitle track in + /// front of it. #[test] - fn empty_middle_slot_does_not_inflate_stream_number() { - // aud="eng,,fra": the empty middle slot is skipped, and the - // second real stream (fra) must be numbered 2, matching - // apply_labels' monotonic counter — not 3 (its raw CSV index). + fn empty_csv_slot_still_occupies_its_stn_slot() { + // Audio: slot 2 is empty; `fra` is STN slot 3 and is the commentary + // the vendor pointed at with the 0-based CSV index 2. + let feature = r#""#; + let labels = labels_from_feature(feature); + let a = audio(&labels); + assert_eq!(a.len(), 2, "the empty slot carries no label"); + assert_eq!(a[0].language, "eng"); + assert_eq!(a[0].stream_number, 1); + assert_eq!(a[1].language, "fra"); + assert_eq!( + a[1].stream_number, 3, + "an empty CSV cell occupies STN slot 2, so `fra` is slot 3" + ); + assert_eq!(a[1].purpose, LabelPurpose::Commentary); + + // Subtitles: same shape, and the consequence is a misplaced forced + // flag. `forced_sub` index 2 is the forced-narrative track; with the + // empty slot renumbered away it would be written onto STN slot 2. + let feature = r#""#; + let labels = labels_from_feature(feature); + let s = subs(&labels); + assert_eq!(s.len(), 2); + assert_eq!(s[0].language, "eng"); + assert_eq!(s[0].stream_number, 1); + assert_eq!(s[0].qualifier, LabelQualifier::None); + assert_eq!(s[1].language, "fra"); + assert_eq!( + s[1].stream_number, 3, + "the forced marker belongs to STN slot 3, not slot 2" + ); + assert_eq!(s[1].qualifier, LabelQualifier::Forced); + } + + #[test] + fn empty_middle_slot_carries_no_label_but_keeps_its_slot() { + // aud="eng,,fra": the empty middle cell yields no label — there is + // nothing to label — but it still owns STN slot 2, so `fra` is slot + // 3. (This test previously asserted 2, pinning the renumbering bug.) let feature = r#""#; let labels = labels_from_feature(feature); let a = audio(&labels); @@ -271,7 +340,7 @@ mod tests { assert_eq!(a[0].language, "eng"); assert_eq!(a[0].stream_number, 1); assert_eq!(a[1].language, "fra"); - assert_eq!(a[1].stream_number, 2); + assert_eq!(a[1].stream_number, 3); } #[test] @@ -279,7 +348,7 @@ mod tests { // Whitespace around the index, and a multi-value list, must both // resolve. com index is positional against the raw CSV, so with // an empty slot at position 1, " 2 " marks the 'fra' track - // (CSV index 2) as commentary. + // (CSV index 2, STN slot 3) as commentary. let feature = r#""#; let labels = labels_from_feature(feature); let a = audio(&labels); @@ -293,7 +362,7 @@ mod tests { fn forced_sub_aligns_with_raw_csv_index() { // sub="eng,eng,zho,ces" forced_sub="0,0,0,1": the forced flag is // positional on the raw CSV, so 'ces' (index 3) is forced; its - // stream_number is its non-empty position (4 here, no gaps). + // stream_number is its 1-based cell position, 4. let feature = r#""#; let labels = labels_from_feature(feature); let s = subs(&labels); @@ -339,10 +408,12 @@ mod tests { assert!(feature.contains(r#"name="MainMovie""#)); } - /// Spec: stream_number for audio is 1-based and increments only on non-empty slots. - /// Mutation: increment for empty slots too → stream numbers inflate. + /// Spec: stream_number for audio is the cell's own 1-based CSV position, + /// because the CSV is the STN list and empty cells are slots too. + /// Mutation: count only non-empty cells → every label behind an empty + /// cell shifts one slot forward. #[test] - fn audio_stream_numbering_skips_empty_slots() { + fn audio_stream_numbering_uses_raw_csv_slot_position() { let feature = r#""#; let labels = labels_from_feature(feature); let a = audio(&labels); @@ -350,9 +421,9 @@ mod tests { assert_eq!(a[0].language, "eng"); assert_eq!(a[0].stream_number, 1); assert_eq!(a[1].language, "fra"); - assert_eq!(a[1].stream_number, 2); + assert_eq!(a[1].stream_number, 3); assert_eq!(a[2].language, "spa"); - assert_eq!(a[2].stream_number, 3); + assert_eq!(a[2].stream_number, 5); } /// Spec: forced subtitle at the last position with gaps in between. @@ -361,7 +432,7 @@ mod tests { #[test] fn forced_sub_uses_raw_csv_index_with_gaps() { // sub="eng,,fra,,spa" forced_sub="0,0,0,0,1" - // raw CSV index 4 = "spa"; stream_number for spa = 3 (3rd non-empty). + // raw CSV index 4 = "spa", i.e. STN slot 5. let feature = r#""#; let labels = labels_from_feature(feature); let s = subs(&labels); @@ -372,6 +443,7 @@ mod tests { assert_eq!(s[1].qualifier, LabelQualifier::None); assert_eq!(s[2].language, "spa"); assert_eq!(s[2].qualifier, LabelQualifier::Forced); + assert_eq!(s[2].stream_number, 5); } /// Spec: aud_com1_idx is positional against the raw CSV. @@ -380,13 +452,14 @@ mod tests { /// Mutation: use stream_number instead of raw CSV index → wrong stream is commentary. #[test] fn audio_commentary_index_raw_csv_position() { - // aud="eng,,fra,spa" aud_com1_idx="2" → CSV index 2 = "fra". - // "fra" is stream_number 2 (second non-empty slot, skipping the empty). + // aud="eng,,fra,spa" aud_com1_idx="2" → CSV index 2 = "fra", + // which is STN slot 3. let feature = r#""#; let labels = labels_from_feature(feature); let a = audio(&labels); assert_eq!(a.len(), 3); assert_eq!(a[1].language, "fra"); + assert_eq!(a[1].stream_number, 3); assert_eq!(a[1].purpose, LabelPurpose::Commentary); assert_eq!(a[0].purpose, LabelPurpose::Normal); assert_eq!(a[2].purpose, LabelPurpose::Normal); @@ -429,10 +502,11 @@ mod tests { assert!(s.is_empty(), "no subtitle labels when sub is absent"); } - /// Spec: audio stream_number uses saturating_add on overflow (per u16 cap). - /// Mutation: use wrapping_add → stream numbers wrap to 0, skipping apply. + /// Spec: audio stream_number is the cell's 1-based position and never + /// wraps; past the u16 space the parser stops emitting. + /// Mutation: cast `i + 1` to u16 → stream numbers wrap to 0, skipping apply. #[test] - fn audio_stream_number_saturates_not_wraps() { + fn audio_stream_number_never_wraps() { // 65535 audio tracks is impossible on a real disc but the parser must // not panic or produce 0. Build a comma-separated list of 65535 "eng"s. // We only run the number-assignment logic via labels_from_feature. diff --git a/src/labels/png_filenames.rs b/src/labels/png_filenames.rs index 7d92292..f444b57 100644 --- a/src/labels/png_filenames.rs +++ b/src/labels/png_filenames.rs @@ -1,8 +1,8 @@ //! Menu-graphic filename language hints. //! //! Some BD-J discs encode per-language menu artwork with the language in the -//! filename, e.g. `Dune_UHD01_Eng_Composite1.png`, -//! `VForVendetta_UHD01_FRE_Composite2.png`. The `_UHD01_{LANG}_Composite` +//! filename, e.g. `Feature_UHD01_Eng_Composite1.png`, +//! `AltFeature_UHD01_FRE_Composite2.png`. The `_UHD01_{LANG}_Composite` //! marker is authored deliberately, so the set of `{LANG}` tokens is the set //! of menu languages the disc ships. //! @@ -91,10 +91,16 @@ mod tests { #[test] fn extracts_confirmed_samples() { - assert_eq!(filename_lang("Dune_UHD01_Eng_Composite1.png"), Some("eng")); - assert_eq!(filename_lang("Dune_UHD01_Ger_Composite2.png"), Some("deu")); assert_eq!( - filename_lang("VForVendetta_UHD01_FRE_Composite2.png"), + filename_lang("Feature_UHD01_Eng_Composite1.png"), + Some("eng") + ); + assert_eq!( + filename_lang("Feature_UHD01_Ger_Composite2.png"), + Some("deu") + ); + assert_eq!( + filename_lang("AltFeature_UHD01_FRE_Composite2.png"), Some("fra") ); } @@ -120,9 +126,9 @@ mod tests { #[test] fn dedups_and_numbers_distinct_languages() { let names = vec![ - "Dune_UHD01_Eng_Composite1.png".to_string(), - "Dune_UHD01_Eng_Composite2.png".to_string(), - "Dune_UHD01_Ger_Composite1.png".to_string(), + "Feature_UHD01_Eng_Composite1.png".to_string(), + "Feature_UHD01_Eng_Composite2.png".to_string(), + "Feature_UHD01_Ger_Composite1.png".to_string(), "LoadingComposite1.png".to_string(), ]; let labels = labels_from_filenames(&names); diff --git a/src/labels/vocab.rs b/src/labels/vocab.rs index 0c0aa15..03ae0eb 100644 --- a/src/labels/vocab.rs +++ b/src/labels/vocab.rs @@ -177,7 +177,7 @@ const BARE_LANGS: &[(&str, &str)] = &[ ]; /// Map a short menu-graphic language token (as embedded in authoring -/// filenames like `Dune_UHD01_Eng_Composite1.png`) to an ISO-639-2/T code. +/// filenames like `Feature_UHD01_Eng_Composite1.png`) to an ISO-639-2/T code. /// /// These filename tokens are compact 2/3-letter abbreviations, NOT the full /// language names [`lang`] handles, so they get their own certain table.