Repository: shssoichiro/oxipng
Pull request #511 (2023-05-23 05:32:32)

Allow APNG with reductions disabled
This is a bit of a hack, not sure if it's something that should actually be added, but it provides rudimentary support for APNG files by disabling all reductions and preserving original chunk order. See #79.
The savings to the test file `apng_file.png` are indeed minimal, achieving only 0.19% with default options and 0.41% with `-o6 -a`.

Note the required chunks are retained with strip safe, though you could still opt to strip all to turn it into a standard png.

TPS: The greatest advantage to this is, when wholesale running over large #s of PNGs, is to avoid absolutely destroying APNGs in the process.  That, in & of itself, should increase OxiPNG's uptake & acceptance immediately.

@javiergutierrezchamorro 🙇🏾‍♂️ Comments, please?
andrews05: @TPS Oxipng currently skips APNGs entirely, so not sure what destruction you're referring to?

I've just added recompression of fdATs which is pretty straightforward and gets us up 0.74% savings on the apng test file. This is still very limited though as reductions are still disabled and it won't re-filter the fdATs. This is all possible in theory and could come some day, but again this is more of a quick hack to get something working.
TPS: @andrews05 I clarified my meaning above; sorry for the confusion.

I mean that, much of the implicit premise of #79 ("Supported" ≠ "Optimized") is that `OxiPNG` lives in a world of static PNGs.  I'm not aware of anything in the docs that mentions it's APNG-aware, much less that it's already safe for such.  When did that happen?

This PR takes it to the next step: the very tiniest beginning of APNG optimization.  That, combined w/ a note in the ReadMe & probably a brief mention in the built-in help text, is really welcome. 🙇🏾‍♂️
andrews05: @TPS Ah, looks like it happened after that issue was created (#88), so understandable that people weren't aware.

Yeah, it could definitely do with a note in the Read Me. There's a bunch of other updates that should be made to the Read Me as well but it's tied in with a benchmark script that I've been reluctant to delve into. I might check it out after #495 lands, which will affect benchmarks.
TPS: @andrews05 I love the iterative nature of how you put together your PRs.  Did https://github.com/shssoichiro/oxipng/commit/070c736d6532eaffa03eb1ab62730e0b81fd02ca crack 1% savings?
andrews05: @TPS Ha, most of the time it's because I keep making mistakes that I need to fix 🤣. In this case the CI failed with the parallel feature disabled because I used a function that hadn't been shimmed for non-parallel yet. The new commit doesn't affect output.
shssoichiro: This looks like a nice improvement, even though the compression savings are relatively small. How safe is this to run in bulk? The primary reason why oxipng has continued to skip APNG files is because there has not been a way to verify the resulting image is identical to the input--above all, I want to ensure oxipng never causes damage to a file (unless the user is explicitly using non-default settings that are noted as possibly altering the image).
andrews05: The sanity checks will cover the main IDAT. I'm not sure how you would verify the rest of the animation though. I tested it on about a dozen images and am reasonably sure it's doing everything right, but hard to be certain 🤷‍♂️
andrews05: Right, I've added validation of all frames 🙂
javiergutierrezchamorro: Cool. Looking forward to updated binaries and see if ready for prime time!
andrews05: I've now tested on 32 files - still not a huge amount but I am pretty confident that it's safe.
I've also added some warnings about stripping APNGs with `--strip all`. 
TPS: > I've now tested on 32 files - still not a huge amount but I am pretty confident that it's safe.

@andrews05 Here's a bunch you could try: https://commons.m.wikimedia.org/wiki/Category:Animated_PNG_files
andrews05: @TPS Oh thanks, that's really helpful. I've now tested on around 350MB of APNGs, all successful 🙂
TPS: In your tests on these, did it still hold to <1% reduction?  Anything noteworthy?
andrews05: Overall still <1% but I think some individual files got up to 8%.
ace-dent: @andrews05 - you probably have this already, but a APNG test corpus is here: https://philip.html5.org/tests/apng/tests.html
andrews05: @ace-dent Thanks, I did come across that. That seems to be more to do with correct decoding and rendering though. I think it will be helpful when full support eventually gets implemented but for now we aren't actually decoding the acTL/fcTL chunks.

Patch:
diff --git a/README.md b/README.md
index d2e7262e..04189a9c 100644
--- a/README.md
+++ b/README.md
@@ -115,6 +115,11 @@ method of usage involves creating an
 passing it, along with an input filename, into the
 [optimize function](https://docs.rs/oxipng/3.0.1/oxipng/fn.optimize.html).
 
+It is recommended to disable the "binary" feature when including oxipng as a library. Currently, there is
+no simple way to just disable one feature in Cargo, it has to be done by disabling default features
+and specifying the desired ones, for example:
+`oxipng = { version = "8.0", features = ["parallel", "zopfli", "filetime"], default-features = false }`
+
 ## History
 
 Oxipng began as a complete rewrite of the OptiPNG project,
diff --git a/README.template.md b/README.template.md
index 265e9f9b..6f14ed8d 100644
--- a/README.template.md
+++ b/README.template.md
@@ -75,6 +75,11 @@ method of usage involves creating an
 passing it, along with an input filename, into the
 [optimize function](https://docs.rs/oxipng/3.0.1/oxipng/fn.optimize.html).
 
+It is recommended to disable the "binary" feature when including oxipng as a library. Currently, there is
+no simple way to just disable one feature in Cargo, it has to be done by disabling default features
+and specifying the desired ones, for example:
+`oxipng = { version = "8.0", features = ["parallel", "zopfli", "filetime"], default-features = false }`
+
 ## History
 
 Oxipng began as a complete rewrite of the OptiPNG project,
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..13242044 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};
@@ -388,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<Vec<u8>> {
         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
@@ -398,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())
     }
@@ -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.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();
 
@@ -844,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<Deadline>,
+    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");
@@ -897,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
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 49f7a89d..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;
@@ -93,8 +94,16 @@ impl PngData {
         let mut aux_chunks: Vec<Chunk> = 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());
                 }
@@ -104,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"
+                        );
                     }
                 }
             }
@@ -165,9 +178,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 +216,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 +224,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/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<Item = Self::Item>;
+    type Item: Send + 'data;
+    fn par_iter_mut(&'data mut self) -> Self::Iter;
+}
+
 impl<I: IntoIterator> 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<I: Iterator> ParallelIterator for I {}
 
 #[allow(dead_code)]
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<DynamicImage, image::ImageError> {
-    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<Vec<RgbaImage>, 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)


Test patch:
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]