craft-audio: a damaged FLAC keeps its timing; AAC uses BitReader::peek (#846)

* craft-audio: AAC Huffman lookahead uses BitReader::peek

`BitReader::peek` reads from one 8-byte load since the bit reader was made
faster, which is what the AAC decoder's private `peek32` did. The private copy
goes; the lookahead is the same zero-padded 32 bits.

Ported from storytold/filmcraft#4232.

* craft-audio: a damaged FLAC keeps its length and timing

symphonia's FLAC demuxer drops a frame whose checksum fails and resyncs on the
next one, so the decoder never saw an error: the file decoded shorter and all
later audio came out one block early, with `damaged` at 0.

The jump in the packet timestamps is the audio that was lost. It is returned
as silence, in blocks of at most 65 536 frames, and counted once in
`AudioInfo::damaged`. It never passes the length the file declares; with no
declared length a jump of more than 2^20 frames is not believed. A seek into
the lost frames lands on silence at the right position.

Ported from storytold/soundcraft#556, onto the streaming decoder.

---------

Co-authored-by: vangel76 <vangel1976@googlemail.com>
Co-authored-by: Gert Pellin <gert@pellin.be>

crates/audio/README.md

@@ -116,7 +116,9 @@ let left: &[f32] = &buf.channels[0];
 ```
 
 `decode` returns `TooLarge` above 2³⁰ samples (4 GiB of `f32`). `info.damaged` counts packets
-that were damaged and replaced by silence of their length.
+that were damaged and replaced by silence of their length. A FLAC frame whose checksum fails is
+dropped by the demuxer; the jump in the timestamps shows it, and the lost frames are silence too (one
+count per run of them), so the audio after them stays in place.
 
 ### Choose a track
 

crates/audio/src/aac/huffman.rs

@@ -104,7 +104,7 @@ impl Lut {
     /// The symbol of the codeword at the reader position; `Eof` if the codeword is cut off.
     #[inline]
     fn decode(&self, br: &mut BitReader) -> Result<u32> {
-        let bits = peek32(br);
+        let bits = br.peek(32);
         let mut e = self.t[(bits >> (32 - L1)) as usize];
         if e & SUB != 0 {
             let w = e & 31;
@@ -119,17 +119,6 @@ impl Lut {
     }
 }
 
-/// The next 32 bits (zero padded past the end), from one 8-byte load when possible.
-#[inline]
-fn peek32(br: &BitReader) -> u32 {
-    let pos = br.position();
-    let byte = pos >> 3;
-    match br.data().get(byte..byte.saturating_add(8)).and_then(|b| <[u8; 8]>::try_from(b).ok()) {
-        Some(b) => ((u64::from_be_bytes(b) << (pos & 7)) >> 32) as u32,
-        None => br.peek(32),
-    }
-}
-
 fn luts() -> &'static [Lut; 12] {
     static LUTS: OnceLock<[Lut; 12]> = OnceLock::new();
     LUTS.get_or_init(|| std::array::from_fn(|i| if i == 0 { Lut::build(&SCALEFACTOR) } else { Lut::build(BOOKS[i - 1].codes) }))

crates/audio/src/lib.rs

@@ -337,7 +337,7 @@ pub struct AudioInfo {
     /// Broadcast Wave metadata from a WAV `bext` chunk.
     pub bwf: Option<BwfInfo>,
     /// Packets that were damaged or failed to decode and were replaced by concealment (silence of
-    /// their length), counted as audio is decoded: by [`decode`] for the whole file, and so far by
+    /// their length), and runs of FLAC frames lost to a failed checksum, counted as audio is decoded: by [`decode`] for the whole file, and so far by
     /// a [`Decoder`]. 0 from [`probe`] when it read only headers.
     pub damaged: u64,
 }

crates/audio/src/read/symph.rs

@@ -27,6 +27,10 @@ use crate::{AudioBuffer, AudioError, AudioInfo, ChannelLayout, CodecId, Containe
 /// takes its frame length from the file (encoders use 4096).
 const MAX_PACKET_FRAMES: u64 = 1 << 20;
 
+/// The most frames of silence one block holds where a FLAC lost frames; a longer loss is several
+/// blocks.
+const GAP_BLOCK_FRAMES: u64 = 1 << 16;
+
 /// The most samples (frames x channels) a packet may declare: the PCM decoder allocates
 /// that many for its output up front, so a CAF with thousands of channels and a long packet could
 /// otherwise claim gigabytes before a sample is decoded (64 MiB of `f32`). Ports soundcraft #1332.
@@ -367,6 +371,22 @@ fn precheck(b: &[u8], start: usize, detected: Option<Container>) -> Result<Prech
     }
 }
 
+/// `block` without the frames still to drop after a seek landed before its target; `None` when
+/// that is all of it.
+fn after_seek(discard: &mut u64, mut block: AudioBuffer) -> Option<AudioBuffer> {
+    if *discard > 0 {
+        let drop = (*discard as usize).min(block.frames());
+        for c in &mut block.channels {
+            c.drain(..drop);
+        }
+        *discard -= drop as u64;
+        if block.frames() == 0 {
+            return None;
+        }
+    }
+    Some(block)
+}
+
 /// Skip leading ID3v2 tags, run the container size preflight, then probe with symphonia; a zero-frame FLAC found by
 /// the preflight comes back as [`Opened::Empty`] without probing. A size-checked container is refused when probing
 /// selects a different reader or one that was never checked, so incidental payload magic cannot bypass those limits.
@@ -576,6 +596,13 @@ pub(crate) struct Symph {
     last_frames: usize,
     /// Frames still to drop after a seek landed before its target.
     discard: u64,
+    /// A FLAC file: the frame the next packet starts at when none was lost. The FLAC demuxer drops
+    /// a frame whose checksum fails and resyncs on the next one, so a later timestamp is audio that
+    /// was lost.
+    flac_next: Option<u64>,
+    /// Frames of silence still to return for lost FLAC frames, then the block decoded after them.
+    gap: u64,
+    held: Option<Vec<Vec<f32>>>,
     ended: bool,
     interleaved: Vec<f32>,
 }
@@ -602,6 +629,9 @@ impl Symph {
                         errors: 0,
                         last_frames: 0,
                         discard: 0,
+                        flac_next: None,
+                        gap: 0,
+                        held: None,
                         ended: true,
                         interleaved: Vec::new(),
                     });
@@ -631,6 +661,9 @@ impl Symph {
             }
             let decoder = symphonia::default::get_codecs().make_audio_decoder(&params, &decoder_options()).map_err(map_err)?;
             let frames_known = info.frames > 0 && info.channels > 0;
+            // Native FLAC timestamps count frames from 0.
+            let counts_frames = t.time_base.is_none_or(|tb| tb.numer.get() == 1 && tb.denom.get() == info.sample_rate);
+            let flac_next = (info.container == Container::Flac && info.codec == CodecId::Flac && counts_frames).then_some(0);
             Ok(Symph {
                 time_base: t.time_base,
                 reader: Some(reader),
@@ -642,6 +675,9 @@ impl Symph {
                 errors: 0,
                 last_frames: 0,
                 discard: 0,
+                flac_next,
+                gap: 0,
+                held: None,
                 ended: false,
                 interleaved: Vec::new(),
             })
@@ -669,6 +705,20 @@ impl Symph {
     fn next_block_inner(&mut self) -> Result<Option<AudioBuffer>> {
         let (Some(reader), Some(decoder)) = (self.reader.as_mut(), self.decoder.as_mut()) else { return Ok(None) };
         loop {
+            // Silence for lost FLAC frames comes first, then the block that was decoded after them.
+            let pending = if self.gap > 0 {
+                let n = self.gap.min(GAP_BLOCK_FRAMES);
+                self.gap -= n;
+                Some(crate::pcm::silence(usize::from(self.info.channels), n as usize)?)
+            } else {
+                self.held.take()
+            };
+            if let Some(block) = pending {
+                match after_seek(&mut self.discard, AudioBuffer { sample_rate: self.info.sample_rate, channels: block }) {
+                    Some(block) => return Ok(Some(block)),
+                    None => continue,
+                }
+            }
             if self.ended {
                 if self.info.channels == 0 {
                     return Err(AudioError::Malformed("stream contains no audio".into()));
@@ -694,6 +744,18 @@ impl Symph {
             if packet.track_id != self.track_id {
                 continue;
             }
+            if let Some(next) = self.flac_next {
+                let pts = u64::try_from(packet.pts.get()).unwrap_or(0);
+                let lost = pts.saturating_sub(next);
+                // Never past the declared length. Without one, a jump longer than any packet is
+                // not believed.
+                let room = if self.frames_known { self.info.frames.saturating_sub(next) } else { MAX_PACKET_FRAMES };
+                if lost > 0 && lost <= room && self.info.channels > 0 {
+                    self.gap = lost;
+                    self.info.damaged += 1;
+                }
+                self.flac_next = Some(pts.saturating_add(packet.dur.get()));
+            }
             let block = match decoder.decode(&packet) {
                 Ok(decoded) => {
                     // The interleaved copy below is laid out by planes: use their count so a
@@ -755,18 +817,13 @@ impl Symph {
                 }
                 Err(e) => return Err(map_err(e)),
             };
-            let mut block = AudioBuffer { sample_rate: self.info.sample_rate, channels: block };
-            if self.discard > 0 {
-                let drop = (self.discard as usize).min(block.frames());
-                for c in &mut block.channels {
-                    c.drain(..drop);
-                }
-                self.discard -= drop as u64;
-                if block.frames() == 0 {
-                    continue;
-                }
+            if self.gap > 0 {
+                self.held = Some(block);
+                continue;
+            }
+            if let Some(block) = after_seek(&mut self.discard, AudioBuffer { sample_rate: self.info.sample_rate, channels: block }) {
+                return Ok(Some(block));
             }
-            return Ok(Some(block));
         }
     }
 
@@ -802,8 +859,13 @@ impl Symph {
             decoder.reset();
             // the packet before the seek says nothing about the length of one lost after it
             self.last_frames = 0;
+            self.gap = 0;
+            self.held = None;
             let actual = u64::try_from(seeked.actual_ts.get()).unwrap_or(0);
             let actual = u64::try_from(u128::from(actual) * u128::from(numer) * u128::from(rate) / u128::from(denom)).unwrap_or(u64::MAX);
+            // A seek into lost frames lands on the next one that survived: the stream is where the
+            // seek says, and that packet's later timestamp is the rest of the loss.
+            self.flac_next = self.flac_next.map(|_| actual);
             self.discard = frame.saturating_sub(actual);
             self.ended = false;
             Ok(frame)

crates/audio/tests/audio/flac_damage.rs

@@ -0,0 +1,155 @@
+//! A damaged FLAC keeps its length and timing: the frames that were lost are silence.
+
+use craft_audio::{AudioBuffer, BitDepth, DecodeOptions, Decoder, EncodeFormat, EncodeOptions, decode, encode};
+
+const FRAMES: usize = 48_000;
+/// The encoder's block size: the frames one FLAC frame holds.
+const BLOCK: usize = 4096;
+
+fn tone() -> AudioBuffer {
+    let mut b = AudioBuffer::new(48_000, 1, FRAMES);
+    for (i, s) in b.channels[0].iter_mut().enumerate() {
+        *s = ((8000.0 * (i as f64 * 0.05).sin()).round() / 32768.0) as f32;
+    }
+    b
+}
+
+fn flac() -> Vec<u8> {
+    encode(&tone(), &EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Int16).with_dither(false)).unwrap()
+}
+
+fn crc(data: &[u8], bits: u32, poly: u32) -> u32 {
+    let mut c = 0u32;
+    for &b in data {
+        c ^= u32::from(b) << (bits - 8);
+        for _ in 0..8 {
+            c = if c & (1 << (bits - 1)) != 0 { (c << 1) ^ poly } else { c << 1 };
+        }
+    }
+    c & ((1 << bits) - 1)
+}
+
+/// Where each frame of `flac()` starts (and the end of the file): a sync code whose header has a
+/// valid CRC-8. Frame numbers below 128 are one byte; the short last block stores its size in two
+/// more.
+fn frame_starts(bytes: &[u8]) -> Vec<usize> {
+    let header = |i: usize| {
+        let len = if bytes.get(i + 2)? >> 4 == 7 { 7 } else { 5 };
+        (bytes.get(i..i + 2)? == [0xFF, 0xF8] && crc(bytes.get(i..i + len)?, 8, 0x07) == u32::from(*bytes.get(i + len)?)).then_some(i)
+    };
+    let mut starts: Vec<usize> = (0..bytes.len()).filter_map(header).collect();
+    assert_eq!(starts.len(), FRAMES.div_ceil(BLOCK));
+    starts.push(bytes.len());
+    starts
+}
+
+/// `flac()` with `shift` added to the number of every frame from `from` on (the new numbers are 128
+/// to 2047: two bytes), checksums corrected: its timestamps jump although no frame is damaged.
+fn renumbered(from: usize, shift: u32) -> Vec<u8> {
+    let bytes = flac();
+    let starts = frame_starts(&bytes);
+    let mut out = bytes[..starts[from]].to_vec();
+    for (at, w) in starts.windows(2).enumerate().skip(from) {
+        let (start, end) = (w[0], w[1]);
+        let header = if bytes[start + 2] >> 4 == 7 { 7 } else { 5 };
+        let number = at as u32 + shift;
+        let mut frame = bytes[start..start + 4].to_vec();
+        frame.extend_from_slice(&[0xC0 | (number >> 6) as u8, 0x80 | (number & 0x3F) as u8]);
+        frame.extend_from_slice(&bytes[start + 5..start + header]);
+        frame.push(crc(&frame, 8, 0x07) as u8);
+        frame.extend_from_slice(&bytes[start + header + 1..end - 2]);
+        frame.extend_from_slice(&(crc(&frame, 16, 0x8005) as u16).to_be_bytes());
+        out.extend_from_slice(&frame);
+    }
+    out
+}
+
+/// `buf` is the tone with exactly the frames in `lost` silent.
+fn assert_silent_in(buf: &AudioBuffer, lost: std::ops::Range<usize>) {
+    let src = tone();
+    assert_eq!(buf.frames(), FRAMES);
+    for (i, (a, b)) in src.channels[0].iter().zip(&buf.channels[0]).enumerate() {
+        assert_eq!(*b, if lost.contains(&i) { 0.0 } else { *a }, "frame {i}");
+    }
+}
+
+#[test]
+fn an_intact_flac_reports_no_damage() {
+    let (info, buf) = decode(&flac(), Some("flac")).unwrap();
+    assert_eq!(info.damaged, 0);
+    assert_silent_in(&buf, 0..0);
+}
+
+/// The FLAC demuxer drops a frame whose checksum fails, and the audio after it used to come out
+/// one block early.
+#[test]
+fn a_corrupt_flac_frame_becomes_silence_and_is_counted() {
+    let mut bytes = flac();
+    let starts = frame_starts(&bytes);
+    bytes[starts[4] + 20] ^= 0xFF;
+    let (info, buf) = decode(&bytes, Some("flac")).unwrap();
+    assert_eq!((info.frames, info.damaged), (FRAMES as u64, 1));
+    assert_silent_in(&buf, 4 * BLOCK..5 * BLOCK);
+}
+
+#[test]
+fn a_run_of_corrupt_flac_frames_is_one_gap() {
+    let mut bytes = flac();
+    let starts = frame_starts(&bytes);
+    for frame in 3..6 {
+        bytes[starts[frame] + 20] ^= 0xFF;
+    }
+    let (info, buf) = decode(&bytes, Some("flac")).unwrap();
+    assert_eq!((info.frames, info.damaged), (FRAMES as u64, 1));
+    assert_silent_in(&buf, 3 * BLOCK..6 * BLOCK);
+}
+
+/// A seek lands on the right audio before the loss, inside it and after it.
+#[test]
+fn seeking_a_damaged_flac_keeps_its_timing() {
+    let mut bytes = flac();
+    let starts = frame_starts(&bytes);
+    bytes[starts[4] + 20] ^= 0xFF;
+    let src = tone();
+    for target in [0, 3 * BLOCK + 7, 4 * BLOCK, 4 * BLOCK + 100, 5 * BLOCK + 1, 9 * BLOCK] {
+        let mut dec = Decoder::from_bytes(&bytes, Some("flac"), &DecodeOptions::default()).unwrap();
+        assert_eq!(dec.seek(target as u64).unwrap(), target as u64);
+        let mut got = Vec::new();
+        while let Some(block) = dec.next_block().unwrap() {
+            got.extend_from_slice(&block.channels[0]);
+        }
+        assert_eq!(got.len(), FRAMES - target, "from {target}");
+        for (i, (a, b)) in got.iter().zip(&src.channels[0][target..]).enumerate() {
+            let lost = (4 * BLOCK..5 * BLOCK).contains(&(target + i));
+            assert_eq!(*a, if lost { 0.0 } else { *b }, "from {target}: frame {}", target + i);
+        }
+    }
+}
+
+/// A timestamp that jumps past the declared length is not padded: the file says how long it is.
+#[test]
+fn a_jump_past_the_declared_length_is_not_silence() {
+    let (info, buf) = decode(&renumbered(5, 300), Some("flac")).unwrap();
+    assert_eq!(info.damaged, 0);
+    assert!(buf.frames() <= FRAMES, "{} frames", buf.frames());
+}
+
+/// With no declared length, a jump of up to 2^20 frames is lost audio; a longer one is not believed.
+#[test]
+fn an_undeclared_length_bounds_the_silence() {
+    let unknown = |mut bytes: Vec<u8>| {
+        // STREAMINFO's 36-bit total sample count: the low nibble of byte 21 and bytes 22 to 25
+        bytes[21] &= 0xF0;
+        bytes[22..26].fill(0);
+        bytes
+    };
+    // 200 frames of 4096 (819 200 samples) are missing before frame 5
+    let (info, buf) = decode(&unknown(renumbered(5, 200)), Some("flac")).unwrap();
+    assert_eq!((info.damaged, buf.frames()), (1, FRAMES + 200 * BLOCK));
+    assert!(buf.channels[0][5 * BLOCK..205 * BLOCK].iter().all(|&s| s == 0.0));
+    assert_eq!(buf.channels[0][205 * BLOCK..], tone().channels[0][5 * BLOCK..]);
+    // 300 frames are 1 228 800 samples, more than 2^20: the audio stays where it was
+    let (info, buf) = decode(&unknown(renumbered(5, 300)), Some("flac")).unwrap();
+    assert_eq!(info.damaged, 0);
+    assert_silent_in(&buf, 0..0);
+}

crates/audio/tests/audio/main.rs

@@ -2,6 +2,7 @@
 
 mod decode_extensions;
 mod encode_format;
+mod flac_damage;
 mod float_headroom;
 mod malformed;
 mod mp4_length;