From c3afb994f945f389820f72d40c8400065dd947f6 Mon Sep 17 00:00:00 2001 From: SteveYu000 <111431635+SteveYu000@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:36:41 +0800 Subject: [PATCH] Preserve source clocks and VFR range-end coverage Refine only progressive non-TS clocks that need it, preserving ordinary CFR grids and timescales. Distinguish an internal segment seam from the end of a kept range so a normal 23.976 picture at 37.996 s is not reduced to a 4 ms flash by OUT at 38 s. Retain existing unbounded-hold semantics. Classify repeated holds against typical display gaps as well as the average, and let auto indexing scan ambiguous progressive B-frame sources whose public container table only supplies decode timestamps. Invalidate old classifications with seek-cache version 19. Keep the latest displayed picture writable at the final refined-clock tail: decoder reorder depth alone does not bound how many B pictures can follow that picture. Reuse the existing seam-buffer limit. Add an 80-case real-media suite and document its scope. Locally compile both v0.8.39 and this CLI with Rust 1.98/MSVC/FFmpeg 7.1.1: 45/80 to 80/80; review reproducer 41.708333 ms in MP4 and 42 ms in MKV; existing VFR 8/8; 574 workspace unit tests passed, four known Windows failures filtered; clippy exits zero with existing warnings. Historical matrix 140/235 to 165/235 without new failures. Twenty ordinary CFR MP4 files and all 70 TS/M2TS files remain byte-identical to the baseline. This is the independent source-clock/VFR split of #10. SAR #11, AV1/MP4 #12 and M2TS arrival-time changes are separate. A forced --index scan on the reviewer's duplicate-PTS synthetic input retains its pre-existing unbounded-hold behavior and is documented as a limitation. --- docs/technical/building.md | 1 + docs/technical/source-clock.md | 95 ++++++++++ rust/crates/core/src/cut.rs | 185 ++++++++++++++++-- rust/crates/core/src/index.rs | 37 ++++ rust/crates/core/src/seek_index.rs | 4 +- tests/ci.sh | 2 +- tests/run_source_clock_tests.sh | 4 + tests/source_clock_tests.py | 295 +++++++++++++++++++++++++++++ 8 files changed, 602 insertions(+), 21 deletions(-) create mode 100644 docs/technical/source-clock.md create mode 100755 tests/run_source_clock_tests.sh create mode 100644 tests/source_clock_tests.py diff --git a/docs/technical/building.md b/docs/technical/building.md index 2543cf6..3349ffc 100644 --- a/docs/technical/building.md +++ b/docs/technical/building.md @@ -86,6 +86,7 @@ bash tests/run_bd_audio_tests.sh # the sound a disc carries, written out bash tests/run_vc1_tests.sh # the VC-1 encoder, put through a decoder 4 bash tests/run_vp9_av1_tests.sh # VP9 and AV1: cut, and still the source's bytes 5 bash tests/run_vfr_tests.sh # variable rate, both ways round 8 +bash tests/run_source_clock_tests.sh # source clocks, range ends and VFR copying 80 bash tests/run_demux_tests.sh # what a reader loses by reading what it does not 9 bash tests/run_transrate_tests.sh # fitting a disc: the identity, the size, the cost 15 bash tests/run_join_tests.sh # several recordings into one file, and the seams 17 diff --git a/docs/technical/source-clock.md b/docs/technical/source-clock.md new file mode 100644 index 0000000..03e7347 --- /dev/null +++ b/docs/technical/source-clock.md @@ -0,0 +1,95 @@ +# Source clocks and variable-rate range ends + +The timestamp clock and the nominal picture rate serve different purposes. +The boundary encoder still uses the existing rate and quality settings. The +output timeline must also represent the source's display timestamps. + +## When the clock changes + +`Grid::of` refines a progressive, non-TS source only when it is variable-rate, +or its nominal frame period is not an integer number of source ticks (for +example, 23.976 fps stored in milliseconds). An ordinary 25 fps MP4, or a +23.976 fps MP4 on a 1/24000 clock, retains its existing grid and timescale. +An already suitable grid is returned unchanged, without fraction reduction. + +`on_clock` chooses a subdivision that represents both the field grid and +the source tick. Only a newly aligned grid reduces its output time base. +Checked arithmetic and the existing 32-bit MP4 step/timescale limits bound +the result; if no such refinement fits, the existing bounded grid remains. +This does not promise exact ticks for every possible clock and file length. + +The walk measures repeated holds against the typical positive display gap +as well as the average. A 40/40/80 ms pattern must not be called constant +merely because its average frame lasts about 53 ms. Millisecond-rounded +NTSC and a 2:3 cadence still do not count as variable by that test. + +An unclassified progressive stream with B pictures and an average over 1% +below its declared rate needs its display times read: the public container +index contains decode timestamps. `ContainerIndex` declines that case, +so the default `auto` mode falls back to the packet scan. This can add a +sequential read for those sources; ordinary CFR keeps the fast table path. +An explicitly required container index reports the reason it cannot answer. +Seek-cache version 19 invalidates the former classification. + +## A seam is not the end of a keep-range + +At a seam inside one kept range, the following segment resumes on a measured +picture. Extending the preceding segment to a nominal frame length would +shift every picture after the seam. `SegmentCtx::last` distinguishes this +from the end of a keep-range. + +At a range end, a normal picture retains the coded-length floor used before +this change. The review reproducer has a picture at 37.996 s and OUT at +38 s. Refining the clock must not turn that picture into a 4 ms flash; its +normal 23.976 frame lasts about 41.708 ms. The existing +`coded_length_unbounded` cases retain their time-based hold behavior, +including a range entirely inside a long held picture. + +For newly refined CFR, the decoder's next measured picture compensates for +the planner's coarse-clock selection margin. A copy has that coverage from +the planner's terminating access point, including its leading pictures. + +On a refined clock, the final displayed picture also stays in the write +queue until a later displayed picture arrives or the final hold is known. +Decoder reorder depth alone is insufficient: a copied H.264 P picture can +precede three B pictures while `has_b_frames` is two. Releasing that P +picture too early left a 40/40/80 ms selection about 27 ms short. The queue +uses the existing 256 MiB seam-buffer limit and reports an error if that +limit is exceeded. Other clocks retain the existing queue behavior. + +## Reproducing the checks + +Build the actual CLI with the project's FFmpeg 7.1 SDK, then run: + +```sh +bash tests/run_source_clock_tests.sh +# Or select the executable and tools explicitly: +python3 tests/source_clock_tests.py --cut rust/target/release/smartcut \ + --ffmpeg ffmpeg --ffprobe ffprobe --work-dir /tmp +bash tests/run_vfr_tests.sh +``` + +The 80-case suite generates AVC, HEVC and AV1 inputs, includes the reviewer's +unchanged 90-second 720p command, and checks single/multiple selections, +copy-only and single-picture cuts, long holds and CFR controls in MP4/MKV. +It checks decoded counts, all display PTS, decode ordering, movie and video +track ends, compressed copied pictures and their decoded pixels. AVC/HEVC +VCL bytes are checked separately from repeated parameter-set NAL units. +Matroska stores no DTS; equal inferred DTS on its millisecond clock are +recorded, while backward DTS are rejected. MP4 DTS must strictly increase. + +Each run writes complete commands, hashes and results to a new directory. +`--fixtures /fixtures` reuses the exact input bytes. FFmpeg +7.1's synthetic B-frame VFR MP4 edit list can hide its last generated +picture: the reference pixel decoder alone uses `ignore_editlist=1` to +read all generated packets. Outputs always undergo normal demuxing and +strict decoding, with their end times checked independently. + +This suite tests timing and copying, not AV1 sample-description compliance, +SAR propagation, M2TS arrival clocks, GUI playback or hardware players. + +The review assertions use the original default indexing. Forcing `--index scan` +on that duplicate-PTS synthetic file retains a pre-existing unbounded-hold +case (796 pictures rather than 794, with an approximately 4 ms range bound) +in both v0.8.39 and this change. That separate selection/hold behavior is not +claimed fixed here. diff --git a/rust/crates/core/src/cut.rs b/rust/crates/core/src/cut.rs index cf8f6e7..4fc269d 100644 --- a/rust/crates/core/src/cut.rs +++ b/rust/crates/core/src/cut.rs @@ -635,17 +635,37 @@ struct Span { /// `seg.end` is the answer, held to the last picture's place and no /// further. A recording of fields of a declared rate (2:3 pulldown in an /// MP4) is on that grid too, and is answered the same way. -fn held_to_the_end(span: Span, anchor: Option, seg: &Segment, src: &Source, field: f64) -> Span { +fn held_to_the_end(span: Span, anchor: Option, seg: &Segment, src: &Source, + ctx: &SegmentCtx, next_picture: Option) -> Span { + let field = ctx.grid.unit(); let Some(a) = anchor else { return span }; - let until = if src.duration > 0.0 { + let mut until = if src.duration > 0.0 { seg.end.min(src.duration) } else { seg.end }; + let own = Grid::of(src); + let clock_cfr = own.divisor.is_some() && !src.video.variable_rate; + if clock_cfr { + // The CFR planner moves coarse-clock bounds just before a picture + // to settle selection. That margin is not display time to remove. + if let Some(next) = next_picture.filter(|t| *t >= until && *t - until < src.video.frame_duration()) { + until = next; + } else if src.duration > until && src.duration - until <= 3.5 * src.video.time_base { + until = src.duration; + } + } // Held to [`FAR`] for the reason the copy's own placing is. let held = ((until - a).min(FAR) / field).round() as i64; - let own = Grid::of(src); - if coded_length_unbounded(&src.video) && own.unit() == field { + // Refining the clock must not remove a normal picture's coded floor: + // a varying 23.976 MP4 can end a range only 4 ms into its last picture. + // Only the original unbounded-length case and newly refined CFR grids + // may replace that floor by measured coverage. Genuine VFR holds keep + // their existing semantics, including cuts entirely inside one hold. + // Inside a kept range the next segment resumes at a measured picture, + // so preserving a nominal floor there would shift every later picture. + let continuing = own.divisor.is_some() && !ctx.last; + if (coded_length_unbounded(&src.video) || clock_cfr || continuing) && own.unit() == field { // The last picture starts at most a frame of this grid before the // end its coded length gives (a field before, for the second field // of a pair), and it is up for at least a unit. @@ -745,6 +765,9 @@ struct SegmentCtx<'a> { /// Whether this is the opening segment of its keep-range, which is where /// the range's audio boundary decision is made. first: bool, + /// The end of a keep-range, rather than a seam inside it. A normal + /// picture retains its coded floor here even on a finer source clock. + last: bool, /// What the recording says about its own colour, read once for the /// whole cut. See [`signalling_of`]. signalling: &'a Signalling, @@ -832,6 +855,10 @@ struct Writer { /// Only over the cut's last range, and only where that is of use; see /// there. keep_last: bool, + /// On a refined clock the last displayed picture must remain writable + /// until its exact hold is known, even if several B pictures follow it + /// in decode order. Decoder reorder depth alone does not bound that run. + hold_latest: bool, /// Where the output's pictures end, in units, for the last of them to /// be up until. /// @@ -1464,6 +1491,19 @@ impl Writer { // pulldown does not move the boundary either. let keep = (self.depth * 2).max(0) + if self.keep_last { 2 } else { 0 }; while self.held_halves() > keep { + // A copied P picture can be followed by three B pictures while + // has_b_frames says two. Writing it now loses the chance to set + // its final hold: a 40/40/80 ms cut ended 27 ms early. Let it go + // when a later displayed picture arrives, or lengthen it in + // flush. Apply only to the clocks this change refines. + if self.hold_latest && self.pending.front().is_some_and(|front| + self.pending.iter().all(|p| p.display <= front.display)) + { + if self.pending.iter().map(|p| p.packet.size()).sum::() > HELD_AT_MOST { + bail!("last-picture reorder buffer exceeds the seam buffer limit"); + } + break; + } self.emit_one()?; } Ok(()) @@ -4664,7 +4704,8 @@ fn copy_segment( } bail!("could not find the entry point at {:.3}s", seg.start); } - Ok(held_to_the_end(span, anchor, seg, src, field)) + let next_picture = seg.copy_until.map(|_| seg.end); + Ok(held_to_the_end(span, anchor, seg, src, ctx, next_picture)) } /// How the pictures of a re-encoded segment are produced. @@ -5404,6 +5445,14 @@ fn open_encoder_as( /// [`Grid`]. const FINE: i64 = 32; +fn needs_source_clock(variable: bool, frame: f64, tick: f64) -> bool { + if !(frame.is_finite() && frame > 0.0 && tick.is_finite() && tick > 0.0) { + return false; + } + let ticks = frame / tick; + variable || (ticks - ticks.round()).abs() > 1e-6 +} + /// The units the output timeline is counted in. /// /// **A field of the rate the recording is built on**, which for everything @@ -5433,12 +5482,60 @@ struct Grid { den: i64, /// Units per field. sub: i64, + /// Reduce only a newly aligned clock. Existing grids retain their exact + /// timescale and sample tables, even where the fraction could be reduced. + divisor: Option, } impl Grid { fn of(src: &Source) -> Grid { let v = &src.video; - Grid::from_rates(v.frame_rate, v.base_rate, v.variable_rate).held_within(src.duration) + let grid = Grid::from_rates(v.frame_rate, v.base_rate, v.variable_rate).held_within(src.duration); + // A fine container clock alone is not a reason to change a CFR + // output. Only varying pictures, or a frame period the source clock + // cannot state exactly (e.g. 23.976 on milliseconds), need it. + if !src.on_a_ts && !v.interlaced() + && needs_source_clock(v.variable_rate, v.frame_duration(), v.time_base) + { + grid.on_clock(v.time_base, src.duration) + } else { + grid + } + } + + /// Keep the container's timestamp ticks representable, including on a + /// nominally constant recording. A rounded average is not its clock. + fn on_clock(self, seconds_per_tick: f64, duration: f64) -> Grid { + if !(seconds_per_tick.is_finite() && seconds_per_tick > 0.0) { + return self; + } + let clock = ff::Rational::from(seconds_per_tick).reduce(); + let (n, d) = (i64::from(clock.numerator()), i64::from(clock.denominator())); + if n <= 0 || d <= 0 { return self; } + let Some(needed) = d.checked_mul(self.den) else { return self }; + let Some(available) = n.checked_mul(2 * self.num) else { return self }; + let step = needed / unsafe { ff::ffi::av_gcd(needed, available) }; + if self.sub % step == 0 { + return self; + } + // Any multiple of `step` represents both the field and source clock. + // Keep at least the existing precision, without taking the LCM with + // FINE: that can needlessly multiply the clock and exhaust the bound + // on a long recording whose GOP-sized timestamps fit comfortably. + let Some(groups) = self.sub.checked_add(step - 1).map(|s| s / step) else { return self }; + let Some(sub) = groups.checked_mul(step) else { return self }; + if self.num.checked_mul(sub).and_then(|v| v.checked_mul(2)).is_none() { + return self; + } + let divisor = unsafe { ff::ffi::av_gcd(2 * self.num * sub, self.den) }; + let aligned = Grid { sub, divisor: Some(divisor), ..self }; + let scale = aligned.timescale(); + if scale > i64::from(i32::MAX) + || (duration.is_finite() && duration > 0.0 && scale as f64 * duration > f64::from(i32::MAX)) + { + return self; + } + aligned } /// The grid, with ticks coarse enough that the longest gap the recording @@ -5477,6 +5574,7 @@ impl Grid { num: i64::from(n).max(1), den: i64::from(d).max(1), sub: self.sub, + divisor: None, } } @@ -5528,7 +5626,7 @@ impl Grid { fn of_rate(num: i64, den: i64, sub: i64) -> Grid { let most = i64::from(i32::MAX) / (2 * sub); if num <= most { - return Grid { num, den, sub }; + return Grid { num, den, sub, divisor: None }; } let (mut n, mut d) = (0i32, 0i32); // Bounds both terms, which is what is wanted: a frame rate is over @@ -5538,12 +5636,17 @@ impl Grid { num: i64::from(n).max(1), den: i64::from(d).max(1), sub, + divisor: None, } } /// Ticks per second in the output's own time base. fn timescale(self) -> i64 { - 2 * self.num * self.sub + 2 * self.num * self.sub / self.divisor.unwrap_or(1) + } + + fn ticks_per_unit(self) -> i64 { + self.den / self.divisor.unwrap_or(1) } fn time_base(self) -> ff::Rational { @@ -5552,7 +5655,7 @@ impl Grid { /// How long one unit lasts, in seconds. fn unit(self) -> f64 { - self.den as f64 / self.timescale() as f64 + self.ticks_per_unit() as f64 / self.timescale() as f64 } /// What a picture shown for `fields` fields occupies. @@ -6014,6 +6117,7 @@ fn reencode_segment( // the gap to the previous picture is a frame by construction and the // range is answered by the picture it already had. let mut still_on_screen: Option = None; + let mut next_picture: Option = None; let least = src.video.least_gap(); macro_rules! place { @@ -6081,6 +6185,7 @@ fn reencode_segment( let Some(pts) = frame.pts() else { continue }; let t = pts as f64 * in_tb - src.start_time; if t >= seg.end - tol { + next_picture = Some(next_picture.map_or(t, |old| old.min(t))); past_end = true; continue; } @@ -6303,7 +6408,7 @@ fn reencode_segment( seg.start, seg.end, span.pictures, ); } - Ok(held_to_the_end(span, anchor, seg, src, field)) + Ok(held_to_the_end(span, anchor, seg, src, ctx, next_picture)) } /// Where the streams go on the way out. @@ -9407,14 +9512,12 @@ fn cut_into( } }; let grid = Grid::of(src); - let den = grid.den; - - // Output timebase: one tick per 1/(2*num*sub) second, so a *unit* is - // exactly `den` ticks, a field is `sub` of them and a normal frame twice - // that. 30000/1001 lands on 1/60000 with 1001 ticks per field -- integer - // arithmetic throughout, no rounding anywhere on the timeline, and - // 3-field pictures are representable. See [`Grid`] for the recording - // that needs the units smaller than a field. + let den = grid.ticks_per_unit(); + + // A unit is `den` output ticks, a field is `sub` units. A newly aligned + // source clock reduces both together; untouched grids retain exactly + // their old time base and sample tables. 30000/1001 remains 1001/60000 s + // per field. See [`Grid`] for the finer units varying pictures need. let mut muxer_opts = ff::Dictionary::new(); let timescale = grid.timescale().to_string(); let to_ts = writing_ts(output); @@ -10897,6 +11000,7 @@ fn cut_into( held: None, opening: false, keep_last: false, + hold_latest: false, ends: None, top_written: None, }; @@ -11409,8 +11513,11 @@ fn cut_into( // [`Writer::ends`] -- on its own grid only, as [`held_to_the_end`]. writer.keep_last = reel_no + 1 == reels.len() && plan_no + 1 == reel_plans[reel_no].len() - && last_held_to_the_end(&rsrc.video) + && (last_held_to_the_end(&rsrc.video) + || (!rsrc.on_a_ts && !rsrc.video.interlaced() + && !rsrc.video.variable_rate && Grid::of(rsrc).divisor.is_some())) && Grid::of(rsrc).unit() == grid.unit(); + writer.hold_latest = writer.keep_last && Grid::of(rsrc).divisor.is_some(); for (n, seg) in plan.segments.iter().enumerate() { let first_segment = n == 0; // A stretch written in the master's shape -- a reel that does not @@ -11436,6 +11543,7 @@ fn cut_into( offset: target_start - plan.t_in, subpictures: thread.graphics, first: first_segment, + last: n + 1 == plan.segments.len(), signalling: &signalling, }; // Each segment reports the span it actually occupied, so the @@ -12037,6 +12145,45 @@ fn cut_into( mod tests { use super::*; + #[test] + fn container_ticks_survive_a_rate_derived_grid() { + for (rate, base, variable, tick) in [ + (6000.0 / 319.0, 25.0, true, 1.0 / 12800.0), + (30000.0 / 1001.0, 30000.0 / 1001.0, false, 0.001), + ] { + let before = Grid::from_rates(rate, base, variable); + let grid = before.on_clock(tick, 6000.0); + assert!(grid.unit() <= before.unit()); + for i in 0..10000 { + let t = i as f64 * tick; + let restored = (t / grid.unit()).round() * grid.unit(); + assert!((restored - t).abs() < 1e-10); + } + assert!(grid.timescale() <= i64::from(i32::MAX)); + } + } + + #[test] + fn ordinary_cfr_does_not_acquire_a_new_clock() { + for (rate, tick) in [(25.0, 1.0 / 12800.0), (25.0, 0.001), + (24000.0 / 1001.0, 1.0 / 24000.0), + (30000.0 / 1001.0, 1.0 / 30000.0)] { + assert!(!needs_source_clock(false, 1.0 / rate, tick)); + } + assert!(needs_source_clock(false, 1001.0 / 24000.0, 0.001)); + assert!(needs_source_clock(true, 1.0 / 25.0, 1.0 / 12800.0)); + let original = Grid::from_rates(25.0, 25.0, false); + let already = original.on_clock(1.0 / 25.0, 60.0); + assert_eq!(already.timescale(), original.timescale()); + assert_eq!(already.sub, original.sub); + for bad in [0.0, -1.0, f64::NAN, f64::INFINITY] { + assert!(!needs_source_clock(true, 0.04, bad)); + assert_eq!(original.on_clock(bad, 60.0).timescale(), original.timescale()); + } + } + + + /// Every `.sup` of one cut has a name of its own. A transport stream's /// are named as they always were, by language or else by PID; a Matroska /// file's, whose streams all sit on 0, by their stream where the language diff --git a/rust/crates/core/src/index.rs b/rust/crates/core/src/index.rs index 0538fce..beb0cd8 100644 --- a/rust/crates/core/src/index.rs +++ b/rust/crates/core/src/index.rs @@ -417,6 +417,13 @@ fn varies(shown: &[f64], fd: f64) -> bool { if fd <= 0.0 || shown.len() < ENOUGH { return false; } + // The average can hide repeated holds: 40/40/80 ms averages to + // 53 ms, below the old 1.75-frame threshold. Use the typical measured + // gap as well; a 2:3 cadence and millisecond rounding still fit. + let mut gaps: Vec = shown.windows(2) + .map(|w| w[1] - w[0]).filter(|g| g.is_finite() && *g > 0.0).collect(); + gaps.sort_by(f64::total_cmp); + let fd = gaps.get(gaps.len().saturating_sub(1) / 2).copied().map_or(fd, |g| fd.min(g)); let (mut held, mut dense, mut counted) = (0usize, 0usize, 0usize); for w in shown.windows(2) { let gap = w[1] - w[0]; @@ -612,6 +619,16 @@ impl IndexSource for ContainerIndex { { bail!("an MPEG program or transport stream keeps no seek table"); } + // An MP4's public table contains decode times. With B pictures, + // outline_of cannot use it to classify display gaps. A progressive + // stream averaging materially below its declared rate may contain + // repeated holds (40/40/80 ms), so let the caller's packet scan ask + // its PTS instead. Constant-rate sources retain the fast table path. + if !video.variable_rate && !video.interlaced() && video.has_b_frames > 0 + && video.base_rate > video.frame_rate * 1.01 && video.base_rate < 1000.0 + { + bail!("reordered pictures below the declared rate need their display times scanned"); + } // How long the recording is, by the container's own reckoning. Only // used to ask whether the table below covers it, so a container that // will not say how long it is simply asks nothing. @@ -2064,6 +2081,26 @@ pub fn sample_bit_rate( mod tests { use super::*; + #[test] + fn repeated_holds_are_not_hidden_by_the_average() { + let mut shown = vec![0.0]; + for i in 0..239 { + shown.push(shown.last().unwrap() + if i % 3 == 2 { 0.08 } else { 0.04 }); + } + assert!(varies(&shown, 319.0 / 6000.0)); + } + + #[test] + fn rounded_ntsc_and_two_three_cadence_are_not_variable() { + let ntsc: Vec<_> = (0..240).map(|i| (i as f64 * 1001.0 / 30.0).round() / 1000.0).collect(); + assert!(!varies(&ntsc, 1001.0 / 30000.0)); + let mut film = vec![0.0]; + for i in 0..239 { + film.push(film.last().unwrap() + if i % 2 == 0 { 0.04 } else { 0.06 }); + } + assert!(!varies(&film, 0.04)); + } + /// Entry points every `every` seconds from `from` to `to`. fn every(from: f64, to: f64, every: f64) -> Vec { let mut out = Vec::new(); diff --git a/rust/crates/core/src/seek_index.rs b/rust/crates/core/src/seek_index.rs index 27972bf..2784e5a 100644 --- a/rust/crates/core/src/seek_index.rs +++ b/rust/crates/core/src/seek_index.rs @@ -144,7 +144,9 @@ static SERIAL: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new( /// the walk sees that its pictures come three times as close: an index /// written before has no such figure, and a cut opened from it would be /// planned on the average where a fresh open plans on the pictures. -pub const VERSION: u32 = 18; +/// 19: repeated holds are measured against the typical gap as well as the +/// average. Older caches may have classified 40/40/80 ms as constant. +pub const VERSION: u32 = 19; const MAGIC: &[u8; 4] = b"SCIX"; diff --git a/tests/ci.sh b/tests/ci.sh index 55166ad..20451e3 100755 --- a/tests/ci.sh +++ b/tests/ci.sh @@ -28,7 +28,7 @@ SUITES=( audio_codec audio_smart audio_format audio_head bilingual index proxy disc bdav udf dvd bd_audio - vp9_av1 vfr demux transrate join + vp9_av1 vfr source_clock demux transrate join ) [ $# -gt 0 ] && SUITES=("$@") diff --git a/tests/run_source_clock_tests.sh b/tests/run_source_clock_tests.sh new file mode 100755 index 0000000..c610837 --- /dev/null +++ b/tests/run_source_clock_tests.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "$0")/.." +exec python3 tests/source_clock_tests.py "$@" diff --git a/tests/source_clock_tests.py b/tests/source_clock_tests.py new file mode 100644 index 0000000..2313b3b --- /dev/null +++ b/tests/source_clock_tests.py @@ -0,0 +1,295 @@ +"""Source clocks and range-end coverage, using real files and the built CLI. + +Python standard library only. All inputs, commands, hashes and results are +kept in a fresh directory; --fixtures reuses exactly the same input bytes. +The 720p reproducer deliberately keeps the reviewer's command unchanged. +""" +import argparse +from fractions import Fraction +import hashlib +import json +import os +from pathlib import Path +import re +import subprocess +import tempfile +import time + + +def sha(path): + with Path(path).open('rb') as f: + return hashlib.file_digest(f, 'sha256').hexdigest() + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument('--cut', type=Path, default=Path('rust/target/release/smartcut' + ('.exe' if os.name == 'nt' else ''))) + ap.add_argument('--ffmpeg', default='ffmpeg') + ap.add_argument('--ffprobe', default='ffprobe') + ap.add_argument('--work-dir', type=Path, default=Path(tempfile.gettempdir())) + ap.add_argument('--fixtures', type=Path) + ap.add_argument('--review-source', type=Path) + args = ap.parse_args() + args.work_dir.mkdir(parents=True, exist_ok=True) + root = Path(tempfile.mkdtemp(prefix='smartcut-source-clock-', dir=args.work_dir)).resolve() + fx = args.fixtures.resolve() if args.fixtures else root / 'fixtures' + fx.mkdir(parents=True, exist_ok=True) + result = {'test_sha256': sha(__file__), 'cut': str(args.cut.resolve()), + 'cut_sha256': sha(args.cut), 'commands': [], 'fixtures': {}, 'cases': [], + 'source_pixel_oracle': 'Generated MP4s: ignore_editlist=1 decodes all generated packets; outputs use normal demux/decoding.'} + + def run(cmd, key, check=True): + cmd = list(map(str, cmd)) + start = time.monotonic() + p = subprocess.run(cmd, capture_output=True) + result['commands'].append({'command': cmd, 'exit_code': p.returncode, + 'seconds': time.monotonic() - start}) + (root / (key + '.log')).write_bytes(p.stdout + b'\nSTDERR\n' + p.stderr) + if check and p.returncode: + raise RuntimeError(f'{key}: {p.stderr.decode(errors="replace")[-1800:]}') + return p + + def inspect(path, key): + p = run([args.ffprobe, '-v', 'error', '-select_streams', 'v:0', + '-show_streams', '-show_format', '-show_packets', '-show_data_hash', 'sha256', + '-of', 'json', path], key) + data = json.loads(p.stdout) + data['sorted'] = sorted(data['packets'], key=lambda x: int(x['pts'])) + tb = Fraction(data['streams'][0]['time_base']) + data['times'] = [float(int(p['pts']) * tb) for p in data['sorted']] + codec = data['streams'][0]['codec_name'] + if codec in ('h264', 'hevc'): + # SmartCut may repeat parameter sets at an entry point. Compare + # the compressed pictures (VCL NALs) separately from those headers. + types = '1-5' if codec == 'h264' else '0-31' + hashes = run([args.ffmpeg, '-v', 'error', '-i', path, '-map', '0:v:0', + '-c:v', 'copy', '-bsf:v', 'filter_units=pass_types=' + types, + '-f', 'framehash', '-hash', 'sha256', '-'], key + '-vcl') + hashes = [line.rsplit(',', 1)[1].strip() for line in hashes.stdout.decode().splitlines() + if line and not line.startswith('#')] + assert len(hashes) == len(data['packets']) + data['picture_hashes'] = [h for _, h in sorted(zip(data['packets'], hashes), + key=lambda item: int(item[0]['pts']))] + else: + data['picture_hashes'] = [p['data_hash'] for p in data['sorted']] + return data + + def pixels(path, key, source=False): + # FFmpeg 7.1's muxer can end the edit list at the final PTS of these + # reordered VFR generators, hiding their last encoded picture from + # normal decoding. All 240 generated packets are the reference here. + # Never bypass an output's edit list: it must decode normally. + options = ['-ignore_editlist', '1'] if source and path.suffix == '.mp4' else [] + p = run([args.ffmpeg, '-v', 'error', *options, '-i', path, '-map', '0:v:0', + '-fps_mode', 'passthrough', '-f', 'framemd5', '-'], key) + return [line.rsplit(',', 1)[1].strip() for line in p.stdout.decode().splitlines() + if line and not line.startswith('#')] + + def generate(name, cmd): + path = fx / name + if not args.fixtures: + run([args.ffmpeg, '-hide_banner', '-v', 'error', '-y', *cmd, path], + 'generate-' + name) + result['fixtures'][name] = {'path': str(path), 'sha256': sha(path)} + return path + + base = ['-f', 'lavfi', '-i', 'testsrc2=size=192x112:rate=25', '-frames:v', '240'] + codec_options = { + 'h264': ['-c:v', 'libx264', '-preset', 'veryfast', '-crf', '20', + '-x264-params', 'keyint=24:min-keyint=24:scenecut=0:bframes=3'], + 'hevc': ['-c:v', 'libx265', '-preset', 'ultrafast', '-crf', '24', + '-x265-params', 'keyint=24:min-keyint=24:scenecut=0:bframes=3:log-level=error'], + 'av1': ['-c:v', 'libaom-av1', '-cpu-used', '8', '-crf', '32', '-b:v', '0', '-g', '24'], + } + specs = [] + for codec, options in codec_options.items(): + mp4 = generate(f'{codec}-vfr.mp4', base + ['-vf', + 'setpts=N+floor(N/3)', '-fps_mode', 'passthrough', + *options, '-video_track_timescale', '12800']) + mkv = generate(f'{codec}-vfr.mkv', ['-i', mp4, '-map', '0:v:0', '-c', 'copy']) + specs.extend([(mp4, 'vfr'), (mkv, 'vfr')]) + for codec in ('h264', 'hevc'): + for label, rate, timescale in [('25', '25', '12800'), ('ntsc', '24000/1001', '24000')]: + mp4 = generate(f'{codec}-{label}.mp4', ['-f', 'lavfi', '-i', + f'testsrc2=size=192x112:rate={rate}', '-frames:v', '240', + *codec_options[codec], '-video_track_timescale', timescale]) + mkv = generate(f'{codec}-{label}.mkv', ['-i', mp4, '-map', '0:v:0', '-c', 'copy']) + specs.extend([(mp4, 'cfr'), (mkv, 'coarse' if label == 'ntsc' else 'cfr')]) + hold = generate('holds.mp4', ['-f', 'lavfi', '-i', 'testsrc2=size=192x112:rate=30', + '-frames:v', '180', '-vf', r'setpts=N+gte(N\,60)*60+gte(N\,120)*60', + '-fps_mode', 'passthrough', *codec_options['h264'], + '-video_track_timescale', '30000']) + hold_mkv = generate('holds.mkv', ['-i', hold, '-map', '0:v:0', '-c', 'copy']) + if args.fixtures and not (fx / 'review.mp4').exists(): + saved = json.loads((fx / 'fixtures.json').read_text(encoding='utf-8')) + review = Path(saved['review.mp4']['path']) + result['fixtures']['review.mp4'] = {'path': str(review), 'sha256': sha(review)} + elif args.review_source and not args.fixtures: + review = args.review_source.resolve() + result['fixtures']['review.mp4'] = {'path': str(review), 'sha256': sha(review)} + else: + review = generate('review.mp4', ['-f', 'lavfi', '-i', + 'testsrc2=rate=24000/1001:size=1280x720', '-t', '90', '-vf', + r'setpts=(N*1001/24000-if(eq(mod(N\,300)\,299)\,0.025\,0))/TB', + '-fps_mode', 'passthrough', '-c:v', 'libx264', '-g', '120', + '-video_track_timescale', '1000']) + # A reference to an existing exact reproducer also survives --fixtures. + manifest = fx / 'fixtures.json' + if args.fixtures: + saved = json.loads(manifest.read_text(encoding='utf-8')) + if not (fx / 'review.mp4').exists(): + review = Path(saved['review.mp4']['path']) + result['fixtures']['review.mp4'] = {'path': str(review), 'sha256': sha(review)} + assert saved == result['fixtures'], 'fixture bytes changed' + else: + manifest.write_text(json.dumps(result['fixtures'], indent=2), encoding='utf-8') + + sources = {} + for path in [p for p, _ in specs] + [hold, hold_mkv, review]: + sources[path] = inspect(path, 'source-' + path.name) + sources[path]['pixels'] = pixels(path, 'pixels-' + path.name, source=True) + assert len(sources[path]['pixels']) == len(sources[path]['times']), path + + def save(): + result['passed'] = sum(c['passed'] for c in result['cases']) + result['total'] = len(result['cases']) + (root / 'results.json').write_text(json.dumps(result, indent=2), encoding='utf-8') + + def case(path, name, ranges, expected, end, suffix=None, kind='', extra=()): + src = sources[path] + out = root / (name + (suffix or path.suffix)) + command = [args.cut.resolve(), path, '-o', out] + for a, b in ranges: + command += ['--keep', f'{a:.12f}-{b:.12f}'] + command += list(extra) + row = {'name': name, 'kind': kind, 'input': str(path), 'input_sha256': sha(path), + 'ranges': ranges, 'output': str(out), 'issues': []} + try: + proc = run(command, name, check=False) + row['exit_code'] = proc.returncode + if proc.returncode: + raise RuntimeError('export failed') + got = inspect(out, name + '-probe') + pts = got['times'] + count = len(expected) + row['frames'] = len(pts) + row['expected_frames'] = count + if len(pts) != count: + row['issues'].append('frame_count') + tolerance = max(1e-6, float(Fraction(got['streams'][0]['time_base']))) + 1e-7 + error = max((abs(t - at) for t, (_, at) in zip(pts, expected)), default=0.0) + row['max_display_error_seconds'] = error + if error > tolerance: + row['issues'].append('display_pts') + packets = got['packets'] + dts = [int(p['dts']) for p in packets if 'dts' in p] + # Matroska stores PTS, not DTS. On the reviewer's duplicate-PTS + # source its millisecond clock can give equal inferred DTS. + row['equal_dts'] = sum(b == a for a, b in zip(dts, dts[1:])) + row['dts_monotonic'] = all((b >= a if out.suffix == '.mkv' else b > a) + for a, b in zip(dts, dts[1:])) + if not row['dts_monotonic']: + row['issues'].append('dts') + strict = run([args.ffmpeg, '-v', 'error', '-xerror', '-err_detect', 'explode', + '-i', out, '-map', '0:v:0', '-f', 'null', '-'], name + '-strict', check=False) + row['strict_decode'] = strict.returncode == 0 + if not row['strict_decode']: + row['issues'].append('strict_decode') + # These outputs contain video only. Check the movie duration as + # well as every display PTS: decoding alone cannot detect a + # final hold shortened by the muxer's sample-table durations. + duration = float(got['format']['duration']) + row['presentation_duration'] = duration + row['stream_duration'] = got['streams'][0].get('duration') + row['expected_duration'] = end + if abs(duration - end) > max(tolerance, .001001): + row['issues'].append('last_picture_duration') + if out.suffix == '.mp4' and abs(float(row['stream_duration']) - end) > max(tolerance, .001001): + row['issues'].append('video_track_duration') + log = proc.stdout + proc.stderr + intervals = [(float(a), float(b)) for a, b in + re.findall(rb'^\s*copy\s+([\d.]+)\s+->\s*([\d.]+)', log, re.M)] + decoded = pixels(out, name + '-pixels') + row['decoded_frames'] = len(decoded) + if len(decoded) != count: + row['issues'].append('decoded_count') + copied = [i for i, (source_index, _) in enumerate(expected) + if any(a + .001 < src['times'][source_index] < b - .001 for a, b in intervals)] + row['copy_intervals'] = intervals + row['copy_interior_frames'] = len(copied) + row['copied_packet_changes'] = [i for i in copied if i >= len(got['sorted']) or + got['sorted'][i]['data_hash'] != src['sorted'][expected[i][0]]['data_hash']] + row['copied_picture_errors'] = [i for i in copied if i >= len(got['picture_hashes']) or + got['picture_hashes'][i] != src['picture_hashes'][expected[i][0]]] + row['copied_pixel_errors'] = [i for i in copied if i >= len(decoded) or + decoded[i] != src['pixels'][expected[i][0]]] + if row['copied_picture_errors'] or row['copied_pixel_errors']: + row['issues'].append('copied_picture') + row['output_sha256'] = sha(out) + row['time_base'] = got['streams'][0]['time_base'] + if kind == 'review' and len(ranges) == 2: + seam = sum(20 <= t < 38 for t in src['times']) + row['seam_gap_seconds'] = pts[seam] - pts[seam-1] + if not .040 <= row['seam_gap_seconds'] <= .043: + row['issues'].append('four_ms_regression') + except Exception as exc: + row['issues'].append('exception') + row['error'] = str(exc) + row['passed'] = not row['issues'] + result['cases'].append(row) + save() + print(('PASS' if row['passed'] else 'FAIL'), name, row['issues'], flush=True) + + cuts = {'partial': [(13, 227)], 'multi': [(13, 81), (100, 227)], + 'aligned': [(24, 216)], 'one': [(23, 24)], 'full': [(0, 240)]} + for path, kind in specs: + src = sources[path] + stream = src['streams'][0] + # The generating cadence gives the final picture 40 ms. A B-frame + # MP4's stream.duration is a decode duration, not its displayed end. + final = ((240 * 1001 / 24000 if 'ntsc' in path.name else 9.6) if kind != 'vfr' + else 12.76) + times = src['times'] + [final] + for label, indices in cuts.items(): + expected, ranges, offset = [], [], 0.0 + for a, b in indices: + ranges.append((times[a], times[b])) + expected.extend((i, offset + times[i] - times[a]) for i in range(a, b)) + offset += times[b] - times[a] + case(path, path.stem + '-' + path.suffix[1:] + '-' + label, + ranges, expected, offset, kind=kind) + + for path in (hold, hold_mkv): + times = sources[path]['times'] + for label, ranges in [('ends-in-hold', [(0.6, 2.7)]), + ('inside-hold', [(2.2, 2.7)]), + ('joins-held-ranges', [(0.6, 2.7), (4.4, 6.8)])]: + expected, offset = [], 0.0 + for a, b in ranges: + indices = [i for i, t in enumerate(times) if a - 1e-8 <= t < b - 1e-8] + if not indices: + indices = [max(i for i, t in enumerate(times) if t < a)] + expected.extend((i, offset + max(times[i], a) - a) for i in indices) + offset += b - a + case(path, label + '-' + path.suffix[1:], ranges, expected, offset, kind='hold') + + for label, ranges in [('multi', [(20, 38), (60, 75)]), ('single', [(20, 38)])]: + times = sources[review]['times'] + expected, offset = [], 0.0 + for a, b in ranges: + indices = [i for i, t in enumerate(times) if a <= t < b] + first = times[indices[0]] + expected.extend((i, offset + times[i] - first) for i in indices) + # This normal 23.976 picture keeps its coded floor, including + # the reviewer's picture at 37.996 s before a bound of 38 s. + offset += max(b - first, times[indices[-1]] - first + 1001/24000) + for suffix in ('.mp4', '.mkv'): + case(review, 'review-' + label + suffix, ranges, expected, offset, + suffix=suffix, kind='review') + save() + print(f"{result['passed']}/{result['total']} passed; {root}", flush=True) + return 0 if result['passed'] == result['total'] else 1 + + +if __name__ == '__main__': + raise SystemExit(main())