From 84bbec0666d732f59940bd0852b970960b89eda1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= Date: Mon, 5 Sep 2022 18:50:13 +0200 Subject: [PATCH] Add initial support for changing Zopfli iterations (#446) * Update and optimize dependencies These changes update the dependencies to their latest versions, fixing some known issues that prevented doing so in the first place. In addition, the direct dependency on byteorder was dropped in favor of stdlib functions that have been stabilized for some time in Rust, and the transitive dependency on chrono, pulled by stderrlog, was also dropped, which had been affected by security issues and improperly maintained in the past: - https://github.com/cardoe/stderrlog-rs/issues/31 - https://www.reddit.com/r/rust/comments/ts84n4/chrono_or_time_03/ * Run rustfmt * Bump MSRV to 1.56.1 Updating to this patch version should not be cumbersome for end-users, and it is required by a transitive dependency. * Bump MSRV to 1.57.0 os_str_bytes requires it. * Add initial support for changing Zopfli iterations PR https://github.com/shssoichiro/oxipng/pull/445 did some dependency updates, which included using the latest zopfli version. The latest version of this crate exposes new options in its API that allow users to choose the desired number of Zopfli compression iterations, which may greatly affect execution time. In fact, other optimizers such as zopflipng dynamically select this number depending on the input file size (see: https://github.com/shssoichiro/oxipng/issues/414). As a first step towards making OxiPNG deal with Zopfli better, let's add the necessary options for libraries to be able to choose the number of iterations. This number is still fixed to 15 as before when using the CLI. * Fix Clippy lint Co-authored-by: Josh Holmer --- benches/zopfli.rs | 14 +++++++++----- src/deflate/mod.rs | 17 ++++++++++++++--- src/lib.rs | 2 +- src/main.rs | 5 ++++- tests/flags.rs | 5 ++++- 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/benches/zopfli.rs b/benches/zopfli.rs index 8c98b350..a9b91e08 100644 --- a/benches/zopfli.rs +++ b/benches/zopfli.rs @@ -4,16 +4,20 @@ extern crate oxipng; extern crate test; use oxipng::internal_tests::*; +use std::num::NonZeroU8; use std::path::PathBuf; use test::Bencher; +// SAFETY: trivially safe. Stopgap solution until const unwrap is stabilized. +const DEFAULT_ZOPFLI_ITERATIONS: NonZeroU8 = unsafe { NonZeroU8::new_unchecked(15) }; + #[bench] fn zopfli_16_bits_strategy_0(b: &mut Bencher) { let input = test::black_box(PathBuf::from("tests/files/rgb_16_should_be_rgb_16.png")); let png = PngData::new(&input, false).unwrap(); b.iter(|| { - zopfli_deflate(png.raw.data.as_ref()).ok(); + zopfli_deflate(png.raw.data.as_ref(), DEFAULT_ZOPFLI_ITERATIONS).ok(); }); } @@ -23,7 +27,7 @@ fn zopfli_8_bits_strategy_0(b: &mut Bencher) { let png = PngData::new(&input, false).unwrap(); b.iter(|| { - zopfli_deflate(png.raw.data.as_ref()).ok(); + zopfli_deflate(png.raw.data.as_ref(), DEFAULT_ZOPFLI_ITERATIONS).ok(); }); } @@ -35,7 +39,7 @@ fn zopfli_4_bits_strategy_0(b: &mut Bencher) { let png = PngData::new(&input, false).unwrap(); b.iter(|| { - zopfli_deflate(png.raw.data.as_ref()).ok(); + zopfli_deflate(png.raw.data.as_ref(), DEFAULT_ZOPFLI_ITERATIONS).ok(); }); } @@ -47,7 +51,7 @@ fn zopfli_2_bits_strategy_0(b: &mut Bencher) { let png = PngData::new(&input, false).unwrap(); b.iter(|| { - zopfli_deflate(png.raw.data.as_ref()).ok(); + zopfli_deflate(png.raw.data.as_ref(), DEFAULT_ZOPFLI_ITERATIONS).ok(); }); } @@ -59,6 +63,6 @@ fn zopfli_1_bits_strategy_0(b: &mut Bencher) { let png = PngData::new(&input, false).unwrap(); b.iter(|| { - zopfli_deflate(png.raw.data.as_ref()).ok(); + zopfli_deflate(png.raw.data.as_ref(), DEFAULT_ZOPFLI_ITERATIONS).ok(); }); } diff --git a/src/deflate/mod.rs b/src/deflate/mod.rs index 7a8af8a6..9894a5b0 100644 --- a/src/deflate/mod.rs +++ b/src/deflate/mod.rs @@ -4,6 +4,9 @@ use crate::Deadline; use crate::PngResult; use indexmap::IndexSet; +#[cfg(feature = "zopfli")] +use std::num::NonZeroU8; + #[doc(hidden)] pub mod miniz_stream; @@ -49,11 +52,14 @@ pub fn deflate( } #[cfg(feature = "zopfli")] -pub fn zopfli_deflate(data: &[u8]) -> PngResult> { +pub fn zopfli_deflate(data: &[u8], iterations: NonZeroU8) -> PngResult> { use std::cmp::max; let mut output = Vec::with_capacity(max(1024, data.len() / 20)); - let options = zopfli::Options::default(); + let options = zopfli::Options { + iteration_count: iterations, + ..Default::default() + }; match zopfli::compress(&options, &zopfli::Format::Zlib, data, &mut output) { Ok(_) => (), Err(_) => return Err(PngError::new("Failed to compress in zopfli")), @@ -85,7 +91,12 @@ pub enum Deflaters { }, #[cfg(feature = "zopfli")] /// Use the better but slower Zopfli implementation - Zopfli, + Zopfli { + /// The number of compression iterations to do. 15 iterations are fine + /// for small files, but bigger files will need to be compressed with + /// less iterations, or else they will be too slow. + iterations: NonZeroU8, + }, #[cfg(feature = "libdeflater")] /// Use libdeflater. Libdeflater, diff --git a/src/lib.rs b/src/lib.rs index 441d8849..4910e50f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -618,7 +618,7 @@ fn optimize_png( &deadline, ), #[cfg(feature = "zopfli")] - Deflaters::Zopfli => deflate::zopfli_deflate(filtered), + Deflaters::Zopfli { iterations } => deflate::zopfli_deflate(filtered, iterations), #[cfg(feature = "libdeflater")] Deflaters::Libdeflater => deflate::libdeflater_deflate(filtered, &best_size), }; diff --git a/src/main.rs b/src/main.rs index 02518118..adf51a4a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,7 @@ use oxipng::Headers; use oxipng::Options; use oxipng::{InFile, OutFile}; use std::fs::DirBuilder; +use std::num::NonZeroU8; use std::path::PathBuf; use std::process::exit; use std::time::Duration; @@ -495,7 +496,9 @@ fn parse_opts_into_struct( } if matches.is_present("zopfli") { - opts.deflate = Deflaters::Zopfli; + opts.deflate = Deflaters::Zopfli { + iterations: NonZeroU8::new(15).unwrap(), + }; } else if matches.is_present("libdeflater") { opts.deflate = Deflaters::Libdeflater; } else if let Deflaters::Zlib { diff --git a/tests/flags.rs b/tests/flags.rs index 07f29d19..f99324bd 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -4,6 +4,7 @@ use oxipng::{InFile, OutFile}; #[cfg(feature = "filetime")] use std::cell::RefCell; use std::fs::remove_file; +use std::num::NonZeroU8; #[cfg(feature = "filetime")] use std::ops::Deref; use std::path::Path; @@ -582,7 +583,9 @@ fn fix_errors() { fn zopfli_mode() { let input = PathBuf::from("tests/files/zopfli_mode.png"); let (output, mut opts) = get_opts(&input); - opts.deflate = Deflaters::Zopfli; + opts.deflate = Deflaters::Zopfli { + iterations: NonZeroU8::new(15).unwrap(), + }; test_it_converts( input,