diff --git a/src/dirimage/encode.rs b/src/dirimage/encode.rs index f7f834f..84d6166 100644 --- a/src/dirimage/encode.rs +++ b/src/dirimage/encode.rs @@ -493,6 +493,14 @@ fn push_fid(buf: &mut Vec, name: &str, icb_lba: u32, is_dir: bool, is_parent chars |= 0x08; } fid[18] = chars; + // The planner refuses any name whose encoding exceeds what this byte can + // hold (`layout::MAX_CS0_NAME_BYTES`), so this cannot wrap in practice. The + // assert states the invariant where it is relied on rather than trusting a + // check three files away; a wrap here would desynchronise the directory. + debug_assert!( + l_fi <= u8::MAX as usize, + "FID name length must fit one byte" + ); fid[19] = l_fi as u8; put_long_ad(&mut fid[20..36], SECTOR as u32, icb_lba); fid[36..38].copy_from_slice(&0u16.to_le_bytes()); // length of implementation use @@ -590,7 +598,10 @@ fn write_dir(out: &mut MetaSectors, layout: &Layout, dir: &DirNode) -> Result<() // A directory's link count is 1 (its own FID in the parent) plus one for // each child directory's parent FID pointing back at it. - let link_count = 1 + dir.dirs.len() as u16; + // The planner caps subdirectory fan-out (`layout::MAX_SUBDIRS`) so this + // cannot overflow; saturating rather than wrapping keeps a future change to + // that cap from silently producing a wrong count. + let link_count = (dir.dirs.len() as u16).saturating_add(1); let fe = file_entry( true, fids.len() as u64, diff --git a/src/dirimage/layout.rs b/src/dirimage/layout.rs index ec6819e..a904e65 100644 --- a/src/dirimage/layout.rs +++ b/src/dirimage/layout.rs @@ -46,6 +46,22 @@ const MAX_DEPTH: u32 = 8; /// `MAX_TOTAL_DIR_ENTRIES`. const MAX_ENTRIES: usize = 100_000; +/// Longest OSTA CS0 encoding a File Identifier Descriptor can describe. +/// +/// The FID's name-length field is one byte, so 255 is the ceiling; 254 leaves +/// the encoder no way to produce a value that wraps to zero. +const MAX_CS0_NAME_BYTES: usize = 254; + +/// Most subdirectories one directory may hold. +/// +/// A directory File Entry's link count is `u16` and counts one per child +/// directory plus one for its own entry in the parent, so the last usable +/// value is `u16::MAX - 1`. +const MAX_SUBDIRS: usize = (u16::MAX - 1) as usize; + +/// The fan-out cap must bite before the global entry cap, or it never fires. +const _: () = assert!(MAX_SUBDIRS < MAX_ENTRIES); + /// One contiguous run of a file's bytes at a partition-relative block. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) struct Extent { @@ -158,12 +174,30 @@ fn walk(dir: &Path, disc_path: &str, depth: u32, entries: &mut usize) -> Result< // loop, and UDF has no link this maps onto. let ft = entry.file_type().map_err(Error::from)?; let child_path = format!("{}/{}", disc_path.trim_end_matches('/'), name); + // A File Identifier Descriptor records the encoded name length in ONE + // byte. A longer name would wrap that field and desynchronise every + // later entry in the directory, so refuse while planning rather than + // encode something unreadable. 255 ASCII bytes is a legal name on + // ext4/APFS/NTFS and already exceeds this once the CS0 compression + // byte is added, so it is reachable without anything exotic. + if super::encode::encode_cs0(&name).len() > MAX_CS0_NAME_BYTES { + return Err(Error::DirNameTooLong { path: child_path }); + } *entries += 1; if *entries > MAX_ENTRIES { return Err(Error::DirImageTooLarge); } names.push(name.to_ascii_uppercase()); if ft.is_dir() { + // A directory's File Entry records its link count in 16 bits: one + // per child directory plus one for its own entry in the parent. + // The global entry cap alone permits a single directory holding + // more than that, which would wrap the count. + if dirs.len() >= MAX_SUBDIRS { + return Err(Error::DirImageFanout { + path: disc_path.to_string(), + }); + } dirs.push(walk(&entry.path(), &child_path, depth + 1, entries)?); } else { let meta = match std::fs::metadata(entry.path()) { @@ -679,4 +713,53 @@ mod tests { assert!(!is_excluded("00000.m2ts")); assert!(!is_excluded("VTS_01_1.VOB")); } + + /// A name too long for the FID's one-byte length field is refused while + /// planning. + /// + /// Audit finding: the length was narrowed with `as u8`, so a 255-byte ASCII + /// name — legal on ext4/APFS/NTFS — encoded to 256 bytes with the CS0 + /// compression byte and wrote a length of ZERO. Every later entry in that + /// directory would then be read from the wrong offset, losing files with no + /// error. The cap must sit below the point where the field wraps. + #[test] + fn an_over_long_name_is_refused_not_truncated() { + let longest_ok = "a".repeat(MAX_CS0_NAME_BYTES - 1); + assert_eq!( + super::super::encode::encode_cs0(&longest_ok).len(), + MAX_CS0_NAME_BYTES, + "fixture: the longest accepted name encodes to exactly the cap" + ); + assert!( + MAX_CS0_NAME_BYTES < u8::MAX as usize, + "the cap must leave the length field unable to wrap" + ); + // The exact case that used to write a length of zero: a 255-byte name + // (the POSIX NAME_MAX, so entirely ordinary) encodes to 256 bytes once + // the CS0 compression byte is prepended, and 256 narrows to 0 in a u8. + let name_max = "a".repeat(255); + let encoded = super::super::encode::encode_cs0(&name_max).len(); + assert_eq!(encoded, 256, "fixture: NAME_MAX encodes to 256 bytes"); + assert_eq!( + encoded as u8, 0, + "fixture: this is the narrowing that silently zeroed the field" + ); + assert!( + encoded > MAX_CS0_NAME_BYTES, + "so the planner must refuse it before the encoder sees it" + ); + } + + /// The subdirectory cap keeps a directory's 16-bit link count from wrapping. + /// + /// Audit finding: the count was `1 + dirs.len() as u16`, and the global + /// entry cap alone permits one directory holding 65,535 subdirectories. + #[test] + fn the_subdir_cap_keeps_the_link_count_representable() { + assert_eq!( + (MAX_SUBDIRS as u16).checked_add(1), + Some(u16::MAX), + "the largest permitted fan-out must still fit the link count" + ); + } } diff --git a/src/error.rs b/src/error.rs index e66c001..1ba69b6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -195,6 +195,8 @@ pub const E_DIR_IMAGE_FILE_CHANGED: u16 = 9065; /// A `dir://` SOURCE folder does not fit a 32-bit sector address space /// (> 2^32 sectors ≈ 8 TiB), or holds more entries than a UDF tree can carry. pub const E_DIR_IMAGE_TOO_LARGE: u16 = 9066; +pub const E_DIR_NAME_TOO_LONG: u16 = 9067; +pub const E_DIR_IMAGE_FANOUT: u16 = 9068; pub const E_M2TS_PACKET_MALFORMED: u16 = 9021; /// A `network://` output target resolved to no address that is safe to /// connect to (every resolved IP was loopback / private / link-local / @@ -777,6 +779,25 @@ pub enum Error { }, /// A `dir://` SOURCE folder exceeds the addressable image size. DirImageTooLarge, + /// A name is too long to record in a UDF directory entry. + /// + /// The File Identifier Descriptor stores the encoded name length in ONE + /// byte, so a name whose OSTA CS0 encoding exceeds 254 bytes cannot be + /// described. Truncating the length field instead would desynchronise the + /// whole directory — every later entry in it would be read from the wrong + /// offset — so an over-long name is refused while the tree is still being + /// planned. + DirNameTooLong { + path: String, + }, + /// One directory holds more subdirectories than a UDF link count can express. + /// + /// A directory's File Entry records its link count in 16 bits, and that + /// count is one per child directory plus one for its own entry in its + /// parent. Beyond that the count silently wraps, so the tree is refused. + DirImageFanout { + path: String, + }, } impl Error { @@ -898,6 +919,8 @@ impl Error { Error::DirImagePlacement { .. } => E_DIR_IMAGE_PLACEMENT, Error::DirImageEncrypted => E_DIR_IMAGE_ENCRYPTED, Error::DirImageUnsupportedTree => E_DIR_IMAGE_UNSUPPORTED_TREE, + Error::DirNameTooLong { .. } => E_DIR_NAME_TOO_LONG, + Error::DirImageFanout { .. } => E_DIR_IMAGE_FANOUT, Error::DirImageFileChanged { .. } => E_DIR_IMAGE_FILE_CHANGED, Error::DirImageTooLarge => E_DIR_IMAGE_TOO_LARGE, } diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index fd80d59..cfa5c1f 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -1704,6 +1704,20 @@ impl MkvMuxer { "frames were discarded before the first cluster opened (no track-0 video keyframe had arrived yet); they are absent from the output" ); } + // Frames the playlist's clip marks excluded are dropped on purpose — a + // join legitimately discards the material a disc stores twice — but the + // count must not be write-only. Same reasoning as the pre-cluster + // counter above: an unexpected VOLUME here is how a title ends up + // quietly short while the run reports success. + let seam_dropped = self.continuity.dropped_total(); + if seam_dropped > 0 { + tracing::info!( + target: "mux", + dropped = seam_dropped, + frames_written = self.frame_count, + "frames outside the playlist's clip marks were dropped at clip joins" + ); + } // The source declared no duration up-front (DURATION was reserved as a // placeholder). Derive the real runtime from the muxed timeline so the // Segment declares it — and so the BPS tags below can be computed. diff --git a/src/mux/timeline.rs b/src/mux/timeline.rs index 8a5da08..3103e82 100644 --- a/src/mux/timeline.rs +++ b/src/mux/timeline.rs @@ -1,12 +1,25 @@ -//! Shared clip-boundary timeline-continuity corrector. +//! Shared clip-boundary timeline corrector. //! -//! A BD/UHD title's clips are read as one concatenated sector stream (clip -//! boundaries / mpls connection_condition are not plumbed to the mux), so at a -//! non-seamless boundary the source PES PTS jumps backward. Left uncorrected, -//! that produces a sustained band of non-monotonic block timestamps. Every -//! muxer/sink that consumes the interleaved per-track PES stream and emits a -//! monotonic timeline (the MKV muxer, the `demux://` elementary-stream sink) -//! uses [`TimelineContinuity`] so the correction lives in exactly one place. +//! A BD/UHD title's clips are read as one concatenated sector stream, so the +//! source PES PTS does not run continuously across a clip join. There are two +//! ways to place them, and this module holds both: +//! +//! - **From the playlist's marks** ([`SeamPlan`]) — when the title carries +//! PlayItem IN/OUT times, each clip contributes exactly `out - in` and the +//! clips are laid end to end. This is exact: it closes forward skips, joins +//! overlaps without rewinding, and drops material the playlist excludes. +//! - **By inference** ([`TimelineContinuity::adjust`]) — when there are no +//! usable marks (DVD, HD-DVD, `mkv://` / `m2ts://` sources), a backward PTS +//! jump larger than [`DISCONTINUITY_BACKSTEP_NS`] is read as a join and +//! rebased. Inference cannot see a forward skip, because a forward gap is +//! indistinguishable from frames lost to damaged media, and cannot see an +//! overlap smaller than the reorder threshold. +//! +//! [`TimelineContinuity::map`] picks between them: marks when present, +//! inference otherwise. Every muxer/sink that consumes the interleaved per-track +//! PES stream and emits a monotonic timeline (the MKV muxer, the `demux://` +//! elementary-stream sink) goes through it, so the correction lives in exactly +//! one place. /// A backward PTS step larger than this is treated as a clip-boundary /// discontinuity (a non-seamless BD clip / dual-layer-break where the source @@ -78,6 +91,14 @@ pub(crate) struct SeamClip { /// and material outside a clip's marks is dropped rather than emitted twice. pub(crate) struct SeamPlan { clips: Vec, + /// Frames dropped because they fell outside every clip's marks, per track. + /// + /// A drop is correct — the playlist does not include that material — but a + /// SILENT drop is how this codebase has produced complete-looking, wrong + /// output before. Counting them means an unexpected volume shows up in the + /// log instead of in someone's file, and gives a caller something to assert + /// on. Indexed by track alongside `cursors`. + dropped: Vec, /// Per-track position: (clip index, last raw PTS seen). /// /// Each track crosses a join on ITS OWN frame, not on video's. The demuxer @@ -132,9 +153,22 @@ impl SeamPlan { Some(Self { clips: out, cursors: Vec::new(), + dropped: Vec::new(), }) } + /// How many frames this track has had dropped for falling outside every + /// clip's marks. + #[cfg(test)] + pub(crate) fn dropped_for(&self, track: usize) -> u64 { + self.dropped.get(track).copied().unwrap_or(0) + } + + /// Frames dropped across every track. + pub(crate) fn dropped_total(&self) -> u64 { + self.dropped.iter().fold(0u64, |a, b| a.saturating_add(*b)) + } + /// Total playable duration (ns) — the sum of every clip's `out − in`. This /// is the length the title actually is, and what the output timeline must /// end at. @@ -150,7 +184,7 @@ impl SeamPlan { /// /// `None` means DROP: the frame lies outside every clip's marks, so the /// playlist does not include it. - fn place(&mut self, raw_ns: i64, track: usize) -> Option { + fn place(&mut self, raw_ns: i64, track: usize, drives: bool) -> Option { if self.cursors.len() <= track { self.cursors.resize( track + 1, @@ -159,6 +193,7 @@ impl SeamPlan { last_raw_ns: None, }, ); + self.dropped.resize(track + 1, 0); } let pos = self.cursors[track]; let mut clip = pos.clip; @@ -184,8 +219,32 @@ impl SeamPlan { // the current position: a bound only from above is satisfied by // every later clip's IN too, and the cursor would run to the end of // the list on a single backward step. + // A backward step means this track has restarted at the next + // clip's IN. What counts as "at" differs by track, and getting it + // wrong strands a track on the previous clip's offset: + // + // - VIDEO is dense (a frame every ~42ms at 24fps), so its first + // frame of the new clip lands ON the mark. Requiring that is what + // keeps a B-frame reorder dip near the end of a clip — which is + // also a backward step, and inside the next clip's range during + // an overlap — from being mistaken for a crossing. + // - PASSIVE tracks have no reorder, so ANY backward step is a + // crossing. They can also be sparse: a subtitle may have no event + // near the mark at all, and its first frame after the join can + // land well past it. Holding those to the video window left them + // on the old clip until their PTS passed its OUT, mistiming them + // by the overlap in between. + // + // Both forms still require the frame to land at or after the next + // IN, which is what stops one backward step walking the cursor to + // the end of the list. let stepped_back = pos.last_raw_ns.is_some_and(|last| raw_ns < last) - && (raw_ns.saturating_sub(next_in)).abs() <= CLIP_START_TOLERANCE_NS; + && if drives { + (raw_ns.saturating_sub(next_in)).abs() <= CLIP_START_TOLERANCE_NS + } else { + raw_ns >= next_in.saturating_sub(CLIP_START_TOLERANCE_NS) + && raw_ns <= self.clips[clip + 1].out_ns + }; if past_out || stepped_back { clip += 1; } else { @@ -199,16 +258,37 @@ impl SeamPlan { last_raw_ns: Some(raw_ns), }; if raw_ns < c.in_ns || raw_ns > c.out_ns { + self.dropped[track] = self.dropped[track].saturating_add(1); + // Once per track, and only on the first drop: a join legitimately + // drops a handful of frames, so this must not become per-frame + // noise on a normal title. The total is available to callers. + if self.dropped[track] == 1 { + tracing::debug!( + target: "freemkv::mux", + track, + clip, + raw_ns, + in_ns = c.in_ns, + out_ns = c.out_ns, + "frame outside the playlist's clip marks; dropping" + ); + } return None; } Some(raw_ns.saturating_add(c.offset_ns)) } } -/// Global timeline-continuity corrector. freemkv reads a BD title's clips as -/// one concatenated sector stream (clip boundaries / mpls connection_condition -/// are not plumbed to the mux), so at a non-seamless boundary the source PES -/// PTS jumps backward. Left uncorrected, that produces a sustained band of +/// Global timeline corrector. +/// +/// Holds a [`SeamPlan`] when the title's PlayItem marks are usable, and falls +/// back to the PTS-jump inference described below when they are not. The +/// inference documentation that follows applies to the FALLBACK path only — +/// under a plan, placement comes from the marks and none of the epoch/frontier +/// reasoning below decides anything. +/// +/// freemkv reads a BD title's clips as one concatenated sector stream, so at a +/// non-seamless boundary the source PES PTS jumps backward. Left uncorrected, that produces a sustained band of /// non-monotonic block timestamps (a downstream muxer then derives /// non-monotonic DTS from them). /// @@ -286,6 +366,15 @@ impl TimelineContinuity { } } + /// Total frames dropped for falling outside the playlist's clip marks. + /// + /// Zero for a title without a seam plan. A muxer reports this when it + /// finishes so a drop is never invisible: dropping is correct at a join, + /// but an unexpected VOLUME of drops is how output ends up quietly short. + pub(crate) fn dropped_total(&self) -> u64 { + self.seams.as_ref().map_or(0, |p| p.dropped_total()) + } + /// Map a raw PES PTS onto the output timeline, or `None` to drop the frame. /// /// Dropping only ever happens under a [`SeamPlan`]: it is material outside @@ -295,7 +384,7 @@ impl TimelineContinuity { // Take the plan out for the call so `place` can borrow `self` // mutably without fighting the borrow checker over the whole struct. let mut plan = self.seams.take().expect("checked is_some"); - let placed = plan.place(raw_pts_ns, track); + let placed = plan.place(raw_pts_ns, track, drives_epoch); self.seams = Some(plan); if let Some(p) = placed { // Keep the frontier meaningful for anything that reads it, and @@ -542,11 +631,13 @@ mod tests { let out_ns = mpls_ticks_to_ns(c.out_time); // First frame of the clip lands at the running total. let got = plan - .place(in_ns, 0) + .place(in_ns, 0, true) .expect("clip start is inside its marks"); assert_eq!(got, expected_start, "clip {i} start misplaced"); // Last frame lands at the running total plus the clip's length. - let end = plan.place(out_ns, 0).expect("clip end is inside its marks"); + let end = plan + .place(out_ns, 0, true) + .expect("clip end is inside its marks"); assert_eq!( end, expected_start + (out_ns - in_ns), @@ -574,8 +665,8 @@ mod tests { c3_in - c2_out > 9_000_000_000, "fixture should contain the ~9.17s skip" ); - let end_of_2 = plan.place(c2_out, 0).expect("in clip 2"); - let start_of_3 = plan.place(c3_in, 0).expect("in clip 3"); + let end_of_2 = plan.place(c2_out, 0, true).expect("in clip 2"); + let start_of_3 = plan.place(c3_in, 0, true).expect("in clip 3"); assert_eq!( start_of_3, end_of_2, "clip 3 must begin exactly where clip 2 ended — the skip is not content" @@ -596,21 +687,23 @@ mod tests { let c1_in = mpls_ticks_to_ns(clips[1].in_time); assert!(c1_in < c0_out, "fixture should contain the overlap"); // Play clip 0 through to its OUT mark. - let last_of_0 = plan.place(c0_out, 0).expect("clip 0 OUT is inside clip 0"); + let last_of_0 = plan + .place(c0_out, 0, true) + .expect("clip 0 OUT is inside clip 0"); // The next clip opens ON its IN mark. Under the old inference this was a // 1.79s backward step, below the reorder threshold, so no seam was // recognised and the join was emitted as duplicate content whose // timestamps then collided. With the marks known, clip 1 is placed to // continue exactly where clip 0 ended: one monotonic timeline, no // rewind, and no collision for the muxer to flatten. - let first_of_1 = plan.place(c1_in, 0).expect("clip 1 IN"); + let first_of_1 = plan.place(c1_in, 0, true).expect("clip 1 IN"); assert_eq!( first_of_1, last_of_0, "clip 1 must continue from clip 0's end, not rewind by the overlap" ); // And the timeline keeps moving forward from there. let into_1 = plan - .place(c1_in + 1_000_000_000, 0) + .place(c1_in + 1_000_000_000, 0, true) .expect("1s into clip 1"); assert_eq!( into_1, @@ -628,14 +721,144 @@ mod tests { let c0_out = mpls_ticks_to_ns(clips[0].out_time); let c1_in = mpls_ticks_to_ns(clips[1].in_time); // Video crosses into clip 1. - plan.place(c1_in + 500_000_000, 0).expect("in clip 1"); + plan.place(c1_in + 500_000_000, 0, true).expect("in clip 1"); // A straggler from clip 0's tail arrives afterwards. let tail = c0_out - 50_000_000; // 50ms before clip 0's OUT - let placed = plan.place(tail, 1).expect("straggler must be placed"); + let placed = plan + .place(tail, 1, false) + .expect("straggler must be placed"); let expected = tail + (0i64 - mpls_ticks_to_ns(clips[0].in_time)); assert_eq!(placed, expected, "straggler must ride clip 0's offset"); } + /// `map()`'s seam-plan branch — the glue between `SeamPlan::place` and the + /// frontier/offset bookkeeping — was untested. Audit finding: a wrong + /// operand in `offset_ns = p - raw_pts_ns`, or a stale `high_ns` across a + /// join, would corrupt downstream cluster timing and no test would notice. + #[test] + fn map_under_a_seam_plan_tracks_offset_and_frontier() { + let clips = seamless_branching_clips(); + let mut tc = TimelineContinuity::with_clips(&clips); + assert!(tc.seams.is_some(), "a multi-clip title must get a plan"); + + let c0_in = mpls_ticks_to_ns(clips[0].in_time); + let c0_out = mpls_ticks_to_ns(clips[0].out_time); + + let first = tc.map(c0_in, true, 0).expect("first frame"); + assert_eq!(first, 0, "clip 0 starts the output timeline at zero"); + assert_eq!( + tc.offset_ns, -c0_in, + "offset is the correction actually applied" + ); + assert_eq!(tc.high_ns, Some(0), "video advances the frontier"); + + let later = tc.map(c0_in + 5_000_000_000, true, 0).expect("later frame"); + assert_eq!(later, 5_000_000_000); + assert_eq!(tc.high_ns, Some(5_000_000_000), "frontier follows video"); + + // A passive track must NOT advance the frontier. + let before = tc.high_ns; + tc.map(c0_in + 1_000_000_000, false, 1).expect("audio"); + assert_eq!(tc.high_ns, before, "passive tracks never move the frontier"); + + // Across the join the frontier keeps rising, never rewinds. + let across = tc.map(c0_out, true, 0).expect("clip 0 OUT"); + assert!( + across >= 5_000_000_000, + "timeline must not rewind at a join" + ); + } + + /// A frame outside every clip's marks is dropped, and the drop is COUNTED. + /// Audit finding: an uncounted drop is the silent-wrong-output shape this + /// project has shipped before. + #[test] + fn frames_outside_the_marks_are_dropped_and_counted() { + 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); + assert_eq!(plan.place(c0_in - 5_000_000_000, 0, true), None, "dropped"); + assert_eq!(plan.dropped_for(0), 1, "and counted"); + assert_eq!(plan.dropped_for(1), 0, "counted per track, not globally"); + plan.place(c0_in, 0, true).expect("inside"); + assert_eq!( + plan.dropped_for(0), + 1, + "a placed frame must not count as dropped" + ); + } + + /// A SPARSE passive track crosses even when its first frame after the join + /// lands well past the mark. + /// + /// Audit finding. A PGS subtitle track may have no event near a clip's IN + /// at all. Holding it to the dense-video window (250ms either side of the + /// mark) left it on the PREVIOUS clip's offset until its PTS finally passed + /// that clip's OUT — mistiming every subtitle in between by the overlap, + /// 1.79s on the measured title. + #[test] + fn a_sparse_passive_track_crosses_late() { + 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); + + // The track's last event in clip 0, near its OUT. + let tail = c0_out - 100_000_000; + let placed_tail = plan.place(tail, 1, false).expect("clip 0 tail"); + assert_eq!(placed_tail, tail - c0_in, "tail rides clip 0's offset"); + + // Its first event in clip 1 steps BACK (the clips overlap) but lands + // 400ms past the IN mark, because the track is sparse and had no event + // sitting on the mark. The old window was +/-250ms, so this was missed + // and the event stayed on clip 0 — mistimed by the overlap. + let late = c1_in + 400_000_000; + assert!(late < tail, "fixture: this is a backward step"); + assert!( + (late - c1_in) > CLIP_START_TOLERANCE_NS, + "fixture: further past the mark than the video window allows" + ); + let got = plan + .place(late, 1, false) + .expect("late event must be placed"); + + let clip1_offset = (c0_out - c0_in) - c1_in; + assert_eq!( + got, + late + clip1_offset, + "sparse track must cross to clip 1" + ); + assert!(got > placed_tail, "and must not rewind the timeline"); + } + + /// A video reorder dip inside the overlap window must NOT be read as a + /// crossing. This is the other side of the sparse-track fix: video keeps + /// the tight window precisely because its backward steps are also reorder. + #[test] + fn a_video_reorder_dip_in_the_overlap_is_not_a_crossing() { + let clips = seamless_branching_clips(); + let mut plan = SeamPlan::from_clips(&clips).expect("plan"); + let c0_out = mpls_ticks_to_ns(clips[0].out_time); + let c1_in = mpls_ticks_to_ns(clips[1].in_time); + // Video near the end of clip 0 — inside the overlap, so these PTS are + // also inside clip 1's range. + let a = c0_out - 300_000_000; + let base = plan.place(a, 0, true).expect("in clip 0"); + // A reorder dip of ~42ms: backward, and >= clip 1's IN. + let dip = a - 42_000_000; + assert!( + dip >= c1_in, + "fixture: the dip is inside clip 1's range too" + ); + let got = plan.place(dip, 0, true).expect("dip placed"); + assert_eq!( + got, + base - 42_000_000, + "a reorder dip must stay on clip 0, not jump to clip 1's offset" + ); + } + /// Each track crosses a join on its OWN frame. /// /// This is the regression for the first attempt at this fix, which gave @@ -657,17 +880,17 @@ mod tests { // Audio (track 1) runs up to near clip 0's OUT. let tail = c0_out - 200_000_000; - plan.place(c0_in, 1).expect("audio start"); - let a_tail = plan.place(tail, 1).expect("audio tail"); + plan.place(c0_in, 1, false).expect("audio start"); + let a_tail = plan.place(tail, 1, false).expect("audio tail"); // Video (track 0) crosses into clip 1 first. - plan.place(c0_out, 0).expect("video at clip 0 OUT"); - plan.place(c1_in, 0).expect("video at clip 1 IN"); + plan.place(c0_out, 0, true).expect("video at clip 0 OUT"); + plan.place(c1_in, 0, true).expect("video at clip 1 IN"); // Audio's NEXT tail frame still belongs to clip 0 and must stay there — // contiguous with the previous one, not thrown forward by the overlap. let a_tail2 = plan - .place(tail + 10_000_000, 1) + .place(tail + 10_000_000, 1, false) .expect("audio tail continues"); assert_eq!( a_tail2 - a_tail, @@ -677,7 +900,7 @@ mod tests { // When audio itself steps back to clip 1's IN, it crosses — and lands // after its own tail, with no rewind and no collision. - let a_new = plan.place(c1_in, 1).expect("audio crosses"); + let a_new = plan.place(c1_in, 1, false).expect("audio crosses"); assert!( a_new > a_tail2, "audio must not rewind at the join (got {a_new} after {a_tail2})" @@ -715,7 +938,7 @@ mod tests { 6_410_000_000_000, ] { assert_eq!( - plan.place(t, 0), + plan.place(t, 0, true), Some(t), "contiguous clips must not move a frame (t={t})" );