Support writing lossless WebP #698

adamsilverstein · 2021-05-05T15:24:38Z

Originally #327

Lossless WebP is a rather interesting alternative to PNG, and already
supported by gdImageCreateFromWebp*(), so we add support for
gdImageWebp*(), too.

We can stick with the existing API, using the quality parameter to
request lossless encoding if it is set to gdWebpLossless, which we
define to PHP_INT (to avoid adding a new dependency to gd.h, we hard-
code the value – we're assuming sizeof(int)==4 anyway).

Lossless WebP is a rather interesting alternative to PNG, and already supported by `gdImageCreateFromWebp*()`, so we add support for `gdImageWebp*()`, too. We can stick with the existing API, using the quality parameter to request lossless encoding if it is set to `gdWebpLossless`, which we define to `PHP_INT` (to avoid adding a new dependency to gd.h, we hard- code the value – we're assuming `sizeof(int)==4` anyway).

# Conflicts: # tests/webp/.gitignore # tests/webp/CMakeLists.txt # tests/webp/Makemodule.am

adamsilverstein · 2021-05-05T20:29:23Z

@cmb69 I'm going to test this out in WordPress, can you confirm that for previous versions of libgd, if I pass a quality value of 101 to imagewebp will it use 100 instead (or would I get an error)?

See libgd/libgd#698.

cmb69 · 2021-05-05T20:45:44Z

The quality is passed to WebPEncodeRGBA(); not sure how that function handles out of range values.

cmb69 · 2021-05-06T10:44:08Z

Thank you!

There is a caveat, though: WebPEncodeLosslessRGBA() is only available as of libwebp 1.2.0, but we do not require any particular libwebp version so far. Either we raise the requirements, or we must use WebPEncodeLosslessRGBA() only if available.

vapier · 2021-05-06T21:01:36Z

libwebp-1.2.0 was only released a few months ago, so we can't require it :/

cmb69 · 2021-05-06T21:17:04Z

Sorry, I've been confused by the GH UI. Actually, it should be available as of 0.2.0, released 2012.

adamsilverstein · 2021-05-11T19:28:46Z

Sorry, I've been confused by the GH UI. Actually, it should be available as of 0.2.0, released 2012.

So does that mean the code is ok as is, or are there additional changes needed?

I'll look into WebPEncodeRGBA, I can't tell from the code, but I can try testing it with WordPress to see how it behaves. I just want to be sure passing 101 to an older version of GD won't break anything, otherwise we would need to check the GD version before using the feature.

cmb69 · 2021-05-11T20:34:51Z

I'm not sure whether not supporting <= 0.1 is an issue; might be okay to just document that fact. Of course, it would be better to check during configure that libwebp >= 0.2 is available, and likely still better to check that WebPEncodeRGBA() is available.

The quality is passed to WebPEncodeRGBA(); not sure how that function handles out of range values.

I still don't know. Better test it. :)

adamsilverstein · 2021-05-11T20:51:26Z

it would be better to check during configure that libwebp >= 0.2 is available, and likely still better to check that WebPEncodeRGBA() is available

I'll see if I can figure out how to add that in configure, thanks for the lead. hopefully I can find a similar capability check already in use for some other part.

cmb69 · 2021-05-11T20:56:10Z

I'll see if I can figure out how to add that in configure, thanks for the lead. hopefully I can find a similar capability check already in use for some other part.

See, for instance,

libgd/CMakeLists.txt

Lines 128 to 133 in 82d2609

    
           	IF (ENABLE_AVIF) 
        
           		FIND_PACKAGE(libavif 0.8.2 REQUIRED CONFIG) 
        
           		SET(HAVE_LIBAVIF 1) 
        
           		SET(AVIF_LIBRARIES avif) 
        
           		SET(AVIF_FOUND 1) 
        
           	ENDIF (ENABLE_AVIF)

Note that GD also has an autotools build chain, which also needs respective checkks. I think we can ignore the Windows build chains (at least for now).

morsssss · 2021-05-11T22:34:53Z

If it helps, we did some similar version checking in the avif PR.

adamsilverstein · 2021-05-12T22:29:05Z

Of course, it would be better to check during configure that libwebp >= 0.2 is available

To clarify - do you mean adding that as a requirement in the existing ENABLE_WEBP check, or adding a new check that sets something like ENABLE_WEBP_LOSSLESS?

vapier · 2021-05-12T23:36:50Z

requiring libwebp 0.2.0 is fine, so just make it the min version all the time and call it a day

adamsilverstein · 2021-05-13T14:51:34Z

I took a pass, however I am working pretty blindly here - meaning I'm not sure what I am doing 🤷🏼 😄 . I question if this change works because I tried higher version number and configure/make still worked for me locally.

cmb69 · 2021-05-13T14:55:20Z

Did you run bootstrap.sh?

adamsilverstein · 2021-05-13T14:57:25Z

Did you run bootstrap.sh?

Aha!

configure.ac

pierrejoye · 2021-07-19T04:01:13Z

good morning :)

Thanks for this PR!

anything left here? Need support?

morsssss · 2021-07-19T13:57:59Z

@adamsilverstein , @pierrejoye , it strikes me that we should port this change over to PHP right away. I don't know if we'll get it in before feature freeze, which is tomorrow, but it's worth a try...

https://wiki.php.net/todo/php81

cmb69

In my opinion, this is still good for PHP-8.1 even after feature freeze (should happen before ABI freeze, though).

morsssss · 2021-07-19T15:28:31Z

In my opinion, this is still good for PHP-8.1 even after feature freeze (should happen before ABI freeze, though).

what is ABI freeze? (I see the next milestone looks like Beta 1 on 22 July)

cmb69 · 2021-07-19T15:42:13Z

what is ABI freeze?

That would be tagging of RC1 (Aug, 31st)

morsssss · 2021-07-23T21:07:35Z

On the libgd side... is this ready to merge?

cmb69 and others added 3 commits October 9, 2016 00:47

Set gdWebpLossless as threshold at 101.

3826c44

Merge branch 'master' into webp-lossless

a2b3ba0

# Conflicts: # tests/webp/.gitignore # tests/webp/CMakeLists.txt # tests/webp/Makemodule.am

adamsilverstein force-pushed the webp-lossless branch from 496734d to 278d1b0 Compare May 5, 2021 17:35

Clean formatting.

b3d1986

adamsilverstein force-pushed the webp-lossless branch from 278d1b0 to b3d1986 Compare May 5, 2021 17:50

adamsilverstein added a commit to adamsilverstein/wordpress-develop that referenced this pull request May 5, 2021

Use WebP lossless for GD when available (quality of 101).

00c854a

See libgd/libgd#698.

adamsilverstein mentioned this pull request May 5, 2021

Use WebP lossless for GD when available (quality of 101). WordPress/wordpress-develop#1230

Closed

adamsilverstein added a commit to adamsilverstein/wordpress-develop that referenced this pull request May 5, 2021

Use WebP lossless for GD when available (quality of 101).

c675d3d

See libgd/libgd#698.

adamsilverstein added a commit to adamsilverstein/wordpress-develop that referenced this pull request May 5, 2021

Use WebP lossless for GD when available (quality of 101).

ad6b57e

See libgd/libgd#698.

Require libwebp >= 0.2.0.

5ef62f0

Remove redundant version check.

97e6c20

adamsilverstein commented May 13, 2021

View reviewed changes

configure.ac Show resolved Hide resolved

vapier approved these changes May 17, 2021

View reviewed changes

cmb69 approved these changes Jul 19, 2021

View reviewed changes

cmb69 merged commit 8accfa9 into libgd:master Jul 23, 2021

morsssss mentioned this pull request Aug 10, 2021

Lossless conversion for webp php/php-src#7348

Closed

adamsilverstein mentioned this pull request Feb 9, 2022

Improve support for WebP lossless WordPress/performance#13

Closed

brandonpayton mentioned this pull request May 13, 2024

PHP: Support libwebp on PHP 7.0 – 7.3 WordPress/wordpress-playground#1381

Closed

Support writing lossless WebP #698

Support writing lossless WebP #698

Uh oh!

Conversation

adamsilverstein commented May 5, 2021

Uh oh!

adamsilverstein commented May 5, 2021

Uh oh!

cmb69 commented May 5, 2021

Uh oh!

cmb69 commented May 6, 2021

Uh oh!

vapier commented May 6, 2021

Uh oh!

cmb69 commented May 6, 2021

Uh oh!

adamsilverstein commented May 11, 2021

Uh oh!

cmb69 commented May 11, 2021

Uh oh!

adamsilverstein commented May 11, 2021

Uh oh!

cmb69 commented May 11, 2021

Uh oh!

morsssss commented May 11, 2021

Uh oh!

adamsilverstein commented May 12, 2021

Uh oh!

vapier commented May 12, 2021

Uh oh!

adamsilverstein commented May 13, 2021

Uh oh!

cmb69 commented May 13, 2021

Uh oh!

adamsilverstein commented May 13, 2021

Uh oh!

Uh oh!

pierrejoye commented Jul 19, 2021

Uh oh!

morsssss commented Jul 19, 2021

Uh oh!

cmb69 left a comment

Choose a reason for hiding this comment

Uh oh!

morsssss commented Jul 19, 2021

Uh oh!

cmb69 commented Jul 19, 2021

Uh oh!

morsssss commented Jul 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants