diff --git a/CHANGELOG.md b/CHANGELOG.md index ba54582..42721a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,44 @@ # Changelog +## [1.6.4] — UNRELEASED + +### Fixed + +- **On a few Blu-ray/UHD titles the sound ran on for half a minute after the + picture had ended.** A disc stores each part of a film as a clip, and the + playlist marks exactly where that clip's content begins and ends. Where a + title is a single clip, freemkv trimmed the picture to those marks but not the + sound — and some discs leave extra audio in the file past the end mark (a + quiet fade authored after the last frame of picture). That trailing audio was + copied through, so the file claimed one running time while carrying up to ~36 + seconds more sound than picture. Measured on `The Bourne Supremacy`: the + picture ends at 1:48:26 as declared, but every sound track ran to 1:49:02. + Single-clip titles are now trimmed to their playlist marks the same way + multi-clip titles already were, so sound and picture end together at the + declared duration. A title that had no extra material past its marks is + byte-for-byte unchanged. Multi-clip titles were never affected. + +- **A multi-title-set CSS DVD could descramble one title set under another + set's key.** When a DVD's second title set resisted the keyless title-key + recovery, the decrypt fell back to the disc-wide key instead of failing — + writing a corrupt title behind an intact header and reporting success at exit + 0. A failed recovery is now a hard error, the same as every other path in the + crate already does; ordering makes the recovery more likely to succeed but + cannot make a failed one safe. + +- **A disc whose stream language field was all-zero could abort the whole track + export.** An all-zero language code (the ordinary "undefined" value on real + discs) put a NUL byte into a demux output filename and failed file creation + before a single track opened. Control bytes in that field are now sanitised + the same way the rest of the name already was. + +### Security + +- **Bounded the last unbounded attacker-controlled list in the DVD label + parser.** A crafted IFO could grow the forced-subtitle index list without + limit; it now carries the same positional cap as the neighbouring command + lists. No effect on a well-formed disc. + ## [1.6.3] — 2026-08-10 ### Changed diff --git a/Cargo.toml b/Cargo.toml index 8121bbe..1ad4a09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libfreemkv" -version = "1.6.3" +version = "1.6.4" edition = "2024" rust-version = "1.97" license = "MIT" diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index 06fedd3..e98ce53 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -1448,7 +1448,9 @@ impl MkvMuxer { // all tracks, A/V-sync-preserving) keeps the boundary from becoming a // band of non-monotonic block timestamps. Only the PRIMARY video track // drives the boundary decision; every other track (audio, subtitle, DV - // EL) rides the current offset. No-op for single-clip titles. + // EL) rides the current offset. A single-clip BD title still trims to + // its one clip's [in, out] marks (dropping trailing audio the m2ts + // carries past OUT); only a source with no marks at all is a true no-op. // `None` means the playlist does not include this frame — material // outside every clip's IN/OUT marks, which only a seam-plan-driven // title can report. Dropping it is the point: emitting it is what put diff --git a/src/mux/timeline.rs b/src/mux/timeline.rs index 61fa878..f151f8e 100644 --- a/src/mux/timeline.rs +++ b/src/mux/timeline.rs @@ -151,14 +151,25 @@ struct TrackPos { impl SeamPlan { /// Build a plan from a title's clips, or `None` when there is nothing to /// place: no clips (DVD, HD-DVD, `mkv://`/`m2ts://` sources — none of which - /// carry PlayItem marks), a single clip (nothing to join), or marks that are - /// not usable (a zero/inverted span means the playlist is not telling us - /// anything we can act on, and guessing is what this type exists to avoid). + /// carry PlayItem marks), or marks that are not usable (a zero/inverted span + /// means the playlist is not telling us anything we can act on, and guessing + /// is what this type exists to avoid). + /// + /// A **single** clip still gets a plan. Joining is not the marks' only job — + /// trimming to `[in, out]` is — and it matters whenever a clip's physical + /// extent runs past its OUT mark. Real discs author trailing audio (a fade + /// after the last video frame) beyond OUT in the m2ts; without a plan the + /// inference path keeps it, leaving audio seconds past the declared duration. + /// One clip needs no cross-clip placement, so the loop below reduces to the + /// `[in, out]` drop filter plus the standard `offset = −in_ns` rebase — the + /// same one clip 0 of a multi-clip title already gets, which the MKV muxer + /// then re-anchors, so every KEPT frame is byte-identical to the no-plan + /// path. The only change is that out-of-mark frames are now dropped. /// /// Returning `None` leaves [`TimelineContinuity`] on its PTS-jump inference, /// which is what every non-BD path has always used. pub(crate) fn from_clips(clips: &[crate::disc::Clip]) -> Option { - if clips.len() < 2 { + if clips.is_empty() { return None; } // Trust the spans only if they tile the feed contiguously from 0. @@ -730,9 +741,10 @@ impl TimelineContinuity { /// Corrector driven by a title's PlayItem marks where they exist. /// - /// Falls back to [`Self::new`]'s inference when the title has fewer than two - /// clips or its marks are unusable — so DVD, HD-DVD, `mkv://` and `m2ts://` - /// sources behave exactly as before. + /// Falls back to [`Self::new`]'s inference when the title has no clips or + /// its marks are unusable — so DVD, HD-DVD, `mkv://` and `m2ts://` sources + /// behave exactly as before. A single BD clip DOES get a plan: its marks + /// still trim trailing/pre-roll material outside `[in, out]`. pub(crate) fn with_clips( clips: &[crate::disc::Clip], content_format: crate::disc::ContentFormat, @@ -2226,10 +2238,6 @@ mod tests { #[test] fn no_seam_plan_without_usable_marks() { assert!(SeamPlan::from_clips(&[]).is_none(), "no clips"); - assert!( - SeamPlan::from_clips(&seamless_branching_clips()[..1]).is_none(), - "a single clip has nothing to join" - ); let mut bad = seamless_branching_clips(); bad[3].out_time = bad[3].in_time; // zero-length span assert!( @@ -2238,6 +2246,52 @@ mod tests { ); } + /// A SINGLE-clip title still gets a plan: joining is not the marks' only job, + /// trimming to `[in, out]` is. Real discs author trailing audio (a fade after + /// the last video frame) PAST the OUT mark in the m2ts; the no-plan inference + /// path kept it, leaving audio seconds past the declared duration — the + /// `The Bourne Supremacy` defect (audio +35.6 s past a single-clip title's + /// end). The plan drops it, and every KEPT frame is placed exactly as the + /// no-plan path would (raw rebased by `−in_ns`, which the MKV muxer already + /// does), so a disc with no out-of-mark content is byte-identical. + #[test] + fn single_clip_trims_content_outside_its_marks() { + let clips = seamless_branching_clips(); // clip 0: in 4199.0s, out 6033.04s + let mut plan = SeamPlan::from_clips(&clips[..1]).expect("a single clip is planned"); + + let in_ns = mpls_ticks_to_ns(clips[0].in_time); + let out_ns = mpls_ticks_to_ns(clips[0].out_time); + + // In-mark frames are kept and rebased to 0 at IN — and drop nothing. + assert_eq!(plan.place(in_ns, 0, true, None), Some(0), "IN rebases to 0"); + assert_eq!( + plan.place(in_ns + 10 * S, 1, false, None), + Some(10 * S), + "a mid-clip audio frame keeps its offset from IN" + ); + assert_eq!( + plan.place(out_ns, 2, false, None), + Some(out_ns - in_ns), + "a frame AT the OUT mark is inside and kept" + ); + assert_eq!( + plan.dropped_total(), + 0, + "no in-mark frame may be dropped (output is unchanged for a clean disc)" + ); + + // Trailing audio past OUT is dropped AND counted, so the finish() gates + // in the sinks can see the volume. + assert!( + plan.place(out_ns + 30 * S, 1, false, None).is_none(), + "trailing audio 30s past OUT must be dropped, not emitted past the end" + ); + assert!( + plan.dropped_for(1) >= 1, + "the dropped tail frame must be counted" + ); + } + /// Without a plan, `map` is exactly the old `adjust` and never drops. #[test] fn map_without_a_plan_is_the_old_behaviour() {