craft-audio: review fixes, round 8 (#442)

- Code wasted bits (RFC 9639 §9.2.2): low bits that are zero in every
  sample of a subframe were stored anyway, so 16-bit audio exported at 24
  or 32 bits grew by the padding.
- Allow residuals up to the 32-bit limit (§9.2.7.3). The limit was half
  that, so at 32 bits a transient forced its whole block to verbatim.
- README: 32-bit FLAC codes stereo channels independently.

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

crates/audio/README.md

@@ -265,6 +265,7 @@ match craft_audio::decode(&bytes, None) {
   without the comment, back surrounds are read as side surrounds. It includes the usual 7-channel
   layout too: without the comment, its back left, back right and back centre are read as back
   centre, side left and side right.
+- FLAC at 32 bits codes stereo channels independently, because a side channel would need 33 bits.
 - Outside RFC 9639's streamable subset (§7), which some decoders require: FLAC at 32 bits, at a
   sample rate the frame header cannot name, or with a layout that needs the mask comment.
 - `ChannelLayout::unordered` is written to FLAC as mask `0x0000`, as RFC 9639 §8.6.2 allows, but
@@ -280,7 +281,7 @@ match craft_audio::decode(&bytes, None) {
 | Codec | Spec | Scope | Verified against |
 |---|---|---|---|
 | WAV, AIFF | Microsoft RIFF, Apple AIFF/AIFF-C | read: integer 8–32-bit, float 32/64-bit, `WAVE_FORMAT_EXTENSIBLE` masks, RF64/BW64, BWF `bext`, truncated files; write: 16/24/32-bit integer and 32-bit float, RF64 above 4 GiB, any layout, TPDF dither | round trips; hostile-input tests |
-| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg |
+| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, wasted bits, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg |
 
 ## symphonia
 

crates/audio/src/write/flac/frame.rs

@@ -120,23 +120,37 @@ fn write_residual(w: &mut BitWriter, residual: &Residual, r: &[i64], block: usiz
 }
 
 fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) {
+    if let Subframe::Wasted { bits, coded } = sub {
+        let shifted: Vec<i32> = x.iter().map(|&v| v >> bits).collect();
+        return write_coded(w, &shifted, coded, bps - bits, *bits);
+    }
+    write_coded(w, x, sub, bps, 0);
+}
+
+/// Subframe header (§9.2.1) and body of samples already without their `wasted` low bits.
+fn write_coded(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8, wasted: u8) {
     let b = u32::from(bps);
     let sample = |w: &mut BitWriter, v: i32| w.write_bits(v as u32, b);
-    w.write_bit(false);
+    // the flag, then the count minus 1 in unary
+    let header = |w: &mut BitWriter, kind: u32| {
+        w.write_bit(false);
+        w.write_bits(kind, 6);
+        w.write_bit(wasted > 0);
+        if wasted > 0 {
+            w.write_bits(1, u32::from(wasted));
+        }
+    };
     match sub {
         Subframe::Constant(v) => {
-            w.write_bits(0, 6);
-            w.write_bit(false);
+            header(w, 0);
             sample(w, *v);
         }
         Subframe::Verbatim => {
-            w.write_bits(1, 6);
-            w.write_bit(false);
+            header(w, 1);
             x.iter().for_each(|&v| sample(w, v));
         }
         Subframe::Fixed { order, residual } => {
-            w.write_bits(0b001000 | u32::from(*order), 6);
-            w.write_bit(false);
+            header(w, 0b001000 | u32::from(*order));
             let o = usize::from(*order);
             x.iter().take(o).for_each(|&v| sample(w, v));
             let r = super::predict::fixed_residual(x, o).unwrap_or_default();
@@ -144,8 +158,7 @@ fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) {
         }
         Subframe::Lpc { coefs, precision, shift, residual } => {
             let o = coefs.len();
-            w.write_bits(0b100000 | (o as u32 - 1), 6);
-            w.write_bit(false);
+            header(w, 0b100000 | (o as u32 - 1));
             x.iter().take(o).for_each(|&v| sample(w, v));
             w.write_bits(u32::from(*precision) - 1, 4);
             w.write_bits(u32::from(*shift), 5);
@@ -153,6 +166,8 @@ fn write_subframe(w: &mut BitWriter, x: &[i32], sub: &Subframe, bps: u8) {
             let r = super::predict::lpc_residual(x, coefs, *shift).unwrap_or_default();
             write_residual(w, residual, &r, x.len(), o);
         }
+        // `choose` does not nest them
+        Subframe::Wasted { coded, .. } => write_coded(w, x, coded, bps, wasted),
     }
 }
 

crates/audio/src/write/flac/oracle_tests.rs

@@ -315,3 +315,30 @@ fn the_frame_limit_stops_a_write_before_it_reaches_the_sink() {
     assert!(w.write(&[[0.0f32; 1]], 1).is_ok());
     assert!(matches!(w.write(&[[0.0f32; 1]], 1), Err(AudioError::TooLarge(_))));
 }
+
+/// Samples with unused low bits, on every kind of subframe and stereo mode: the decoder must put
+/// the bits back before it undoes the stereo decorrelation.
+#[test]
+fn wasted_bits_round_trip() {
+    for (bps, shift) in [(16u8, 3u32), (24, 8), (32, 16), (32, 8)] {
+        let source = 16.min(bps - shift as u8);
+        let pad = |kind, seed| -> Vec<i32> { signal(kind, 9_000, source, seed).iter().map(|&v| v << shift).collect() };
+        for (kind, other) in [("music", "music"), ("noise", "noise"), ("square", "music"), ("click", "sweep")] {
+            let planar = [pad(kind, 51), pad(other, 52)];
+            if round_trip(&format!("wasted-{bps}-{shift}-{kind}"), &planar, EncoderConfig::new(48_000, 2, bps), 4096).is_none() {
+                return;
+            }
+        }
+    }
+    // only the top bit can vary: one bit per sample remains
+    let lowest: Vec<i32> = (0..9_000).map(|i| if i % 3 == 0 { -(1 << 23) } else { 0 }).collect();
+    round_trip("wasted-to-one-bit", &[lowest], EncoderConfig::new(48_000, 1, 24), 4096);
+}
+
+/// A click at full scale needs a residual near the limit; a block is not stored verbatim for it.
+#[test]
+fn a_full_scale_click_at_32_bits_is_still_predicted() {
+    let planar = [signal("click", 4_096, 32, 61)];
+    let Some(size) = round_trip("click-32", &planar, EncoderConfig::new(48_000, 1, 32), 4096) else { return };
+    assert!(size < 4_096 * 4 / 2, "{size}");
+}

crates/audio/src/write/flac/predict.rs

@@ -2,8 +2,8 @@
 //! predictors, linear prediction from the windowed autocorrelation (Levinson–Durbin), and
 //! partitioned Rice codes. Everything here only measures and chooses; [`super::frame`] writes.
 
-/// Residuals larger than this are not Rice coded (a 5-bit parameter tops out at 30).
-const MAX_RESIDUAL: i64 = (1 << 30) - 1;
+/// Residuals must fit a 32-bit integer, leaving out its most negative value (§9.2.7.3).
+const MAX_RESIDUAL: i64 = i32::MAX as i64;
 
 /// A residual coded with partitioned Rice codes.
 #[derive(Clone, Debug, PartialEq)]
@@ -27,8 +27,21 @@ impl Residual {
 pub enum Subframe {
     Constant(i32),
     Verbatim,
-    Fixed { order: u8, residual: Residual },
-    Lpc { coefs: Vec<i32>, precision: u8, shift: u8, residual: Residual },
+    Fixed {
+        order: u8,
+        residual: Residual,
+    },
+    Lpc {
+        coefs: Vec<i32>,
+        precision: u8,
+        shift: u8,
+        residual: Residual,
+    },
+    /// `coded` is of the samples without their `bits` zero low bits, at that many bits fewer (§9.2.2).
+    Wasted {
+        bits: u8,
+        coded: Box<Subframe>,
+    },
 }
 
 /// Folded residual: 0, -1, 1, -2, 2 … → 0, 1, 2, 3, 4 ….
@@ -243,6 +256,13 @@ pub fn choose(x: &[i32], bps: u8, effort: Effort) -> (Subframe, u64) {
     if x.iter().all(|&v| v == first) {
         return (Subframe::Constant(first), head + b);
     }
+    // not constant, so not all zero: at least one bit per sample stays
+    let wasted = x.iter().fold(0, |bits, &v| bits | v).trailing_zeros();
+    if wasted > 0 {
+        let shifted: Vec<i32> = x.iter().map(|&v| v >> wasted).collect();
+        let (coded, bits) = choose(&shifted, bps - wasted as u8, effort);
+        return (Subframe::Wasted { bits: wasted as u8, coded: Box::new(coded) }, bits + u64::from(wasted));
+    }
     let mut best = (Subframe::Verbatim, head + n as u64 * b);
     let mut consider = |sub: Subframe, bits: u64| {
         if bits < best.1 {
@@ -292,6 +312,30 @@ mod tests {
         assert_eq!(choose(&[7; 100], 16, Effort::level(5)), (Subframe::Constant(7), 24));
     }
 
+    #[test]
+    fn residuals_may_reach_the_limit_of_a_32_bit_integer() {
+        assert!(fixed_residual(&[i32::MAX, -i32::MAX], 0).is_some());
+        assert!(fixed_residual(&[0, i32::MAX], 1).is_some());
+        // the most negative value is out of range (§9.2.7.3)
+        assert!(fixed_residual(&[i32::MIN], 0).is_none());
+        assert!(fixed_residual(&[0, 1 << 30, -(1 << 30)], 1).is_none());
+        assert!(lpc_residual(&[0, i32::MAX], &[1], 0).is_some());
+    }
+
+    #[test]
+    fn wasted_bits_are_not_coded() {
+        let x = tones(4096);
+        let padded: Vec<i32> = x.iter().map(|&s| s << 8).collect();
+        let effort = Effort::level(5);
+        let (sub, bits) = choose(&x, 16, effort);
+        // the same coding, plus the 8 bits that say how many are wasted
+        assert_eq!(choose(&padded, 24, effort), (Subframe::Wasted { bits: 8, coded: Box::new(sub) }, bits + 8));
+        // at most what leaves one bit per sample, and nothing for silence, a constant keeps its bits
+        let lowest = [0, -(1 << 23)];
+        assert!(matches!(choose(&lowest, 24, effort).0, Subframe::Wasted { bits: 23, .. }));
+        assert_eq!(choose(&[256; 10], 24, effort).0, Subframe::Constant(256));
+    }
+
     /// Two tones and a little noise, from a fixed seed.
     fn tones(n: usize) -> Vec<i32> {
         let mut seed = 1u32;

crates/audio/tests/audio/roundtrip.rs

@@ -117,6 +117,25 @@ fn flac_handles_silence_noise_full_scale_and_odd_rates() {
     }
 }
 
+/// 16-bit audio exported at a larger depth costs no more: the padding is not coded.
+#[test]
+fn flac_does_not_store_padding() {
+    let mut src = sine(48_000, 2, 20_000);
+    for s in src.channels.iter_mut().flatten() {
+        *s = (*s * 32768.0).round() / 32768.0;
+    }
+    let size = |depth| {
+        let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(depth).with_dither(false);
+        let flac = encode(&src, &opts).unwrap();
+        let (_, wav) = decode(&encode(&src, &opts.clone().with_format(EncodeFormat::Wav)).unwrap(), None).unwrap();
+        assert_eq!(decode(&flac, None).unwrap().1, wav, "{depth:?}");
+        flac.len()
+    };
+    let base = size(BitDepth::Int16);
+    assert!(size(BitDepth::Int24) < base * 11 / 10);
+    assert!(size(BitDepth::Int32) < base * 11 / 10);
+}
+
 #[test]
 fn flac_rejects_unsupported_layouts() {
     let opts = EncodeOptions::new(EncodeFormat::Flac).with_bit_depth(BitDepth::Float32).with_dither(false);