From f436ef62a3bdb5a7d99a9cf50a7c290cd1a66ad5 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 23 May 2023 17:19:57 +1200 Subject: [PATCH 1/5] Allow APNG with reductions disabled --- src/headers.rs | 4 +++- src/lib.rs | 18 ++++++++++++++++-- src/png/mod.rs | 28 +++++++++++++++++++++------- tests/lib.rs | 6 +++--- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/headers.rs b/src/headers.rs index 274173d7..8a70a85e 100644 --- a/src/headers.rs +++ b/src/headers.rs @@ -86,7 +86,9 @@ pub enum StripChunks { impl StripChunks { /// List of chunks that will be kept when using the `Safe` option - pub const KEEP_SAFE: [[u8; 4]; 4] = [*b"cICP", *b"iCCP", *b"sRGB", *b"pHYs"]; + pub const KEEP_SAFE: [[u8; 4]; 7] = [ + *b"cICP", *b"iCCP", *b"sRGB", *b"pHYs", *b"acTL", *b"fcTL", *b"fdAT", + ]; pub(crate) fn keep(&self, name: &[u8; 4]) -> bool { match &self { diff --git a/src/lib.rs b/src/lib.rs index d870da55..aa5b69bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,6 +32,7 @@ use crate::png::PngImage; use crate::reduction::*; use log::{debug, info, trace, warn}; use rayon::prelude::*; +use std::borrow::Cow; use std::fmt; use std::fs::{copy, File, Metadata}; use std::io::{stdin, stdout, BufWriter, Read, Write}; @@ -564,17 +565,30 @@ fn optimize_png( debug!(" IDAT size = {} bytes", idat_original_size); debug!(" File size = {} bytes", file_original_size); + // Check for APNG by presence of acTL chunk + let opts = if png.aux_chunks.iter().any(|c| &c.name == b"acTL") { + warn!("APNG detected, disabling all reductions"); + let mut opts = opts.to_owned(); + opts.interlace = None; + opts.bit_depth_reduction = false; + opts.color_type_reduction = false; + opts.palette_reduction = false; + opts.grayscale_reduction = false; + Cow::Owned(opts) + } else { + Cow::Borrowed(opts) + }; let max_size = if opts.force { None } else { Some(png.estimated_output_size()) }; - if let Some(new_png) = optimize_raw(raw.clone(), opts, deadline, max_size) { + if let Some(new_png) = optimize_raw(raw.clone(), &opts, deadline, max_size) { png.raw = new_png.raw; png.idat_data = new_png.idat_data; } - postprocess_chunks(png, opts, &raw.ihdr); + postprocess_chunks(png, &opts, &raw.ihdr); let output = png.output(); diff --git a/src/png/mod.rs b/src/png/mod.rs index 49f7a89d..c2008098 100644 --- a/src/png/mod.rs +++ b/src/png/mod.rs @@ -93,8 +93,16 @@ impl PngData { let mut aux_chunks: Vec = Vec::new(); while let Some(chunk) = parse_next_chunk(byte_data, &mut byte_offset, opts.fix_errors)? { match &chunk.name { - b"IDAT" => idat_data.extend_from_slice(chunk.data), - b"acTL" => return Err(PngError::APNGNotSupported), + b"IDAT" => { + if idat_data.is_empty() { + // Keep track of where the first IDAT sits relative to other chunks + aux_chunks.push(Chunk { + name: chunk.name, + data: Vec::new(), + }) + } + idat_data.extend_from_slice(chunk.data); + } b"IHDR" | b"PLTE" | b"tRNS" => { key_chunks.insert(chunk.name, chunk.data.to_owned()); } @@ -165,9 +173,10 @@ impl PngData { ihdr_data.write_all(&[0]).ok(); // Filter method -- 5-way adaptive filtering ihdr_data.write_all(&[self.raw.ihdr.interlaced as u8]).ok(); write_png_block(b"IHDR", &ihdr_data, &mut output); - // Ancillary chunks - for chunk in self - .aux_chunks + // Ancillary chunks - split into those that come before IDAT and those that come after + let mut aux_split = self.aux_chunks.split(|c| &c.name == b"IDAT"); + let aux_pre = aux_split.next().unwrap(); + for chunk in aux_pre .iter() .filter(|c| !(&c.name == b"bKGD" || &c.name == b"hIST" || &c.name == b"tRNS")) { @@ -202,8 +211,7 @@ impl PngData { _ => {} } // Special ancillary chunks that need to come after PLTE but before IDAT - for chunk in self - .aux_chunks + for chunk in aux_pre .iter() .filter(|c| &c.name == b"bKGD" || &c.name == b"hIST" || &c.name == b"tRNS") { @@ -211,6 +219,12 @@ impl PngData { } // IDAT data write_png_block(b"IDAT", &self.idat_data, &mut output); + // Ancillary chunks that come after IDAT + for aux_post in aux_split { + for chunk in aux_post { + write_png_block(&chunk.name, &chunk.data, &mut output); + } + } // Stream end write_png_block(b"IEND", &[], &mut output); diff --git a/tests/lib.rs b/tests/lib.rs index a32de8b1..3c27f8be 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -30,7 +30,7 @@ fn optimize_from_memory_apng() { in_file.read_to_end(&mut in_file_buf).unwrap(); let result = oxipng::optimize_from_memory(&in_file_buf, &Options::default()); - assert!(result.is_err()); + assert!(result.is_ok()); } #[test] @@ -58,9 +58,9 @@ fn optimize_apng() { let result = oxipng::optimize( &"tests/files/apng_file.png".into(), &OutFile::Path(None), - &Options::default(), + &Options::from_preset(0), ); - assert!(result.is_err()); + assert!(result.is_ok()); } #[test] From 16ca58b175d089d2cd992d80a8d0e6a8c0c23204 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 24 May 2023 10:37:19 +1200 Subject: [PATCH 2/5] Recompress fdATs --- src/lib.rs | 47 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index aa5b69bf..13242044 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -389,7 +389,7 @@ impl RawImage { /// Create an optimized png from the raw image data using the options provided pub fn create_optimized_png(&self, opts: &Options) -> PngResult> { let deadline = Arc::new(Deadline::new(opts.timeout)); - let mut png = optimize_raw(self.png.clone(), opts, deadline, None) + let mut png = optimize_raw(self.png.clone(), opts, deadline.clone(), None) .ok_or_else(|| PngError::new("Failed to optimize input data"))?; // Process aux chunks @@ -399,7 +399,7 @@ impl RawImage { .filter(|c| opts.strip.keep(&c.name)) .cloned() .collect(); - postprocess_chunks(&mut png, opts, &self.png.ihdr); + postprocess_chunks(&mut png, opts, deadline, &self.png.ihdr); Ok(png.output()) } @@ -583,12 +583,12 @@ fn optimize_png( } else { Some(png.estimated_output_size()) }; - if let Some(new_png) = optimize_raw(raw.clone(), &opts, deadline, max_size) { + 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; } - postprocess_chunks(png, &opts, &raw.ihdr); + postprocess_chunks(png, &opts, deadline, &raw.ihdr); let output = png.output(); @@ -858,7 +858,12 @@ 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, orig_ihdr: &IhdrData) { +fn postprocess_chunks( + png: &mut PngData, + opts: &Options, + deadline: Arc, + 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"); @@ -911,6 +916,38 @@ fn postprocess_chunks(png: &mut PngData, opts: &Options, orig_ihdr: &IhdrData) { !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 !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); + } + }) + } } /// Check if an image was already optimized prior to oxipng's operations From 4ab941e305307de2242abf85d93e0358365265c5 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 24 May 2023 10:47:06 +1200 Subject: [PATCH 3/5] Add par_iter_mut shim --- src/rayon.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/rayon.rs b/src/rayon.rs index ae1914c2..21010668 100644 --- a/src/rayon.rs +++ b/src/rayon.rs @@ -26,6 +26,12 @@ pub trait IntoParallelRefIterator<'data> { fn par_iter(&'data self) -> Self::Iter; } +pub trait IntoParallelRefMutIterator<'data> { + type Iter: ParallelIterator; + type Item: Send + 'data; + fn par_iter_mut(&'data mut self) -> Self::Iter; +} + impl IntoParallelIterator for I where I::Item: Send, @@ -50,6 +56,18 @@ where } } +impl<'data, I: 'data + ?Sized> IntoParallelRefMutIterator<'data> for I +where + &'data mut I: IntoParallelIterator, +{ + type Iter = <&'data mut I as IntoParallelIterator>::Iter; + type Item = <&'data mut I as IntoParallelIterator>::Item; + + fn par_iter_mut(&'data mut self) -> Self::Iter { + self.into_par_iter() + } +} + impl ParallelIterator for I {} #[allow(dead_code)] From 041e5f084b095506bfc6bec58f75b287f1f652b7 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 30 May 2023 18:34:28 +1200 Subject: [PATCH 4/5] Validate all frames in APNGs --- src/sanity_checks.rs | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/sanity_checks.rs b/src/sanity_checks.rs index 496b5dfd..8150d72d 100644 --- a/src/sanity_checks.rs +++ b/src/sanity_checks.rs @@ -1,15 +1,14 @@ -use image::{DynamicImage, GenericImageView, ImageFormat, Pixel}; +use image::{codecs::png::PngDecoder, *}; use log::{error, warn}; -use std::io::Cursor; /// Validate that the output png data still matches the original image pub fn validate_output(output: &[u8], original_data: &[u8]) -> bool { - let (old_png, new_png) = rayon::join( + let (old_frames, new_frames) = rayon::join( || load_png_image_from_memory(original_data), || load_png_image_from_memory(output), ); - match (new_png, old_png) { + match (new_frames, old_frames) { (Err(new_err), _) => { error!("Failed to read output image for validation: {}", new_err); false @@ -21,26 +20,40 @@ pub fn validate_output(output: &[u8], original_data: &[u8]) -> bool { warn!("Failed to read input image for validation: {}", old_err); true } - (Ok(new_png), Ok(old_png)) => images_equal(&old_png, &new_png), + (Ok(new_frames), Ok(old_frames)) if new_frames.len() != old_frames.len() => false, + (Ok(new_frames), Ok(old_frames)) => { + for (a, b) in old_frames.iter().zip(new_frames) { + if !images_equal(&a, &b) { + return false; + } + } + true + } } } -/// Loads a PNG image from memory to a [DynamicImage] -fn load_png_image_from_memory(png_data: &[u8]) -> Result { - let mut reader = image::io::Reader::new(Cursor::new(png_data)); - reader.set_format(ImageFormat::Png); - reader.no_limits(); - reader.decode() +/// Loads a PNG image from memory to frames of [RgbaImage] +fn load_png_image_from_memory(png_data: &[u8]) -> Result, image::ImageError> { + let decoder = PngDecoder::new(png_data)?; + if decoder.is_apng() { + decoder + .apng() + .into_frames() + .map(|f| f.map(|f| f.into_buffer())) + .collect() + } else { + DynamicImage::from_decoder(decoder).map(|i| vec![i.into_rgba8()]) + } } /// Compares images pixel by pixel for equivalent content -fn images_equal(old_png: &DynamicImage, new_png: &DynamicImage) -> bool { +fn images_equal(old_png: &RgbaImage, new_png: &RgbaImage) -> bool { let a = old_png.pixels().filter(|x| { - let p = x.2.channels(); + let p = x.channels(); !(p.len() == 4 && p[3] == 0) }); let b = new_png.pixels().filter(|x| { - let p = x.2.channels(); + let p = x.channels(); !(p.len() == 4 && p[3] == 0) }); a.eq(b) From ff06573c9fb0db5420f4d35d69c3646d7b2f9599 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 31 May 2023 08:43:07 +1200 Subject: [PATCH 5/5] Add warnings about stripping APNGs --- src/main.rs | 2 +- src/png/mod.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index d848d5ef..519d68b4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -120,7 +120,7 @@ fn main() { ) .arg( Arg::new("strip") - .help("Strip metadata objects ['safe', 'all', or comma-separated list]") + .help("Strip metadata objects ['safe', 'all', or comma-separated list]\nCAUTION: stripping 'all' will convert APNGs to standard PNGs") .long("strip") .value_name("mode") .conflicts_with("strip-safe"), diff --git a/src/png/mod.rs b/src/png/mod.rs index c2008098..46ccc04a 100644 --- a/src/png/mod.rs +++ b/src/png/mod.rs @@ -7,6 +7,7 @@ use crate::interlace::{deinterlace_image, interlace_image, Interlacing}; use crate::Options; use bitvec::bitarr; use libdeflater::{CompressionLvl, Compressor}; +use log::warn; use rgb::ComponentSlice; use rustc_hash::FxHashMap; use std::fs::File; @@ -112,6 +113,10 @@ impl PngData { 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" + ); } } }