diff --git a/crates/codecs/src/gop.rs b/crates/codecs/src/gop.rs --- a/crates/codecs/src/gop.rs +++ b/crates/codecs/src/gop.rs @@ -123,6 +123,38 @@ pub trait VideoSamples: Sync { fn make_decoder(&self) -> crate::Result>; } +/// The track time (units of `num / den` s) to look up the frame shown at `t` by, for a track whose +/// nominal rate is `rate`. `snap`: move the request back to the start of its frame first, see +/// [`snap_coarse_units`]. +/// +/// Rounded to the nearest unit (#73): muxers round frame times to the timescale (Matroska usually +/// to 1 ms), so a stamp can sit up to half a unit after the true time, and flooring missed every +/// frame stamped late. +pub(crate) fn lookup_time(t: filmcraft_time::Tick, num: i64, den: i64, rate: filmcraft_time::FrameRate, snap: bool) -> i64 { + (if snap { rate.snap(t) } else { t }).to_rational_round(num, den) +} + +/// Whether to look frames up by first moving the request back to the start of its frame (nominal +/// rate `rate`), for a track whose samples are stamped `stamps` (units of `num / den` s). +/// +/// #73's tolerance of half a unit is only harmless while a unit is short against a frame. With a +/// unit of half a frame or more (an MP4 whose timescale is its frame rate, one unit per frame), +/// half a unit is a quarter of a frame or more, and from its middle on every frame was looked up +/// as the next one. Such a track can't place frames between its units anyway, so the request +/// first moves back to the start of its frame, which then rounds onto that frame's own stamp. +/// +/// Only when every stamp is where that lookup lands for its nearest nominal frame: a coarse unit +/// is no constant rate. A variable-rate track with samples 2, 1, 2 units long at two units a frame +/// has a stamp between two nominal frames, and snapping would move a request at it onto the +/// frame before. Such a track keeps #73's lookup by the nearest unit. +pub(crate) fn snap_coarse_units(stamps: impl IntoIterator, num: i64, den: i64, rate: filmcraft_time::FrameRate) -> bool { + let unit = filmcraft_time::Tick::from_rational(1, num, den); + if unit.0.saturating_mul(2) < rate.frame_duration().0 { + return false; + } + stamps.into_iter().all(|p| lookup_time(rate.snap_nearest(filmcraft_time::Tick::from_rational(p, num, den)), num, den, rate, true) == p) +} + /// A container's sync sample for `i` when leading pictures need the GOP before: `sync` is the /// index's own sync sample, `leads(i, key)` says `i` is shown before `key`. One step per GOP, so a /// loop: a crafted index can chain tens of thousands of GOPs (#2803). A `sync` past `i` stops the diff --git a/crates/codecs/src/mkv.rs b/crates/codecs/src/mkv.rs --- a/crates/codecs/src/mkv.rs +++ b/crates/codecs/src/mkv.rs @@ -58,6 +58,9 @@ pub struct MkvSource { /// Video codec as an ISO-BMFF sample entry (for the decoder factories). ventry: Option, video: GopCache, + /// Look frames up by the start of their frame: the video track's timestamp unit is half a frame + /// or longer and its samples sit on the nominal frame grid ([`crate::gop::snap_coarse_units`]). + snap: bool, /// The alpha layer of a VP8 / VP9 track with `AlphaMode` 1 (WebM transparency, #402). alpha: Option>, /// The playable audio tracks in file order: `info.audio_streams[k]` describes `audios[k]`. @@ -221,10 +224,11 @@ fn to_tick(t: &filmcraft_matroska::Track, pts: i64) -> Tick { /// A time as the track timestamp to look a frame up by: rounded to the nearest tick, as muxers /// round frame times to `TimestampScale` (usually 1 ms). Flooring skipped every frame whose -/// timestamp was rounded up (every third frame at 30 fps). -fn from_tick(t: &filmcraft_matroska::Track, time: Tick) -> i64 { +/// timestamp was rounded up (every third frame at 30 fps). See [`crate::gop::lookup_time`] for +/// `snap`, timestamp units as long as a frame. +fn from_tick(t: &filmcraft_matroska::Track, rate: FrameRate, time: Tick, snap: bool) -> i64 { let (n, d) = tb(t); - time.to_rational_round(n, d) + crate::gop::lookup_time(time, n, d, rate, snap) } /// VP9 configuration from the Matroska `CodecPrivate` feature list (ID / length / value triples: @@ -460,7 +464,12 @@ impl MkvSource { && t.samples.iter().any(|s| s.addition.is_some()) }); let alpha = alpha.map(|t| Arc::new(crate::webm_alpha::AlphaLayer::new(bytes.clone(), t.samples.iter().map(|s| (s.pts, s.addition))))); - Ok(Self { info, bytes, file, vtrack, ventry, video: GopCache::new(explicit_color).with_rotation(rotation), alpha, audios }) + let snap = vtrack.zip(info.video.as_ref()).is_some_and(|(i, v)| { + let t = &file.tracks[i]; + let (n, d) = tb(t); + crate::gop::snap_coarse_units(t.samples.iter().map(|s| s.pts), n, d, v.frame_rate) + }); + Ok(Self { info, bytes, file, vtrack, ventry, video: GopCache::new(explicit_color).with_rotation(rotation), snap, alpha, audios }) } fn read(&self, track: usize, i: usize) -> crate::Result> { @@ -616,8 +625,9 @@ impl MediaSource for MkvSource { fn video_frame(&self, req: FrameRequest) -> Result, MediaError> { let ti = self.vtrack.ok_or(MediaError::NoStream("video"))?; let t = req.time.max(Tick::ZERO); - let target = from_tick(&self.file.tracks[ti], t); - let late = filmcraft_media::cancel::catch_up().map(|m| from_tick(&self.file.tracks[ti], t - m)); + let rate = self.info.video.as_ref().map(|v| v.frame_rate).unwrap_or_default(); + let target = from_tick(&self.file.tracks[ti], rate, t, self.snap); + let late = filmcraft_media::cancel::catch_up().map(|m| from_tick(&self.file.tracks[ti], rate, t - m, self.snap)); Ok(self.video.frame_late(&MkvVideo { src: self, track: ti }, target, late)?) } diff --git a/crates/codecs/src/mp4.rs b/crates/codecs/src/mp4.rs --- a/crates/codecs/src/mp4.rs +++ b/crates/codecs/src/mp4.rs @@ -53,6 +53,9 @@ pub struct Mp4Source { file: Mp4File, vtrack: Option, video: GopCache, + /// Look frames up by the start of their frame: the video track's timestamp unit is half a frame + /// or longer and its samples sit on the nominal frame grid ([`crate::gop::snap_coarse_units`]). + snap: bool, /// The playable audio tracks in file order: `info.audio_streams[k]` describes `audios[k]`. audios: Vec, } @@ -368,7 +371,11 @@ impl Mp4Source { .collect(); snap_to_frame_grid(&mut file, vtrack); rebase_unedited_video(&mut file, vtrack, &mut audios); - Ok(Self { info, bytes, file, vtrack, video: GopCache::new(explicit_color).with_rotation(rotation).with_crop(aperture), audios }) + let snap = vtrack.zip(info.video.as_ref()).is_some_and(|(i, v)| { + let t = &file.tracks[i]; + crate::gop::snap_coarse_units(t.samples.iter().map(|s| s.pts.saturating_add(t.edit_offset)), 1, i64::from(t.timescale), v.frame_rate) + }); + Ok(Self { info, bytes, file, vtrack, video: GopCache::new(explicit_color).with_rotation(rotation).with_crop(aperture), snap, audios }) } fn read(&self, track: usize, i: usize) -> crate::Result> { @@ -378,10 +385,10 @@ impl Mp4Source { fn video_at(&self, t: Tick) -> crate::Result> { let ti = self.vtrack.ok_or_else(|| CodecError::Unsupported("no video".into()))?; let ts = self.file.tracks[ti].timescale as i64; - // Nearest, not floor: with a coarse timescale (1000) sample times are rounded, and a - // floored lookup lands one unit short of every frame whose time was rounded up. - let late = filmcraft_media::cancel::catch_up().map(|m| (t - m).to_rational_round(1, ts)); - self.video.frame_late(&Mp4Video { src: self, track: ti }, t.to_rational_round(1, ts), late) + let rate = self.info.video.as_ref().map(|v| v.frame_rate).unwrap_or_default(); + let at = |t: Tick| crate::gop::lookup_time(t, 1, ts, rate, self.snap); + let late = filmcraft_media::cancel::catch_up().map(|m| at(t - m)); + self.video.frame_late(&Mp4Video { src: self, track: ti }, at(t), late) } /// Decoded packet `i` of audio stream `a` (`ainfo` is its `AudioStreamInfo`; `st` its decoder state). diff --git a/crates/codecs/src/rounded_pts_tests.rs b/crates/codecs/src/rounded_pts_tests.rs --- a/crates/codecs/src/rounded_pts_tests.rs +++ b/crates/codecs/src/rounded_pts_tests.rs @@ -174,8 +174,13 @@ fn mp4_with_frames_on_the_grid_shows_every_frame_once() { /// An MP4 at 90 kHz with these sample durations. fn mp4_90k(durations: &[u32]) -> Arc<[u8]> { + mp4_durations(90_000, durations) +} + +/// An MP4 at timescale `ts` with the given sample durations. +fn mp4_durations(ts: u32, durations: &[u32]) -> Arc<[u8]> { let mut w = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mov)).unwrap(); - let t = w.add_track(TrackConfig::new(SampleEntry::jpeg((BITS * BLOCK) as u16, BLOCK as u16), 90_000)).unwrap(); + let t = w.add_track(TrackConfig::new(SampleEntry::jpeg((BITS * BLOCK) as u16, BLOCK as u16), ts)).unwrap(); for (k, &duration) in durations.iter().enumerate() { w.write_sample(t, WriteSample { data: &picture(k), duration, composition_offset: 0, is_sync: true }).unwrap(); } @@ -235,3 +240,121 @@ fn mp4_variable_rate_and_on_grid_frame_times_stay_put() { } assert!(failures.is_empty(), "{}", failures[..failures.len().min(8)].join("\n")); } + +/// Asks for every frame at its start, its middle and nine tenths in, expects that frame each time. +fn inside_frame_problems(src: &dyn MediaSource, rate: FrameRate) -> Vec { + let frame = rate.frame_duration().0; + let mut wrong = Vec::new(); + for k in 0..frame_count(rate) { + for tenths in [0, 5, 9] { + let t = rate.tick_of(k as i64) + Tick(frame / 10 * tenths); + let got = index_of(&src.video_frame(FrameRequest::full(t)).unwrap()); + if got != k { + wrong.push(format!("frame {k} + {tenths}/10: got {got}")); + } + } + } + wrong +} + +/// An MP4 whose timescale is `per_frame` units per frame of `rate` (an integer rate), every sample +/// that many units long. +fn mp4_units_per_frame(rate: FrameRate, per_frame: u32) -> Arc<[u8]> { + let ts = (rate.num / rate.den) as u32 * per_frame; + let mut w = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mov)).unwrap(); + let t = w.add_track(TrackConfig::new(SampleEntry::jpeg((BITS * BLOCK) as u16, BLOCK as u16), ts)).unwrap(); + for k in 0..frame_count(rate) { + w.write_sample(t, WriteSample { data: &picture(k), duration: per_frame, composition_offset: 0, is_sync: true }).unwrap(); + } + w.finish().unwrap().into_inner().into() +} + +/// An MP4 muxed with timescale = frame rate (one unit per frame, as penumbra writes its recordings): +/// half a unit is half a frame, so rounding to the nearest unit showed the next frame from every +/// frame's middle on. Two units per frame had the same problem from three quarters in. +#[test] +fn mp4_with_a_timescale_of_one_or_two_units_per_frame_finds_the_frame_inside_it() { + let mut failures = Vec::new(); + for rate in [FrameRate::FPS_24, FrameRate::FPS_25, FrameRate::FPS_30, FrameRate::FPS_60] { + for per_frame in [1, 2] { + let src = crate::open_bytes("synthetic.mov", mp4_units_per_frame(rate, per_frame)).unwrap(); + for p in inside_frame_problems(src.as_ref(), rate).into_iter().take(3) { + failures.push(format!("{rate}, {per_frame} units per frame: {p}")); + } + } + } + assert!(failures.is_empty(), "mov:\n{}", failures.join("\n")); +} + +/// A Matroska file whose `TimestampScale` is one frame of `rate` in whole nanoseconds, `scale_ns` +/// (rounded down or up where a frame is no whole number of them). +fn mkv_scale(rate: FrameRate, scale_ns: u64) -> Arc<[u8]> { + let mut spec = TrackSpec::new(TrackKind::Video, "V_MJPEG"); + spec.video_size = Some((BITS * BLOCK, BLOCK)); + spec.default_duration_ns = Some(scale_ns); + let opts = MuxOptions { timestamp_scale: scale_ns, ..MuxOptions::default() }; + let mut w = MkvWriter::new(Cursor::new(Vec::new()), vec![spec], opts).unwrap(); + for k in 0..frame_count(rate) { + let ns = (k as i128 * 1_000_000_000 * rate.den as i128 / rate.num as i128) as i64; + w.write_frame(0, ns, true, &picture(k), None).unwrap(); + } + w.finish().unwrap().into_inner().into() +} + +/// The same for Matroska with a timestamp unit of one frame: exact at 25 fps, a fraction of a +/// nanosecond short of or past the frame at 24, 30 and 60 fps. +#[test] +fn mkv_with_a_timestamp_unit_of_one_frame_finds_the_frame_inside_it() { + let mut failures = Vec::new(); + for rate in [FrameRate::FPS_24, FrameRate::FPS_25, FrameRate::FPS_30, FrameRate::FPS_60] { + let exact = 1_000_000_000 * rate.den as u64; + let floor = exact / rate.num as u64; + let ceil = exact.div_ceil(rate.num as u64); + for scale_ns in if floor == ceil { vec![floor] } else { vec![floor, ceil] } { + let src = crate::open_bytes("synthetic.mkv", mkv_scale(rate, scale_ns)).unwrap(); + for p in inside_frame_problems(src.as_ref(), rate).into_iter().take(3) { + failures.push(format!("{rate}, {scale_ns} ns units: {p}")); + } + } + } + assert!(failures.is_empty(), "mkv:\n{}", failures.join("\n")); +} + +/// A Matroska file with `TimestampScale` `scale_ns` and a frame at each of `starts` (units of it). +fn mkv_starts(scale_ns: u64, starts: &[i64]) -> Arc<[u8]> { + let mut spec = TrackSpec::new(TrackKind::Video, "V_MJPEG"); + spec.video_size = Some((BITS * BLOCK, BLOCK)); + let opts = MuxOptions { timestamp_scale: scale_ns, ..MuxOptions::default() }; + let mut w = MkvWriter::new(Cursor::new(Vec::new()), vec![spec], opts).unwrap(); + for (k, &s) in starts.iter().enumerate() { + w.write_frame(0, s * scale_ns as i64, true, &picture(k), None).unwrap(); + } + w.finish().unwrap().into_inner().into() +} + +/// A coarse unit is no constant rate: at two units a frame (timescale 50, nominal 25 fps), a track +/// with one sample a unit long has a frame starting between two nominal frames. Moving a request +/// at that start back to its nominal frame would show the frame before; such a track keeps the +/// lookup by the nearest unit, in both containers. +#[test] +fn variable_rate_track_with_a_coarse_unit_keeps_its_off_grid_frame() { + let durations = [2u32, 1, 2, 2, 2, 2, 2, 2]; + let starts: Vec = durations.iter().scan(0i64, |t, &d| Some(std::mem::replace(t, *t + i64::from(d)))).collect(); + let sources = [ + ("mov", crate::open_bytes("synthetic.mov", mp4_durations(50, &durations)).unwrap()), + ("mkv", crate::open_bytes("synthetic.mkv", mkv_starts(20_000_000, &starts)).unwrap()), + ]; + let mut failures = Vec::new(); + for (name, src) in &sources { + assert_eq!(src.info().video.as_ref().unwrap().frame_rate, FrameRate::FPS_25, "{name}: nominal rate"); + // every unit up to the last frame: the last frame starting at or before it + for t in 0..=*starts.last().unwrap() { + let want = starts.iter().rposition(|&s| s <= t).unwrap(); + let got = index_of(&src.video_frame(FrameRequest::full(Tick::from_rational(t, 1, 50))).unwrap()); + if got != want { + failures.push(format!("{name}: at {t}/50 s wanted frame {want}, got {got}")); + } + } + } + assert!(failures.is_empty(), "{}", failures.join("\n")); +}