From 6b67c522499f0e3eb40cd5ec37f9d5918af24049 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:02:48 -0700 Subject: [PATCH] Round 6: a large backstep only crosses when it would be misplaced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-5 fix advanced the clip cursor on ANY backward step over 3s. That also fires for a corrupt PTS, and for a legitimate STC discontinuity inside one clip — and nothing moves the cursor back, because a forward step matches neither past_out nor stepped_back. Every later frame then sits below the new clip IN and is dropped: on the fixture table that is ~17 minutes of one track gone, and the only volume gate compares total drops against ALL tracks frames, so it exits 0. The branch now also requires the frame to be INSIDE the current clip marks, which is the only case that would otherwise be silently placed at the old offset — the rewind. A frame outside them needs no help: the containment check drops and counts it, the cursor stays put, and the next good frame is placed normally. Both behaviours have a test, each confirmed to fail without the guard. Separately, the dir:// PES input path scanned the folder and never applied the encryption verdict scan_dir exists to produce, so the same folder ripped through one door and failed through the other asking for a key it does not need. That logic now lives in one function, session::apply_folder_encryption_verdict, called by both. --- src/mux/resolve.rs | 28 ++++++++++++++++++----- src/mux/timeline.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++- src/session.rs | 25 +++++++++++++++++--- 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/mux/resolve.rs b/src/mux/resolve.rs index 65184db..6110df1 100644 --- a/src/mux/resolve.rs +++ b/src/mux/resolve.rs @@ -392,9 +392,12 @@ pub fn input(url: &str, opts: &InputOptions) -> io::Result io::Result { @@ -467,6 +473,9 @@ fn image_input( mut reader: S, opts: &InputOptions, reopen: F, + // A FOLDER's `encrypted` flag comes from tree shape and can be wrong; an + // image's cannot. See `session::apply_folder_encryption_verdict`. + is_folder: bool, ) -> io::Result where S: SectorSource + Send + 'static, @@ -476,6 +485,13 @@ where let mut disc = crate::disc::Disc::scan_image(&mut reader, capacity, &crate::disc::ScanOptions::default()) .map_err(|e| -> io::Error { e.into() })?; + // Without this a folder reached here with a tree-shape verdict while + // `session::scan_dir` reached the opposite one from its CONTENT, so the + // same folder ripped through one door and failed through the other. + if is_folder { + crate::session::apply_folder_encryption_verdict(&mut reader, &mut disc) + .map_err(|e| -> io::Error { e.into() })?; + } // Apply the caller-resolved keys (lookup-free); decrypt_keys() then // yields them for the stream below. Propagate a failed application // rather than silently muxing an undecryptable stream. diff --git a/src/mux/timeline.rs b/src/mux/timeline.rs index 8fc9087..f1b4818 100644 --- a/src/mux/timeline.rs +++ b/src/mux/timeline.rs @@ -259,7 +259,24 @@ impl SeamPlan { // skip any clip the frame is already past. Terminates at the first clip // that could contain it; a frame still below that clip's IN is // pre-mark material and the containment check drops and counts it. - if big_backstep && clip + 1 < self.clips.len() { + // ...and ONLY when the frame would otherwise be wrongly PLACED, i.e. it + // falls inside the current clip's marks. + // + // That qualifier is the whole safety of this branch. Advancing on any + // large backstep strands the track permanently: a corrupt PTS, or a + // legitimate STC discontinuity inside one clip, is also a >3s backstep, + // and once the cursor moves forward nothing moves it back — a forward + // step matches neither `past_out` nor `stepped_back`. Every later frame + // then sits below the new clip's IN and is dropped, so a track can lose + // a clip's worth of content (~17 minutes on the fixture table) while the + // only volume gate compares total drops against ALL tracks' frames. + // + // A frame OUTSIDE the current clip's marks needs no help: the + // containment check at the end drops and counts it, and the cursor stays + // put so the next good frame is placed normally. Only a frame INSIDE + // them would be silently placed at the old offset, which is the rewind. + let inside_current = raw_ns >= self.clips[clip].in_ns && raw_ns <= self.clips[clip].out_ns; + if big_backstep && inside_current && clip + 1 < self.clips.len() { clip += 1; while clip + 1 < self.clips.len() && raw_ns > self.clips[clip].out_ns { clip += 1; @@ -703,6 +720,43 @@ mod tests { } } + /// A glitched PTS must not strand a track on a later clip forever. + /// + /// Audit finding against the large-backstep branch: advancing on ANY >3s + /// backward step also fires for a corrupt PTS, or a legitimate STC + /// discontinuity inside one clip. Nothing moves the cursor back — a forward + /// step matches neither `past_out` nor `stepped_back` — so every later + /// frame sits below the new clip's IN and is dropped. On this table that is + /// ~17 minutes of one track, and the only volume gate compares total drops + /// against ALL tracks' frames, so it exits 0. + /// + /// The branch now requires the frame to be INSIDE the current clip's marks, + /// which is the only case that would otherwise be wrongly placed. + #[test] + fn a_glitched_pts_does_not_strand_a_track_on_a_later_clip() { + let clips = seamless_branching_clips(); + let mut plan = SeamPlan::from_clips(&clips).expect("plan"); + // Clip 0 is (188955000, 271486824) -> 4199.0s .. 6033.04s + let good = plan + .place(5_000_000_000_000, 1, false) + .expect("a frame inside clip 0 is placed"); + // A damaged PTS well below clip 0's IN: a >3s backstep that is NOT a + // clip change. It should be dropped, and the cursor must not move. + assert!( + plan.place(4_000_000_000_000, 1, false).is_none(), + "a frame before the first clip's IN is not placeable" + ); + // The very next good frame must still be placed, on the same clip. + let after = plan + .place(5_001_000_000_000, 1, false) + .expect("the track must recover on the next good frame, not be stranded"); + assert_eq!( + after - good, + 1_000_000_000, + "the recovered frame must land 1s after the last good one, on the same clip" + ); + } + /// Build a real seamless-branching clip table (`00801.mpls`, 11 PlayItems, marks /// read off the disc) so the arithmetic is pinned to measured ground truth /// rather than to invented numbers. diff --git a/src/session.rs b/src/session.rs index 8131795..aa857d6 100644 --- a/src/session.rs +++ b/src/session.rs @@ -475,16 +475,35 @@ pub fn scan_dir(path: &Path, opts: ScanOptions) -> Result<(Disc, Box Result<()> { // `css.is_some()` is the DVD path, and that verdict came from actually // cracking scrambled sectors — real evidence about content, not tree shape. // Only the AACS-by-tree-shape verdict is re-judged here. if disc.encrypted && disc.css.is_none() && disc.css_error.is_none() { - match probe_folder_encryption(&mut reader, &disc)? { + match probe_folder_encryption(reader, disc)? { true => return Err(Error::DirImageEncrypted), false => { tracing::warn!( target: "freemkv::scan", - phase = "scan_dir", + phase = "folder_verdict", "folder carries an AACS directory but its sampled content units \ are already in the clear; treating it as decrypted" ); @@ -494,7 +513,7 @@ pub fn scan_dir(path: &Path, opts: ScanOptions) -> Result<(Disc, Box