From 681089f8c7c85662c88c642dbdca9d60ba0eb820 Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 1 Apr 2020 00:28:22 +0100 Subject: [PATCH 1/2] Remove the concept of size bias (#199) Closes #198 --- src/evaluate.rs | 31 ++++++------------------------- src/lib.rs | 10 +++++----- src/reduction/alpha.rs | 2 +- 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/src/evaluate.rs b/src/evaluate.rs index c6be4d9a..cc083b7f 100644 --- a/src/evaluate.rs +++ b/src/evaluate.rs @@ -24,8 +24,6 @@ use std::thread; struct Candidate { image: PngData, - // compressed size multiplier. Fudge factor to prefer more promising formats. - bias: f32, // if false, that's baseline file to throw away is_reduction: bool, filter: u8, @@ -42,30 +40,16 @@ impl Comparator { fn evaluate(&mut self, new: Candidate) { // a tie-breaker is required to make evaluation deterministic let is_best = if let Some(ref old) = self.best_result { - // ordering is important - later file gets to use bias over earlier, but not the other way - // (this way bias=0 replaces, but doesn't forbid later optimizations) - let new_len = (new.image.idat_data.len() as f64 - * if new.nth > old.nth { - f64::from(new.bias) - } else { - 1.0 - }) as usize; - let old_len = (old.image.idat_data.len() as f64 - * if new.nth < old.nth { - f64::from(old.bias) - } else { - 1.0 - }) as usize; // choose smallest compressed, or if compresses the same, smallest uncompressed, or cheaper filter let new = ( - new_len, + new.image.idat_data.len(), new.image.raw.data.len(), new.image.raw.ihdr.bit_depth, new.filter, new.nth, ); let old = ( - old_len, + old.image.idat_data.len(), old.image.raw.data.len(), old.image.raw.ihdr.bit_depth, old.filter, @@ -141,17 +125,15 @@ impl Evaluator { /// Set baseline image. It will be used only to measure minimum compression level required pub fn set_baseline(&self, image: Arc) { - self.try_image_inner(image, 1.0, false) + self.try_image_inner(image, false) } /// Check if the image is smaller than others - /// Bias is a value in 0..=1 range. Compressed size is multiplied by - /// this fraction when comparing to the best, so 0.95 allows 5% larger size. - pub fn try_image(&self, image: Arc, bias: f32) { - self.try_image_inner(image, bias, true) + pub fn try_image(&self, image: Arc) { + self.try_image_inner(image, true) } - fn try_image_inner(&self, image: Arc, bias: f32, is_reduction: bool) { + fn try_image_inner(&self, image: Arc, is_reduction: bool) { let nth = self.nth.fetch_add(1, SeqCst); // These clones are only cheap refcounts let deadline = self.deadline.clone(); @@ -186,7 +168,6 @@ impl Evaluator { idat_data, raw: Arc::clone(&image), }, - bias, filter, is_reduction, nth, diff --git a/src/lib.rs b/src/lib.rs index 5227bc0f..d56f264d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -740,7 +740,7 @@ fn perform_reductions( if let Some(interlacing) = opts.interlace { if let Some(reduced) = png.change_interlacing(interlacing) { png = Arc::new(reduced); - eval.try_image(png.clone(), 0.); + eval.try_image(png.clone()); } if deadline.passed() { return; @@ -750,7 +750,7 @@ fn perform_reductions( if opts.palette_reduction { if let Some(reduced) = reduced_palette(&png) { png = Arc::new(reduced); - eval.try_image(png.clone(), 0.95); + eval.try_image(png.clone()); if opts.verbosity == Some(1) { report_reduction(&png); } @@ -765,13 +765,13 @@ fn perform_reductions( let previous = png.clone(); let bits = reduced.ihdr.bit_depth; png = Arc::new(reduced); - eval.try_image(png.clone(), 1.0); + eval.try_image(png.clone()); if (bits == BitDepth::One || bits == BitDepth::Two) && previous.ihdr.bit_depth != BitDepth::Four { // Also try 16-color mode for all lower bits images, since that may compress better if let Some(reduced) = reduce_bit_depth(&previous, 4) { - eval.try_image(Arc::new(reduced), 0.98); + eval.try_image(Arc::new(reduced)); } } if opts.verbosity == Some(1) { @@ -786,7 +786,7 @@ fn perform_reductions( if opts.color_type_reduction { if let Some(reduced) = reduce_color_type(&png) { png = Arc::new(reduced); - eval.try_image(png.clone(), 0.96); + eval.try_image(png.clone()); if opts.verbosity == Some(1) { report_reduction(&png); } diff --git a/src/reduction/alpha.rs b/src/reduction/alpha.rs index 95b03744..9c4a0446 100644 --- a/src/reduction/alpha.rs +++ b/src/reduction/alpha.rs @@ -24,7 +24,7 @@ pub(crate) fn try_alpha_reductions( let alphas_iter = alphas.par_iter().with_max_len(1); alphas_iter .filter_map(|&alpha| filtered_alpha_channel(&png, *alpha)) - .for_each(|image| eval.try_image(Arc::new(image), 0.99)); + .for_each(|image| eval.try_image(Arc::new(image))); } pub fn filtered_alpha_channel(png: &PngImage, optim: AlphaOptim) -> Option { From 121558bf5cb3883296a74b09b40b70bbd64930a2 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 1 Apr 2020 01:24:57 +0100 Subject: [PATCH 2/2] Add libdeflater as an option (#203) libdeflater is a Rust wrapper around [libdeflate](https://github.com/ebiggers/libdeflate) - an alternative heavily optimised library for deflate/zlib/gzip compression and decompression that is intended for situations where upper bounds of the output are well-known. In my benchmarks on test files in the repo it has shown to be usually both slightly faster and providing better compressed output than cloudflare-zlib, but in some cases showing the opposite, so rather than swapping defaults, it's currently provided as another option, similarly to zopfli. Since it's not strictly better in all cases, I'm not providing median numbers, but you can check distribution histograms for time and size differences here (all using `oxipng -o 6 -t 6 -P`): https://docs.google.com/spreadsheets/d/1WOKgeYZBhLkQvMGAC36snN4azilElzOFhx63RJu0EZY/edit?usp=sharing --- Cargo.lock | 10 +++++++ Cargo.toml | 1 + benches/libdeflater.rs | 64 +++++++++++++++++++++++++++++++++++++++++ src/deflate/deflater.rs | 31 ++++++++++++++++++++ src/deflate/mod.rs | 5 ++++ src/lib.rs | 13 +++++---- src/main.rs | 8 ++++++ tests/flags.rs | 18 ++++++++++++ 8 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 benches/libdeflater.rs create mode 100644 src/deflate/deflater.rs diff --git a/Cargo.lock b/Cargo.lock index 6be5ec18..365ae348 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -218,6 +218,14 @@ name = "libc" version = "0.2.67" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "libdeflater" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cc 1.0.50 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "maybe-uninit" version = "2.0.0" @@ -296,6 +304,7 @@ dependencies = [ "crc 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "image 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libdeflater 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "miniz_oxide 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -464,6 +473,7 @@ dependencies = [ "checksum itertools 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "284f18f85651fe11e8a991b2adb42cb078325c996ed026d994719efcfca1d54b" "checksum lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" "checksum libc 0.2.67 (registry+https://github.com/rust-lang/crates.io-index)" = "eb147597cdf94ed43ab7a9038716637d2d1bf2bc571da995d0028dec06bd3018" +"checksum libdeflater 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "66dca08b13369865b2f6dca1dd05f833985cbe6c12a676b04d55f78b85e80246" "checksum maybe-uninit 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00" "checksum memoffset 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "75189eb85871ea5c2e2c15abbdd541185f63b408415e5051f5cac122d8c774b9" "checksum miniz_oxide 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "aa679ff6578b1cddee93d7e82e263b94a575e0bfced07284eb0c037c1d2416a5" diff --git a/Cargo.toml b/Cargo.toml index d37fa5b6..c7afb897 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ num_cpus = "^1.0.0" zopfli = "^0.4.0" miniz_oxide = "0.3" rgb = "0.8.11" +libdeflater = "0.2.0" [dependencies.rayon] optional = true diff --git a/benches/libdeflater.rs b/benches/libdeflater.rs new file mode 100644 index 00000000..d3323253 --- /dev/null +++ b/benches/libdeflater.rs @@ -0,0 +1,64 @@ +#![feature(test)] + +extern crate oxipng; +extern crate test; + +use oxipng::internal_tests::*; +use std::path::PathBuf; +use test::Bencher; + +#[bench] +fn libdeflater_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(|| { + libdeflater_deflate(png.raw.data.as_ref()).ok(); + }); +} + +#[bench] +fn libdeflater_8_bits_strategy_0(b: &mut Bencher) { + let input = test::black_box(PathBuf::from("tests/files/rgb_8_should_be_rgb_8.png")); + let png = PngData::new(&input, false).unwrap(); + + b.iter(|| { + libdeflater_deflate(png.raw.data.as_ref()).ok(); + }); +} + +#[bench] +fn libdeflater_4_bits_strategy_0(b: &mut Bencher) { + let input = test::black_box(PathBuf::from( + "tests/files/palette_4_should_be_palette_4.png", + )); + let png = PngData::new(&input, false).unwrap(); + + b.iter(|| { + libdeflater_deflate(png.raw.data.as_ref()).ok(); + }); +} + +#[bench] +fn libdeflater_2_bits_strategy_0(b: &mut Bencher) { + let input = test::black_box(PathBuf::from( + "tests/files/palette_2_should_be_palette_2.png", + )); + let png = PngData::new(&input, false).unwrap(); + + b.iter(|| { + libdeflater_deflate(png.raw.data.as_ref()).ok(); + }); +} + +#[bench] +fn libdeflater_1_bits_strategy_0(b: &mut Bencher) { + let input = test::black_box(PathBuf::from( + "tests/files/palette_1_should_be_palette_1.png", + )); + let png = PngData::new(&input, false).unwrap(); + + b.iter(|| { + libdeflater_deflate(png.raw.data.as_ref()).ok(); + }); +} diff --git a/src/deflate/deflater.rs b/src/deflate/deflater.rs new file mode 100644 index 00000000..19f93db8 --- /dev/null +++ b/src/deflate/deflater.rs @@ -0,0 +1,31 @@ +use crate::{PngError, PngResult}; +use crate::atomicmin::AtomicMin; +use libdeflater::{CompressionError, CompressionLvl, Compressor}; + +pub fn deflate(data: &[u8], max_size: &AtomicMin) -> PngResult> { + let mut compressor = Compressor::new(CompressionLvl::best()); + let capacity = max_size.get().unwrap_or(data.len() / 2); + let mut dest = Vec::with_capacity(capacity); + unsafe { + // This is ok because the Vec contains Copy-able data (u8) + // and because libdeflater wrapper doesn't try to read + // the bytes from the target. + // + // That said, it should be able to accept MaybeUninit instead, + // so I raised an upstream issue that should make this safer: + // https://github.com/adamkewley/libdeflater/issues/1 + dest.set_len(capacity); + } + let len = compressor + .zlib_compress(data, &mut dest) + .map_err(|err| match err { + CompressionError::InsufficientSpace => PngError::DeflatedDataTooLong(capacity), + })?; + if let Some(max) = max_size.get() { + if len > max { + return Err(PngError::DeflatedDataTooLong(max)); + } + } + dest.truncate(len); + Ok(dest) +} diff --git a/src/deflate/mod.rs b/src/deflate/mod.rs index c04a8e4a..e7b5dcca 100644 --- a/src/deflate/mod.rs +++ b/src/deflate/mod.rs @@ -9,6 +9,9 @@ use zopfli; #[doc(hidden)] pub mod miniz_stream; +mod deflater; +pub use deflater::deflate as libdeflater_deflate; + #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] pub mod cfzlib; @@ -62,4 +65,6 @@ pub enum Deflaters { Zlib, /// Use the better but slower Zopfli implementation Zopfli, + /// Use libdeflater. + Libdeflater, } diff --git a/src/lib.rs b/src/lib.rs index d56f264d..6e22d03f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -353,6 +353,7 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<( data } }; + let mut png = PngData::from_slice(&in_data, opts.fix_errors)?; // Run the optimizer on the decoded PNG. @@ -565,7 +566,7 @@ fn optimize_png( } } } else { - // Zopfli compression has no additional options + // Zopfli and Libdeflater compression have no additional options. results.push(TrialOptions { filter: *f, compression: 0, @@ -600,17 +601,17 @@ fn optimize_png( return None; } let filtered = &filters[&trial.filter]; - let new_idat = if opts.deflate == Deflaters::Zlib { - deflate::deflate( + let new_idat = match opts.deflate { + Deflaters::Zlib => deflate::deflate( filtered, trial.compression, trial.strategy, opts.window, &best_size, &deadline, - ) - } else { - deflate::zopfli_deflate(filtered) + ), + Deflaters::Zopfli => deflate::zopfli_deflate(filtered), + Deflaters::Libdeflater => deflate::libdeflater_deflate(filtered, &best_size), }; let new_idat = match new_idat { diff --git a/src/main.rs b/src/main.rs index 29631f58..d39d83d1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -193,6 +193,10 @@ fn main() { .help("Use the slower but better compressing Zopfli algorithm, overrides zlib-specific options") .short("Z") .long("zopfli")) + .arg(Arg::with_name("libdeflater") + .help("Use an alternative Libdeflater algorithm, overrides zlib-specific options") + .short("D") + .long("libdeflater")) .arg(Arg::with_name("timeout") .help("Maximum amount of time, in seconds, to spend on optimizations") .takes_value(true) @@ -481,6 +485,10 @@ fn parse_opts_into_struct( opts.deflate = Deflaters::Zopfli; } + if matches.is_present("libdeflater") { + opts.deflate = Deflaters::Libdeflater; + } + if let Some(x) = matches.value_of("threads") { opts.threads = x.parse::().unwrap(); } diff --git a/tests/flags.rs b/tests/flags.rs index 1e9a43f7..d9f7b477 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -438,3 +438,21 @@ fn zopfli_mode() { BitDepth::Eight, ); } + + +#[test] +fn libdeflater_mode() { + let input = PathBuf::from("tests/files/zopfli_mode.png"); + let (output, mut opts) = get_opts(&input); + opts.deflate = Deflaters::Libdeflater; + + test_it_converts( + input, + &output, + &opts, + ColorType::RGB, + BitDepth::Eight, + ColorType::RGB, + BitDepth::Eight, + ); +}