craft-audio: SBR review fixes

- A VARFIX pointer of 1 marks no transient envelope, like 0.
- A start at QMF channel 32 is valid; kx is checked once the crossover is known.
- The noise and harmonic gains take q / (1 + q) first, so two clamped levels no longer
  overflow to infinity and NaN.
- A header that runs past its fill element is refused before it replaces the one in use.
- A new program config layout drops the SBR decoders, whose history was the old order's.
- The ffmpeg oracle requires a high band to compare on every channel but the 5.1 LFE.

Refs #1121

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

crates/audio/src/aac/decoder.rs

@@ -135,6 +135,10 @@ impl Decoder {
         }
         self.layout = layout;
         self.offsets = offsets;
+        // the SBR decoders' history belongs to the old element order: `apply_sbr` rebuilds them
+        if let Some(e) = self.sbr_out.as_mut() {
+            e.clear();
+        }
         self.channels = vec![ChannelState { overlap: vec![0.0; 1024], prev_shape: WindowShape::Sine }; n];
     }
 
@@ -700,7 +704,7 @@ mod tests {
     /// its SBR fill element describes is there; with SBR off, the core alone (silence).
     #[test]
     fn sbr_fill_elements_rebuild_the_high_band() {
-        let payload = sbr::test_payload(44_100, |_| 40, 0).unwrap();
+        let payload = sbr::test_payload(44_100, |_| 40, |_| 0).unwrap();
         let mut bw = BitWriter::new();
         bw.write_bits(ID_SCE, 3);
         bw.write_bits(0, 4);
@@ -736,6 +740,11 @@ mod tests {
         assert_eq!(out.len(), 4 * 2048);
         let rms = (out[2048..].iter().map(|s| s * s).sum::<f32>() / (out.len() - 2048) as f32).sqrt();
         assert!(rms > 1e-4, "{rms}");
+        // a new layout drops the SBR decoders (their history is the old element order's) and
+        // keeps SBR on
+        dec.set_layout(vec![(ElementType::Sce, 0)]);
+        assert_eq!(dec.sbr_out.as_ref().map(Vec::len), Some(0));
+        assert_eq!(dec.decode(&au).unwrap()[0].len(), 2048);
     }
 
     /// An access unit whose elements are all empty: `elements` are (id, tag), then ID_END.

crates/audio/src/aac/sbr/bitstream.rs

@@ -204,8 +204,9 @@ impl Grid {
         if t[0] < 0 || t[..=n].windows(2).any(|w| w[0] >= w[1]) {
             return Err(bad());
         }
+        // l_A (ISO/IEC 14496-3 §4.6.18.3.3): VARFIX pointer 1, like 0, marks no envelope
         let la = match (pointer, class) {
-            (0, _) => None,
+            (0, _) | (1, VARFIX) => None,
             (p, FIXVAR | VARVAR) => Some((n + 1).checked_sub(p).ok_or_else(bad)?),
             (p, _) => Some(p - 1),
         };
@@ -468,6 +469,9 @@ mod tests {
         // VARFIX: leading border 1, relative 2, pointer 2: l_A = 1
         let g = grid(&[(VARFIX as u32, 2), (1, 2), (1, 2), (0, 2), (2, 2), (1, 1), (0, 1)]).unwrap();
         assert_eq!((g.num_env, &g.t_env[..3], g.la, g.t_noise), (2, &[1, 3, 16][..], Some(1), [1, 3, 16]));
+        // the same with pointer 1: no transient envelope, the noise floors still split at 3
+        let g = grid(&[(VARFIX as u32, 2), (1, 2), (1, 2), (0, 2), (1, 2), (1, 1), (0, 1)]).unwrap();
+        assert_eq!((g.la, g.t_noise), (None, [1, 3, 16]));
         // VARVAR: 1 .. 17 with one border from each end
         let g = grid(&[(VARVAR as u32, 2), (1, 2), (1, 2), (1, 2), (1, 2), (0, 2), (0, 2), (0, 2), (0, 1), (0, 1), (0, 1)]).unwrap();
         assert_eq!((g.num_env, &g.t_env[..4], g.la), (3, &[1, 3, 15, 17][..], None));

crates/audio/src/aac/sbr/freq.rs

@@ -75,8 +75,9 @@ fn start_stop(h: &Header, fs: u32) -> Option<(usize, usize)> {
         }
     }
     .min(64);
-    // the low band reaches only the analysis bank's 32 channels
-    (1..32).contains(&k0).then_some(())?;
+    // the low band reaches only the analysis bank's 32 channels (`kx` is checked against them
+    // once the crossover is known)
+    (1..=32).contains(&k0).then_some(())?;
     (k2 > k0).then_some((k0 as usize, k2 as usize))
 }
 
@@ -341,6 +342,14 @@ mod tests {
         assert!(valid > 10_000, "{valid}");
     }
 
+    /// The highest start a header can give, 32 at 24 kHz (start 15), is valid: with stop 14
+    /// (twice the start) and linear bands it covers channels 32 to 64.
+    #[test]
+    fn a_start_at_channel_32_is_accepted() {
+        let t = FreqTables::new(&header(15, 14, 0, 0, false, 2), 24_000).unwrap();
+        assert_eq!((t.k0, t.kx, t.m), (32, 32, 32));
+    }
+
     /// A typical 44.1 kHz header: start channel 12 + offset, stop near 64, three noise bands.
     #[test]
     fn a_44k_header_has_the_expected_layout() {

crates/audio/src/aac/sbr/hf.rs

@@ -201,13 +201,14 @@ fn envelope_gains(f: &Frame, l: usize, sine_index: &[bool; 64], delta: bool) ->
     }
     let mut out = EnvelopeGains { gain: [0.0; 64], noise: [0.0; 64], sine: [0.0; 64] };
     for c in 0..m {
+        // `q / (1 + q)` first: energy and noise ratio are each clamped, their product is not
         let (eo, ec, q) = (e_orig[c], e_curr[c], q[c]);
-        out.noise[c] = (eo * q / (1.0 + q)).sqrt();
+        let share = q / (1.0 + q);
+        out.noise[c] = (eo * share).sqrt();
         if sine_index[c] {
             out.sine[c] = (eo / (1.0 + q)).sqrt();
         }
-        out.gain[c] =
-            if s_band[c] { (eo / (EPS + ec) * q / (1.0 + q)).sqrt() } else { (eo / ((EPS + ec) * (1.0 + if delta { q } else { 0.0 }))).sqrt() };
+        out.gain[c] = if s_band[c] { (eo / (EPS + ec) * share).sqrt() } else { (eo / ((EPS + ec) * (1.0 + if delta { q } else { 0.0 }))).sqrt() };
     }
     // limit per limiter band, then boost back the energy the limiter took
     let lim_gain = LIMITER_GAINS[usize::from(hdr.limiter_gains & 3)];

crates/audio/src/aac/sbr/mod.rs

@@ -285,6 +285,8 @@ impl Element {
         }
         if br.read_flag().ok()? {
             let h = Header::read(&mut br).ok()?;
+            // a header that runs past its fill element is damaged: keep the one in use
+            (br.position() <= p.end).then_some(())?;
             if self.header != Some(h) {
                 if self.header.is_none_or(|o| !o.same_tables(&h)) {
                     for c in &mut self.channels {
@@ -377,13 +379,13 @@ pub(crate) const TEST_HEADER: Header = Header {
 
 /// An SBR single channel element for tests at output rate `rate`: [`TEST_HEADER`] (3 dB steps,
 /// start 5, stop 9, the default extra fields), then one FIXFIX envelope at high resolution with
-/// band `k` at `env(k)` (1.5 dB steps, as a single FIXFIX envelope has) and every noise floor
-/// value `noise`.
+/// band `k` at `env(k)` (1.5 dB steps, as a single FIXFIX envelope has) and noise band `k` at
+/// `noise(k)`.
 #[cfg(test)]
-pub(crate) fn test_payload(rate: u32, env: impl Fn(usize) -> i32, noise: u32) -> Option<Vec<u8>> {
+pub(crate) fn test_payload(rate: u32, env: impl Fn(usize) -> i32, noise: impl Fn(usize) -> i32) -> Option<Vec<u8>> {
     use craft_media::bitstream::BitWriter;
     let t = FreqTables::new(&TEST_HEADER, rate)?;
-    let (env, n_q): (Vec<i32>, usize) = ((0..t.n_high()).map(env).collect(), t.n_q());
+    let (env, noise): (Vec<i32>, Vec<i32>) = ((0..t.n_high()).map(env).collect(), (0..t.n_q()).map(noise).collect());
     let mut bw = BitWriter::new();
     bw.write_bits(1, 1); // bs_header_flag
     for (v, n) in [(1, 1), (5, 4), (9, 4), (0, 3), (0, 2), (0, 1), (0, 1)] {
@@ -394,7 +396,7 @@ pub(crate) fn test_payload(rate: u32, env: impl Fn(usize) -> i32, noise: u32) ->
     bw.write_bits(0, 2); // one envelope
     bw.write_bits(1, 1); // high resolution
     bw.write_bits(0, 2); // frequency-direction envelope and noise floor
-    for _ in 0..n_q {
+    for _ in &noise {
         bw.write_bits(0, 2); // no inverse filtering
     }
     bw.write_bits(env[0] as u32, 7);
@@ -402,9 +404,9 @@ pub(crate) fn test_payload(rate: u32, env: impl Fn(usize) -> i32, noise: u32) ->
         let (code, len) = tables::F_ENV_1_5DB[(w[1] - w[0] + 60) as usize];
         bw.write_bits(code, u32::from(len));
     }
-    bw.write_bits(noise, 5);
-    for _ in 1..n_q {
-        let (code, len) = tables::F_ENV_3_0DB[31];
+    bw.write_bits(noise[0] as u32, 5);
+    for w in noise.windows(2) {
+        let (code, len) = tables::F_ENV_3_0DB[(w[1] - w[0] + 31) as usize];
         bw.write_bits(code, u32::from(len));
     }
     bw.write_bits(0, 2); // no sinusoids, no extended data
@@ -460,7 +462,7 @@ mod tests {
     fn the_high_band_follows_the_envelope() {
         let t = tables();
         let mid = t.n_high() / 2;
-        let data = test_payload(RATE, |k| if k < mid { 30 } else { 42 }, 0).unwrap();
+        let data = test_payload(RATE, |k| if k < mid { 30 } else { 42 }, |_| 0).unwrap();
         let mut e = Element::new(1, RATE);
         let out = run(&mut e, &data, 6);
         let tail = &out[out.len() - 4096..];
@@ -481,7 +483,7 @@ mod tests {
     /// finite and of full length.
     #[test]
     fn damaged_sbr_data_is_survived() {
-        let good = test_payload(RATE, |k| 20 + k as i32, 3).unwrap();
+        let good = test_payload(RATE, |k| 20 + k as i32, |_| 3).unwrap();
         let mut seed = 0x1234_5678u32;
         let mut next = || {
             seed ^= seed << 13;
@@ -510,4 +512,33 @@ mod tests {
             assert!(out.iter().all(|c| c.len() == 2048 && c.iter().all(|s| s.is_finite())));
         }
     }
+
+    /// The largest envelope (127 steps of 1.5 dB) with the largest noise floor (each band 31
+    /// steps below the last) keeps the output finite: the gains are formed so that no
+    /// intermediate product of two clamped levels overflows.
+    #[test]
+    fn extreme_levels_stay_finite() {
+        let rate =
+            [22_050, 24_000, 32_000, 44_100, 48_000].into_iter().find(|&r| FreqTables::new(&TEST_HEADER, r).is_some_and(|t| t.n_q() >= 3)).unwrap();
+        let data = test_payload(rate, |_| 127, |k| -31 * k as i32).unwrap();
+        let mut e = Element::new(1, rate);
+        run(&mut e, &data, 3);
+    }
+
+    /// A header that runs past the end of its fill element is not taken: the one in use stays,
+    /// and the frame is dropped.
+    #[test]
+    fn a_header_cut_short_is_not_taken() {
+        let data = test_payload(RATE, |_| 30, |_| 0).unwrap();
+        let mut e = Element::new(1, RATE);
+        run(&mut e, &data, 1);
+        assert_eq!(e.header, Some(TEST_HEADER));
+        // another start frequency (bits 2..6), in an element that ends 6 bits in
+        let mut cut = data.clone();
+        cut[0] ^= 0x04;
+        assert!(e.read(Payload { data: &cut, start: 0, end: 6, crc: false }).is_none());
+        assert_eq!(e.header, Some(TEST_HEADER));
+        assert!(e.read(Payload { data: &cut, start: 0, end: 8 * cut.len(), crc: false }).is_some());
+        assert_ne!(e.header, Some(TEST_HEADER));
+    }
 }

crates/audio/tests/codecs/aac/he_aac_oracle.rs

@@ -149,8 +149,12 @@ fn matches_ffmpeg_he_aac_decoder() {
                     }
                 }
             }
-            let mean = diffs.iter().sum::<f64>() / diffs.len().max(1) as f64;
-            let mad = diffs.iter().map(|d| d.abs()).sum::<f64>() / diffs.len().max(1) as f64;
+            // the LFE (WAV channel 3 of 5.1) has no SBR, so no high band; every other channel must
+            // have one to compare
+            let lfe = case.channels == 6 && c == 3;
+            assert!(lfe || !diffs.is_empty(), "{} ch{c}: no high band to compare", case.name);
+            let mean = diffs.iter().sum::<f64>() / diffs.len() as f64;
+            let mad = diffs.iter().map(|d| d.abs()).sum::<f64>() / diffs.len() as f64;
             println!(
                 "{} ch{c}: full SNR {full:.1} dB, low band SNR {low:.1} dB, high band level {mean:+.2} dB (mean |Δ| {mad:.2} dB over {})",
                 case.name,
@@ -159,7 +163,7 @@ fn matches_ffmpeg_he_aac_decoder() {
             // The low band differs by the core decoders' rounding; the high band's noise comes
             // from the same table, so whole frames match too.
             assert!(full > 45.0 && low > 55.0, "{} ch{c}: SNR {full:.1} / {low:.1} dB", case.name);
-            assert!(mad < 0.1, "{} ch{c}: high band off by {mad:.2} dB", case.name);
+            assert!(lfe || mad < 0.1, "{} ch{c}: high band off by {mad:.2} dB", case.name);
         }
     }
 }