Merge PR #1829: fix(codecs): read the frame rate phone clips average to (#1700)

# Conflicts:
#	crates/codecs/src/mp4.rs
This commit is contained in:
aiongg
2026-10-31 02:14:21 +00:00
3 changed files with 77 additions and 20 deletions
+30 -20
View File
@@ -9,7 +9,7 @@ use std::sync::{Arc, Mutex};
use filmcraft_color::ColorInfo;
use filmcraft_frame::{AudioBuffer, Region, VideoFrame};
use filmcraft_isobmff::{ByteSource, CleanAperture, CodecConfig, Mp4File, TrackKind};
use filmcraft_isobmff::{ByteSource, CleanAperture, CodecConfig, Mp4File, Sample, TrackKind};
use filmcraft_media::{AudioStreamInfo, FrameRequest, MediaError, MediaInfo, MediaKind, MediaSource, SharedSource, VideoStreamInfo};
use filmcraft_time::{FrameRate, Tick};
@@ -26,6 +26,30 @@ struct DecodedAudioPacket {
failed: bool,
}
/// How far a track's nominal rate may sit from a standard rate and still be read as it. Timestamps
/// rounded to a coarse unit (whole milliseconds in a 90 kHz track, as phones and messaging apps
/// write them) move the average by less than this at every standard rate (#1700).
const STANDARD_RATE_TOLERANCE: f64 = 0.005;
/// A video track's nominal frame rate: the mean of the sample durations within a quarter frame of
/// the median duration, read as the closest standard rate when close enough.
///
/// A duration near the median is the same frame period as rounding has quantised it; one much
/// longer belongs to a variable-rate frame and stays out of the mean. The mean sees through a
/// millisecond rounder the median cannot: 30 fps over 90 kHz is 2970 and 3060 units in turn, whose
/// median reads 30.303 fps but whose mean is 30 fps (#1700).
fn nominal_rate(timescale: u32, samples: &[Sample]) -> Option<FrameRate> {
let mut durs: Vec<u32> = samples.iter().map(|s| s.duration).filter(|&d| d > 0).collect();
durs.sort_unstable();
let &median = durs.get(durs.len() / 2)?;
let near: Vec<f64> = durs.iter().filter(|&&d| d.abs_diff(median) * 4 <= median).map(|&d| f64::from(d)).collect();
let mean = near.iter().sum::<f64>() / near.len().max(1) as f64;
if mean <= 0.0 {
return None;
}
Some(FrameRate::from_f64_within(f64::from(timescale) / mean, STANDARD_RATE_TOLERANCE).sane())
}
struct AudioState {
decoder: Option<PacketDecoder>,
/// Decoded packets by sample index.
@@ -266,13 +290,8 @@ impl Mp4Source {
};
aperture = vp.clean_aperture.and_then(|c| clean_aperture(c, (w, h)));
let (w, h) = aperture.map_or((w, h), |r| (r.w as u32, r.h as u32));
// frame rate from the median sample duration
let mut durs: Vec<u32> = t.samples.iter().take(240).map(|s| s.duration).collect();
durs.sort_unstable();
let d = durs.get(durs.len() / 2).copied().unwrap_or(1).max(1);
let rate = FrameRate::from_f64(t.timescale as f64 / d as f64);
// A rate that rounds to zero (one sample per huge duration) can't be divided by.
let rate = if rate.num > 0 && rate.den > 0 { rate } else { FrameRate::default() };
let rate = nominal_rate(t.timescale, &t.samples).unwrap_or_default();
color = color_from(entry, w, h);
if entry
.video
@@ -744,19 +763,10 @@ pub fn opener(name: &str, bytes: Arc<[u8]>) -> Option<Result<SharedSource, Media
fn snap_to_frame_grid(file: &mut Mp4File, vtrack: Option<usize>) {
let Some(t) = vtrack.and_then(|i| file.tracks.get_mut(i)) else { return };
let Some(first) = t.samples.iter().map(|s| s.pts).min() else { return };
// The nominal duration: the median sample duration, averaged with the durations one unit from
// it. Those are the same duration rounded to the timescale (1501 and 1502 at 59.94 fps over
// 90 kHz): the median alone drifts half a unit a frame, enough to snap a short clip onto the
// wrong points.
let mut durs: Vec<u32> = t.samples.iter().map(|s| s.duration).collect();
durs.sort_unstable();
let Some(&median) = durs.get(durs.len() / 2) else { return };
let near: Vec<f64> = durs.iter().filter(|&&d| d.abs_diff(median) <= 1).map(|&d| f64::from(d)).collect();
let mean = near.iter().sum::<f64>() / near.len().max(1) as f64;
if mean <= 0.0 {
return;
}
let rate = FrameRate::from_f64(f64::from(t.timescale) / mean);
// The nominal duration: the mean of the samples near the median, with the rate snapped to its
// standard form when rounding moved it. Whole-millisecond timestamps turn 30 fps into 33/34 ms
// durations; the median alone picks either side, while the recovered mean keeps the whole grid.
let Some(rate) = nominal_rate(t.timescale, &t.samples) else { return };
// one frame lasts `step_num / step_den` track units
let (step_num, step_den) = (i128::from(t.timescale) * i128::from(rate.den), i128::from(rate.num));
if step_num <= 0 || step_den <= 0 {
+32
View File
@@ -190,6 +190,38 @@ fn mp4_durations(ts: u32, durations: &[u32]) -> Arc<[u8]> {
/// Files that are not a constant rate gone slightly off its grid keep their own times: whatever is
/// asked (each frame's start, the unit before it, every 30 fps grid point), the frame shown is the
/// last one starting at or before it.
///
/// A clip whose frame times were rounded to whole milliseconds inside a 90 kHz track (#1700). A
/// 30 fps clip's durations are 2970 and 3060 units in turn, and a 60 fps clip's 1440 and 1530; the
/// old median reads the first as 30.303 fps and the second as 58.824 fps. Both mean to 3000/1500.
fn mp4_ms_rounded(rate: FrameRate) -> 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 n = frame_count(rate);
let nominal_scale = i128::from(rate.num) / i128::from(rate.den);
let grid = 90_000 / nominal_scale as i64;
let stamp = |k: usize| ((2 * k as i64 * grid + 90) / 180) * 90;
for k in 0..n {
w.write_sample(t, WriteSample { data: &picture(k), duration: (stamp(k + 1) - stamp(k)) as u32, composition_offset: 0, is_sync: true }).unwrap();
}
w.finish().unwrap().into_inner().into()
}
/// A phone-style 90 kHz track with whole-millisecond frame times keeps its rate and every frame.
#[test]
fn mp4_ms_rounded_frame_times_get_back_to_a_standard_rate() {
let mut failures = Vec::new();
for rate in [FrameRate::FPS_30, FrameRate::FPS_60] {
let src = crate::open_bytes("synthetic.mov", mp4_ms_rounded(rate)).unwrap();
let got = src.info().video.as_ref().map(|v| v.frame_rate);
if got != Some(rate) {
failures.push(format!("{rate}: reads as {got:?}"));
}
failures.extend(problems(src.as_ref(), rate).into_iter().map(|p| format!("{rate}: {p}")));
}
assert!(failures.is_empty(), "mov:\n{}", failures.join("\n"));
}
#[test]
fn mp4_variable_rate_and_on_grid_frame_times_stay_put() {
let cases: [(&str, Vec<u32>); 5] = [
+15
View File
@@ -210,6 +210,21 @@ impl FrameRate {
FrameRate::new((fps * 1000.0).round() as i64, 1000)
}
/// Closest standard rate when `fps` is within `tolerance` of it, `tolerance` a fraction of the
/// rate (`0.005` is half a percent), else the nearest 1/1000 rate. A rate a container reports
/// from timestamps rounded to a coarse unit lands slightly off a standard rate: at 90 kHz,
/// whole milliseconds leave a 30 fps clip's average at 30.02 or 29.95 (#1700).
pub fn from_f64_within(fps: f64, tolerance: f64) -> Self {
let mut best: Option<(f64, FrameRate)> = None;
for r in Self::COMMON {
let off = (r.as_f64() - fps).abs();
if off <= tolerance * r.as_f64() && best.is_none_or(|(closest, _)| off < closest) {
best = Some((off, r));
}
}
best.map_or_else(|| Self::from_f64(fps), |(_, r)| r)
}
/// This rate if it is positive, else the default (a damaged file or project can carry a zero
/// or negative rate; every frame computation uses this so it can't divide by zero).
pub fn sane(self) -> FrameRate {