shifathosn
/
filmcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #12 from storytold/pr/never-crash-h264 

h264: never crash (no unwrap/expect, deny the panicking lints)
This commit is contained in:
Brandon Thomas 2026-10-04 16:15:39 -04:00 committed by GitHub
parent 51e5adc627 0f678aa105
commit 4436d4d887
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 73 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/h264/src/cabac_mb.rs
View File

@ -527,7 +527,9 @@ impl SliceDecoder<'_> {
let pos = c.bit_pos().div_ceil(8);
let data = c.data();
ensure!(data.len() >= pos + 384, "truncated I_PCM macroblock");
let samples: [u8; 384] = data[pos..pos + 384].try_into().unwrap();
let Some(&samples) = data.get(pos..).and_then(|d| d.first_chunk::<384>()) else {
return invalid("truncated I_PCM macroblock");
};
c.set_bit_pos((pos + 384) * 8);
c.init_engine()?;
self.finish_pcm();

2
crates/h264/src/cavlc.rs
View File

@ -32,7 +32,7 @@ impl VlcTable {
}
let suffix = &code[lz + 1..];
assert!(suffix.len() <= 5, "suffix too long in {code}");
let sbits = u32::from_str_radix(if suffix.is_empty() { "0" } else { suffix }, 2).unwrap();
let sbits = suffix.bytes().fold(0u32, |a, b| (a << 1) | u32::from(b == b'1'));
let free = 5 - suffix.len();
for ext in 0..(1u32 << free) {
let s = (sbits << free) | ext;

13
crates/h264/src/deblock.rs
View File

@ -232,7 +232,10 @@ fn filter_vertical<const N: usize>(pix: &mut [u8], stride: usize, x: usize, y: u
let mut w = [0u64; N];
for (i, w) in w.iter_mut().enumerate() {
let o = (y + i) * stride + x - 4;
*w = u64::from_le_bytes(pix[o..o + 8].try_into().unwrap());
let Some(&b) = pix.get(o..).and_then(|p| p.first_chunk::<8>()) else {
return;
};
*w = u64::from_le_bytes(b);
}
let mut s = [[0i16; N]; 8];
for (k, row) in s.iter_mut().enumerate() {
@ -256,7 +259,9 @@ fn filter_horizontal<const N: usize>(pix: &mut [u8], stride: usize, x: usize, y:
let base = (y - 4) * stride + x;
let mut s = [[0i16; N]; 8];
for (k, row) in s.iter_mut().enumerate() {
let src: &[u8; N] = pix[base + k * stride..base + k * stride + N].try_into().unwrap();
let Some(src) = pix.get(base + k * stride..).and_then(|p| p.first_chunk::<N>()) else {
return;
};
for i in 0..N {
row[i] = src[i] as i16;
}
@ -264,7 +269,9 @@ fn filter_horizontal<const N: usize>(pix: &mut [u8], stride: usize, x: usize, y:
filter_lanes(&mut s, ep, chroma);
let (k0, k1) = if chroma { (3, 5) } else { (1, 7) };
for (k, row) in s.iter().enumerate().take(k1).skip(k0) {
let dst: &mut [u8; N] = (&mut pix[base + k * stride..base + k * stride + N]).try_into().unwrap();
let Some(dst) = pix.get_mut(base + k * stride..).and_then(|p| p.first_chunk_mut::<N>()) else {
return;
};
for i in 0..N {
dst[i] = row[i] as u8;
}

10
crates/h264/src/decoder.rs
View File

@ -267,7 +267,7 @@ impl Decoder {
if !o.frame.is_published(o.frame.mb_h() - 1) && self.out_queue.len() <= max_queue {
break;
}
let o = self.out_queue.pop_front().expect("front exists");
let Some(o) = self.out_queue.pop_front() else { break };
pics.push(make_picture(&o));
}
Ok(pics)
@ -353,7 +353,9 @@ impl Decoder {
found = Some((pps.clone(), sps.clone()));
Ok((&**pps, &**sps))
})?;
let (pps, sps) = found.expect("lookup succeeded");
let Some((pps, sps)) = found else {
return Err(Error::MissingParameterSet("PPS".into()));
};
sps.check_supported()?;
if pps.num_slice_groups > 1 {
return unsupported("slice groups (FMO)");
@ -379,7 +381,9 @@ impl Decoder {
self.start_picture(&sh, &sps, pts)?;
}
let ls = self.level_scale(&pps);
let pending = self.pending.as_mut().expect("picture started");
let Some(pending) = self.pending.as_mut() else {
return invalid("slice without a started picture");
};
let refs = self.dpb.build_ref_lists(&sh, pending.poc.frame(), sps.max_frame_num())?;
if !sh.slice_type.is_intra() {
ensure!(!refs[0].is_empty(), "no reference pictures available for inter slice");

6
crates/h264/src/inter.rs
View File

@ -54,9 +54,9 @@ fn clip1(v: i32) -> u8 {
#[inline(always)]
fn copy_n(dst: &mut [u8], src: &[u8], n: usize) {
fn fixed<const N: usize>(dst: &mut [u8], src: &[u8]) {
let s: &[u8; N] = src[..N].try_into().unwrap();
let d: &mut [u8; N] = (&mut dst[..N]).try_into().unwrap();
*d = *s;
if let (Some(d), Some(s)) = (dst.first_chunk_mut::<N>(), src.first_chunk::<N>()) {
*d = *s;
}
}
match n {
16 => fixed::<16>(dst, src),

1
crates/h264/src/lib.rs
View File

@ -17,6 +17,7 @@
// Index loops over fixed-size blocks read more clearly than iterator chains in codec code.
#![allow(clippy::needless_range_loop, clippy::too_many_arguments)]
#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
mod cabac;
mod cabac_mb;

8
crates/h264/src/picture.rs
View File

@ -277,9 +277,11 @@ impl Frame {
/// bytes is copied instead (cheaper than a variable-length copy; `out` must then have room for `N`).
#[inline(always)]
fn copy_line<const N: usize>(line: &[u8], x0: i32, out: &mut [u8], w: usize) {
if w <= N && x0 >= 0 && x0 as usize + N <= line.len() && out.len() >= N {
let src: &[u8; N] = line[x0 as usize..x0 as usize + N].try_into().unwrap();
let dst: &mut [u8; N] = (&mut out[..N]).try_into().unwrap();
if w <= N
&& x0 >= 0
&& let Some(src) = line.get(x0 as usize..).and_then(|l| l.first_chunk::<N>())
&& let Some(dst) = out.first_chunk_mut::<N>()
{
*dst = *src;
} else {
copy_clamped(line, x0, &mut out[..w]);

16
crates/h264/src/slicedec.rs
View File

@ -888,7 +888,9 @@ impl<'a> SliceDecoder<'a> {
let stride = self.pic.planes.width;
let px = self.mb_x * 16 + (raster & 3) * 4;
let py = self.mb_y * 16 + (raster >> 2) * 4;
let blk: &mut [i32; 16] = (&mut self.s.coef[raster * 16..raster * 16 + 16]).try_into().unwrap();
let Some(blk) = self.s.coef.get_mut(raster * 16..).and_then(|c| c.first_chunk_mut::<16>()) else {
return;
};
transform::idct4_add(blk, &mut self.pic.planes.y[py * stride + px..], stride);
blk.fill(0);
}
@ -900,7 +902,9 @@ impl<'a> SliceDecoder<'a> {
let stride = self.pic.planes.width;
let px = self.mb_x * 16 + (b8 & 1) * 8;
let py = self.mb_y * 16 + (b8 >> 1) * 8;
let blk: &mut [i32; 64] = (&mut self.s.coef[b8 * 64..b8 * 64 + 64]).try_into().unwrap();
let Some(blk) = self.s.coef.get_mut(b8 * 64..).and_then(|c| c.first_chunk_mut::<64>()) else {
return;
};
transform::idct8_add(blk, &mut self.pic.planes.y[py * stride + px..], stride);
blk.fill(0);
}
@ -917,7 +921,9 @@ impl<'a> SliceDecoder<'a> {
}
let px = self.mb_x * 8 + (b & 1) * 4;
let py = self.mb_y * 8 + (b >> 1) * 4;
let blk: &mut [i32; 16] = (&mut self.s.coef_c[c][b * 16..b * 16 + 16]).try_into().unwrap();
let Some(blk) = self.s.coef_c[c].get_mut(b * 16..).and_then(|c| c.first_chunk_mut::<16>()) else {
continue;
};
let plane = if c == 0 { &mut self.pic.planes.cb } else { &mut self.pic.planes.cr };
transform::idct4_add(blk, &mut plane[py * stride + px..], stride);
blk.fill(0);
@ -1216,7 +1222,9 @@ impl<'a> SliceDecoder<'a> {
let pos = r.byte_pos();
let data = r.data();
ensure!(data.len() >= pos + 384, "truncated I_PCM macroblock");
let samples: [u8; 384] = data[pos..pos + 384].try_into().unwrap();
let Some(&samples) = data.get(pos..).and_then(|d| d.first_chunk::<384>()) else {
return invalid("truncated I_PCM macroblock");
};
r.skip(384 * 8)?;
self.finish_pcm();
self.write_pcm(&samples);

44
crates/h264/src/transform.rs
View File

@ -32,13 +32,13 @@ impl LevelScale {
/// Scale one 4x4 AC coefficient (8.5.12.1, not the DC of Intra16x16 / chroma blocks).
#[inline(always)]
pub fn scale4(c: i32, ls: i32, qp: i32) -> i32 {
if qp >= 24 { (c * ls) << (qp / 6 - 4) } else { (c * ls + (1 << (3 - qp / 6))) >> (4 - qp / 6) }
if qp >= 24 { c.wrapping_mul(ls) << (qp / 6 - 4) } else { c.wrapping_mul(ls).wrapping_add(1 << (3 - qp / 6)) >> (4 - qp / 6) }
}
/// Scale one 8x8 coefficient (8.5.13.1).
#[inline(always)]
pub fn scale8(c: i32, ls: i32, qp: i32) -> i32 {
if qp >= 36 { (c * ls) << (qp / 6 - 6) } else { (c * ls + (1 << (5 - qp / 6))) >> (6 - qp / 6) }
if qp >= 36 { c.wrapping_mul(ls) << (qp / 6 - 6) } else { c.wrapping_mul(ls).wrapping_add(1 << (5 - qp / 6)) >> (6 - qp / 6) }
}
/// Intra16x16 luma DC: inverse Hadamard + scaling (8.5.10). `c` is raster order (row*4+col);
@ -65,20 +65,21 @@ pub fn luma_dc_dequant(c: &mut [i32; 16], qp: i32, ls00: i32) {
let d23 = t[8 + j] - t[12 + j];
let f = [s01 + s23, s01 - s23, d01 - d23, d01 + d23];
for i in 0..4 {
c[i * 4 + j] = if qp >= 36 { (f[i] * ls00) << (qp / 6 - 6) } else { (f[i] * ls00 + (1 << (5 - qp / 6))) >> (6 - qp / 6) };
c[i * 4 + j] =
if qp >= 36 { f[i].wrapping_mul(ls00) << (qp / 6 - 6) } else { f[i].wrapping_mul(ls00).wrapping_add(1 << (5 - qp / 6)) >> (6 - qp / 6) };
}
}
}
/// 4:2:0 chroma DC: 2x2 transform + scaling (8.5.11). `c` = [c00, c01, c10, c11].
pub fn chroma_dc_dequant_420(c: &mut [i32; 4], qp: i32, ls00: i32) {
let s0 = c[0] + c[1];
let d0 = c[0] - c[1];
let s1 = c[2] + c[3];
let d1 = c[2] - c[3];
let f = [s0 + s1, d0 + d1, s0 - s1, d0 - d1];
let s0 = c[0].wrapping_add(c[1]);
let d0 = c[0].wrapping_sub(c[1]);
let s1 = c[2].wrapping_add(c[3]);
let d1 = c[2].wrapping_sub(c[3]);
let f = [s0.wrapping_add(s1), d0.wrapping_add(d1), s0.wrapping_sub(s1), d0.wrapping_sub(d1)];
for k in 0..4 {
c[k] = ((f[k] * ls00) << (qp / 6)) >> 5;
c[k] = (f[k].wrapping_mul(ls00) << (qp / 6)) >> 5;
}
}
@ -113,7 +114,9 @@ pub fn idct4_add(d: &[i32; 16], dst: &mut [u8], stride: usize) {
o[3][j] = e - h;
}
for (i, row) in o.iter().enumerate() {
let line: &mut [u8; 4] = (&mut dst[i * stride..i * stride + 4]).try_into().unwrap();
let Some(line) = dst.get_mut(i * stride..).and_then(|d| d.first_chunk_mut::<4>()) else {
return;
};
for (p, &v) in line.iter_mut().zip(row) {
*p = clip_u8(*p as i32 + ((v + 32) >> 6));
}
@ -156,8 +159,8 @@ fn idct8_1d(d: [i32; 8]) -> [i32; 8] {
/// 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 mut t = [[0i32; 8]; 8];
for (i, row) in t.iter_mut().enumerate() {
*row = idct8_1d(d[i * 8..i * 8 + 8].try_into().unwrap());
for (row, src) in t.iter_mut().zip(d.as_chunks::<8>().0) {
*row = idct8_1d(*src);
}
let mut o = [[0i32; 8]; 8];
for j in 0..8 {
@ -167,7 +170,9 @@ pub fn idct8_add(d: &[i32; 64], dst: &mut [u8], stride: usize) {
}
}
for (i, row) in o.iter().enumerate() {
let line: &mut [u8; 8] = (&mut dst[i * stride..i * stride + 8]).try_into().unwrap();
let Some(line) = dst.get_mut(i * stride..).and_then(|d| d.first_chunk_mut::<8>()) else {
return;
};
for (p, &v) in line.iter_mut().zip(row) {
*p = clip_u8(*p as i32 + ((v + 32) >> 6));
}
@ -283,4 +288,17 @@ mod tests {
assert_eq!(scale8(1, 320, 36), 320);
assert_eq!(scale8(1, 320, 30), (320 + 1) >> 1);
}
/// Corrupt streams can carry huge coefficient levels: dequantisation wraps (as release
/// builds always did) instead of panicking with "attempt to multiply with overflow" in
/// debug builds (found by mutation fuzzing an H.264 transport stream).
#[test]
fn huge_levels_dequantise_without_overflow_panics() {
let mut c = [i32::MAX / 2, i32::MAX / 3, -i32::MAX / 2, 7];
chroma_dc_dequant_420(&mut c, 51, 16 * 25);
let _ = scale4(i32::MAX, 16 * 25, 51);
let _ = scale4(i32::MAX, 16 * 25, 0);
let _ = scale8(i32::MIN, 16 * 25, 51);
let _ = scale8(i32::MIN, 16 * 25, 0);
}
}