diff --git a/apps/filmcraft-web/src/webcodecs.rs b/apps/filmcraft-web/src/webcodecs.rs index 650999733..612a782b2 100644 --- a/apps/filmcraft-web/src/webcodecs.rs +++ b/apps/filmcraft-web/src/webcodecs.rs @@ -502,7 +502,8 @@ impl MediaSource for WcSource { } let t = &self.file.tracks[self.track]; let n = t.samples.len(); - let target = req.time.max(Tick::ZERO).to_rational_floor(1, t.timescale.max(1) as i64); + // nearest, as `Mp4Source` looks frames up (sample times are rounded to the timescale) + let target = req.time.max(Tick::ZERO).to_rational_round(1, t.timescale.max(1) as i64); let i = t.sample_at_presentation_time(target).unwrap_or(n - 1).min(n - 1); let want = t.samples[i].pts; self.collect(&mut c, want); diff --git a/crates/codecs/src/lib.rs b/crates/codecs/src/lib.rs index d1598de75..8f8de4bc4 100644 --- a/crates/codecs/src/lib.rs +++ b/crates/codecs/src/lib.rs @@ -169,5 +169,7 @@ pub fn open_bytes(name: &str, bytes: Arc<[u8]>) -> std::result::Result Tick { Tick::from_rational(pts * n, 1, d) } +/// 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 { let (n, d) = tb(t); - time.to_rational_floor(n, d) + time.to_rational_round(n, d) } /// VP9 configuration from the Matroska `CodecPrivate` feature list (ID / length / value triples: diff --git a/crates/codecs/src/mp4.rs b/crates/codecs/src/mp4.rs index 9820fbc87..fe2e0f606 100644 --- a/crates/codecs/src/mp4.rs +++ b/crates/codecs/src/mp4.rs @@ -261,8 +261,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; - let late = filmcraft_media::cancel::catch_up().map(|m| (t - m).to_rational_floor(1, ts)); - self.video.frame_late(&Mp4Video { src: self, track: ti }, t.to_rational_floor(1, ts), late) + // 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) } fn audio_packet(&self, st: &mut AudioState, i: usize) -> crate::Result>>> { diff --git a/crates/codecs/src/rounded_pts_tests.rs b/crates/codecs/src/rounded_pts_tests.rs new file mode 100644 index 000000000..169c7a836 --- /dev/null +++ b/crates/codecs/src/rounded_pts_tests.rs @@ -0,0 +1,136 @@ +//! Frame lookup against timestamps rounded to the container's timebase (issue #73). +//! +//! WebM stores timestamps in 1 ms units, so at 30 fps frame 2 (66.67 ms) is stored as 67. Flooring +//! the requested time to 66 picked frame 1 instead: every third frame was skipped and the one +//! before it shown twice. MP4 has the same problem when a muxer picks a 1 ms track timescale. +//! +//! The streams are synthetic Motion-JPEG: each picture shows its index as a row of black and +//! white blocks, so a decoded frame says which sample it came from. + +use std::io::Cursor; +use std::sync::Arc; + +use filmcraft_frame::{PixelData, VideoFrame}; +use filmcraft_isobmff::{Brand, Mp4Writer, SampleEntry, TrackConfig, WriteSample, WriterOptions}; +use filmcraft_matroska::{MkvWriter, MuxOptions, TrackKind, TrackSpec}; +use filmcraft_media::{FrameRequest, MediaSource}; +use filmcraft_time::{FrameRate, TICKS_PER_SECOND, Tick}; + +/// Bits of the frame index drawn per picture, one `BLOCK`-pixel square each. +const BITS: u32 = 8; +const BLOCK: u32 = 16; +const SECONDS: i64 = 3; + +/// The rates that don't divide 1 ms evenly (affected), and 25 fps (never was). +const RATES: [FrameRate; 7] = + [FrameRate::FPS_30, FrameRate::FPS_29_97, FrameRate::FPS_60, FrameRate::FPS_59_94, FrameRate::FPS_24, FrameRate::FPS_23_976, FrameRate::FPS_25]; + +/// How the muxer quantised the exact frame times to milliseconds. +#[derive(Clone, Copy, Debug)] +enum Stamp { + /// To the nearest millisecond, halves up (mkvmerge, ffmpeg). + Nearest, + /// Truncated. + Floor, +} + +fn picture(index: usize) -> Vec { + let img = image::GrayImage::from_fn(BITS * BLOCK, BLOCK, |x, _| image::Luma([if (index >> (x / BLOCK)) & 1 == 1 { 255 } else { 0 }])); + let mut out = Vec::new(); + image::codecs::jpeg::JpegEncoder::new_with_quality(&mut out, 90).encode_image(&img).unwrap(); + out +} + +fn index_of(f: &VideoFrame) -> usize { + let PixelData::Rgba8(px) = &f.data else { panic!("MJPEG decodes to RGBA8") }; + (0..BITS).filter(|b| px[((BLOCK / 2 * f.width + b * BLOCK + BLOCK / 2) * 4) as usize] > 127).map(|b| 1 << b).sum() +} + +fn frame_count(rate: FrameRate) -> usize { + rate.frame_at(Tick(SECONDS * TICKS_PER_SECOND)) as usize +} + +/// Frame `k`'s start in milliseconds, as the muxer stores it. +fn stamp_ms(rate: FrameRate, k: usize, stamp: Stamp) -> i64 { + let (num, den) = (rate.num as i128, rate.den as i128); + let exact2 = 2 * k as i128 * 1000 * den; // twice the time in ms, times num + (match stamp { + Stamp::Nearest => (exact2 + num) / (2 * num), + Stamp::Floor => exact2 / (2 * num), + }) as i64 +} + +fn webm(rate: FrameRate, stamp: Stamp) -> Arc<[u8]> { + let mut spec = TrackSpec::new(TrackKind::Video, "V_MJPEG"); + spec.video_size = Some((BITS * BLOCK, BLOCK)); + spec.default_duration_ns = Some((1_000_000_000 * rate.den / rate.num) as u64); + let opts = MuxOptions { doc_type: "webm".into(), ..MuxOptions::default() }; + assert_eq!(opts.timestamp_scale, 1_000_000, "1 ms timestamps"); + let mut w = MkvWriter::new(Cursor::new(Vec::new()), vec![spec], opts).unwrap(); + for k in 0..frame_count(rate) { + w.write_frame(0, stamp_ms(rate, k, stamp) * 1_000_000, true, &picture(k), None).unwrap(); + } + w.finish().unwrap().into_inner().into() +} + +fn mp4_1ms(rate: FrameRate, stamp: Stamp) -> 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), 1000)).unwrap(); + let n = frame_count(rate); + for k in 0..n { + let duration = (stamp_ms(rate, k + 1, stamp) - stamp_ms(rate, k, stamp)) as u32; + w.write_sample(t, WriteSample { data: &picture(k), duration, composition_offset: 0, is_sync: true }).unwrap(); + } + w.finish().unwrap().into_inner().into() +} + +/// Plays the stream back at its own rate (every frame's start), and asks for every frame's +/// midpoint too. Returns what went wrong: frames never shown, frames shown twice, wrong midpoints. +fn problems(src: &dyn MediaSource, rate: FrameRate) -> Vec { + let n = frame_count(rate); + let at = |t: Tick| index_of(&src.video_frame(FrameRequest::full(t)).unwrap()); + let mut shown = vec![0usize; n]; + for k in 0..n { + shown[at(rate.tick_of(k as i64)).min(n - 1)] += 1; + } + let mut out = Vec::new(); + let dropped: Vec = (0..n).filter(|&k| shown[k] == 0).collect(); + let repeated: Vec = (0..n).filter(|&k| shown[k] > 1).collect(); + if !dropped.is_empty() || !repeated.is_empty() { + out.push(format!( + "{} of {n} frames never shown {:?}…, {} shown twice {:?}…", + dropped.len(), + &dropped[..dropped.len().min(4)], + repeated.len(), + &repeated[..repeated.len().min(4)] + )); + } + let mid: Vec<(usize, usize)> = (0..n).map(|k| (k, at(rate.tick_of(k as i64) + Tick(rate.frame_duration().0 / 2)))).filter(|(k, got)| k != got).collect(); + if !mid.is_empty() { + out.push(format!("{} mid-frame requests wrong, e.g. (wanted, got) {:?}", mid.len(), &mid[..mid.len().min(4)])); + } + out +} + +fn check(container: &str, open: fn(FrameRate, Stamp) -> Arc<[u8]>) { + let mut failures = Vec::new(); + for rate in RATES { + for stamp in [Stamp::Nearest, Stamp::Floor] { + let src = crate::open_bytes(&format!("synthetic.{container}"), open(rate, stamp)).unwrap(); + for p in problems(src.as_ref(), rate) { + failures.push(format!("{rate}, {stamp:?} ms stamps: {p}")); + } + } + } + assert!(failures.is_empty(), "{container}:\n{}", failures.join("\n")); +} + +#[test] +fn webm_with_1ms_timestamps_shows_every_frame_once() { + check("webm", webm); +} + +#[test] +fn mp4_with_a_1ms_timescale_shows_every_frame_once() { + check("mov", mp4_1ms); +} diff --git a/crates/time/src/lib.rs b/crates/time/src/lib.rs index 82a2089b0..231120a3e 100644 --- a/crates/time/src/lib.rs +++ b/crates/time/src/lib.rs @@ -52,6 +52,19 @@ impl Tick { pub fn to_rational_floor(self, num: i64, den: i64) -> i64 { (self.0 as i128 * den as i128).checked_div_euclid(num as i128 * TICKS_PER_SECOND as i128).unwrap_or(0) as i64 } + /// Inverse of [`Tick::from_rational`], rounded to the nearest timebase unit (halves up). + /// + /// For finding a sample by its timestamp: containers store timestamps rounded to their + /// timebase (Matroska usually to 1 ms), so a stamp can sit up to half a unit after the true + /// time. Flooring the requested time misses every frame whose stamp was rounded up. + pub fn to_rational_round(self, num: i64, den: i64) -> i64 { + let n = self.0 as i128 * den as i128; + let unit = num as i128 * TICKS_PER_SECOND as i128; + let (n, unit) = if unit < 0 { (-n, -unit) } else { (n, unit) }; + let (Some(q), Some(r)) = (n.checked_div_euclid(unit), n.checked_rem_euclid(unit)) else { return 0 }; + // 0 <= r < unit: round up from the halfway point + (if r * 2 >= unit { q + 1 } else { q }) as i64 + } pub fn abs(self) -> Tick { Tick(self.0.abs()) } @@ -474,6 +487,12 @@ mod tests { assert_eq!(Tick::from_rational(100, 1, 0), Tick::ZERO); assert_eq!(Tick::from_units(100, 0), Tick::ZERO); assert_eq!(Tick(5).to_rational_floor(0, 1), 0); + assert_eq!(Tick(5).to_rational_round(0, 1), 0); + for t in [i64::MIN, -1, 0, 1, i64::MAX] { + for (n, d) in [(i64::MIN, 1), (-1, 1000), (1, i64::MAX), (i64::MAX, i64::MIN), (1, 0)] { + let _ = Tick(t).to_rational_round(n, d); + } + } assert_eq!(Tick(5).mul_ratio(1, 0), Tick::ZERO); assert_eq!(Tick(5).clamp(Tick(10), Tick(0)), Tick(0)); let _ = format_timecode_frames(i64::MIN, FrameRate::FPS_29_97, true); @@ -535,6 +554,19 @@ mod tests { let t = Tick::from_rational(90_000, 1, 90_000); assert_eq!(t.0, TICKS_PER_SECOND); assert_eq!(t.to_rational_floor(1, 90_000), 90_000); + assert_eq!(t.to_rational_round(1, 90_000), 90_000); + // 1 ms timebase (Matroska): to the nearest millisecond, halves up + let ms = |t: Tick| t.to_rational_round(1, 1000); + assert_eq!(ms(FrameRate::FPS_30.tick_of(1)), 33); + assert_eq!(ms(FrameRate::FPS_30.tick_of(2)), 67); + assert_eq!(FrameRate::FPS_30.tick_of(2).to_rational_floor(1, 1000), 66); + assert_eq!(ms(FrameRate::FPS_29_97.tick_of(15)), 501, "500.5 ms"); + assert_eq!(ms(Tick::from_rational(-4, 1, 10_000)), 0, "-0.4 ms"); + assert_eq!(ms(Tick::from_rational(-5, 1, 10_000)), 0, "-0.5 ms"); + assert_eq!(ms(Tick::from_rational(-6, 1, 10_000)), -1, "-0.6 ms"); + // a negative timebase is nonsense, but rounds the same way + assert_eq!(FrameRate::FPS_30.tick_of(2).to_rational_round(-1, -1000), 67); + assert_eq!(FrameRate::FPS_29_97.tick_of(15).to_rational_round(-1, -1000), 501); assert_eq!(FrameRate::from_f64(29.97), FrameRate::FPS_29_97); assert_eq!(FrameRate::FPS_23_976.label(), "23.976"); }