Merge branch 'fix/label-pid-binding' into dev

CHANGELOG.md

@@ -104,6 +104,61 @@
   whose authoring sets `forced_on_flag` at all. Across the disc-image corpus
   this cleared every cross-title label conflict, on both affected vendor
   formats.
+- **Streams borrowed from the playlists were merged into the vendor label list
+  by a number that meant something else, so a bonus clip could be labelled from
+  the feature's tracks.** A vendor label's `stream_number` is a slot in the one
+  stream table its config blob describes. The labels merged in from the
+  playlists to cover streams the vendor named nothing for carried a different
+  number entirely: a dense counter over every distinct stream found while
+  scanning the whole disc in directory order, related to no playlist's slot
+  numbering at all. The merge matched the two by equality, and the binder then
+  counted streams against the result. Measured across the 44-image corpus: 22
+  discs merge such labels, and of the 566 places one lands on a stream, 443
+  (78%) are a stream it does not describe — the label states which PID it read
+  itself from, and it is a different one. 142 of those were already stopped by
+  the language check added alongside the ordinal binding; 301 were applied. The
+  streams-only labels carry no editorial payload, so the direct damage is
+  confined to codec text, but the polluted list is also what the anchor gate
+  reads, and on 11 disc/stream-type pairs it is what decides the anchor — which
+  is how it reaches the vendor's forced and SDH flags. The same defect ran
+  through the clip-info orphan streams, numbered from `max + 1` of a list they
+  share no coordinate system with.
+
+  A label now either NAMES the elementary stream it describes — `(clip, PID)`,
+  read out of the very table the stream itself is built from — or it does not,
+  and only the ones that do not are ever reached by counting. Playlist- and
+  clip-info-derived labels bind by that name and by nothing else; the vendor's
+  bind through the language-sequence anchor as before, over the vendor's own
+  slots only. A named stream outranks a guessed one, so an editorial flag
+  reaches a stream only where the disc's own numbering puts it there. The
+  presence of the name is the provenance marker, which is what the anchor gate
+  was missing: it can now tell the vendor's slots from the borrowed ones, so a
+  slot the vendor never named no longer breaks the sequence, and a title with
+  fewer streams than the list has slots is no longer eligible to hold it. An
+  orphan stream, being in no playlist, is in no title, and now binds to nothing
+  rather than to whatever counted its way.
+
+  Over the corpus, 41 of 44 images are byte-identical and every one of the
+  three that move loses a label it should never have had: a dozen featurette
+  playlists stop reporting a feature subtitle's SDH marking on their own
+  single, unrelated subtitle; eleven menu and bonus titles stop advertising the
+  feature's object-audio format on plain stereo tracks; and on a disc whose
+  every title carries one audio stream, a regional-variant tag that had been
+  asserted on all seventeen titles is asserted on none — that disc's tables are
+  too short to anchor anything, so the tag is no longer claimed anywhere, and
+  in exchange every title now states the codec it actually carries, which none
+  of them did before. No feature title changes on any image. Six of the crate's
+  own tests had been asserting the invented numbering, including one pinning
+  the disc-global counter as a deliberate property.
+- **A subtitle the content probe demoted went on calling itself forced.** The
+  probe writes its verdict to the stream's `forced` flag but left the
+  qualifier alone, and those are two renderings of one fact for two different
+  consumers: the muxer writes Matroska `FlagForced` from the flag, the JSON
+  metadata sidecar writes its qualifier string from the qualifier. A track the
+  probe cleared therefore shipped with a sidecar calling it forced next to a
+  header saying it is not. The demotion now clears the qualifier with the flag.
+  Only a forced claim is cleared — an SDH marking says something the probe
+  neither confirmed nor refuted, and is left alone.
 - **Vendor stream labels were numbered by parsed entry, not by stream slot.**
   Label blobs contain entries the parser deliberately does not interpret, but
   those entries still occupy a stream-number slot. Counting only the parsed

src/disc/pgs_forced_probe.rs

@@ -854,6 +854,21 @@ fn apply_verdicts(title: &mut DiscTitle, verdicts: &HashMap<u16, bool>) {
             && let Some(&forced) = verdicts.get(&sub.pid)
         {
             sub.forced = forced;
+            // A demoted track must not go on describing itself as forced. The
+            // flag and the qualifier are two renderings of one fact for
+            // different consumers — the muxer writes `FlagForced` from
+            // `forced`, the JSON sidecar writes its qualifier string from
+            // `qualifier` — so leaving `Forced` behind here published a track
+            // whose sidecar said "forced" and whose Matroska header said it was
+            // not. The probe read the content; it outranks the vendor's claim.
+            //
+            // Only this direction is a contradiction. `qualifier == None` on a
+            // track the probe promoted is not one: `None` is the absence of an
+            // editorial qualifier, not an assertion that the track is ordinary,
+            // and the vendor never claimed otherwise.
+            if !forced && sub.qualifier == crate::disc::LabelQualifier::Forced {
+                sub.qualifier = crate::disc::LabelQualifier::None;
+            }
         }
     }
 }
@@ -2045,6 +2060,52 @@ mod tests {
         })
     }
 
+    /// Spec: a verdict that DEMOTES a track clears a `Forced` qualifier with
+    /// it. The two fields are one fact rendered for two consumers — the muxer
+    /// writes Matroska `FlagForced` from `forced`, the JSON metadata sidecar
+    /// writes its qualifier string from `qualifier` — so leaving the qualifier
+    /// behind publishes a track that calls itself forced next to a header that
+    /// says it is not.
+    ///
+    /// Mutation: delete the qualifier assignment in `apply_verdicts` — the
+    /// track comes out `forced == false, qualifier == Forced`.
+    #[test]
+    fn a_demoted_track_stops_calling_itself_forced() {
+        let mut title = pgs_title(0x1200, true);
+        if let Stream::Subtitle(sub) = &mut title.streams[0] {
+            sub.qualifier = LabelQualifier::Forced;
+        }
+        apply_verdicts(&mut title, &HashMap::from([(0x1200u16, false)]));
+        let Stream::Subtitle(sub) = &title.streams[0] else {
+            unreachable!()
+        };
+        assert!(!sub.forced);
+        assert_eq!(
+            sub.qualifier,
+            LabelQualifier::None,
+            "the content outranks the vendor's claim, and both renderings of it move together"
+        );
+    }
+
+    /// Spec: a qualifier that is not a forced claim is not the probe's to
+    /// touch. `Sdh` says something about the track's content that a
+    /// forced-narrative verdict neither confirms nor refutes.
+    ///
+    /// Mutation: clear the qualifier unconditionally on demotion — the SDH
+    /// marking is lost.
+    #[test]
+    fn a_demoted_track_keeps_a_qualifier_that_is_not_a_forced_claim() {
+        let mut title = pgs_title(0x1200, true);
+        if let Stream::Subtitle(sub) = &mut title.streams[0] {
+            sub.qualifier = LabelQualifier::Sdh;
+        }
+        apply_verdicts(&mut title, &HashMap::from([(0x1200u16, false)]));
+        let Stream::Subtitle(sub) = &title.streams[0] else {
+            unreachable!()
+        };
+        assert_eq!(sub.qualifier, LabelQualifier::Sdh);
+    }
+
     fn forced_flag(title: &DiscTitle, pid: u16) -> bool {
         title
             .streams

src/labels/criterion.rs

@@ -47,6 +47,7 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult>
     let mut labels = Vec::new();
     for (info, &stream_num) in stream_infos.iter().zip(stream_nums.iter()) {
         labels.push(StreamLabel {
+            stream_id: None,
             stream_number: stream_num,
             stream_type: info.stream_type,
             language: info.language.clone(),

src/labels/ctrm.rs

@@ -207,6 +207,7 @@ fn parse_language_streams_text(text: &str) -> Vec<StreamLabel> {
         }
 
         labels.push(StreamLabel {
+            stream_id: None,
             stream_number: stream_num,
             stream_type,
             language,
@@ -401,6 +402,7 @@ mod tests {
 
     fn lbl(t: StreamLabelType, n: u16, name: &str) -> StreamLabel {
         StreamLabel {
+            stream_id: None,
             stream_number: n,
             stream_type: t,
             language: String::new(),
@@ -910,6 +912,7 @@ fn parse_menu_base_text(text: &str) -> Vec<StreamLabel> {
             .unwrap_or_default();
 
         labels.push(StreamLabel {
+            stream_id: None,
             stream_number: stream_num,
             stream_type,
             language,

src/labels/dbp.rs

@@ -171,6 +171,7 @@ fn make_label(num: u16, label: String, stream_type: StreamLabelType) -> StreamLa
     let qualifier = vocab::qualifier(&label);
     let purpose = vocab::purpose(&label);
     StreamLabel {
+        stream_id: None,
         stream_number: num,
         stream_type,
         language,

src/labels/deluxe.rs

@@ -1066,6 +1066,7 @@ fn interpret_streams(constructions: &[Construction], master: &MasterEnumTable) -
         }
 
         out.push(StreamLabel {
+            stream_id: None,
             stream_number,
             stream_type,
             language,

src/labels/mpls_universal.rs

@@ -86,44 +86,50 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult>
     Some(ParseResult::low(labels))
 }
 
-/// Convert every stream entry across `playlists` into deduped
+/// Convert every stream entry across `playlists` into one [`StreamLabel`] per
-/// [`StreamLabel`]s. Factored out of [`parse`] so unit tests can drive
+/// physical stream. Factored out of [`parse`] so unit tests can drive the
-/// the actual conversion logic (stream-type mapping, dedup key, dense
+/// actual conversion logic (stream-type mapping, identity, slot numbering)
-/// global counters) directly from already-parsed [`crate::mpls::Playlist`]
+/// directly from already-parsed [`crate::mpls::Playlist`] values, without
-/// values, without needing a synthetic on-disc UDF image.
+/// needing a synthetic on-disc UDF image.
+///
+/// Identity is `(clip, PID)` — what the STN entry states — and it is both the
+/// dedup key and the label's [`StreamId`]. A stream twenty playlists list is
+/// one label; two clips that both open their first audio at 0x1100 are two.
+/// This replaced a disc-global dense counter that numbered surviving entries
+/// 1, 2, 3, … in playlist-directory order: that number was not an STN slot in
+/// anything, but it was handed to a binder that reads `stream_number` as one.
 fn build_labels(playlists: &[crate::mpls::Playlist]) -> Vec<StreamLabel> {
+    use std::collections::HashSet;
     let mut labels: Vec<StreamLabel> = Vec::new();
-    // (stream_type_tag, language, codec_hint, pid) — PID is the
+    let mut seen: HashSet<super::StreamId> = HashSet::new();
-    // canonical "same physical stream" key; type+lang+codec round
-    // out the rare case where two distinct logical streams happen
-    // to share a PID across playlists with different metadata.
-    let mut seen: Vec<(StreamLabelType, String, String, u16)> = Vec::new();
 
-    // Global 1-based counters keyed by StreamLabelType. Incremented
+    for playlist in playlists {
-    // only when an entry survives dedup, so stream_numbers are dense
+        // `Playlist::streams` is the FIRST play item's STN table, so every
-    // (1, 2, 3, ...) per type across the whole disc — not reset per
+        // entry here is a stream of that play item's clip — the same clip
-    // playlist. A disc with 2 MPLS files that each list the same
+        // `disc::bluray` records as the title's `clips[0]`. That pairing is
-    // 8 audio streams ends up with audio_1..audio_8, not audio_1..
+        // what makes the PID an identity rather than a 16-bit number.
-    // audio_16 or audio_1..audio_8 with audio_1 duplicated.
+        //
+        // Streams cannot be non-empty without a play item to have read them
+        // from, so the empty case is unreachable on a real disc; entries we
+        // cannot identify are skipped rather than emitted as unbindable
+        // labels.
+        let Some(clip_id) = playlist.play_items.first().map(|pi| pi.clip_id.clone()) else {
+            continue;
+        };
+
+        // 1-based STN slot within THIS playlist's table, per type — the
+        // `stream_number` field's documented meaning, counted the same way
+        // `disc::bluray` counts the stream list it builds from these entries.
+        // Nothing binds through it (these labels bind by id); it is stated
+        // truthfully rather than invented so that a reader of the label list
+        // sees where on its own playlist each stream sits.
         let mut audio_idx: u16 = 0;
         let mut sub_idx: u16 = 0;
 
-    for playlist in playlists {
         for entry in &playlist.streams {
             let Some(label_type) = label_type_for(entry) else {
                 continue;
             };
-
-            let language = normalize_language(&entry.language);
-            let name = language_display_name(&language);
-            let codec_hint = build_codec_hint(label_type, entry);
-
-            let key = (label_type, language.clone(), codec_hint.clone(), entry.pid);
-            if seen.contains(&key) {
-                continue;
-            }
-            seen.push(key);
-
             let stream_number = match label_type {
                 StreamLabelType::Audio => {
                     audio_idx += 1;
@@ -135,7 +141,20 @@ fn build_labels(playlists: &[crate::mpls::Playlist]) -> Vec<StreamLabel> {
                 }
             };
 
+            let stream_id = super::StreamId {
+                clip_id: clip_id.clone(),
+                pid: entry.pid,
+            };
+            if !seen.insert(stream_id.clone()) {
+                continue;
+            }
+
+            let language = normalize_language(&entry.language);
+            let name = language_display_name(&language);
+            let codec_hint = build_codec_hint(label_type, entry);
+
             labels.push(StreamLabel {
+                stream_id: Some(stream_id),
                 stream_number,
                 stream_type: label_type,
                 language,
@@ -365,10 +384,24 @@ mod tests {
         }
     }
 
+    /// A playlist over clip "00001". `Playlist::streams` is read out of the
+    /// first play item's STN table, so a playlist that has streams always has
+    /// a play item to have read them from — the fixture carries one so tests
+    /// exercise the shape production sees, and so each label gets the
+    /// `(clip, PID)` identity it is bound by.
     fn playlist_with(streams: Vec<StreamEntry>) -> Playlist {
+        playlist_on("00001", streams)
+    }
+
+    fn playlist_on(clip_id: &str, streams: Vec<StreamEntry>) -> Playlist {
         Playlist {
             version: "0200".to_string(),
-            play_items: Vec::new(),
+            play_items: vec![crate::mpls::PlayItem {
+                clip_id: clip_id.to_string(),
+                in_time: 0,
+                out_time: 0,
+                connection_condition: 1,
+            }],
             streams,
             marks: Vec::new(),
         }
@@ -503,31 +536,48 @@ mod tests {
         );
     }
 
+    /// Two playlists over the SAME clip that both list PID 0x1100: one
+    /// physical stream, so one label. Identity is `(clip, PID)`, and each
+    /// label states the STN slot it holds in its own playlist.
     #[test]
-    fn dedup_streams_across_playlists() {
+    fn one_label_per_stream_across_playlists_on_the_same_clip() {
-        // Two playlists, same English TrueHD 7.1 PID 0x1100 in both.
+        let pl1 = playlist_on(
-        // Expect one Audio label, not two.
+            "00001",
-        let pl1 = playlist_with(vec![
+            vec![
                 audio_entry(0x1100, 0x83, 12, 1, "eng"),
                 audio_entry(0x1101, 0x81, 6, 1, "fra"),
-        ]);
+            ],
-        let pl2 = playlist_with(vec![
+        );
-            audio_entry(0x1100, 0x83, 12, 1, "eng"), // duplicate
+        let pl2 = playlist_on(
+            "00001",
+            vec![
+                audio_entry(0x1100, 0x83, 12, 1, "eng"), // same stream
                 audio_entry(0x1102, 0x82, 6, 1, "deu"),  // new
-        ]);
+            ],
+        );
         let labels = labels_from_playlists(&[pl1, pl2]);
-        // Expected: eng@0x1100, fra@0x1101, deu@0x1102 — three uniques.
+        assert_eq!(
-        assert_eq!(labels.len(), 3);
+            labels.len(),
-        // PID isn't stored on StreamLabel, so assert on the surviving
+            3,
-        // language set instead.
+            "eng/fra/deu — the duplicate eng is one stream"
-        let mut langs: Vec<String> = labels.iter().map(|l| l.language.clone()).collect();
+        );
-        langs.sort();
-        assert_eq!(langs, vec!["deu", "eng", "fra"]);
 
-        // Stream numbers must be DENSE and GLOBAL across playlists, not
+        let id = |lang: &str| {
-        // reset per playlist. eng (pl1) = 1, fra (pl1) = 2, the duplicate
+            labels
-        // eng in pl2 is deduped (no number consumed), and deu (pl2) = 3.
+                .iter()
-        // Regression guard for the per-playlist counter-reset divergence.
+                .find(|l| l.language == lang)
+                .and_then(|l| l.stream_id.clone())
+                .map(|i| (i.clip_id, i.pid))
+        };
+        assert_eq!(id("eng"), Some(("00001".into(), 0x1100)));
+        assert_eq!(id("fra"), Some(("00001".into(), 0x1101)));
+        assert_eq!(id("deu"), Some(("00001".into(), 0x1102)));
+
+        // `stream_number` is the entry's slot in ITS OWN playlist's STN table
+        // — deu is pl2's second audio, so 2, not "the third distinct stream
+        // seen while scanning the disc". It used to be the latter: a dense
+        // disc-global counter that named no table anyone could count against,
+        // handed to a binder that reads the field as an STN slot.
         let num = |lang: &str| {
             labels
                 .iter()
@@ -536,7 +586,25 @@ mod tests {
         };
         assert_eq!(num("eng"), Some(1));
         assert_eq!(num("fra"), Some(2));
-        assert_eq!(num("deu"), Some(3));
+        assert_eq!(num("deu"), Some(2), "pl2's second audio slot");
+    }
+
+    /// The same PID in two DIFFERENT clips is two different streams — a PID is
+    /// only unique within one clip. Deduping on the PID alone (as the old
+    /// key's `(type, language, codec_hint, pid)` did across clips) collapses
+    /// them into one label, and the second clip's stream is then described by
+    /// the first clip's.
+    #[test]
+    fn same_pid_in_two_clips_is_two_streams() {
+        let pl1 = playlist_on("00001", vec![audio_entry(0x1100, 0x83, 12, 1, "eng")]);
+        let pl2 = playlist_on("00002", vec![audio_entry(0x1100, 0x83, 12, 1, "eng")]);
+        let labels = labels_from_playlists(&[pl1, pl2]);
+        assert_eq!(labels.len(), 2, "different clips: two distinct streams");
+        let clips: Vec<String> = labels
+            .iter()
+            .filter_map(|l| l.stream_id.as_ref().map(|i| i.clip_id.clone()))
+            .collect();
+        assert_eq!(clips, vec!["00001", "00002"]);
     }
 
     #[test]

src/labels/paramount.rs

@@ -157,6 +157,7 @@ fn labels_from_feature(feature: &str) -> Vec<StreamLabel> {
                 LabelPurpose::Normal
             };
             labels.push(StreamLabel {
+                stream_id: None,
                 stream_number,
                 stream_type: StreamLabelType::Audio,
                 language: lang.to_string(),
@@ -210,6 +211,7 @@ fn labels_from_feature(feature: &str) -> Vec<StreamLabel> {
             };
 
             labels.push(StreamLabel {
+                stream_id: None,
                 stream_number,
                 stream_type: StreamLabelType::Subtitle,
                 language: lang.to_string(),

src/labels/pixelogic.rs

@@ -272,6 +272,7 @@ fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec<StreamLa
                     }
                     audio_num += 1;
                     labels.push(StreamLabel {
+                        stream_id: None,
                         stream_number: audio_num,
                         ..label
                     });
@@ -282,6 +283,7 @@ fn assign_labels(strings: &[String], unknown: &mut UnknownParts) -> Vec<StreamLa
                     }
                     sub_num += 1;
                     labels.push(StreamLabel {
+                        stream_id: None,
                         stream_number: sub_num,
                         ..label
                     });
@@ -464,6 +466,7 @@ fn parse_token_inner(s: &str, mut unknown: Option<&mut UnknownParts>) -> Option<
     };
 
     Some(StreamLabel {
+        stream_id: None,
         stream_number: 0,
         stream_type,
         language: lang.to_string(),

src/labels/png_filenames.rs

@@ -50,6 +50,7 @@ fn labels_from_filenames(names: &[String]) -> Vec<StreamLabel> {
     seen.into_iter()
         .enumerate()
         .map(|(i, code)| StreamLabel {
+            stream_id: None,
             stream_number: (i as u16).saturating_add(1),
             stream_type: StreamLabelType::Audio,
             language: code.to_string(),