Merge pull request #792 from vangel76/gap-mkv-codec-delay

#790: Matroska AAC / MP3 / AC-3 trims the exact CodecDelay

crates/codecs/src/audio_timing_tests.rs

@@ -176,3 +176,32 @@ fn playing_at_another_rate_leaves_no_holes() {
     let tail: Vec<Vec<f32>> = 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<Vec<f32>> = 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:?}");
+    }
+}

crates/codecs/src/mkv.rs

@@ -71,15 +71,14 @@ pub struct MkvSource {
 /// length (AAC, MPEG audio, AC-3, E-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<i64> {
     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 {
@@ -98,18 +97,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<i64> {
     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),
@@ -118,6 +112,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 {

crates/codecs/tests/eac3_oracle.rs

@@ -53,12 +53,18 @@ fn eac3_in_mp4_matroska_and_transport_streams() {
         };
         let ours = src.audio(start, n, 48_000).unwrap().channels;
         assert_eq!(ours.len(), ch, "{name}");
-        // Matroska stores CodecDelay (the encoder's 256-sample priming) in ns and timestamps in
-        // ms, and we place packets at the rounded value (240 samples): a shared Matroska timing
-        // limit (AAC and AC-3 alike), not a decoding one. Elsewhere the samples line up exactly.
-        let (worst, lag) = (-32..=32).map(|lag| (snr(&ours, &want, ch, lag), lag)).fold((f64::MIN, 0), |a, b| if b.0 > a.0 { b } else { a });
-        let max_lag = if name.ends_with(".mkv") { 16 } else { 0 };
-        assert!(lag.abs() <= max_lag, "{name}: decode is {lag} samples off ffmpeg's");
+        // Matroska stores the encoder's 256-sample priming as CodecDelay, which we trim exactly
+        // (checked to the sample against the encoded signal in `matroska_audio_oracle`). Whether
+        // ffmpeg's CLI trims it too depends on its version (9.x does; 2023-24 builds keep the
+        // priming samples), so for .mkv its decode may start 256 samples earlier than ours.
+        // Elsewhere the samples line up exactly, whatever the version.
+        let lags: Vec<i64> = match name.ends_with(".mkv") {
+            true => (-32..=32).chain(-256 - 32..=-256 + 32).collect(),
+            false => (-32..=32).collect(),
+        };
+        let (worst, lag) = lags.into_iter().map(|lag| (snr(&ours, &want, ch, lag), lag)).fold((f64::MIN, 0), |a, b| if b.0 > a.0 { b } else { a });
+        let ok = if name.ends_with(".mkv") { lag == 0 || lag == -256 } else { lag == 0 };
+        assert!(ok, "{name}: decode is {lag} samples off ffmpeg's");
         // a window read on a fresh source (random access) gives the same samples
         let fresh = filmcraft_codecs::open_bytes(name, bytes(&f)).unwrap();
         let at = n / 3;

crates/codecs/tests/matroska_audio_oracle.rs

@@ -48,3 +48,100 @@ 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})");
     }
 }
+
+/// Mean |difference| between `ours[i]` and `theirs[i + lag]` over `from..from + len`.
+fn mean_err(ours: &[f32], theirs: &[f32], from: usize, len: usize, lag: i64) -> f64 {
+    (from..from + len).map(|i| (ours[i] as f64 - theirs[(i as i64 + lag) as usize] as f64).abs()).sum::<f64>() / len as f64
+}
+
+/// The lag in `lags` at which `ours` best matches `theirs` (least [`mean_err`]), and that error.
+fn best_lag(ours: &[f32], theirs: &[f32], from: usize, len: usize, lags: std::ops::RangeInclusive<i64>) -> (i64, f64) {
+    lags.map(|lag| (lag, mean_err(ours, theirs, from, len, lag))).min_by(|a, b| a.1.total_cmp(&b.1)).unwrap()
+}
+
+/// A reference signal we control: low-passed white noise from a fixed xorshift seed. Its
+/// autocorrelation dies out within a few samples, so even a one-sample misalignment shows.
+fn noise(n: usize) -> Vec<f32> {
+    let (mut state, mut y) = (0x9E37_79B9_7F4A_7C15u64, 0f32);
+    (0..n)
+        .map(|_| {
+            state ^= state << 13;
+            state ^= state >> 7;
+            state ^= state << 17;
+            let white = (state >> 40) as f32 / (1u64 << 24) as f32 - 0.5;
+            y = 0.5 * y + 0.4 * white;
+            y
+        })
+        .collect()
+}
+
+/// #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 strict check is version-independent: ffmpeg encodes a signal we generate, and our decode
+/// must line up with that signal to the sample (the stream's `CodecDelay` trimmed exactly). ffmpeg's
+/// own decode is compared only after a bounded best-lag search: how its CLI handles Matroska
+/// priming depends on the version (9.x trims `CodecDelay`, 2023-24 builds keep the priming
+/// samples), so its lag is reported, not asserted.
+#[test]
+fn matroska_audio_is_sample_aligned_with_ffmpeg() {
+    let ff = filmcraft_testkit::require_ffmpeg!();
+    let cases: [(&str, u32, &[&str]); 6] = [
+        ("ref_aac_48k.mkv", 48_000, &["-c:a", "aac", "-b:a", "256k"]),
+        ("ref_aac_44k.mkv", 44_100, &["-c:a", "aac", "-b:a", "256k"]),
+        ("ref_ac3_44k.mkv", 44_100, &["-c:a", "ac3", "-b:a", "448k"]),
+        ("ref_ac3_48k.mkv", 48_000, &["-c:a", "ac3", "-b:a", "448k"]),
+        ("ref_eac3_48k.mkv", 48_000, &["-c:a", "eac3", "-b:a", "384k"]),
+        ("ref_mp3_44k.mkv", 44_100, &["-c:a", "libmp3lame", "-b:a", "320k"]),
+    ];
+    for (name, rate, codec) in cases {
+        let reference = noise(rate as usize * 2);
+        let raw = format!("ref_noise_{rate}.f32");
+        let raw_path = filmcraft_testkit::fixtures::generate(&dir().join(&raw), |tmp| {
+            std::fs::write(tmp, reference.iter().flat_map(|v| v.to_le_bytes()).collect::<Vec<u8>>()).is_ok()
+        })
+        .unwrap();
+        let (raw_arg, rate_arg) = (raw_path.to_str().unwrap().to_string(), rate.to_string());
+        let args: Vec<&str> = ["-f", "f32le", "-ar", &rate_arg, "-ac", "1", "-i", &raw_arg].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 file = bytes(&path);
+        let mkv = filmcraft_matroska::open(file.clone()).unwrap();
+        let delay_ns = mkv.tracks.first().map_or(0, |t| t.codec_delay_ns) as i64;
+        let delay = (delay_ns * rate as i64 + 500_000_000) / 1_000_000_000;
+        assert!(delay > 0, "{name}: no CodecDelay");
+        let src = filmcraft_codecs::open_bytes(name, file).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 (from, len, bound) = (rate as usize / 4, rate as usize / 4, delay + 32);
+        // strict: the encoder's input, to the sample
+        let (lag, err) = best_lag(&ours, &reference, from, len, -bound..=bound);
+        let level = reference[from..from + len].iter().map(|v| v.abs() as f64).sum::<f64>() / len as f64;
+        assert_eq!(lag, 0, "{name}: our decode is {lag} samples off the encoded signal (CodecDelay {delay} samples)");
+        assert!(err < 0.25 * level, "{name}: decode error {err:.2e} against a signal level of {level:.2e}");
+        // ffmpeg's decode, wherever its version puts the priming
+        let theirs = ffmpeg_audio_f32(&ff, &path, &[]);
+        assert!(theirs.len() >= frames, "{name}: ffmpeg decoded {} samples", theirs.len());
+        let (ff_lag, ff_err) = best_lag(&ours, &theirs, from, len, -bound..=bound);
+        let priming = match ff_lag {
+            0 => "trims CodecDelay".to_string(),
+            l if l == delay => "keeps the priming samples".to_string(),
+            l => format!("is {l} samples off"),
+        };
+        println!("{name}: CodecDelay {delay} samples; vs the encoded signal lag 0, error {err:.2e} (level {level:.2e}); ffmpeg {priming}, error {ff_err:.2e}");
+        assert!(ff_err < 1e-3, "{name}: differs from ffmpeg's decode by {ff_err:.2e} at its best lag {ff_lag}");
+        // 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}");
+    }
+}

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 the encoded signal (#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% |

docs/gaps.md

@@ -172,6 +172,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 / E-AC-3 sample-aligned with the signal the encoder was
+  given (the exact `CodecDelay` is trimmed, not the 1 ms-rounded one; #790).
 
 ## G11. No plugin hosting