From 3b1fa296633d2eec2d9fd01a32fb3f9c7eb39251 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 10 Oct 2026 23:59:15 +0200 Subject: [PATCH] #790: Matroska AAC / MP3 / AC-3 trims the exact CodecDelay Problem: ffmpeg stores the encoder priming as CodecDelay in ns (1024 AAC samples at 48 kHz = 21.333 ms, 256 AC-3 samples at 44.1 kHz = 5.805 ms) and block timestamps in ms. The demuxer subtracts the delay rounded to 1 ms, and fixed-length packets were anchored on that rounded stamp, so the whole track played up to 16 samples late against ffmpeg's decode. Only Opus undid the rounding. Fix: one stamp-to-samples conversion for every Matroska audio codec that undoes the demuxer's rounding and removes the exact CodecDelay (rounded to the nearest sample; the element holds whole ns), with checked math against hostile timebases. Co-Authored-By: Claude Opus 5.5 --- crates/codecs/src/audio_timing_tests.rs | 29 ++++++++++++ crates/codecs/src/mkv.rs | 42 +++++++++++------ crates/codecs/tests/matroska_audio_oracle.rs | 47 ++++++++++++++++++++ docs/file-format-parity.md | 2 +- docs/gaps.md | 2 + 5 files changed, 108 insertions(+), 14 deletions(-) diff --git a/crates/codecs/src/audio_timing_tests.rs b/crates/codecs/src/audio_timing_tests.rs index 40e43a2d8..62f25cb10 100644 --- a/crates/codecs/src/audio_timing_tests.rs +++ b/crates/codecs/src/audio_timing_tests.rs @@ -176,3 +176,32 @@ fn playing_at_another_rate_leaves_no_holes() { let tail: Vec> = got.iter().map(|c| c[2048..].to_vec()).collect(); assert_eq!(holes(&tail), 0); } + +/// #790: the encoder priming stored as `CodecDelay` (1024 samples: 21.333 ms at 48 kHz, 23.220 ms +/// at 44.1 kHz) is trimmed exactly. The demuxer takes it off the millisecond timestamps rounded to +/// whole milliseconds; starting the first packet there played the track 16 (48 kHz) or 10 (44.1 kHz) +/// samples late. +#[test] +fn matroska_codec_delay_is_trimmed_to_the_sample() { + for rate in [48_000u32, 44_100] { + let (asc, units) = aac(rate); + let want = continuous(&asc, rate, &units); + let delay_ns = (1024 * 1_000_000_000 + rate as i64 / 2) / rate as i64; + let mut spec = TrackSpec::new(TrackKind::Audio, "A_AAC"); + spec.codec_private = asc.clone(); + spec.audio = Some((rate as f64, 2, None)); + spec.codec_delay_ns = delay_ns as u64; + let mut w = MkvWriter::new(Cursor::new(Vec::new()), vec![spec], MuxOptions::default()).unwrap(); + for (k, u) in units.iter().enumerate() { + let start_ns = (k as i64 * 1024 * 1_000_000_000 + rate as i64 / 2) / rate as i64; + w.write_frame(0, start_ns - delay_ns, true, u, None).unwrap(); + } + let bytes: Arc<[u8]> = w.finish().unwrap().into_inner().into(); + let src = MkvSource::open("primed.mkv", bytes).unwrap(); + let frames = units.len().saturating_sub(2) * 1024; + let got = played(&src, rate, frames); + let trimmed: Vec> = want.iter().map(|c| c[1024..].to_vec()).collect(); + let (n, first) = mismatches(&got, &trimmed, 0); + assert_eq!(n, 0, "{rate} Hz: {n} samples differ from the decode with the priming trimmed, first (channel, sample) {first:?}"); + } +} diff --git a/crates/codecs/src/mkv.rs b/crates/codecs/src/mkv.rs index 97de67431..c3ca0355d 100644 --- a/crates/codecs/src/mkv.rs +++ b/crates/codecs/src/mkv.rs @@ -71,15 +71,14 @@ pub struct MkvSource { /// length (AAC, MPEG audio, AC-3) therefore run on from one another, resynchronising to the /// timestamp only across gaps of more than half a packet (and more than two ticks); see /// [`crate::audio::contiguous_starts`]. Opus packets: [`opus_starts`]. Other codecs start at their -/// timestamps. +/// timestamps. Timestamps are taken with the exact `CodecDelay` off (see [`stamp_to_samples`]). fn audio_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, rate: i64) -> Vec { use crate::audio::{FixedFrames, PacketTime, contiguous_starts, fixed_packet_samples}; let Some(t) = file.tracks.get(ti) else { return Vec::new() }; if let Some(h) = opus_head(&t.codec) { return opus_starts(file, bytes, ti, &h, rate); } - let (n, d) = tb(t); - let at = |pts: i64| (pts as i128 * n as i128 * rate as i128 / d as i128) as i64; + let at = stamp_to_samples(t, rate, codec_delay_samples(t, rate)); let first = || file.read_sample(bytes, ti, 0).unwrap_or_default(); let out_rate = u32::try_from(rate).unwrap_or(0); let frame = match &t.codec { @@ -97,18 +96,13 @@ fn audio_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, rate: i64) -> Vec /// Sample-exact Opus packet start positions (48 kHz frames, pre-skip removed). /// -/// The demuxer also subtracts a rounded `CodecDelay` from the timestamps. Starts are accumulated -/// from each packet's TOC duration, resynchronising to the timestamp only across real gaps (more -/// than two ticks off). Pre-skip is the exact `CodecDelay` (or the header's pre-skip when it is -/// absent). +/// Starts are accumulated from each packet's TOC duration, resynchronising to the timestamp only +/// across real gaps (more than two ticks off). Pre-skip is the exact `CodecDelay` (or the header's +/// pre-skip when it is absent). fn opus_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, head: &filmcraft_opus::OpusHead, rate: i64) -> Vec { let Some(t) = file.tracks.get(ti) else { return Vec::new() }; - let (n, d) = tb(t); - let delay_ns = t.codec_delay_ns as i128; - // The demuxer's rounding of CodecDelay to ticks, undone here. - let delay_ticks = codec_delay_ticks(t); - let skip = if t.codec_delay_ns > 0 { (delay_ns * rate as i128 / 1_000_000_000) as i64 } else { head.pre_skip as i64 }; - let at = |pts: i64| ((pts.saturating_add(delay_ticks) as i128 * n as i128 * rate as i128 / d as i128) as i64).saturating_sub(skip); + let skip = if t.codec_delay_ns > 0 { codec_delay_samples(t, rate) } else { head.pre_skip as i64 }; + let at = stamp_to_samples(t, rate, skip); let packets = t.samples.iter().enumerate().map(|(i, s)| crate::audio::PacketTime { stamp: at(s.pts), stamped: own_stamp(s), @@ -117,6 +111,28 @@ fn opus_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, head: &filmcraft_o crate::audio::contiguous_starts(packets, stamp_tolerance(t, rate)) } +/// A block timestamp (with the demuxer's rounded `CodecDelay` taken off) as a position in sample +/// frames at `rate`, `skip` frames of priming removed (#790). The demuxer's rounding is undone first: +/// `CodecDelay` is rarely a whole number of ticks (1024 AAC samples at 48 kHz are 21.333 ms, 256 +/// AC-3 samples at 44.1 kHz 5.805 ms), so starting the first packet at the rounded delay played the +/// whole track up to 16 samples late. +fn stamp_to_samples(t: &filmcraft_matroska::Track, rate: i64, skip: i64) -> impl Fn(i64) -> i64 { + let (n, d) = tb(t); + let delay_ticks = codec_delay_ticks(t); + move |pts: i64| { + let ticks = pts.saturating_add(delay_ticks) as i128; + let at = ticks.checked_mul(n as i128).and_then(|x| x.checked_mul(rate as i128)).map_or(ticks.signum() * i128::MAX, |x| x / d as i128); + i64::try_from(at).unwrap_or(if at < 0 { i64::MIN } else { i64::MAX }).saturating_sub(skip) + } +} + +/// `CodecDelay` in sample frames at `rate`, rounded to the nearest (the element holds whole +/// nanoseconds: 1024 samples at 48 kHz are stored as 21 333 333 ns). +fn codec_delay_samples(t: &filmcraft_matroska::Track, rate: i64) -> i64 { + let ns = (t.codec_delay_ns as i128).saturating_mul(rate.max(0) as i128); + i64::try_from(ns.saturating_add(500_000_000) / 1_000_000_000).unwrap_or(i64::MAX) +} + /// `CodecDelay` in timestamp ticks, rounded as the demuxer rounds it (half away from zero) before /// subtracting it from every timestamp of the track. fn codec_delay_ticks(t: &filmcraft_matroska::Track) -> i64 { diff --git a/crates/codecs/tests/matroska_audio_oracle.rs b/crates/codecs/tests/matroska_audio_oracle.rs index 1298ccd48..ff7822740 100644 --- a/crates/codecs/tests/matroska_audio_oracle.rs +++ b/crates/codecs/tests/matroska_audio_oracle.rs @@ -48,3 +48,50 @@ fn matroska_audio_plays_without_holes() { assert!(r_ours < 2.0 * r_theirs + 0.005, "{name}: clicks between packets (|Δ²| {r_ours} vs ffmpeg's {r_theirs})"); } } + +/// The lag in `-max..=max` at which `ours` best matches `theirs` (least mean |difference| over +/// `from..from + len`). +fn best_lag(ours: &[f32], theirs: &[f32], from: usize, len: usize, max: i64) -> i64 { + let err = |lag: i64| -> f64 { (from..from + len).map(|i| (ours[i] as f64 - theirs[(i as i64 + lag) as usize] as f64).abs()).sum::() / len as f64 }; + (-max..=max).min_by(|&a, &b| err(a).total_cmp(&err(b))).unwrap() +} + +/// #790: ffmpeg stores the encoder priming as `CodecDelay` (1024 samples = 21.333 ms for AAC at +/// 48 kHz, 256 = 5.805 ms for AC-3 at 44.1 kHz, 1105 = 25.057 ms for MP3), and block timestamps to +/// the millisecond. Taking the rounded delay as the start of the first packet played the whole +/// track up to 16 samples late; the decode must line up with ffmpeg's to the sample. +#[test] +fn matroska_audio_is_sample_aligned_with_ffmpeg() { + let ff = filmcraft_testkit::require_ffmpeg!(); + let cases: [(&str, u32, &[&str]); 5] = [ + ("noise_aac_48k.mkv", 48_000, &["-c:a", "aac", "-b:a", "256k"]), + ("noise_aac_44k.mkv", 44_100, &["-c:a", "aac", "-b:a", "256k"]), + ("noise_ac3_44k.mkv", 44_100, &["-c:a", "ac3", "-b:a", "448k"]), + ("noise_ac3_48k.mkv", 48_000, &["-c:a", "ac3", "-b:a", "448k"]), + ("noise_mp3_44k.mkv", 44_100, &["-c:a", "libmp3lame", "-b:a", "320k"]), + ]; + for (name, rate, codec) in cases { + let input = format!("anoisesrc=color=pink:seed=7:amplitude=0.5:sample_rate={rate}:duration=2"); + let args: Vec<&str> = ["-f", "lavfi", "-i", input.as_str(), "-ac", "1"].iter().copied().chain(codec.iter().copied()).collect(); + let Some(path) = fixture(&ff, name, &args) else { + eprintln!("SKIPPED ({name}): this ffmpeg can't encode it"); + continue; + }; + let src = filmcraft_codecs::open_bytes(name, bytes(&path)).unwrap(); + let frames = rate as usize * 3 / 2; + let mut ours = Vec::with_capacity(frames); + while ours.len() < frames { + let n = 1000.min(frames - ours.len()); + ours.extend_from_slice(&src.audio(ours.len() as i64, n, rate).unwrap().channels[0]); + } + let theirs = ffmpeg_audio_f32(&ff, &path, &[]); + assert!(theirs.len() >= frames, "{name}: ffmpeg decoded {} samples", theirs.len()); + let lag = best_lag(&ours, &theirs, rate as usize / 4, rate as usize, 40); + assert_eq!(lag, 0, "{name}: our decode is {lag} samples off ffmpeg's"); + // read from the middle, as a seek does + let at = rate as i64 / 2; + let mid = src.audio(at, 2000, rate).unwrap().channels[0].clone(); + let diff = mid.iter().zip(&ours[at as usize..]).map(|(a, b)| (a - b).abs()).fold(0f32, f32::max); + assert!(diff < 1e-3, "{name}: a read from {at} differs from continuous decoding by {diff}"); + } +} diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index b19262c84..125eec7dc 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -39,7 +39,7 @@ only). "Tested" names the evidence. | QuickTime MOV | read + write | yes | yes (H.264, HEVC, ProRes, DNxHR, APV, MJPEG, PCM) | ffprobe | 90% | | MXF OP1a / OP-Atom | read + write (XDCAM, AVC-Intra, P2, AS-10 / AS-11, JPEG 2000, DNx) | yes (AVC, DNx, ProRes, MPEG-2, PCM / AES3, timecode) | OP1a (DNxHR, ProRes, H.264), OP-Atom | index-table seeking tests | 70% | | MPEG TS / PS (AVCHD `.mts`, `.m2ts`, `.ts`, `.mpg`, `.vob`, `.mod`) | read; write MPEG-2 / Blu-ray / DVD | yes | **no** | fixtures | 65% | -| Matroska / WebM | read (limited) | yes (H.264, HEVC, VP9, AV1, ProRes, MJPEG + AAC, Opus, FLAC, MP3, Vorbis, PCM); WebM black report (#432), MKV multi-audio (#601) | no | fixtures | 75% | +| Matroska / WebM | read (limited) | yes (H.264, HEVC, VP9, AV1, ProRes, MJPEG + AAC, Opus, FLAC, MP3, Vorbis, PCM); WebM black report (#432), MKV multi-audio (#601) | no | fixtures; audio sample-aligned with ffmpeg (#790) | 75% | | **AVI** | read + write | **no** (extension listed, no demuxer, #598) | no | — | 0% | | WMV / ASF | read; write on Windows | no | no | — | 0% | | FLV / F4V | read + write | no | no | — | 0% | diff --git a/docs/gaps.md b/docs/gaps.md index b479a6d1b..ba593bcbb 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -163,6 +163,8 @@ platforms** G4, G9, G17; **performance** G16; **ecosystem** G11; **measurement** - **Estimate:** 120–200 h (camera RAW alone 60–100 h, and some SDKs have licences we cannot use: clean-room from public specs only where specs exist). Docs: [codec-parity.md](codec-parity.md), [file-format-parity.md](file-format-parity.md). +- **Have:** Matroska AAC / MP3 / AC-3 sample-aligned with ffmpeg's decode (the exact + `CodecDelay` is trimmed, not the 1 ms-rounded one; #790). ## G11. No plugin hosting