From 2f68c79db05d6b4a6c64508b8956bfa92cec9ae0 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 Jan 2025 19:06:19 +1300 Subject: [PATCH 1/6] Add read_be_u16 and tidy parse_next_chunk --- src/headers.rs | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/headers.rs b/src/headers.rs index 4cead1d9..3478b1a3 100644 --- a/src/headers.rs +++ b/src/headers.rs @@ -134,7 +134,7 @@ fn parse_jumbf_box(data: &[u8]) -> Option<(&[u8], &[u8])> { return None; } let (len, rest) = data.split_at(4); - let len = u32::from_be_bytes(len.try_into().unwrap()) as usize; + let len = read_be_u32(len) as usize; if len < 8 || len > data.len() { return None; } @@ -153,32 +153,25 @@ pub fn parse_next_chunk<'a>( .get(*byte_offset..*byte_offset + 4) .ok_or(PngError::TruncatedData)?, ); + if byte_data.len() < *byte_offset + 12 + length as usize { + return Err(PngError::TruncatedData); + } *byte_offset += 4; let chunk_start = *byte_offset; - let chunk_name = byte_data - .get(chunk_start..chunk_start + 4) - .ok_or(PngError::TruncatedData)?; + let chunk_name = &byte_data[chunk_start..chunk_start + 4]; if chunk_name == b"IEND" { // End of data return Ok(None); } *byte_offset += 4; - let data = byte_data - .get(*byte_offset..*byte_offset + length as usize) - .ok_or(PngError::TruncatedData)?; + let data = &byte_data[*byte_offset..*byte_offset + length as usize]; *byte_offset += length as usize; - let crc = read_be_u32( - byte_data - .get(*byte_offset..*byte_offset + 4) - .ok_or(PngError::TruncatedData)?, - ); + let crc = read_be_u32(&byte_data[*byte_offset..*byte_offset + 4]); *byte_offset += 4; - let chunk_bytes = byte_data - .get(chunk_start..chunk_start + 4 + length as usize) - .ok_or(PngError::TruncatedData)?; + let chunk_bytes = &byte_data[chunk_start..chunk_start + 4 + length as usize]; if !fix_errors && crc32(chunk_bytes) != crc { return Err(PngError::new(&format!( "CRC Mismatch in {} chunk; May be recoverable by using --fix", @@ -202,13 +195,13 @@ pub fn parse_ihdr_chunk( 0 => ColorType::Grayscale { transparent_shade: trns_data .filter(|t| t.len() >= 2) - .map(|t| u16::from_be_bytes([t[0], t[1]])), + .map(|t| read_be_u16(&t[0..2])), }, 2 => ColorType::RGB { transparent_color: trns_data.filter(|t| t.len() >= 6).map(|t| RGB16 { - r: u16::from_be_bytes([t[0], t[1]]), - g: u16::from_be_bytes([t[2], t[3]]), - b: u16::from_be_bytes([t[4], t[5]]), + r: read_be_u16(&t[0..2]), + g: read_be_u16(&t[2..4]), + b: read_be_u16(&t[4..6]), }), }, 3 => ColorType::Indexed { @@ -245,7 +238,12 @@ fn palette_to_rgba( } #[inline] -fn read_be_u32(bytes: &[u8]) -> u32 { +pub fn read_be_u16(bytes: &[u8]) -> u16 { + u16::from_be_bytes(bytes.try_into().unwrap()) +} + +#[inline] +pub fn read_be_u32(bytes: &[u8]) -> u32 { u32::from_be_bytes(bytes.try_into().unwrap()) } From c2717ad92916db281e97ee7c8575e974c4ac1ab9 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 Jan 2025 19:44:28 +1300 Subject: [PATCH 2/6] Parse fcTL/fdAT into frames --- src/apng.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/error.rs | 2 ++ src/lib.rs | 3 ++ src/png/mod.rs | 71 +++++++++++++++++++++++++++++++++++------------ 4 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 src/apng.rs diff --git a/src/apng.rs b/src/apng.rs new file mode 100644 index 00000000..8b99d0f1 --- /dev/null +++ b/src/apng.rs @@ -0,0 +1,75 @@ +use std::io::Write; + +use crate::{ + error::PngError, + headers::{read_be_u16, read_be_u32}, + PngResult, +}; + +#[derive(Debug, Clone)] +/// Animated PNG frame +pub struct Frame { + /// Width of the frame + pub width: u32, + /// Height of the frame + pub height: u32, + /// X offset of the frame + pub x_offset: u32, + /// Y offset of the frame + pub y_offset: u32, + /// Frame delay numerator + pub delay_num: u16, + /// Frame delay denominator + pub delay_den: u16, + /// Frame disposal operation + pub dispose_op: u8, + /// Frame blend operation + pub blend_op: u8, + /// Frame data, from fdAT chunks + pub data: Vec, +} + +impl Frame { + /// Construct a new Frame from the data in a fcTL chunk + pub fn from_fctl_data(byte_data: &[u8]) -> PngResult { + if byte_data.len() < 26 { + return Err(PngError::TruncatedData); + } + Ok(Frame { + width: read_be_u32(&byte_data[4..8]), + height: read_be_u32(&byte_data[8..12]), + x_offset: read_be_u32(&byte_data[12..16]), + y_offset: read_be_u32(&byte_data[16..20]), + delay_num: read_be_u16(&byte_data[20..22]), + delay_den: read_be_u16(&byte_data[22..24]), + dispose_op: byte_data[24], + blend_op: byte_data[25], + data: vec![], + }) + } + + /// Construct the data for a fcTL chunk using the given sequence number + #[must_use] + pub fn fctl_data(&self, sequence_number: u32) -> Vec { + let mut byte_data = Vec::with_capacity(26); + byte_data.write_all(&sequence_number.to_be_bytes()).unwrap(); + byte_data.write_all(&self.width.to_be_bytes()).unwrap(); + byte_data.write_all(&self.height.to_be_bytes()).unwrap(); + byte_data.write_all(&self.x_offset.to_be_bytes()).unwrap(); + byte_data.write_all(&self.y_offset.to_be_bytes()).unwrap(); + byte_data.write_all(&self.delay_num.to_be_bytes()).unwrap(); + byte_data.write_all(&self.delay_den.to_be_bytes()).unwrap(); + byte_data.push(self.dispose_op); + byte_data.push(self.blend_op); + byte_data + } + + /// Construct the data for a fdAT chunk using the given sequence number + #[must_use] + pub fn fdat_data(&self, sequence_number: u32) -> Vec { + let mut byte_data = Vec::with_capacity(4 + self.data.len()); + byte_data.write_all(&sequence_number.to_be_bytes()).unwrap(); + byte_data.write_all(&self.data).unwrap(); + byte_data + } +} diff --git a/src/error.rs b/src/error.rs index b2079d1b..2c7d88a9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,6 +9,7 @@ pub enum PngError { TimedOut, NotPNG, APNGNotSupported, + APNGOutOfOrder, InvalidData, TruncatedData, ChunkMissing(&'static str), @@ -33,6 +34,7 @@ impl fmt::Display for PngError { f.write_str("Missing data in the file; the file is truncated") } PngError::APNGNotSupported => f.write_str("APNG files are not (yet) supported"), + PngError::APNGOutOfOrder => f.write_str("APNG chunks are out of order"), PngError::ChunkMissing(s) => write!(f, "Chunk {s} missing or empty"), PngError::InvalidDepthForType(d, ref c) => { write!(f, "Invalid bit depth {d} for color type {c}") diff --git a/src/lib.rs b/src/lib.rs index 90944618..1efc8251 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,7 @@ pub use crate::{ options::{InFile, Options, OutFile}, }; +mod apng; mod atomicmin; mod colors; mod deflate; @@ -530,6 +531,7 @@ fn optimize_raw( raw: png, idat_data, aux_chunks: Vec::new(), + frames: Vec::new(), }; if image.estimated_output_size() < max_size.unwrap_or(usize::MAX) { debug!("Found better combination:"); @@ -549,6 +551,7 @@ fn optimize_raw( raw: result.image, idat_data: result.idat_data, aux_chunks: Vec::new(), + frames: Vec::new(), }; if image.estimated_output_size() < max_size.unwrap_or(usize::MAX) { debug!("Found better combination:"); diff --git a/src/png/mod.rs b/src/png/mod.rs index 9d98c63a..9f856f94 100644 --- a/src/png/mod.rs +++ b/src/png/mod.rs @@ -12,6 +12,7 @@ use rgb::ComponentSlice; use rustc_hash::FxHashMap; use crate::{ + apng::*, colors::{BitDepth, ColorType}, deflate, error::PngError, @@ -47,6 +48,8 @@ pub struct PngData { pub idat_data: Vec, /// All non-critical chunks from the PNG are stored here pub aux_chunks: Vec, + /// APNG frames + pub frames: Vec, } impl PngData { @@ -97,6 +100,8 @@ impl PngData { let mut idat_data: Vec = Vec::new(); let mut key_chunks: FxHashMap<[u8; 4], Vec> = FxHashMap::default(); let mut aux_chunks: Vec = Vec::new(); + let mut frames: Vec = Vec::new(); + let mut sequence_number = 0; while let Some(chunk) = parse_next_chunk(byte_data, &mut byte_offset, opts.fix_errors)? { match &chunk.name { b"IDAT" => { @@ -112,27 +117,46 @@ impl PngData { b"IHDR" | b"PLTE" | b"tRNS" => { key_chunks.insert(chunk.name, chunk.data.to_owned()); } - _ => { - if opts.strip.keep(&chunk.name) { - if chunk.is_c2pa() { - // StripChunks::None is the default value, so to keep optimizing by default, - // interpret it as stripping the C2PA metadata. - // The C2PA metadata is invalidated if the file changes, so it shouldn't be kept. - if opts.strip == StripChunks::None { - continue; - } - return Err(PngError::C2PAMetadataPreventsChanges); + _ if opts.strip.keep(&chunk.name) => { + if chunk.is_c2pa() { + // StripChunks::None is the default value, so to keep optimizing by default, + // interpret it as stripping the C2PA metadata. + // The C2PA metadata is invalidated if the file changes, so it shouldn't be kept. + if opts.strip == StripChunks::None { + continue; + } + return Err(PngError::C2PAMetadataPreventsChanges); + } + if chunk.name == *b"fcTL" || chunk.name == *b"fdAT" { + // Validate the sequence number + if read_be_u32(&chunk.data[0..4]) != sequence_number { + return Err(PngError::APNGOutOfOrder); + } + sequence_number += 1; + if chunk.name == *b"fcTL" && !idat_data.is_empty() { + // Only create a Frame if it's after the IDAT (else store it as an aux chunk) + frames.push(Frame::from_fctl_data(chunk.data)?); + continue; + } else if chunk.name == *b"fdAT" { + // Append the data to the last frame + frames + .last_mut() + .ok_or(PngError::APNGOutOfOrder)? + .data + .extend_from_slice(&chunk.data[4..]); + continue; } - aux_chunks.push(Chunk { - name: chunk.name, - data: chunk.data.to_owned(), - }); - } else if chunk.name == *b"acTL" { - warn!( - "Stripping animation data from APNG - image will become standard PNG" - ); } + // Regular ancillary chunk + aux_chunks.push(Chunk { + name: chunk.name, + data: chunk.data.to_owned(), + }); + } + b"acTL" => { + warn!("Stripping animation data from APNG - image will become standard PNG") } + _ => (), } } @@ -166,6 +190,7 @@ impl PngData { idat_data, raw: Arc::new(raw), aux_chunks, + frames, }) } @@ -235,14 +260,24 @@ impl PngData { _ => {} } // Special ancillary chunks that need to come after PLTE but before IDAT + let mut sequence_number = 0; for chunk in aux_pre .iter() .filter(|c| matches!(&c.name, b"bKGD" | b"hIST" | b"tRNS" | b"fcTL")) { write_png_block(&chunk.name, &chunk.data, &mut output); + if &chunk.name == b"fcTL" { + sequence_number += 1; + } } // IDAT data write_png_block(b"IDAT", &self.idat_data, &mut output); + // APNG frames + for frame in self.frames.iter() { + write_png_block(b"fcTL", &frame.fctl_data(sequence_number), &mut output); + write_png_block(b"fdAT", &frame.fdat_data(sequence_number + 1), &mut output); + sequence_number += 2; + } // Ancillary chunks that come after IDAT for aux_post in aux_split { for chunk in aux_post { From ab0dfaf2d2f5eab6ad410f51805f0146ecfede6f Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 Jan 2025 20:00:24 +1300 Subject: [PATCH 3/6] Recompress frames --- src/lib.rs | 69 ++++++++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1efc8251..aa43bf2d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -167,7 +167,7 @@ impl RawImage { .filter(|c| opts.strip.keep(&c.name)) .cloned() .collect(); - postprocess_chunks(&mut png, opts, deadline, &self.png.ihdr); + postprocess_chunks(&mut png, opts, &self.png.ihdr); Ok(png.output()) } @@ -367,7 +367,8 @@ fn optimize_png( png.idat_data = new_png.idat_data; } - postprocess_chunks(png, &opts, deadline, &raw.ihdr); + postprocess_chunks(png, &opts, &raw.ihdr); + recompress_frames(png, &opts, deadline); let output = png.output(); @@ -659,12 +660,7 @@ fn report_format(prefix: &str, png: &PngImage) { } /// Perform cleanup of certain chunks from the `PngData` object, after optimization has been completed -fn postprocess_chunks( - png: &mut PngData, - opts: &Options, - deadline: Arc, - orig_ihdr: &IhdrData, -) { +fn postprocess_chunks(png: &mut PngData, opts: &Options, orig_ihdr: &IhdrData) { if let Some(iccp_idx) = png.aux_chunks.iter().position(|c| &c.name == b"iCCP") { // See if we can replace an iCCP chunk with an sRGB chunk let may_replace_iccp = opts.strip != StripChunks::None && opts.strip.keep(b"sRGB"); @@ -719,38 +715,35 @@ fn postprocess_chunks( !invalid }); } +} - // Find fdAT chunks and attempt to recompress them - // Note if there are multiple fdATs per frame then decompression will fail and nothing will change - let mut fdat: Vec<_> = png - .aux_chunks - .iter_mut() - .filter(|c| &c.name == b"fdAT") - .collect(); - if opts.idat_recoding && !fdat.is_empty() { - let buffer_size = orig_ihdr.raw_data_size(); - fdat.par_iter_mut() - .with_max_len(1) - .enumerate() - .for_each(|(i, c)| { - if deadline.passed() || c.data.len() <= 4 { - return; - } - if let Ok(mut data) = deflate::inflate(&c.data[4..], buffer_size).and_then(|data| { - let max_size = AtomicMin::new(Some(c.data.len() - 5)); - opts.deflate.deflate(&data, &max_size) - }) { - debug!( - "Recompressed fdAT #{:<2}: {} ({} bytes decrease)", - i, - c.data.len(), - c.data.len() - 4 - data.len() - ); - c.data.truncate(4); - c.data.append(&mut data); - } - }); +/// Recompress the additional frames of an APNG +fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) { + if !opts.idat_recoding || png.frames.is_empty() { + return; } + let buffer_size = png.raw.ihdr.raw_data_size(); + png.frames + .par_iter_mut() + .with_max_len(1) + .enumerate() + .for_each(|(i, frame)| { + if deadline.passed() { + return; + } + if let Ok(data) = deflate::inflate(&frame.data, buffer_size).and_then(|data| { + let max_size = AtomicMin::new(Some(frame.data.len() - 1)); + opts.deflate.deflate(&data, &max_size) + }) { + debug!( + "Recompressed fdAT #{:<2}: {} ({} bytes decrease)", + i, + data.len(), + frame.data.len() - data.len() + ); + frame.data = data; + } + }); } /// Check if an image was already optimized prior to oxipng's operations From 000ddd758439cf332d5ae19a6c7a971e80a4cb63 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 Jan 2025 20:14:09 +1300 Subject: [PATCH 4/6] Retain filter applied --- src/lib.rs | 3 +++ src/png/mod.rs | 30 ++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index aa43bf2d..42f8fbd9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -365,6 +365,7 @@ fn optimize_png( if let Some(new_png) = optimize_raw(raw.clone(), &opts, deadline.clone(), max_size) { png.raw = new_png.raw; png.idat_data = new_png.idat_data; + png.filter = new_png.filter; } postprocess_chunks(png, &opts, &raw.ihdr); @@ -533,6 +534,7 @@ fn optimize_raw( idat_data, aux_chunks: Vec::new(), frames: Vec::new(), + filter: Some(filter), }; if image.estimated_output_size() < max_size.unwrap_or(usize::MAX) { debug!("Found better combination:"); @@ -553,6 +555,7 @@ fn optimize_raw( idat_data: result.idat_data, aux_chunks: Vec::new(), frames: Vec::new(), + filter: Some(result.filter), }; if image.estimated_output_size() < max_size.unwrap_or(usize::MAX) { debug!("Found better combination:"); diff --git a/src/png/mod.rs b/src/png/mod.rs index 9f856f94..a67fe1e0 100644 --- a/src/png/mod.rs +++ b/src/png/mod.rs @@ -50,6 +50,8 @@ pub struct PngData { pub aux_chunks: Vec, /// APNG frames pub frames: Vec, + /// The filter strategy applied to the idat_data (initially unknown) + pub filter: Option, } impl PngData { @@ -173,24 +175,16 @@ impl PngData { key_chunks.remove(b"PLTE"), key_chunks.remove(b"tRNS"), )?; - let raw_data = deflate::inflate(idat_data.as_ref(), ihdr.raw_data_size())?; - // Reject files with incorrect width/height or truncated data - if raw_data.len() != ihdr.raw_data_size() { - return Err(PngError::TruncatedData); - } + let raw = PngImage::new(ihdr, &idat_data)?; - let mut raw = PngImage { - ihdr, - data: raw_data, - }; - raw.data = raw.unfilter_image()?; // Return the PngData Ok(Self { idat_data, raw: Arc::new(raw), aux_chunks, frames, + filter: None, }) } @@ -292,6 +286,22 @@ impl PngData { } impl PngImage { + pub fn new(ihdr: IhdrData, compressed_data: &[u8]) -> Result { + let raw_data = deflate::inflate(compressed_data, ihdr.raw_data_size())?; + + // Reject files with incorrect width/height or truncated data + if raw_data.len() != ihdr.raw_data_size() { + return Err(PngError::TruncatedData); + } + + let mut image = Self { + ihdr, + data: raw_data, + }; + image.data = image.unfilter_image()?; + Ok(image) + } + /// Convert the image to the specified interlacing type /// Returns true if the interlacing was changed, false otherwise /// The `interlace` parameter specifies the *new* interlacing mode From 369ca2cd1b79f113b3a86a1b00156a5b85bf9404 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 Jan 2025 20:20:49 +1300 Subject: [PATCH 5/6] Refilter frames --- src/lib.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 42f8fbd9..f38251a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -725,7 +725,11 @@ fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) if !opts.idat_recoding || png.frames.is_empty() { return; } - let buffer_size = png.raw.ihdr.raw_data_size(); + // Use the same filter chosen for the main image + // No filter means we failed to optimise the main image and we shouldn't bother trying here + let Some(filter) = png.filter else { + return; + }; png.frames .par_iter_mut() .with_max_len(1) @@ -734,9 +738,13 @@ fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) if deadline.passed() { return; } - if let Ok(data) = deflate::inflate(&frame.data, buffer_size).and_then(|data| { + let mut ihdr = png.raw.ihdr.clone(); + ihdr.width = frame.width; + ihdr.height = frame.height; + if let Ok(data) = PngImage::new(ihdr, &frame.data).and_then(|image| { + let filtered = image.filter_image(filter, opts.optimize_alpha); let max_size = AtomicMin::new(Some(frame.data.len() - 1)); - opts.deflate.deflate(&data, &max_size) + opts.deflate.deflate(&filtered, &max_size) }) { debug!( "Recompressed fdAT #{:<2}: {} ({} bytes decrease)", From 638e9a149415ed8d3056952e597d168dea35993d Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 17 Jan 2025 16:58:36 +1300 Subject: [PATCH 6/6] Error if fdAT invalid --- src/lib.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f38251a3..71c76de6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -369,7 +369,7 @@ fn optimize_png( } postprocess_chunks(png, &opts, &raw.ihdr); - recompress_frames(png, &opts, deadline); + recompress_frames(png, &opts, deadline)?; let output = png.output(); @@ -721,31 +721,30 @@ fn postprocess_chunks(png: &mut PngData, opts: &Options, orig_ihdr: &IhdrData) { } /// Recompress the additional frames of an APNG -fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) { +fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) -> PngResult<()> { if !opts.idat_recoding || png.frames.is_empty() { - return; + return Ok(()); } // Use the same filter chosen for the main image // No filter means we failed to optimise the main image and we shouldn't bother trying here let Some(filter) = png.filter else { - return; + return Ok(()); }; png.frames .par_iter_mut() .with_max_len(1) .enumerate() - .for_each(|(i, frame)| { + .try_for_each(|(i, frame)| { if deadline.passed() { - return; + return Ok(()); } let mut ihdr = png.raw.ihdr.clone(); ihdr.width = frame.width; ihdr.height = frame.height; - if let Ok(data) = PngImage::new(ihdr, &frame.data).and_then(|image| { - let filtered = image.filter_image(filter, opts.optimize_alpha); - let max_size = AtomicMin::new(Some(frame.data.len() - 1)); - opts.deflate.deflate(&filtered, &max_size) - }) { + let image = PngImage::new(ihdr, &frame.data)?; + let filtered = image.filter_image(filter, opts.optimize_alpha); + let max_size = AtomicMin::new(Some(frame.data.len() - 1)); + if let Ok(data) = opts.deflate.deflate(&filtered, &max_size) { debug!( "Recompressed fdAT #{:<2}: {} ({} bytes decrease)", i, @@ -754,7 +753,8 @@ fn recompress_frames(png: &mut PngData, opts: &Options, deadline: Arc) ); frame.data = data; } - }); + Ok(()) + }) } /// Check if an image was already optimized prior to oxipng's operations