From 57acc28df30cd6590f9561d2b77aa77bfbdaebea Mon Sep 17 00:00:00 2001 From: SusieeTheFemboy <96988330+SusieeTheLinuxUser@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:00:01 +0200 Subject: [PATCH] H.264: the inverse transforms clamp coefficients to the spec r range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dequantisation wraps by design (huge_levels_dequantise_without_ overflow_panics), so idct4_add and idct8_add receive any i32 coefficient, and their plain-i32 butterflies panicked a debug build with "attempt to add with overflow" on extreme values. A dequantised r outside -2^15..2^15-1 leaves the transforms' output unspecified (ITU-T H.264 8.5.12.1, 8.5.13.1), so those inputs are clamped to that range — bit-exact for every conforming stream — instead of panicking. Debug-only (release wraps), memory-safe either way; the sibling SoundCraft decoder fixed the same class (its audit's H.264 arithmetic findings). Extends the grouped robustness audit's codec-overflow list (#1183), which covers the POC/MMCO/latency sites but not these transforms. Co-Authored-By: GLM-5.3 --- crates/h264/src/transform.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/h264/src/transform.rs b/crates/h264/src/transform.rs index f87a1c98..e9dacb69 100644 --- a/crates/h264/src/transform.rs +++ b/crates/h264/src/transform.rs @@ -91,11 +91,21 @@ fn clip_u8(v: i32) -> u8 { v.clamp(0, 255) as u8 } +/// A dequantised coefficient the inverse transforms are defined for: the dequantisation wraps +/// by design (see [`huge_levels_dequantise_without_overflow_panics`]), so their butterflies +/// clamp an r outside −2^15..2^15−1 to this range — where the spec leaves the output +/// unspecified anyway (8.5.12.1, 8.5.13.1) — instead of overflowing the plain i32 additions. +#[inline(always)] +fn r_clamp(v: i32) -> i32 { + v.clamp(-(1 << 15), (1 << 15) - 1) +} + /// Inverse 4x4 transform of raster-order `d` and add to `dst` (8.5.12.2, 8.5.14). /// /// Rows first, then columns (the spec's order: the `>> 1` steps make it matter). The column /// pass and the reconstruction run across all columns at once so they vectorise. pub fn idct4_add(d: &[i32; 16], dst: &mut [u8], stride: usize) { + let d: [i32; 16] = std::array::from_fn(|i| r_clamp(d[i])); let mut t = [[0i32; 4]; 4]; for (i, row) in t.iter_mut().enumerate() { let r = &d[i * 4..i * 4 + 4]; @@ -161,6 +171,7 @@ fn idct8_1d(d: [i32; 8]) -> [i32; 8] { /// Inverse 8x8 transform of raster-order `d` and add to `dst` (8.5.13.2): rows, then columns, /// the column pass and reconstruction across all columns at once (they vectorise). pub fn idct8_add(d: &[i32; 64], dst: &mut [u8], stride: usize) { + let d: [i32; 64] = std::array::from_fn(|i| r_clamp(d[i])); let mut t = [[0i32; 8]; 8]; for (row, src) in t.iter_mut().zip(d.as_chunks::<8>().0) { *row = idct8_1d(*src); @@ -304,4 +315,28 @@ mod tests { let _ = scale8(i32::MIN, 16 * 25, 51); let _ = scale8(i32::MIN, 16 * 25, 0); } + + /// The same wrapping dequantisation hands the inverse transforms any i32 coefficient, and + /// their butterflies add in plain i32: `i32::MAX + i32::MAX` panicked a debug build. The + /// spec leaves the transforms' output unspecified when a dequantised value falls outside + /// −2^15..2^15−1 (8.5.12.1, 8.5.13.1), so those inputs are clamped to that range — + /// bit-exact for every stream inside it — instead of panicking. + #[test] + fn huge_coefficients_are_clamped_to_the_spec_r_range() { + let clamp = |v: i32| v.clamp(-(1 << 15), (1 << 15) - 1); + for d in [[i32::MAX; 16], [i32::MIN; 16], std::array::from_fn(|i| if i % 2 == 0 { i32::MIN } else { i32::MAX })] { + let clamped = std::array::from_fn(|i| clamp(d[i])); + let (mut a, mut b) = ([0u8; 64], [0u8; 64]); + idct4_add(&d, &mut a, 8); + idct4_add(&clamped, &mut b, 8); + assert_eq!(a, b, "idct4 of extremes equals idct4 of the spec range"); + } + for d in [[i32::MIN; 64], [i32::MAX; 64]] { + let clamped = std::array::from_fn(|i| clamp(d[i])); + let (mut a, mut b) = ([0u8; 64], [0u8; 64]); + idct8_add(&d, &mut a, 8); + idct8_add(&clamped, &mut b, 8); + assert_eq!(a, b, "idct8 of extremes equals idct8 of the spec range"); + } + } }