Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

High-level compression options API #503

Merged
merged 41 commits into from
Dec 29, 2024
Merged
Changes from 31 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6e258b5
Introduce advanced compression settings
Shnatsel Sep 24, 2024
b431bbf
Make roundtrip tests try various compression modes
Shnatsel Sep 24, 2024
72df368
Rename AdvancedCompression to DeflateCompression, so that Compression…
Shnatsel Sep 24, 2024
54b89cf
Refactor the high-level API for setting the compression level. Semver…
Shnatsel Sep 25, 2024
c767438
Make the Compression setting also influence the filter choice
Shnatsel Sep 25, 2024
e7dd3dc
Fix compilation of the roundtrip fuzzer
Shnatsel Sep 25, 2024
8b08c9a
Merge branch 'master' into advanced-compression
Shnatsel Sep 26, 2024
1e83f78
Merge branch 'advanced-compression' into high-level-compression-control
Shnatsel Sep 26, 2024
6b06728
Disable fuzzing Compression::None to see if it's the culprit for non-…
Shnatsel Sep 26, 2024
b702a94
Compression::None was not the impostor.
Shnatsel Sep 26, 2024
aed727a
Rename None to NoCompression
Shnatsel Sep 27, 2024
dd547b0
Rename Default to Balanced
Shnatsel Sep 27, 2024
17e7229
Drop misleading mentions of photos
Shnatsel Sep 27, 2024
3194474
Correct the description of ultrafast mode
Shnatsel Sep 27, 2024
739a93a
Merge branch 'master' into high-level-compression-control
Shnatsel Sep 27, 2024
cc8956a
Switch the default filter over to adaptive
Shnatsel Sep 27, 2024
104a048
Add Fastest mode for Fdeflate+Up, switch Fast mode to Fdeflate+Adaptive
Shnatsel Sep 27, 2024
c8b19c9
Refactor FilterType and AdaptiveFilterType into a single public enum
Shnatsel Sep 27, 2024
30192da
Drop vestigial AdaptiveFilterType enum
Shnatsel Sep 27, 2024
f07106b
Rename FilterType to Filter
Shnatsel Sep 27, 2024
085fba9
Refactor set_compression: split the conversion from Compression to Fi…
Shnatsel Sep 27, 2024
571ca49
Make adaptive filtering the default in one more place
Shnatsel Sep 27, 2024
eecf56c
Fix compilation of benchmakring example
Shnatsel Sep 27, 2024
f7adc08
Fix doclinks
Shnatsel Sep 27, 2024
2eaf588
Fix one more doclink
Shnatsel Sep 27, 2024
d157df1
Update doc comments on the oddly numerous set_filter
Shnatsel Sep 27, 2024
98f0c29
Document the default on the Filter struct too
Shnatsel Sep 27, 2024
a96a3a2
Fix compilation of benchmark reexport module
Shnatsel Sep 27, 2024
111f7c2
Make Filter enum non-exhaustive to facilitate further experimentation…
Shnatsel Sep 27, 2024
7ba0fda
Document stability guarantees for in-depth compression controls
Shnatsel Sep 27, 2024
4c336eb
Use `Filter::None` for `Compression::Fastest` preset
Shnatsel Dec 5, 2024
df73363
Revert "Use `Filter::None` for `Compression::Fastest` preset"
Shnatsel Dec 5, 2024
76b219a
Record the rationale for Filter::Up in a comment
Shnatsel Dec 5, 2024
a10a266
Clarify comment
Shnatsel Dec 5, 2024
43489af
cargo fmt
Shnatsel Dec 5, 2024
5af8a95
Switch flate2 mode from using u32 compression level to u8, for a hope…
Shnatsel Dec 22, 2024
ca24d55
Correct comment
Shnatsel Dec 23, 2024
131d010
Merge branch 'master' into high-level-compression-control
Shnatsel Dec 29, 2024
a5990f3
cargo fmt
Shnatsel Dec 29, 2024
9a398c2
Update roundtrip target to the new encoding API. improves encoding pa…
Shnatsel Dec 29, 2024
f9d27be
drop unused imports
Shnatsel Dec 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions benches/unfilter.rs
Original file line number Diff line number Diff line change
@@ -9,17 +9,12 @@

use criterion::{criterion_group, criterion_main, Criterion, Throughput};
use png::benchable_apis::unfilter;
use png::FilterType;
use png::Filter;
use rand::Rng;

fn unfilter_all(c: &mut Criterion) {
let bpps = [1, 2, 3, 4, 6, 8];
let filters = [
FilterType::Sub,
FilterType::Up,
FilterType::Avg,
FilterType::Paeth,
];
let filters = [Filter::Sub, Filter::Up, Filter::Avg, Filter::Paeth];
for &filter in filters.iter() {
for &bpp in bpps.iter() {
bench_unfilter(c, filter, bpp);
@@ -30,7 +25,7 @@ fn unfilter_all(c: &mut Criterion) {
criterion_group!(benches, unfilter_all);
criterion_main!(benches);

fn bench_unfilter(c: &mut Criterion, filter: FilterType, bpp: u8) {
fn bench_unfilter(c: &mut Criterion, filter: Filter, bpp: u8) {
let mut group = c.benchmark_group("unfilter");

fn get_random_bytes<R: Rng>(rng: &mut R, n: usize) -> Vec<u8> {
20 changes: 8 additions & 12 deletions examples/corpus-bench.rs
Original file line number Diff line number Diff line change
@@ -43,20 +43,16 @@ fn run_encode(
encoder.set_depth(bit_depth);
encoder.set_compression(match args.speed {
Speed::Fast => png::Compression::Fast,
Speed::Default => png::Compression::Default,
Speed::Best => png::Compression::Best,
Speed::Default => png::Compression::Balanced,
Speed::Best => png::Compression::High,
});
encoder.set_filter(match args.filter {
Filter::None => png::FilterType::NoFilter,
Filter::Sub => png::FilterType::Sub,
Filter::Up => png::FilterType::Up,
Filter::Average => png::FilterType::Avg,
Filter::Paeth => png::FilterType::Paeth,
Filter::Adaptive => png::FilterType::Paeth,
});
encoder.set_adaptive_filter(match args.filter {
Filter::Adaptive => png::AdaptiveFilterType::Adaptive,
_ => png::AdaptiveFilterType::NonAdaptive,
Filter::None => png::Filter::NoFilter,
Filter::Sub => png::Filter::Sub,
Filter::Up => png::Filter::Up,
Filter::Average => png::Filter::Avg,
Filter::Paeth => png::Filter::Paeth,
Filter::Adaptive => png::Filter::Adaptive,
});
let mut encoder = encoder.write_header().unwrap();
encoder.write_image_data(image).unwrap();
9 changes: 4 additions & 5 deletions fuzz/fuzz_targets/roundtrip.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![no_main]

use libfuzzer_sys::fuzz_target;
use png::{FilterType, ColorType, BitDepth};
use png::{Filter, ColorType, BitDepth};

fuzz_target!(|data: (u8, u8, u8, u8, u8, Vec<u8>, Vec<u8>)| {
if let Some((raw, encoded)) = encode_png(data.0, data.1, data.2, data.3, data.4, &data.5, &data.6) {
@@ -16,7 +16,7 @@ fn encode_png<'a>(width: u8, filter: u8, compression: u8, color_type: u8, raw_bi
// Convert untyped bytes to the correct types and validate them:
let width = width as u32;
if width == 0 { return None };
let filter = FilterType::from_u8(filter)?;
let filter = Filter::from_u8(filter)?;
let bit_depth = BitDepth::from_u8(raw_bit_depth)?;
let max_palette_length = 3 * u32::pow(2, raw_bit_depth as u32) as usize;
let mut palette = raw_palette;
@@ -31,9 +31,8 @@ fn encode_png<'a>(width: u8, filter: u8, compression: u8, color_type: u8, raw_bi
let compression = match compression {
0 => png::Compression::Default,
1 => png::Compression::Fast,
2 => png::Compression::Best,
3 => png::Compression::Huffman,
4 => png::Compression::Rle,
2 => png::Compression::High,
3 => png::Compression::None,
_ => return None,
};

5 changes: 3 additions & 2 deletions src/benchable_apis.rs
Original file line number Diff line number Diff line change
@@ -2,12 +2,13 @@
//! This module is gated behind the "benchmarks" feature.

use crate::common::BytesPerPixel;
use crate::filter::FilterType;
use crate::filter::{Filter, RowFilter};
use crate::{BitDepth, ColorType, Info};

/// Re-exporting `unfilter` to make it easier to benchmark, despite some items being only
/// `pub(crate)`: `fn unfilter`, `enum BytesPerPixel`.
pub fn unfilter(filter: FilterType, tbpp: u8, previous: &[u8], current: &mut [u8]) {
pub fn unfilter(filter: Filter, tbpp: u8, previous: &[u8], current: &mut [u8]) {
let filter = RowFilter::from_method(filter).unwrap(); // RowFilter type is private
let tbpp = BytesPerPixel::from_usize(tbpp as usize);
crate::filter::unfilter(filter, tbpp, previous, current)
}
119 changes: 95 additions & 24 deletions src/common.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Common types shared between the encoder and decoder
use crate::text_metadata::{EncodableTextChunk, ITXtChunk, TEXtChunk, ZTXtChunk};
#[allow(unused_imports)] // used by doc comments only
use crate::Filter;
use crate::{chunk, encoder};
use io::Write;
use std::{borrow::Cow, convert::TryFrom, fmt, io};
@@ -303,33 +305,103 @@ impl AnimationControl {
}

/// The type and strength of applied compression.
///
/// This is a simple, high-level interface that will automatically choose
/// the appropriate DEFLATE compression mode and PNG filter.
///
/// If you need more control over the encoding paramters,
/// you can set the [DeflateCompression] and [Filter] manually.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum Compression {
/// Default level
Default,
/// Fast minimal compression
Fast,
/// Higher compression level
/// No compression whatsoever. Fastest, but results in large files.
NoCompression,
/// Extremely fast but light compression.
Fastest,
/// Extremely fast compression with a decent compression ratio.
///
/// Best in this context isn't actually the highest possible level
/// the encoder can do, but is meant to emulate the `Best` setting in the `Flate2`
/// library.
Best,
#[deprecated(
since = "0.17.6",
note = "use one of the other compression levels instead, such as 'fast'"
)]
Huffman,
#[deprecated(
since = "0.17.6",
note = "use one of the other compression levels instead, such as 'fast'"
)]
Rle,
/// Significantly outperforms libpng and other popular encoders
/// by using a [specialized DEFLATE implementation tuned for PNG](https://crates.io/crates/fdeflate),
Comment on lines +333 to +334
Copy link
Contributor

@okaneco okaneco Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A link backing up these claims would be nice here and in the new updated readme. It requires a bit of digging and research on the reader's part to confirm this.

/// while still providing better compression ratio than the fastest modes of other encoders.
Fast,
/// Balances encoding speed and compression ratio
Balanced,
/// Spend more time to produce a slightly smaller file than with `Default`
High,
}

impl Default for Compression {
fn default() -> Self {
Self::Default
Self::Balanced
}
}

/// Advanced compression settings with more customization options than [Compression].
///
/// Note that this setting only affects DEFLATE compression.
/// Another setting that influences the compression ratio and lets you choose
/// between encoding speed and compression ratio is the [Filter].
///
/// ### Stability guarantees
///
/// The implementation details of DEFLATE compression may evolve over time,
/// even without a semver-breaking change to the version of `png` crate.
///
/// If a certain compression setting is superseded by other options,
/// it may be marked deprecated and remapped to a different option.
/// You will see a deprecation notice when compiling code relying on such options.
#[non_exhaustive]
#[derive(Debug, Clone, Copy)]
pub enum DeflateCompression {
/// Do not compress the data at all.
///
/// Useful for incompressible images,
/// or when speed is paramount and you don't care about size at all.
///
/// This mode also disables filters, forcing [Filter::NoFilter].
NoCompression,

/// Excellent for creating lightly compressed PNG images very quickly.
///
/// Uses the [fdeflate](https://crates.io/crates/fdeflate) crate under the hood
/// to achieve speeds far exceeding what libpng is capable of
/// while still providing a decent compression ratio.
///
/// Images encoded in this mode can also be decoded by the `png` crate slightly faster than usual.
/// Other decoders (e.g. libpng) do not get a decoding speed boost from this mode.
FdeflateUltraFast,

/// Uses [flate2](https://crates.io/crates/flate2) crate with the specified [compression level](flate2::Compression::new).
///
/// Flate2 has several backends that make different trade-offs.
/// See the flate2 documentation for the available backends for more information.
Flate2(u32),
Copy link
Contributor

@fintelia fintelia Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After more experimentation with adding higher compression levels to fdeflate, I'm pretty confident that we'd be able to expose "standard" compression levels ranging from 1 to 9 for any alternative we switch to.

One aspect is that the various different flate2 backends actually have a somewhat significant amount of variability about what each compression level means, so we wouldn't have to worry too much about exactly calibrating to match existing behavior. But additionally, these algorithms have like 5 to 10 different knobs to tune. So the challenge is really condensing them into a single number (which you want to do regardless, because users won't want to deal with setting all the parameters!)

Long story short, I think we should drop the Flate2 naming and just call this Level or something to that effect. And perhaps switch to a u8 given that values above 9 have no impact.

Copy link
Contributor Author

@Shnatsel Shnatsel Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: since this is the advanced API, not the high-level one, I'd prefer to keep it Flate2 and also keep the documentation about the way it's implemented. We would add a deprecation notice to it if we ever start emulating it like we did with Huffman and Rle.

I am going to calibrate wondermagick for specific compression ratios against a known reference with a specific flate2 backend, zlib-rs. If/when the underlying implementation changes away from flate2, I would much rather get a warning from the compiler and a deprecation message saying that it's now emulated so that I could re-evaluate the tagets for the new backend and adjust the mapping on my end, as opposed to the targets silently changing.

Knowing that flate2 is no longer used would also let me drop the direct flate2 dependency from my crate that I added to select the backend. Although perhaps png should just expose a zlib-rs feature and not have users add a direct flate2 dependency at all? And we would just make it a no-op feature if we ever move away from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On u32 vs u8: originally I chose to stick to the flate2 API which used u32, but you're right - it makes no sense and just confuses people. I've switched it over to u8.

// TODO: Zopfli?
}

impl Default for DeflateCompression {
fn default() -> Self {
Self::from_simple(Compression::Balanced)
}
}

impl DeflateCompression {
pub(crate) fn from_simple(value: Compression) -> Self {
match value {
Compression::NoCompression => Self::NoCompression,
Compression::Fastest => Self::FdeflateUltraFast,
Compression::Fast => Self::FdeflateUltraFast,
Compression::Balanced => Self::Flate2(flate2::Compression::default().level()),
Compression::High => Self::Flate2(flate2::Compression::best().level()),
}
}

pub(crate) fn closest_flate2_level(&self) -> flate2::Compression {
match self {
DeflateCompression::NoCompression => flate2::Compression::none(),
DeflateCompression::FdeflateUltraFast => flate2::Compression::new(1),
DeflateCompression::Flate2(level) => flate2::Compression::new(*level),
}
}
}

@@ -494,7 +566,8 @@ pub struct Info<'a> {

pub frame_control: Option<FrameControl>,
pub animation_control: Option<AnimationControl>,
pub compression: Compression,
/// Controls the DEFLATE compression options. Influences the trade-off between compression speed and ratio, along with filters.
pub compression_deflate: DeflateCompression,
/// Gamma of the source system.
/// Set by both `gAMA` as well as to a replacement by `sRGB` chunk.
pub source_gamma: Option<ScaledFloat>,
@@ -530,9 +603,7 @@ impl Default for Info<'_> {
pixel_dims: None,
frame_control: None,
animation_control: None,
// Default to `deflate::Compression::Fast` and `filter::FilterType::Sub`
// to maintain backward compatible output.
compression: Compression::Fast,
compression_deflate: DeflateCompression::default(),
source_gamma: None,
source_chromaticities: None,
srgb: None,
4 changes: 2 additions & 2 deletions src/decoder/mod.rs
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ use crate::chunk;
use crate::common::{
BitDepth, BytesPerPixel, ColorType, Info, ParameterErrorKind, Transformations,
};
use crate::filter::{unfilter, FilterType};
use crate::filter::{unfilter, RowFilter};

pub use interlace_info::InterlaceInfo;
use interlace_info::InterlaceInfoIter;
@@ -723,7 +723,7 @@ impl<R: Read> Reader<R> {
let (prev, row) = self.data_stream.split_at_mut(self.current_start);

// Unfilter the row.
let filter = FilterType::from_u8(row[0]).ok_or(DecodingError::Format(
let filter = RowFilter::from_u8(row[0]).ok_or(DecodingError::Format(
FormatErrorInner::UnknownFilterMethod(row[0]).into(),
))?;
unfilter(
Loading