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

Add EncoderParams to make predictor transform optional #111

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

w-flo
Copy link

@w-flo w-flo commented Sep 18, 2024

Hi,

Thank you for your work on this!

I noticed map tiles rendered from SVG compress much better with two tweaks: disable the predictor transform, and use a color cache (almost 40% smaller files with 11 bit color cache).

Now I guess that only works for some types of image, and I can't come up with good heuristics for this. And testing all kinds of encoding parameters every time the encoder runs to pick the best combination seems expensive – I want these map tiles to compress fast, which they currently do.

So I think the API needs some way for the user to specify encoding parameters. I'm not sure if my PR offers a good design for this. It's just something that works for me, but I guess with possible "automatic parameter choice heuristics" to be added in the future by someone else, maybe a better way to design the API can be found, that allows users to set params that they care about, while leaving other params to be determined automatically (probably offering different speed settings).

I'm marking this as a draft for this reason, since I'm very open to suggestions on how to change the API.

Anyway, this PR contains a new "EncoderParams" struct that currently has only one member for disabling or enabling the predictor transform, and there's a second commit that adds a image buffer comparison to the libwebp roundtrip test to hopefully notice if the encoding is buggy. The color cache is not included, to focus on API first.

This shrinks file size when compressing map tiles (rendered from SVG) by
approx. 10%.
Make sure the roundtrip actually results in the same bytes. Also test
roundtrip using non-default EncoderParams.
/// Allows fine-tuning some encoder parameters.
///
/// Pass to [`WebPEncoder::set_params()`].
#[non_exhaustive]
Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure if I should mark this #[non_exhaustive].

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's a good idea to have non-exhaustive in public APIs.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, but it makes using this struct a bit less ergonomic, because

EncoderParams {
    use_predictor_transform: false,
    ..Default::default()
}

(like in the test in my second commit) is forbidden outside of the image-webp crate with non-exhaustive structs. It needs to be

let mut p = EncoderParams::default();
p.use_predictor_transform = false;

instead.

@fintelia
Copy link
Contributor

I like this approach.

libwebp uses heuristics to select one of six combinations of transforms (They call the predictor transform "spatial" in that list). Except for the lowest compression level, where libwebp just has a single hard-code pick, and the highest level, which literally does try all of them.

There's separate logic to select whether (and how large) of a color cache to include. I haven't looked through precisely how it works yet, though.

@w-flo
Copy link
Author

w-flo commented Sep 19, 2024

I like this approach.

I had considered some more, like using a Builder pattern, or only offering a params_mut() method like encoder.params_mut().use_predictor_pattern = false;. I wasn't sure which would work best so I just picked one randomly and thought I'd leave it up to you to figure out what the best way forward would be, and then update my PR as needed. :-) So let me know if you want me to change anything.

libwebp uses heuristics to select one of six combinations of transforms (They call the predictor transform "spatial" in that list). Except for the lowest compression level, where libwebp just has a single hard-code pick, and the highest level, which literally does try all of them.

There's separate logic to select whether (and how large) of a color cache to include. I haven't looked through precisely how it works yet, though.

Well, that does seem like a lot of work to get right, i.e. picking good trade-offs for the various speed settings. I'm glad the "only subtract green transform + big color cache" seems to work pretty decently for all the files I want to compress! Even at -z 9 (slowest), cwebp only shrinks the file size of one test file by 6% more and is really slow. So going forward, I hope image-webp still supports a fast way to say "don't do heuristics, just use these params, trust me!".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants