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 WebP encoding support #1784

Merged
merged 3 commits into from
Sep 22, 2022
Merged

Add WebP encoding support #1784

merged 3 commits into from
Sep 22, 2022

Conversation

cycraig
Copy link
Contributor

@cycraig cycraig commented Sep 7, 2022

Description

This PR adds WebPEncoder, which wraps the Simple Encoding API from the native libwebp library using the webp crate.

The encoding functionality is gated behind a new webp-encoder feature, which is not enabled by default and completely separate from the webp feature. This follows the convention of the current avif/avif-encoder/avif-decoder features which are separate due to the native depedency.

With webp-encoder enabled, the following just works now:

let im = image::open("Lenna.png").unwrap();
im.save("Lenna.webp").unwrap();

The above defaults to a lossy encoding value of 80. The value was chosen since image defaults to 75 for JPEG encoding, and this blog post suggests an equivalent quality level for WebP would be around 77, hence rounded up to 80. On the other hand, the official cwebp encoder utility uses a default quality of 75. Both options still output smaller sizes than JPEG.

WebPEncoder can also be used directly, to set the desired quality:

let file = File::create("Lenna.webp").unwrap();
let quality = WebPQuality::lossless(); // or WebPQuality::lossy(70) etc.
let encoder = WebPEncoder::new_with_quality(file, quality);
encoder.write_image(im.as_bytes(), im.width(), im.height(), im.color()).unwrap();

Motivation

WebP encoding support has been a long-requested feature for image (#582), and it would be good to provide canonical support for it.

It's been 6 years since the original issue with no sign of a pure Rust implementation for WebP encoding (it's a lot of work), so using bindings to libwebp seems like the only viable approach for the foreseeable future. The Simple Encoding API is sufficient for basic use-cases (no animation etc.) and is unlikely to change, so this shouldn't require much maintenance.

The webp crate (which uses libwebp-sys) was chosen because it's maintained and "just works", in that it builds and runs on every target and architecture I've tested on without any problems or additional dependencies required, unlike Avif.

Even though formats like Avif are intended to supplant WebP, they still lack the complete browser support that WebP has, making it preferable for a while longer.

Relevant issues

This fixes the following issue: #582.

Type of change

  • Non-breaking addition.

How the change has been tested

  • Added new unit tests: cargo test --no-default-features --features webp-encoder
  • Built and tested on Ubuntu, MacOS, Windows: https://github.com/cycraig/image/actions/runs/3010465918
  • Statically cross-compiled to musl: cargo build --release --target=x86_64-unknown-linux-musl --no-default-features --features webp-encoder (requires musl-gcc).
  • Used im.save to convert images on my website from JPEG to WebP successfully.

License

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to chose either at their option.

Note also the licenses of the new dependencies:

  • webp crate is dual licensed under MIT/Apache-2.0.
  • libwebp-sys crate uses the MIT license.
  • The native libwebp library from Google (compiled via libwebp-sys) uses a modified BSD 3-clause license. It's still permissive, just with a non-endorsement clause. Again, this only affects developers that ship code with the non-default webp-encoder feature enabled, and it's mostly compatible as far as I know.

@cycraig
Copy link
Contributor Author

cycraig commented Sep 10, 2022

Rebased the changes after #1787 was merged, the CI should pass now.

Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

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

👍 with nits addressed (some just warrant short discussion). Reasoning: for accepting: The native dependency isn't required by default, and we keep it an implementaiton detail in terms of API. So yeah, C dependency is okayish and as you say, most practical.

src/codecs/webp/encoder.rs Show resolved Hide resolved
src/codecs/webp/encoder.rs Outdated Show resolved Hide resolved
src/codecs/webp/encoder.rs Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@djc
Copy link
Contributor

djc commented Sep 22, 2022

Friendly ping, I think this is ready to go?

@fintelia fintelia merged commit 8624b71 into image-rs:master Sep 22, 2022
@djc
Copy link
Contributor

djc commented Sep 22, 2022

Thanks! Is there any known ETA for getting this out to crates.io?

@HeroicKatora
Copy link
Member

@djc The evening of saturday, ping me if not. There's another release outstanding too

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.

4 participants