Make the drop count gate the verdict, not just the log

Round 3 counted frames the clip marks excluded and reported them at
finish. Counting is not bounding: the only other gate was a global
zero-frame check, and its error is additionally classified as a skippable
nav stub, so a title whose marks do not line up with its PES clock could
discard almost all of itself and still exit 0 — a two-hour feature
emitting seconds, which is the defect this change set already shipped
once. Dropping more than was kept is never a real join, so it now fails.

The demux sink had no zero-output guard at all, so a fully-dropped title
finished cleanly: a directory of zero-byte track files beside a populated
chapters document. It now refuses, keyed on frames having been OFFERED —
a chapters-only export, or a track class the title does not carry,
legitimately writes none, and two existing tests correctly said so.

A title set's placement group is the parsed number, so VTS_01_0.IFO and
VTS_1_0.IFO land on one key and the second silently overwrote the first's
constraint, placing a VOB where the IFO the reader uses does not point.
Refused rather than resolved by arrival order.
This commit is contained in:
Matthew Jackson
2026-08-05 18:25:47 -07:00
parent f4fb5c65e0
commit 0f61be00b7
4 changed files with 69 additions and 0 deletions
+10
View File
@@ -433,6 +433,16 @@ fn place_video_ts(vts: &mut DirNode, start: u32) -> Result<u32> {
{ {
let head = read_head(&vts.files[i].host, 0xC8)?; let head = read_head(&vts.files[i].host, 0xC8)?;
let menu = be_u32(&head, 0xC0).unwrap_or(0); let menu = be_u32(&head, 0xC0).unwrap_or(0);
// One group per title set. Two files whose names differ only in
// how the number is written — `VTS_01_0.IFO` and `VTS_1_0.IFO` —
// 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) {
return Err(Error::DirNameCollision {
host: vts.files[i].disc_path.clone(),
});
}
if menu != 0 { if menu != 0 {
menu_req.insert(c.group, lba.saturating_add(menu)); menu_req.insert(c.group, lba.saturating_add(menu));
} }
+20
View File
@@ -201,6 +201,10 @@ pub const E_DIR_NAME_TOO_LONG: u16 = 9067;
/// One directory in the folder holds more subdirectories than a UDF link count /// One directory in the folder holds more subdirectories than a UDF link count
/// can express (it is 16 bits, one per child plus one for its own entry). /// can express (it is 16 bits, one per child plus one for its own entry).
pub const E_DIR_IMAGE_FANOUT: u16 = 9068; pub const E_DIR_IMAGE_FANOUT: u16 = 9068;
/// A title's clip marks excluded more frames than they kept.
pub const E_SEAM_PLAN_DROPPED_MOST: u16 = 9069;
/// A sink finished having written no frames at all.
pub const E_SINK_WROTE_NOTHING: u16 = 9070;
pub const E_M2TS_PACKET_MALFORMED: u16 = 9021; pub const E_M2TS_PACKET_MALFORMED: u16 = 9021;
/// A `network://` output target resolved to no address that is safe to /// A `network://` output target resolved to no address that is safe to
/// connect to (every resolved IP was loopback / private / link-local / /// connect to (every resolved IP was loopback / private / link-local /
@@ -802,6 +806,20 @@ pub enum Error {
DirImageFanout { DirImageFanout {
path: String, path: String,
}, },
/// A title's PlayItem marks excluded more frames than they kept.
///
/// Placing clips by their marks drops whatever falls outside them, which is
/// correct at a join — a disc stores the join twice. Discarding the
/// majority of a title is not a join; it means the marks do not describe
/// the clock the frames are on. Refused rather than written, because the
/// result otherwise looks like a complete file containing seconds of a
/// feature.
SeamPlanDroppedMost {
dropped: u64,
written: u64,
},
/// A sink finished having written no frames at all.
SinkWroteNothing,
} }
impl Error { impl Error {
@@ -925,6 +943,8 @@ impl Error {
Error::DirImageUnsupportedTree => E_DIR_IMAGE_UNSUPPORTED_TREE, Error::DirImageUnsupportedTree => E_DIR_IMAGE_UNSUPPORTED_TREE,
Error::DirNameTooLong { .. } => E_DIR_NAME_TOO_LONG, Error::DirNameTooLong { .. } => E_DIR_NAME_TOO_LONG,
Error::DirImageFanout { .. } => E_DIR_IMAGE_FANOUT, Error::DirImageFanout { .. } => E_DIR_IMAGE_FANOUT,
Error::SeamPlanDroppedMost { .. } => E_SEAM_PLAN_DROPPED_MOST,
Error::SinkWroteNothing => E_SINK_WROTE_NOTHING,
Error::DirImageFileChanged { .. } => E_DIR_IMAGE_FILE_CHANGED, Error::DirImageFileChanged { .. } => E_DIR_IMAGE_FILE_CHANGED,
Error::DirImageTooLarge => E_DIR_IMAGE_TOO_LARGE, Error::DirImageTooLarge => E_DIR_IMAGE_TOO_LARGE,
} }
+23
View File
@@ -671,6 +671,11 @@ pub struct DemuxSink {
ref_first_pts_ns: Option<i64>, ref_first_pts_ns: Option<i64>,
timeline: TimelineContinuity, timeline: TimelineContinuity,
finished: bool, finished: bool,
/// Frames actually placed on the timeline and written.
///
/// A sink that wrote nothing must not report success, and a sink that
/// dropped more than it kept is not looking at a real join.
frames_written: u64,
} }
impl DemuxSink { impl DemuxSink {
@@ -744,6 +749,7 @@ impl DemuxSink {
ref_first_pts_ns: None, ref_first_pts_ns: None,
timeline: TimelineContinuity::with_clips(&title.clips, title.content_format), timeline: TimelineContinuity::with_clips(&title.clips, title.content_format),
finished: false, finished: false,
frames_written: 0,
}) })
} }
@@ -880,6 +886,7 @@ impl Stream for DemuxSink {
let Some(pts) = self.timeline.map(frame.pts, drives, frame.track, is_video) else { let Some(pts) = self.timeline.map(frame.pts, drives, frame.track, is_video) else {
return Ok(()); return Ok(());
}; };
self.frames_written = self.frames_written.saturating_add(1);
if drives { if drives {
// Delay reference: recorded here, not in the track's `TrackOut`, so // Delay reference: recorded here, not in the track's `TrackOut`, so
// it survives the `audio://` / `sub://` kind filter dropping the // it survives the `audio://` / `sub://` kind filter dropping the
@@ -903,6 +910,22 @@ impl Stream for DemuxSink {
// write-only in one sink and reported in the other — an unexpected // write-only in one sink and reported in the other — an unexpected
// volume here is how a demux ends up quietly short. // volume here is how a demux ends up quietly short.
let seam_dropped = self.timeline.dropped_total(); let seam_dropped = self.timeline.dropped_total();
// Frames ARRIVED and every one was dropped. That is a fully-dropped
// title, which this sink used to finish cleanly: a directory of
// zero-byte track files beside a populated chapters document, at exit
// 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 {
return Err(crate::error::Error::SinkWroteNothing.into());
}
if seam_dropped > self.frames_written {
return Err(crate::error::Error::SeamPlanDroppedMost {
dropped: seam_dropped,
written: self.frames_written,
}
.into());
}
if seam_dropped > 0 { if seam_dropped > 0 {
tracing::info!( tracing::info!(
target: "mux", target: "mux",
+16
View File
@@ -1718,6 +1718,22 @@ impl<W: Write + Seek> MkvMuxer<W> {
// counter above: an unexpected VOLUME here is how a title ends up // counter above: an unexpected VOLUME here is how a title ends up
// quietly short while the run reports success. // quietly short while the run reports success.
let seam_dropped = self.continuity.dropped_total(); let seam_dropped = self.continuity.dropped_total();
// Counting a drop is not the same as bounding it. A join legitimately
// discards the material a disc stores twice — tens of frames — but if a
// title's marks do not line up with its PES clock the plan can discard
// most of it, and the only other gate is a GLOBAL zero-frame check
// whose error is additionally classified as a skippable stub. Between
// them, a title emitting seconds of a two-hour feature exits 0. That is
// the defect this change set already shipped once.
//
// More dropped than kept is never a real join, so it fails.
if seam_dropped > self.frame_count {
return Err(crate::error::Error::SeamPlanDroppedMost {
dropped: seam_dropped,
written: self.frame_count,
}
.into());
}
if seam_dropped > 0 { if seam_dropped > 0 {
tracing::info!( tracing::info!(
target: "mux", target: "mux",