Round 5: fix the gates added in round 4, and two placement holes

The zero-frame check ran before the seam gate, and its error is
classified as a skippable nav stub — so a title the plan dropped
ENTIRELY was reported as an empty stub and an all-titles rip would omit
a real feature and finish the rest at exit 0. The seam case is decided
first now, with a code that is not skippable.

The demux sink read a frame's track kind out of the FILTERED slot, which
is empty for a class the export drops. On an audio:// or sub:// export
the video track was therefore called non-video and handed the permissive
crossing rule — the same defect round 4 fixed for a Dolby Vision layer,
reintroduced one file over. Video tracks are now recorded before the kind
filter, beside the primary-video reference that exists for this reason.

Its frame counter counted frames PLACED, not written, while its name and
doc claimed otherwise. Renamed and documented for what it is, including
that it cannot see a single lost track among many.

Placement: files in a subdirectory of VIDEO_TS were never given data.
They were declared at full size with no extents, so they appeared in the
tree and read as nothing. The same folder under BDMV was always placed
correctly. And the duplicate title-set guard keyed on the constraint maps,
so an IFO declaring no offsets inserted nothing and a colliding second IFO
went undetected — it keys on the groups seen now.

Display for SeamPlanDroppedMost and ShortImageRead discarded their
payloads, and four new variants were missing from the code-uniqueness
test.

src/dirimage/layout.rs

@@ -403,6 +403,11 @@ fn place_video_ts(vts: &mut DirNode, start: u32) -> Result<u32> {
     // Required start blocks, resolved as each IFO is placed.
     let mut menu_req: std::collections::HashMap<u32, u32> = std::collections::HashMap::new();
     let mut title_req: std::collections::HashMap<u32, u32> = std::collections::HashMap::new();
+    // Groups whose IFO has already been placed. Kept separately from the
+    // constraint maps because an IFO that declares no offsets inserts into
+    // neither, so keying the duplicate check on those maps would miss a second
+    // IFO for the same set — which is the collision the check exists for.
+    let mut seen_ifo: std::collections::HashSet<u32> = std::collections::HashSet::new();
 
     let mut cursor = start;
     for &i in &order {
@@ -438,7 +443,7 @@ fn place_video_ts(vts: &mut DirNode, start: u32) -> Result<u32> {
             // parse to the same group, and the second insert would overwrite
             // the first's constraint, placing a VOB at an address the IFO the
             // reader uses does not point to. Refuse instead of picking one.
-            if menu_req.contains_key(&c.group) || title_req.contains_key(&c.group) {
+            if !seen_ifo.insert(c.group) {
                 return Err(Error::DirNameCollision {
                     host: vts.files[i].disc_path.clone(),
                 });
@@ -639,6 +644,16 @@ pub(super) fn plan(root: &Path) -> Result<Layout> {
         .position(|d| d.name.eq_ignore_ascii_case("VIDEO_TS"))
     {
         cursor = place_video_ts(&mut tree.dirs[idx], cursor)?;
+        // `place_video_ts` places only the FILES directly inside VIDEO_TS,
+        // because only those carry the IFO-relative constraints. Anything in a
+        // subdirectory of VIDEO_TS still needs data placed: without this its
+        // File Entry is written with the file's real size but no extents, so it
+        // appears in the tree at full length and reads back as nothing, at
+        // exit 0. The same folder under BDMV/ was always placed correctly,
+        // which is what made the gap easy to miss.
+        for sub in tree.dirs[idx].dirs.iter_mut() {
+            place_generic(sub, &mut cursor)?;
+        }
         for f in &mut tree.files {
             cursor = place_file(f, cursor)?;
         }

src/dirimage/tests.rs

@@ -447,6 +447,35 @@ fn an_oversized_vob_offset_leaves_a_gap_rather_than_failing() {
     assert!(buf.iter().all(|&b| b == 0));
 }
 
+/// A file inside a SUBDIRECTORY of `VIDEO_TS` must still have its data placed.
+///
+/// Audit finding. The DVD branch places only the files directly inside
+/// `VIDEO_TS`, because only those carry the IFO-relative constraints — and the
+/// follow-up loop skipped that directory entirely, so anything one level deeper
+/// got a File Entry declaring the file's real size with no extents behind it.
+/// It appeared in the tree at full length and read back as nothing, at exit 0.
+/// The identical folder under `BDMV/` was always placed correctly, which is
+/// what made the gap easy to miss.
+#[test]
+fn a_file_below_video_ts_is_placed_not_just_declared() {
+    let s = Scratch::new("dvdsubdir");
+    s.file("VIDEO_TS/VIDEO_TS.IFO", &vec![0u8; SECTOR]);
+    s.file("VIDEO_TS/VTS_01_0.IFO", &vts_ifo(SECTOR, 0, 2));
+    s.file("VIDEO_TS/VTS_01_1.VOB", &pattern(1, SECTOR));
+    let payload = pattern(7, SECTOR);
+    s.file("VIDEO_TS/EXTRA/notes.bin", &payload);
+
+    let mut img = DirImage::open(s.path()).unwrap();
+    let fs = udf::read_filesystem(&mut img).unwrap();
+    let got = fs
+        .read_file(&mut img, "/VIDEO_TS/EXTRA/notes.bin")
+        .expect("the file must be readable");
+    assert_eq!(
+        got, payload,
+        "a file below VIDEO_TS must read back as its real contents, not zeros"
+    );
+}
+
 /// A VOBS offset far past the content must be REFUSED, not honoured.
 ///
 /// Audit finding. The planner honours a title set's declared VOBS offset

src/error.rs

@@ -1053,6 +1053,12 @@ impl std::fmt::Display for Error {
             },
             Error::Halted => write!(f, "E{}", self.code()),
             Error::UdfNotFound { path } => write!(f, "E{}: {}", self.code(), path),
+            Error::SeamPlanDroppedMost { dropped, written } => {
+                write!(f, "E{} {dropped}/{written}", self.code())
+            }
+            Error::ShortImageRead { lba, expected, got } => {
+                write!(f, "E{} {lba} {expected} {got}", self.code())
+            }
             Error::DirImagePlacement { path }
             | Error::DirImageFileChanged { path }
             | Error::DirNameTooLong { path }
@@ -1572,6 +1578,19 @@ mod tests {
             Error::DirImageTooLarge.code(),
             Error::DirNameTooLong { path: "x".into() }.code(),
             Error::DirImageFanout { path: "x".into() }.code(),
+            Error::SeamPlanDroppedMost {
+                dropped: 1,
+                written: 0,
+            }
+            .code(),
+            Error::SinkWroteNothing.code(),
+            Error::ShortImageRead {
+                lba: 0,
+                expected: 1,
+                got: 0,
+            }
+            .code(),
+            Error::EmptyImage.code(),
         ];
         let mut sorted = codes.to_vec();
         sorted.sort();

src/mux/demux_sink.rs

@@ -660,6 +660,12 @@ pub struct DemuxSink {
     /// Index = track id; `None` for unselected tracks.
     tracks: Vec<Option<TrackOut>>,
     ref_video_track: Option<usize>,
+    /// Every VIDEO track index, recorded before the kind filter.
+    ///
+    /// The filtered `tracks` slots are `None` for a class this export drops, so
+    /// they cannot answer "is this video" for a frame that still flows through
+    /// `write()`.
+    video_tracks: std::collections::HashSet<usize>,
     /// First PTS observed on `ref_video_track`, recorded in `write()` REGARDLESS
     /// of whether that track has a `TrackOut`. The DELAY reference cannot live in
     /// `TrackOut::first_pts_ns`: `audio://` / `sub://` filter the video track's
@@ -671,11 +677,15 @@ pub struct DemuxSink {
     ref_first_pts_ns: Option<i64>,
     timeline: TimelineContinuity,
     finished: bool,
-    /// Frames actually placed on the timeline and written.
+    /// Frames the timeline PLACED (not necessarily persisted).
     ///
-    /// A sink that wrote nothing must not report success, and a sink that
+    /// A frame for a track this export filters out still flows through
-    /// dropped more than it kept is not looking at a real join.
+    /// `write()` and still counts here, so this is "the timeline placed
-    frames_written: u64,
+    /// something" rather than "a file received bytes". That is the right
+    /// denominator for the drop gate below — it asks whether placement worked
+    /// at all — but it is deliberately NOT a per-track written count, so a
+    /// single lost track among many is not visible to it.
+    frames_mapped: u64,
 }
 
 impl DemuxSink {
@@ -684,6 +694,7 @@ impl DemuxSink {
         std::fs::create_dir_all(dir)?;
         let mut tracks: Vec<Option<TrackOut>> = Vec::with_capacity(title.streams.len());
         let mut ref_video_track = None;
+        let mut video_tracks: std::collections::HashSet<usize> = std::collections::HashSet::new();
 
         for (idx, stream) in title.streams.iter().enumerate() {
             let selected = opts
@@ -710,6 +721,15 @@ impl DemuxSink {
             if kind == TrackKind::Video && ref_video_track.is_none() {
                 ref_video_track = Some(idx);
             }
+            // Also BEFORE the kind filter, and for the same reason: the seam
+            // crossing rule differs for a track that carries B-frame reorder,
+            // and a video track still flows through write() on an `audio://`
+            // or `sub://` export even though nothing persists it. Reading the
+            // kind back out of the filtered slot would call it non-video and
+            // hand it the permissive rule.
+            if kind == TrackKind::Video {
+                video_tracks.insert(idx);
+            }
             // Kind filter: `audio://` / `sub://` keep only their class.
             if opts.kind_filter.is_some_and(|k| k != kind) {
                 tracks.push(None);
@@ -746,10 +766,11 @@ impl DemuxSink {
             opts: opts.clone(),
             tracks,
             ref_video_track,
+            video_tracks,
             ref_first_pts_ns: None,
             timeline: TimelineContinuity::with_clips(&title.clips, title.content_format),
             finished: false,
-            frames_written: 0,
+            frames_mapped: 0,
         })
     }
 
@@ -876,17 +897,13 @@ impl Stream for DemuxSink {
         // (a Dolby Vision enhancement layer) does not drive epochs but does
         // carry B-frame reorder, and the seam-crossing rule differs on exactly
         // that property.
-        let is_video = self
+        let is_video = self.video_tracks.contains(&frame.track);
-            .tracks
-            .get(frame.track)
-            .and_then(|s| s.as_ref())
-            .is_some_and(|t| t.kind == TrackKind::Video);
         // See `MkvMuxer::write_frame`: `None` is material outside the
         // playlist's clip marks and is dropped rather than emitted.
         let Some(pts) = self.timeline.map(frame.pts, drives, frame.track, is_video) else {
             return Ok(());
         };
-        self.frames_written = self.frames_written.saturating_add(1);
+        self.frames_mapped = self.frames_mapped.saturating_add(1);
         if drives {
             // Delay reference: recorded here, not in the track's `TrackOut`, so
             // it survives the `audio://` / `sub://` kind filter dropping the
@@ -916,13 +933,13 @@ impl Stream for DemuxSink {
         // 0. Keyed on frames having been offered, because a sink that is never
         // given any — a chapters-only export, or a track class the title does
         // not carry — legitimately writes none.
-        if self.frames_written == 0 && seam_dropped > 0 {
+        if self.frames_mapped == 0 && seam_dropped > 0 {
             return Err(crate::error::Error::SinkWroteNothing.into());
         }
-        if seam_dropped > self.frames_written {
+        if seam_dropped > self.frames_mapped {
             return Err(crate::error::Error::SeamPlanDroppedMost {
                 dropped: seam_dropped,
-                written: self.frames_written,
+                written: self.frames_mapped,
             }
             .into());
         }

src/mux/mkv.rs

@@ -1697,6 +1697,15 @@ impl<W: Write + Seek> MkvMuxer<W> {
         // would otherwise yield a structurally-empty MKV with no clusters or
         // cues. Surface that as an error rather than writing valid-but-empty
         // output.
+        // Order matters. A title the seam plan dropped ENTIRELY also has a zero
+        // frame count, and `MkvInvalid` is classified by
+        // `is_skippable_title_stub` as an empty nav/menu stub — so reporting it
+        // that way would make an all-titles rip drop a real feature and finish
+        // the rest at exit 0. Decide the seam case FIRST, with a code that is
+        // not skippable.
+        if self.frame_count == 0 && self.continuity.dropped_total() > 0 {
+            return Err(crate::error::Error::SinkWroteNothing.into());
+        }
         if self.frame_count == 0 {
             return Err(crate::error::Error::MkvInvalid.into());
         }

src/mux/timeline.rs

@@ -1022,7 +1022,6 @@ mod tests {
     fn a_second_video_track_keeps_the_reorder_safe_window() {
         let clips = seamless_branching_clips();
         let mut plan = SeamPlan::from_clips(&clips).expect("plan");
-        let c0_in = mpls_ticks_to_ns(clips[0].in_time);
         let c0_out = mpls_ticks_to_ns(clips[0].out_time);
         let c1_in = mpls_ticks_to_ns(clips[1].in_time);
 
@@ -1040,7 +1039,6 @@ mod tests {
             base - 42_000_000,
             "an enhancement layer's reorder dip must not be read as a join"
         );
-        let _ = c0_in;
     }
 
     /// A clip table that is not one advancing clock must fall back to