From 9a348da8bb98b2cc57770ed7ca397a8d12b638bb Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 27 Oct 2026 23:47:06 +0000 Subject: [PATCH] Refactor codecs for strict Clippy checks --- crates/codecs/src/codecs/tiff.rs | 2 +- crates/codecs/src/codecs/tiff/bands.rs | 2 +- crates/codecs/src/codecs/tiff/decode.rs | 211 +++++++++++++----------- crates/codecs/src/codecs/tiff/encode.rs | 7 +- crates/codecs/src/codecs/tiff/tags.rs | 2 +- crates/codecs/src/codecs/vp8.rs | 121 +++++++------- crates/codecs/src/codecs/vp8/predict.rs | 4 +- 7 files changed, 192 insertions(+), 157 deletions(-) diff --git a/crates/codecs/src/codecs/tiff.rs b/crates/codecs/src/codecs/tiff.rs index 1db3b0c44c..e3ccfbc952 100644 --- a/crates/codecs/src/codecs/tiff.rs +++ b/crates/codecs/src/codecs/tiff.rs @@ -27,7 +27,7 @@ //! (see [`writes_little_endian`]), which the layer data must match. use std::borrow::Cow; -use std::io::{self, Cursor, Read, Seek, SeekFrom}; +use std::io::{self, Read, Seek, SeekFrom}; use half::f16; use tiff::decoder::{ChunkType, Decoder}; diff --git a/crates/codecs/src/codecs/tiff/bands.rs b/crates/codecs/src/codecs/tiff/bands.rs index fe35cc98d2..8eaeadf8a6 100644 --- a/crates/codecs/src/codecs/tiff/bands.rs +++ b/crates/codecs/src/codecs/tiff/bands.rs @@ -1,5 +1,5 @@ use super::decode::{Layout, Patched, Src, is_eof}; -use super::*; +use super::{CodecError, Decoder, SampleType, err, f16, map_err}; /// The a* and b* samples (1 and 2 of every pixel) of a Lab image between the file's encoding and /// PhotoCraft's (8 bits: `a + 128`; 16 bits: `(a + 128) ยท 257`). In the file, CIELab holds them diff --git a/crates/codecs/src/codecs/tiff/decode.rs b/crates/codecs/src/codecs/tiff/decode.rs index 4b5d38cb59..0872739f3e 100644 --- a/crates/codecs/src/codecs/tiff/decode.rs +++ b/crates/codecs/src/codecs/tiff/decode.rs @@ -1,6 +1,9 @@ use super::bands::{convert_ab, decode_bands, extra_sample_indices, split_extra_samples, zeroed}; use super::tags::{F, TAG_IMAGE_SOURCE_DATA, TAG_PHOTOSHOP, TAG_XMP, TEXT_TAGS, photoshop_tags}; -use super::*; +use super::{ + ChannelLayout, ChunkType, CodecError, DecodeWarning, Decoder, ExifDirs, File, Ifd, Image, Limits, Metadata, Read, SampleType, Seek, SeekFrom, Tag, + camera_raw, err, ifd, io, map_err, tiff_exif, +}; /// Decodes the page Photoshop would open: the first full-resolution directory. pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result { @@ -235,6 +238,112 @@ pub(super) fn is_eof(e: &tiff::TiffError) -> bool { matches!(e, tiff::TiffError::IoError(io) if io.kind() == io::ErrorKind::UnexpectedEof) } +/// The sample type of a (SampleFormat, BitsPerSample) pair. +fn sample_src(format: u64, bits: u64) -> Result { + Ok(match (format, bits) { + (1, 1 | 2 | 4) => Src::Bits(bits as u8), + (1, 8) => Src::U8, + (1, 16) => Src::U16, + (1, 32) => Src::U32, + (1, 64) => Src::U64, + (3, 16) => Src::F16, + (3, 32) => Src::F32, + (3, 64) => Src::F64, + (2, _) => return Err(CodecError::unsupported(F, "signed integer samples")), + _ => return Err(CodecError::unsupported(F, format!("{bits}-bit samples (sample format {format})"))), + }) +} + +/// The ColorMap of a palette image as RGB entries. +fn read_palette(f: &File<'_>, dir: &Ifd, spp: u64, bits: u64, src: Src) -> Result, CodecError> { + if spp != 1 || !matches!(src, Src::Bits(_) | Src::U8 | Src::U16) { + return Err(CodecError::unsupported(F, format!("palette image with {spp} samples of {bits} bits"))); + } + let n = 1u64 << bits; + let map = dir.get(ifd::TAG_COLOR_MAP).filter(|e| e.ty == 3 && e.count == 3 * n).ok_or_else(|| err("palette image without a valid ColorMap"))?; + let at = |i: u64| f.uint(map, i).and_then(|v| u16::try_from(v).ok()); + let table: Option> = (0..n).map(|i| Some([at(i)?, at(n + i)?, at(2 * n + i)?])).collect(); + table.ok_or_else(|| err("palette ColorMap cut off")) +} + +/// The layout of a multiband read that was asked for every sample. +fn multiband_layout(photometric: u64, has_alpha: bool, gray_alpha: bool) -> ChannelLayout { + match (photometric, has_alpha) { + (2 | 8 | 9, true) => ChannelLayout::Rgba, + (2 | 8 | 9, false) => ChannelLayout::Rgb, + (5, true) => ChannelLayout::CmykA, + (5, false) => ChannelLayout::Cmyk, + _ if gray_alpha => ChannelLayout::GrayA, + _ => ChannelLayout::Gray, + } +} + +/// The chunk tables must match the geometry. Checked in 64 bits, before the `tiff` crate does +/// the same arithmetic in narrower types. +fn check_chunk_tables(f: &File<'_>, dir: &Ifd, (w, h): (u32, u32), spp: u64, planar: bool, tiled: bool) -> Result<(u64, u64), CodecError> { + let need = |tag: u16, name: &str| f.tag_uint(dir, tag).ok_or_else(|| err(format!("missing or unreadable {name}"))); + let (cw, ch) = if tiled { + let tw = need(ifd::TAG_TILE_WIDTH, "TileWidth")?; + let tl = need(ifd::TAG_TILE_LENGTH, "TileLength")?; + (tw, tl) + } else { + (u64::from(w), f.tag_uint(dir, ifd::TAG_ROWS_PER_STRIP).unwrap_or(u64::from(h)).min(u64::from(h))) + }; + if cw == 0 || ch == 0 { + return Err(err("zero tile or strip size")); + } + // The chunk tables must match the geometry. Checked here, in 64 bits, before the `tiff` + // crate does the same arithmetic in narrower types. + let planes_in_file = if planar { spp } else { 1 }; + let expected = u64::from(w) + .div_ceil(cw) + .checked_mul(u64::from(h).div_ceil(ch)) + .and_then(|n| n.checked_mul(planes_in_file)) + .filter(|&n| n <= u64::from(u32::MAX)) + .ok_or_else(|| err("too many strips or tiles"))?; + let (offsets_tag, counts_tag) = if tiled { (324, 325) } else { (273, 279) }; + let counts = (dir.get(offsets_tag).map(|e| e.count), dir.get(counts_tag).map(|e| e.count)); + if counts != (Some(expected), Some(expected)) { + return Err(err(format!("{expected} strips or tiles expected, the tables list {counts:?}"))); + } + Ok((cw, ch)) +} + +/// Byte patches the `tiff` crate reads through: the directory is seen as the last of its chain, +/// and PhotometricInterpretation is handed over as black-is-zero (or RGB for Lab) when asked to. +fn header_patches( + f: &File<'_>, + dir: &Ifd, + photometric_entry: Option, + patch_photometric: bool, + lab_only: bool, +) -> Result)>, CodecError> { + let mut patches: Vec<(u64, Vec)> = Vec::new(); + let encode_u = |v: u64, len: usize| -> Vec { + let b = if f.le { v.to_le_bytes() } else { v.to_be_bytes() }; + if f.le { b.get(..len).map(<[u8]>::to_vec).unwrap_or_default() } else { b.get(8 - len..).map(<[u8]>::to_vec).unwrap_or_default() } + }; + if f.big { + patches.push((8, encode_u(dir.at, 8))); + } else { + let at = u32::try_from(dir.at).map_err(|_| err("directory offset out of range"))?; + patches.push((4, encode_u(u64::from(at), 4))); + } + // The directory is seen as the last of its chain: the chain was walked already, and a + // cycle through it must not fail this page. + patches.push((dir.next_at, vec![0; if f.big { 8 } else { 4 }])); + if patch_photometric { + let e = photometric_entry.ok_or_else(|| err("missing PhotometricInterpretation"))?; + let len = match e.ty { + 3 if e.count == 1 => 2, + 4 if e.count == 1 => 4, + _ => return Err(err("malformed PhotometricInterpretation")), + }; + patches.push((e.value_at, encode_u(if lab_only { 2 } else { 1 }, len))); + } + Ok(patches) +} + /// Decodes the pixels of one directory. fn decode_ifd(f: &File<'_>, dir: &Ifd, limits: &Limits) -> Result { // The values the `tiff` crate reads must exist: a count past the end of the file is @@ -263,59 +372,13 @@ fn decode_ifd(f: &File<'_>, dir: &Ifd, limits: &Limits) -> Result Src::Bits(bits as u8), - (1, 8) => Src::U8, - (1, 16) => Src::U16, - (1, 32) => Src::U32, - (1, 64) => Src::U64, - (3, 16) => Src::F16, - (3, 32) => Src::F32, - (3, 64) => Src::F64, - (2, _) => return Err(CodecError::unsupported(F, "signed integer samples")), - _ => return Err(CodecError::unsupported(F, format!("{bits}-bit samples (sample format {format})"))), - }; - + let (cw, ch) = check_chunk_tables(f, dir, (w, h), spp, planar, tiled)?; + let src = sample_src(format, bits)?; // WhiteIsZero and palette images are handed to the `tiff` crate as BlackIsZero: it then // returns the stored samples, and the interpretation is applied here (for every depth and // with alpha, which the crate's own inversion does not cover). let invert = photometric == 0; - let palette = if photometric == 3 { - if spp != 1 || !matches!(src, Src::Bits(_) | Src::U8 | Src::U16) { - return Err(CodecError::unsupported(F, format!("palette image with {spp} samples of {bits} bits"))); - } - let n = 1u64 << bits; - let map = dir.get(ifd::TAG_COLOR_MAP).filter(|e| e.ty == 3 && e.count == 3 * n).ok_or_else(|| err("palette image without a valid ColorMap"))?; - let at = |i: u64| f.uint(map, i).and_then(|v| u16::try_from(v).ok()); - let table: Option> = (0..n).map(|i| Some([at(i)?, at(n + i)?, at(2 * n + i)?])).collect(); - Some(table.ok_or_else(|| err("palette ColorMap cut off"))?) - } else { - None - }; + let palette = if photometric == 3 { Some(read_palette(f, dir, spp, bits, src)?) } else { None }; // CIE L*a*b*: CIELab (8; a* and b* signed) and ICCLab (9; a* and b* offset) are handed to the // `tiff` crate as RGB, then labelled Lab with a* and b* in PhotoCraft's encoding // (`convert_ab`). @@ -337,29 +400,7 @@ fn decode_ifd(f: &File<'_>, dir: &Ifd, limits: &Limits) -> Result color.saturating_add(usize::from(alpha_at.is_some())); let all_samples = palette.is_none() && matches!(photometric, 2 | 5 | 8 | 9) && (alpha_at.is_some_and(|a| a > 0) || (!planar && unused)); - let mut patches: Vec<(u64, Vec)> = Vec::new(); - let encode_u = |v: u64, len: usize| -> Vec { - let b = if f.le { v.to_le_bytes() } else { v.to_be_bytes() }; - if f.le { b.get(..len).map(<[u8]>::to_vec).unwrap_or_default() } else { b.get(8 - len..).map(<[u8]>::to_vec).unwrap_or_default() } - }; - if f.big { - patches.push((8, encode_u(dir.at, 8))); - } else { - let at = u32::try_from(dir.at).map_err(|_| err("directory offset out of range"))?; - patches.push((4, encode_u(u64::from(at), 4))); - } - // The directory is seen as the last of its chain: the chain was walked already, and a - // cycle through it must not fail this page. - patches.push((dir.next_at, vec![0; if f.big { 8 } else { 4 }])); - if invert || palette.is_some() || all_samples || lab { - let e = photometric_entry.ok_or_else(|| err("missing PhotometricInterpretation"))?; - let len = match e.ty { - 3 if e.count == 1 => 2, - 4 if e.count == 1 => 4, - _ => return Err(err("malformed PhotometricInterpretation")), - }; - patches.push((e.value_at, encode_u(if lab && !all_samples { 2 } else { 1 }, len))); - } + let patches = header_patches(f, dir, photometric_entry, invert || palette.is_some() || all_samples || lab, lab && !all_samples)?; let tl = { let mut l = tiff::decoder::Limits::default(); @@ -380,29 +421,7 @@ fn decode_ifd(f: &File<'_>, dir: &Ifd, limits: &Limits) -> Result (ChannelLayout::Gray, 1), tiff::ColorType::Multiband { num_samples, .. } if all_samples => { // Lab reads as RGB here and is relabelled below. - let layout = match photometric { - 2 | 8 | 9 => { - if alpha_at.is_some() { - ChannelLayout::Rgba - } else { - ChannelLayout::Rgb - } - } - 5 => { - if alpha_at.is_some() { - ChannelLayout::CmykA - } else { - ChannelLayout::Cmyk - } - } - _ => { - if gray_alpha { - ChannelLayout::GrayA - } else { - ChannelLayout::Gray - } - } - }; + let layout = multiband_layout(photometric, alpha_at.is_some(), gray_alpha); (layout, usize::from(num_samples)) } tiff::ColorType::Multiband { num_samples, .. } if photometric <= 1 => { diff --git a/crates/codecs/src/codecs/tiff/encode.rs b/crates/codecs/src/codecs/tiff/encode.rs index d71c32efde..a0b7539b86 100644 --- a/crates/codecs/src/codecs/tiff/encode.rs +++ b/crates/codecs/src/codecs/tiff/encode.rs @@ -1,6 +1,11 @@ use super::bands::convert_ab; use super::tags::{F, TAG_IMAGE_SOURCE_DATA, TAG_PHOTOSHOP, TAG_XMP, TEXT_TAGS}; -use super::*; +use super::{ + ChannelLayout, CmykA16, CmykA32F, CodecError, Cow, EncodeOptions, ExifDirs, GrayA8, GrayA16, GrayA32F, Image, Lab8, Lab16, LabA8, LabA16, + PhotometricInterpretation, Plan, Rational, ResolutionUnit, SampleFormat, SampleType, Tag, TiffColorType, TiffCompression, TiffEncoder, TiffKind, + TiffKindBig, TiffKindStandard, TiffValue, Type, colortype, io, tiff_exif, +}; +use std::io::Cursor; // --------------------------------------------------------------------------- // Encoding diff --git a/crates/codecs/src/codecs/tiff/tags.rs b/crates/codecs/src/codecs/tiff/tags.rs index df110f777a..a41b154137 100644 --- a/crates/codecs/src/codecs/tiff/tags.rs +++ b/crates/codecs/src/codecs/tiff/tags.rs @@ -1,4 +1,4 @@ -use super::*; +use super::{Format, Tag}; pub(super) const F: Format = Format::Tiff; diff --git a/crates/codecs/src/codecs/vp8.rs b/crates/codecs/src/codecs/vp8.rs index ba75bdd3e9..a24fb727cc 100644 --- a/crates/codecs/src/codecs/vp8.rs +++ b/crates/codecs/src/codecs/vp8.rs @@ -19,7 +19,11 @@ mod transform; use bool_encoder::BoolEncoder; use predict::{Plane, SubEdges, predict_block, predict_sub}; -use tables::*; +use tables::{ + AC_QLOOKUP, B_DC_PRED, B_HE_PRED, B_PRED, B_TM_PRED, B_VE_PRED, BMODE_TREE, CAT_BASE, COEFF_BANDS, COEFF_TREE, COEFF_UPDATE_PROBS, DC_PRED, DC_QLOOKUP, + DCT_0, DCT_1, DCT_2, DCT_3, DCT_4, DCT_CAT1, DCT_CAT2, DCT_CAT3, DCT_CAT4, DCT_CAT5, DCT_CAT6, DCT_EOB, DEFAULT_COEFF_PROBS, H_PRED, KF_BMODE_PROBS, + KF_UV_MODE_PROB, KF_YMODE_PROB, KF_YMODE_TREE, PCAT, TM_PRED, UV_MODE_TREE, V_PRED, ZIGZAG, +}; use transform::{fdct, fwht, idct, iwht}; /// The largest dimension a VP8 frame header can hold (14 bits). @@ -353,6 +357,65 @@ impl<'a> Encoder<'a> { .sum() } + /// Chroma of one macroblock: one mode for both planes; returns it with the U and V blocks. + fn encode_chroma(&mut self, mbx: usize, mby: usize) -> (u8, Vec, Vec) { + let (cx0, cy0) = (mbx * 8, mby * 8); + let mut src_u = [0i32; 64]; + let mut src_v = [0i32; 64]; + for r in 0..8 { + for c in 0..8 { + src_u[r * 8 + c] = self.src_c(&self.src.u, cx0 + c, cy0 + r); + src_v[r * 8 + c] = self.src_c(&self.src.v, cx0 + c, cy0 + r); + } + } + let mut uv_mode = DC_PRED; + let mut uv_best = u64::MAX; + let mut pred_u = [0u8; 64]; + let mut pred_v = [0u8; 64]; + let mut pu = [0u8; 64]; + let mut pv = [0u8; 64]; + for mode in [DC_PRED, V_PRED, H_PRED, TM_PRED] { + predict_block(&self.rec_u, cx0, cy0, 8, mode, &mut pu); + predict_block(&self.rec_v, cx0, cy0, 8, mode, &mut pv); + let e = Self::sse(&pu, &src_u) + Self::sse(&pv, &src_v); + if e < uv_best { + uv_best = e; + uv_mode = mode; + pred_u = pu; + pred_v = pv; + } + } + let mut u_blocks = Vec::with_capacity(4); + let mut v_blocks = Vec::with_capacity(4); + for (plane_src, pred, blocks, is_u) in [(&src_u, &pred_u, &mut u_blocks, true), (&src_v, &pred_v, &mut v_blocks, false)] { + for b in 0..4 { + let mut residual = [0i32; 16]; + for r in 0..4 { + for c in 0..4 { + let i = ((b >> 1) * 4 + r) * 8 + (b & 1) * 4 + c; + residual[r * 4 + c] = plane_src[i] - i32::from(pred[i]); + } + } + let (quantized, dequantized) = Self::code_block(&residual, self.q.uv_dc, self.q.uv_ac, true); + let recon = idct(&dequantized); + let (bx, by) = (cx0 + (b & 1) * 4, cy0 + (b >> 1) * 4); + for r in 0..4 { + for c in 0..4 { + let i = ((b >> 1) * 4 + r) * 8 + (b & 1) * 4 + c; + let value = (i32::from(pred[i]) + recon[r * 4 + c]).clamp(0, 255) as u8; + if is_u { + self.rec_u.set(bx + c, by + r, value); + } else { + self.rec_v.set(bx + c, by + r, value); + } + } + } + blocks.push(Block::from_raster(&quantized, 0)); + } + } + (uv_mode, u_blocks, v_blocks) + } + // Sub-block indices address several arrays by geometry; ranges read better than iterators here. #[allow(clippy::needless_range_loop)] fn encode_macroblock(&mut self, mbx: usize, mby: usize) { @@ -476,61 +539,7 @@ impl<'a> Encoder<'a> { (best_mode, Some(Block::from_raster(&y2_q, 0)), blocks) }; - // Chroma: one mode for both planes. - let (cx0, cy0) = (mbx * 8, mby * 8); - let mut src_u = [0i32; 64]; - let mut src_v = [0i32; 64]; - for r in 0..8 { - for c in 0..8 { - src_u[r * 8 + c] = self.src_c(&self.src.u, cx0 + c, cy0 + r); - src_v[r * 8 + c] = self.src_c(&self.src.v, cx0 + c, cy0 + r); - } - } - let mut uv_mode = DC_PRED; - let mut uv_best = u64::MAX; - let mut pred_u = [0u8; 64]; - let mut pred_v = [0u8; 64]; - let mut pu = [0u8; 64]; - let mut pv = [0u8; 64]; - for mode in [DC_PRED, V_PRED, H_PRED, TM_PRED] { - predict_block(&self.rec_u, cx0, cy0, 8, mode, &mut pu); - predict_block(&self.rec_v, cx0, cy0, 8, mode, &mut pv); - let e = Self::sse(&pu, &src_u) + Self::sse(&pv, &src_v); - if e < uv_best { - uv_best = e; - uv_mode = mode; - pred_u = pu; - pred_v = pv; - } - } - let mut u_blocks = Vec::with_capacity(4); - let mut v_blocks = Vec::with_capacity(4); - for (plane_src, pred, blocks, is_u) in [(&src_u, &pred_u, &mut u_blocks, true), (&src_v, &pred_v, &mut v_blocks, false)] { - for b in 0..4 { - let mut residual = [0i32; 16]; - for r in 0..4 { - for c in 0..4 { - let i = ((b >> 1) * 4 + r) * 8 + (b & 1) * 4 + c; - residual[r * 4 + c] = plane_src[i] - i32::from(pred[i]); - } - } - let (quantized, dequantized) = Self::code_block(&residual, self.q.uv_dc, self.q.uv_ac, true); - let recon = idct(&dequantized); - let (bx, by) = (cx0 + (b & 1) * 4, cy0 + (b >> 1) * 4); - for r in 0..4 { - for c in 0..4 { - let i = ((b >> 1) * 4 + r) * 8 + (b & 1) * 4 + c; - let value = (i32::from(pred[i]) + recon[r * 4 + c]).clamp(0, 255) as u8; - if is_u { - self.rec_u.set(bx + c, by + r, value); - } else { - self.rec_v.set(bx + c, by + r, value); - } - } - } - blocks.push(Block::from_raster(&quantized, 0)); - } - } + let (uv_mode, u_blocks, v_blocks) = self.encode_chroma(mbx, mby); let skip = y2.as_ref().is_none_or(|b| !b.has_coeffs()) && y_blocks.iter().all(|b| !b.has_coeffs()) diff --git a/crates/codecs/src/codecs/vp8/predict.rs b/crates/codecs/src/codecs/vp8/predict.rs index 47319fdd7a..7ddea04238 100644 --- a/crates/codecs/src/codecs/vp8/predict.rs +++ b/crates/codecs/src/codecs/vp8/predict.rs @@ -2,7 +2,9 @@ //! sub-block modes. Predictions read the frame's *unfiltered* reconstruction, which is what the //! decoder predicts from too (the loop filter runs on the finished frame). -use super::tables::*; +#[cfg(test)] +use super::tables::{B_DC_PRED, DC_PRED}; +use super::tables::{B_HD_PRED, B_HE_PRED, B_HU_PRED, B_LD_PRED, B_RD_PRED, B_TM_PRED, B_VE_PRED, B_VL_PRED, B_VR_PRED, H_PRED, TM_PRED, V_PRED}; /// A plane of the reconstruction with the virtual border VP8 defines: the row above the frame /// reads as 127, the column left of it as 129, and the pixel above-left of the frame as 127.