Merge branch 'fix/pgs-forced-probe-sampling' into dev

CHANGELOG.md

@@ -4,6 +4,45 @@
 
 ### Fixed
 
+- **Content-based forced-subtitle detection never observed anything on a
+  feature-length disc.** The PGS probe spent its entire 256 MiB budget on the
+  first sectors of a title, where a feature has no subtitles at all — it hit
+  the budget, observed zero display sets, and contributed nothing to any
+  verdict, so the vendor label was always the only input. The same budget is
+  now SPREAD across each extent as ~16 MiB sample windows sized in proportion
+  to the extent (still on the AACS aligned-unit grid, still bounded by the same
+  ceiling): a track is disproven by any single non-forced display set anywhere,
+  and a genuine forced track is small enough to be caught by the spread. Cost
+  is unchanged; placement is not. The probe also stops spending budget on a
+  track the moment it is disproven, and skips an extent that owes evidence only
+  for such tracks.
+- **A partially-read extent's evidence was memoised as if the whole extent had
+  been read.** A budget-cut (and now sampled) read covers a fraction of an
+  extent, but its result was filed under the extent's full key and replayed to
+  every other playlist sharing the clip — turning a prefix into an absence
+  claim about the whole extent. Cache entries now carry the coverage behind
+  them, and an entry only answers for a run that intended to read no more than
+  it did. Positive evidence (a non-forced display set was seen) still answers
+  regardless, being irretractable.
+- **A forced verdict could rest on a single display set.** Calling a track
+  forced is an absence claim — "no display set here was un-flagged" — and a
+  sampled read sees a fraction of a track. Measured on real discs: tracks exist
+  that carry `forced_on_flag` on about a quarter of their display sets and not
+  on the rest, so catching one flagged set and nothing else is exactly what a
+  wrong promotion looks like. A sampled run now needs at least two display sets
+  before it may assert forced; a run that read every extent end to end has no
+  unread gap and may still promote off one (single-sign forced tracks exist).
+- **Content could never correct a wrong vendor forced label.** The muxer only
+  ever promoted `FlagForced` 0 → 1, so a track labelled forced stayed forced in
+  the output even when the mux had seen every one of its two thousand display
+  sets and not one was forced. Content may now clear the flag too — in the
+  muxer and in the scan-time probe — behind a single shared guard: the absence
+  of `forced_on_flag` means nothing on a disc whose authoring never sets it, so
+  a demotion requires that some other track demonstrably uses the flag AND that
+  the track have the shape of a full dialogue track rather than of a
+  forced-narrative one. Where no track on the disc uses the flag, nothing is
+  demotable.
+
 - **An `.fvi` index named itself as its own source.** The `fvi://` sink was
   handed the DESTINATION path as its `source_path`, so every index reported
   `source.path` as the file it was writing. `source.medium` was always `file`

src/mux/codec/pgs.rs

@@ -69,6 +69,8 @@ pub fn display_set_is_forced(frame_data: &[u8]) -> Option<bool> {
 pub struct ForcedTracker {
     has_display: bool,
     all_forced: bool,
+    displays: u32,
+    forced_displays: u32,
 }
 
 impl Default for ForcedTracker {
@@ -76,10 +78,88 @@ impl Default for ForcedTracker {
         Self {
             has_display: false,
             all_forced: true,
+            displays: 0,
+            forced_displays: 0,
         }
     }
 }
 
+/// The disc-shaped facts about ONE subtitle track that a demotion decision
+/// rests on — how many display sets were seen, and how many of them carried the
+/// HDMV `forced_on_flag`.
+///
+/// Split out from [`ForcedTracker`] so the two places that can contradict a
+/// vendor label (the scan-time probe, which accumulates per-extent evidence,
+/// and the muxer, which holds a live tracker per track) feed the SAME rule.
+#[derive(Clone, Copy, Default, PartialEq, Eq, Debug)]
+pub struct ForcedFacts {
+    /// Display sets observed on this track.
+    pub displays: u32,
+    /// How many of them carried `forced_on_flag`.
+    pub forced_displays: u32,
+}
+
+/// A track must have shown at least this many display sets before "none of them
+/// was forced" is allowed to contradict a vendor forced label.
+///
+/// Absence is weak evidence on a handful of sets: a genuine forced-narrative
+/// track is SMALL (measured shape: tens of display sets for a whole feature), so
+/// a couple of unflagged sets is exactly what one looks like on a disc whose
+/// authoring never sets the flag.
+pub const DEMOTE_MIN_DISPLAY_SETS: u32 = 8;
+
+/// ...and it must carry at least this fraction (1/N) of the display sets of the
+/// BUSIEST subtitle track on the disc.
+///
+/// This is the shape test that separates the two populations. Measured: a
+/// dedicated forced track carries a low-tens count of display sets for a whole
+/// feature, a full dialogue track carries one to two thousand — two orders of
+/// magnitude apart. A track sitting within a quarter of the busiest track's
+/// count is a full track, whatever its label says; a track at one percent of it
+/// is the forced-narrative track its label claims and must keep that label.
+pub const DEMOTE_MIN_DISPLAY_SHARE_DIVISOR: u32 = 4;
+
+/// Whether content evidence is strong enough to CONTRADICT a vendor label that
+/// says a track is forced — i.e. to demote 1 → 0.
+///
+/// Promotion (0 → 1) needs no such gate: it rests on positive evidence (every
+/// display set carried `forced_on_flag`). Demotion rests on an ABSENCE, and an
+/// absence is only meaningful if the flag is in use at all. Measured: discs
+/// exist on which NO track carries `forced_on_flag`; there, "this track has no
+/// forced display sets" is a fact about the authoring house, not about the
+/// track, and demoting on it would strip a correct forced label from every
+/// track on the disc.
+///
+/// So the rule is, in order:
+///   * something must have been observed at all;
+///   * the flag must be IN USE — on some other track (`disc_uses_forced_flag`) or
+///     on this very track, which is the stronger form: a track carrying the flag
+///     on some of its sets and not others shows the authoring house making that
+///     distinction deliberately;
+///   * and the track must have the SHAPE of a full dialogue track
+///     ([`DEMOTE_MIN_DISPLAY_SETS`] and [`DEMOTE_MIN_DISPLAY_SHARE_DIVISOR`])
+///     rather than of a forced-narrative one. This applies to the mixed case too:
+///     a SMALL track with a couple of flagged sets is a forced track whose
+///     authoring flagged some of its signs, and demoting it would be exactly the
+///     mistake the shape test exists to prevent.
+///
+/// `busiest_displays` is the largest `displays` over every subtitle track judged
+/// together (the same title's tracks for the probe, the same file's tracks for
+/// the muxer).
+pub fn demotable(facts: ForcedFacts, disc_uses_forced_flag: bool, busiest_displays: u32) -> bool {
+    if facts.displays == 0 {
+        return false;
+    }
+    let flag_in_use = disc_uses_forced_flag || facts.forced_displays > 0;
+    if !flag_in_use || facts.displays < DEMOTE_MIN_DISPLAY_SETS {
+        return false;
+    }
+    // `displays >= busiest / DIVISOR`, multiplied out (u64: `displays` is a
+    // disc-derived count, so the product must not be able to wrap).
+    u64::from(facts.displays) * u64::from(DEMOTE_MIN_DISPLAY_SHARE_DIVISOR)
+        >= u64::from(busiest_displays)
+}
+
 impl ForcedTracker {
     pub fn new() -> Self {
         Self::default()
@@ -91,6 +171,22 @@ impl ForcedTracker {
         if let Some(forced) = display_set_is_forced(frame_data) {
             self.has_display = true;
             self.all_forced &= forced;
+            // Saturating: the counts drive a shape comparison between tracks, so
+            // a pathological stream must pin them, never wrap (and never panic
+            // on an overflow in a debug build).
+            self.displays = self.displays.saturating_add(1);
+            if forced {
+                self.forced_displays = self.forced_displays.saturating_add(1);
+            }
+        }
+    }
+
+    /// The counts behind the verdict: how many display sets were seen and how
+    /// many carried `forced_on_flag`. Feeds [`demotable`].
+    pub fn facts(&self) -> ForcedFacts {
+        ForcedFacts {
+            displays: self.displays,
+            forced_displays: self.forced_displays,
         }
     }
 
@@ -716,4 +812,93 @@ mod tests {
         let f = parser.parse(&make_pes(pcs_bytes(0), Some(180000)));
         assert_eq!(f[0].data, display, "display PCS data emitted verbatim");
     }
+
+    // ── the demotion guard ──────────────────────────────────────────────────
+
+    fn facts(displays: u32, forced: u32) -> ForcedFacts {
+        ForcedFacts {
+            displays,
+            forced_displays: forced,
+        }
+    }
+
+    /// The case the guard exists for: a disc whose authoring never sets
+    /// `forced_on_flag`. Nothing about the absence of a flag nobody uses can
+    /// contradict a vendor label, however many display sets confirm the absence.
+    #[test]
+    fn nothing_is_demotable_on_a_disc_that_never_sets_the_flag() {
+        for displays in [1u32, DEMOTE_MIN_DISPLAY_SETS, 2_000, u32::MAX] {
+            assert!(
+                !demotable(facts(displays, 0), false, displays),
+                "{displays} unflagged display sets on a flagless disc prove nothing"
+            );
+        }
+    }
+
+    /// A track that itself mixes forced and non-forced display sets needs no
+    /// corroboration from a sibling: the flag is demonstrably in use ON THIS
+    /// TRACK. Measured shape this models: a busy track labelled forced that
+    /// flags one or two of its hundred-odd display sets.
+    #[test]
+    fn a_mixed_track_corroborates_the_flag_itself() {
+        assert!(demotable(facts(108, 2), false, 137));
+    }
+
+    /// ...but the shape test still applies to it. A SMALL track with a couple of
+    /// flagged sets is a forced track whose authoring flagged some of its signs —
+    /// demoting that is the exact mistake the shape test exists to prevent.
+    #[test]
+    fn a_small_mixed_track_is_not_demotable_against_a_busy_disc() {
+        assert!(!demotable(facts(30, 1), true, 2_000));
+        assert!(
+            !demotable(facts(4, 1), true, 4),
+            "and too few sets to say anything either way"
+        );
+    }
+
+    /// With the flag in use elsewhere on the disc, the shape decides. Measured:
+    /// a forced-narrative track carries tens of display sets, a full dialogue
+    /// track one to two thousand.
+    #[test]
+    fn shape_decides_once_the_disc_is_known_to_use_the_flag() {
+        assert!(
+            demotable(facts(2_000, 0), true, 2_000),
+            "the busiest track on the disc, with no forced set on it, is a full track"
+        );
+        assert!(
+            !demotable(facts(20, 0), true, 2_000),
+            "a track at one percent of the busiest is the forced track its label claims"
+        );
+        assert!(
+            !demotable(facts(DEMOTE_MIN_DISPLAY_SETS - 1, 0), true, 8),
+            "too few display sets for their absence of flags to mean anything"
+        );
+        assert!(
+            demotable(facts(DEMOTE_MIN_DISPLAY_SETS, 0), true, 8),
+            "at the threshold, with the shape of the busiest track, it is demotable"
+        );
+    }
+
+    /// Never on no evidence at all: a track nobody observed cannot contradict
+    /// anything.
+    #[test]
+    fn an_unobserved_track_is_never_demotable() {
+        assert!(!demotable(facts(0, 0), true, 2_000));
+    }
+
+    /// Saturating counters: a pathological stream must pin the counts, never wrap
+    /// them (and never panic on overflow in a debug build).
+    #[test]
+    fn display_counts_saturate_instead_of_wrapping() {
+        let mut t = ForcedTracker::new();
+        t.displays = u32::MAX;
+        t.forced_displays = u32::MAX;
+        let mut pcs = vec![0u8; 18];
+        pcs[0] = SEGMENT_PCS;
+        pcs[PCS_NUM_OBJECTS_OFFSET] = 1;
+        pcs[PCS_FIRST_OBJECT_FLAGS_OFFSET] = PCS_FORCED_ON_FLAG;
+        t.observe(&pcs);
+        assert_eq!(t.facts().displays, u32::MAX);
+        assert_eq!(t.facts().forced_displays, u32::MAX);
+    }
 }

src/mux/mkv.rs

@@ -741,7 +741,8 @@ pub struct MkvMuxer<W: Write + Seek> {
     /// display set carried the HDMV `forced_on_flag` (a dedicated forced/narrative
     /// track). At `finish()` the reserved byte is promoted to 1 for such tracks —
     /// so forced subs are flagged even on discs without vendor label metadata.
-    /// Only ever promotes (0→1); a scan/vendor forced flag is never demoted.
+    /// A vendor forced flag is cleared only under the cross-track guard in
+    /// `finish()` (see `super::codec::pgs::demotable`).
     pgs_forced_fixups: std::collections::HashMap<usize, PgsForcedFixup>,
     /// `--log-level 3` opening-frame capture: the first ~100 coded frames per
     /// track are written (raw) to a `<output>.opening.bin` side file with a
@@ -767,6 +768,10 @@ struct Ac3ChannelFixup {
 struct PgsForcedFixup {
     /// Absolute file offset of the 1-byte `FlagForced` value in the Tracks element.
     value_offset: u64,
+    /// The value written up-front — the scan/vendor-label flag. Kept so
+    /// `finish()` can tell a promotion from a demotion and rewrite only the byte
+    /// that actually changes.
+    initial_forced: bool,
     /// Shared forced-narrative classifier fed the track's display sets. The same
     /// type drives the `info`-time forced probe, so both classify identically.
     tracker: super::codec::pgs::ForcedTracker,
@@ -1045,6 +1050,7 @@ impl<W: Write + Seek> MkvMuxer<W> {
                     i,
                     PgsForcedFixup {
                         value_offset,
+                        initial_forced: track.is_forced,
                         tracker: super::codec::pgs::ForcedTracker::new(),
                     },
                 );
@@ -1686,22 +1692,64 @@ impl<W: Write + Seek> MkvMuxer<W> {
         // Close final cluster
         self.end_cluster()?;
 
-        // Promote FlagForced for PGS subtitle tracks that proved to be forced
-        // narrative (displayed subtitles, every one forced). In-place single-byte
-        // rewrite of the reserved value, then restore the append position for the
-        // Cues that follow. Only promotes (0→1); a track already forced from the
-        // scan/vendor flag stays forced.
-        let forced_offsets: Vec<u64> = self
+        // Correct FlagForced for PGS subtitle tracks from what the mux actually
+        // saw. In-place single-byte rewrite of the reserved value, then restore
+        // the append position for the Cues that follow.
+        //
+        // PROMOTE (0→1) a track that proved to be forced narrative: it displayed
+        // subtitles and every one carried `forced_on_flag`. Positive evidence,
+        // no further justification needed.
+        //
+        // DEMOTE (1→0) a track whose vendor label claims forced but whose content
+        // contradicts it — the case a whole-file mux is uniquely entitled to
+        // judge, because unlike the scan-time probe it has seen EVERY display set
+        // on the track. Gated by
+        // [`super::codec::pgs::demotable`]: an absence of `forced_on_flag` proves
+        // nothing on a disc whose authoring never sets it, so the gate demands
+        // that some track here demonstrably does, and that this track have the
+        // shape of a full dialogue track rather than of a forced-narrative one.
+        let disc_uses_forced_flag = self
             .pgs_forced_fixups
             .values()
-            .filter(|f| f.tracker.is_forced())
-            .map(|f| f.value_offset)
+            .any(|f| f.tracker.facts().forced_displays > 0);
+        let busiest = self
+            .pgs_forced_fixups
+            .values()
+            .map(|f| f.tracker.facts().displays)
+            .max()
+            .unwrap_or(0);
+        let rewrites: Vec<(u64, u8)> = self
+            .pgs_forced_fixups
+            .values()
+            .filter_map(|f| {
+                if f.tracker.is_forced() && !f.initial_forced {
+                    return Some((f.value_offset, 1u8));
+                }
+                if f.initial_forced
+                    && !f.tracker.is_forced()
+                    && super::codec::pgs::demotable(
+                        f.tracker.facts(),
+                        disc_uses_forced_flag,
+                        busiest,
+                    )
+                {
+                    tracing::info!(
+                        target: "mux",
+                        displays = f.tracker.facts().displays,
+                        forced_displays = f.tracker.facts().forced_displays,
+                        busiest,
+                        "PGS track labelled forced showed no forced display sets on a disc that uses the flag; clearing FlagForced"
+                    );
+                    return Some((f.value_offset, 0u8));
+                }
+                None
+            })
             .collect();
-        if !forced_offsets.is_empty() {
+        if !rewrites.is_empty() {
             let here = self.writer.stream_position()?;
-            for off in forced_offsets {
+            for (off, value) in rewrites {
                 self.writer.seek(std::io::SeekFrom::Start(off))?;
-                self.writer.write_all(&[1u8])?;
+                self.writer.write_all(&[value])?;
             }
             self.writer.seek(std::io::SeekFrom::Start(here))?;
         }
@@ -3210,6 +3258,132 @@ mod tests {
         pcs
     }
 
+    /// Every `FlagForced` value byte in the file, in track order — the two-PGS-track
+    /// tests need per-track values, not just the first.
+    fn all_flag_forced_values(data: &[u8]) -> Vec<u8> {
+        let needle = ebml::FLAG_FORCED.to_be_bytes();
+        let needle = &needle[2..]; // FlagForced is a 2-byte EBML ID
+        data.windows(needle.len())
+            .enumerate()
+            .filter(|(_, w)| *w == needle)
+            .filter_map(|(i, _)| data.get(i + 3).copied())
+            .collect()
+    }
+
+    /// A PGS subtitle track carrying the vendor/scan forced flag `forced`.
+    fn pgs_subtitle_track(pid: u16, forced: bool) -> MkvTrack {
+        MkvTrack::subtitle(&crate::disc::SubtitleStream {
+            pid,
+            codec: Codec::Pgs,
+            language: "eng".into(),
+            forced,
+            qualifier: crate::disc::LabelQualifier::None,
+            codec_data: None,
+        })
+    }
+
+    /// A wrong vendor forced label is CLEARED by the content — but only where the
+    /// content can carry that argument. During a full mux every display set on the
+    /// track is seen, so "this track has hundreds of display sets and not one of
+    /// them is forced" is as complete as evidence gets; and a sibling track that
+    /// does carry `forced_on_flag` proves the authoring house sets it, so the
+    /// absence on this track means something.
+    ///
+    /// Before this, `finish()` only ever promoted 0→1, so a track wrongly labelled
+    /// forced stayed forced in the output no matter what the disc contained.
+    #[test]
+    fn mkv_pgs_wrong_forced_label_is_cleared_when_a_sibling_uses_the_flag() {
+        use std::sync::{Arc, Mutex};
+
+        let shared = Arc::new(Mutex::new(Cursor::new(Vec::new())));
+        let tracks = [
+            make_video_track(),
+            pgs_subtitle_track(0x1200, true), // mislabelled full track
+            pgs_subtitle_track(0x1201, false), // genuine forced track
+        ];
+        let mut muxer =
+            MkvMuxer::new(SharedWriter(shared.clone()), &tracks, None, 60.0, &[]).unwrap();
+        muxer
+            .write_frame(0, 0, true, &[0u8; 16], Some(40_000_000), None)
+            .unwrap();
+        for i in 0..12 {
+            muxer
+                .write_frame(
+                    1,
+                    1_000_000 * (i + 1),
+                    true,
+                    &pgs_display_set(false),
+                    Some(2_000_000),
+                    None,
+                )
+                .unwrap();
+        }
+        for i in 0..3 {
+            muxer
+                .write_frame(
+                    2,
+                    1_000_000 * (i + 1),
+                    true,
+                    &pgs_display_set(true),
+                    Some(2_000_000),
+                    None,
+                )
+                .unwrap();
+        }
+        muxer.finish().unwrap();
+
+        let data = shared.lock().unwrap().clone().into_inner();
+        assert_eq!(
+            all_flag_forced_values(&data),
+            vec![0, 1],
+            "the mislabelled track loses FlagForced; the genuinely forced one keeps it"
+        );
+    }
+
+    /// ...and the guard that stops that from being reckless. On a disc whose
+    /// authoring never sets `forced_on_flag` — they exist — no track has any
+    /// forced display set, so "no forced display set here" is a fact about the
+    /// authoring, not about the track. The vendor label is then the only
+    /// information there is and must survive.
+    #[test]
+    fn mkv_pgs_forced_label_survives_a_disc_that_never_sets_the_flag() {
+        use std::sync::{Arc, Mutex};
+
+        let shared = Arc::new(Mutex::new(Cursor::new(Vec::new())));
+        let tracks = [
+            make_video_track(),
+            pgs_subtitle_track(0x1200, true),
+            pgs_subtitle_track(0x1201, false),
+        ];
+        let mut muxer =
+            MkvMuxer::new(SharedWriter(shared.clone()), &tracks, None, 60.0, &[]).unwrap();
+        muxer
+            .write_frame(0, 0, true, &[0u8; 16], Some(40_000_000), None)
+            .unwrap();
+        for track in 1..=2 {
+            for i in 0..12 {
+                muxer
+                    .write_frame(
+                        track,
+                        1_000_000 * (i + 1),
+                        true,
+                        &pgs_display_set(false),
+                        Some(2_000_000),
+                        None,
+                    )
+                    .unwrap();
+            }
+        }
+        muxer.finish().unwrap();
+
+        let data = shared.lock().unwrap().clone().into_inner();
+        assert_eq!(
+            all_flag_forced_values(&data),
+            vec![1, 0],
+            "with the flag unused disc-wide, both labels stand as authored"
+        );
+    }
+
     #[test]
     fn mkv_pgs_forced_promoted_when_all_display_sets_forced() {
         // End-to-end: a PGS subtitle track whose every display set is forced is