codecs: video frames are looked up by the nearest timestamp unit, so a 1 ms WebM shows every frame

Problem (#73): a 30 fps WebM played and exported with every third frame
missing and the frame before it shown twice (20 unique frames a second).
The same happens at 29.97, 60, 59.94, 24 and 23.976 fps, and with MP4
files whose video track has a 1 ms timescale. 25 and 50 fps were fine.

Cause: Matroska stores block timestamps in TimestampScale units (usually
1 ms), rounded to the nearest unit, so at 30 fps frame 2 (66.67 ms) is
stored as 67. MkvSource::video_frame converted the requested time to track
units with a floor (66), and the lookup (last sample with pts <= target)
returned frame 1. Every frame whose timestamp was rounded up was skipped.
Mp4Source::video_at, and the web build's WebCodecs MP4 path, floored the
same way against the track timescale; normal MP4 timescales hold frame
times exactly, so only coarse (1000) timescales were hit.

Fix: new Tick::to_rational_round (nearest unit, halves up, total for zero
and negative timebases), used for the video lookup target and the
catch-up threshold in mkv.rs, mp4.rs and webcodecs.rs. A requested time
now matches a frame whose stored timestamp is within half a unit of it,
the same precision the timestamps have. Truncating muxers (stamps rounded
down) still work. Times on exact timebases (90 kHz, 15360, 30000/1001…)
convert exactly as before.

Test: crates/codecs/src/rounded_pts_tests.rs builds synthetic Motion-JPEG
streams in the test (each picture shows its index as black/white blocks),
muxed as WebM with 1 ms timestamps and as MOV with a 1000 timescale, at
30, 29.97, 60, 59.94, 24, 23.976 and 25 fps with nearest and truncated
stamps. Requesting every frame's start over 3 s must show every frame
exactly once, and every frame's midpoint must show that frame. Before the
fix both tests failed ("30 fps, Nearest ms stamps: 30 of 90 frames never
shown [2, 5, 8, 11]…, 30 shown twice [1, 4, 7, 10]…"). The time crate
tests cover the rounding, ties and hostile timebases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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);

crates/codecs/src/lib.rs

@@ -169,5 +169,7 @@ pub fn open_bytes(name: &str, bytes: Arc<[u8]>) -> std::result::Result<filmcraft
     filmcraft_media::open_bytes(name, bytes, &openers())
 }
 
+#[cfg(test)]
+mod rounded_pts_tests;
 #[cfg(test)]
 mod tests;

crates/codecs/src/mkv.rs

@@ -100,9 +100,12 @@ fn to_tick(t: &filmcraft_matroska::Track, pts: i64) -> 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:

crates/codecs/src/mp4.rs

@@ -261,8 +261,10 @@ impl Mp4Source {
     fn video_at(&self, t: Tick) -> crate::Result<Arc<VideoFrame>> {
         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<Arc<Vec<Vec<f32>>>> {

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<u8> {
+    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<String> {
+    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<usize> = (0..n).filter(|&k| shown[k] == 0).collect();
+    let repeated: Vec<usize> = (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);
+}

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");
     }