Lossless conversion for webp by morsssss · Pull Request #7348 · php/php-src

morsssss · 2021-08-06T22:48:46Z

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd. Changing "quantization" to "quality" as in libgd. Adding test.

ext/gd/tests/webp_basic.phpt

morsssss · 2021-08-10T14:40:58Z

P.S. This is the parallel PR in libgd: libgd/libgd#698

ext/gd/tests/webp_basic.phpt

nikic

Looks good to me. @cmb69?

cmb69 · 2021-08-12T08:59:01Z

ext/gd/gd.c

+	/* constant for webp encoding */
+	REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
+


What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change

/* constant for webp encoding */

REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

#ifdef gdWebpLossless

/* constant for webp encoding */

REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);

#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Now I see precedent for this in the code:

#ifdef gdEffectMultiply REGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT); #endif

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

cmb69

Thank you!

morsssss · 2021-08-12T16:24:21Z

Thank you!

Thank you for your consistent patience and good cheer!

> GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Includes unit tests. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb

> GD: > * Avif support is now available through the `imagecreatefromavif()` and > `imageavif()` functions, if libgd has been built with avif support. While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration... Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247 * php/php-src#7026 * php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373 > GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb

* PHP 8.1 | MigrationGuide/New constants: add missing constants [1] > * Added CURLOPT_DOH_URL option > * Added certificate blob options when for libcurl >= 7.71.0: > > CURLOPT_ISSUERCERT_BLOB > CURLOPT_PROXY_ISSUERCERT > CURLOPT_PROXY_ISSUERCERT_BLOB > CURLOPT_PROXY_SSLCERT_BLOB > CURLOPT_PROXY_SSLKEY_BLOB > CURLOPT_SSLCERT_BLOB > CURLOPT_SSLKEY_BLOB Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229 * php/php-src#6612 * php/php-src@3dad63b * php/php-src#7194 * php/php-src@b11785c * PHP 8.1 | MigrationGuide/New constants: add missing constants [2] > GD: > * Avif support is now available through the `imagecreatefromavif()` and > `imageavif()` functions, if libgd has been built with avif support. While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration... Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247 * php/php-src#7026 * php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373 > GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb * PHP 8.1 | MigrationGuide/New constants: add missing constants [3] > Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix * php/php-src#6608 * php/php-src@ebca8de * PHP 8.1 | MigrationGuide/New constants: add missing constants [4] Refs: * https://wiki.php.net/rfc/readonly_properties_v2 * php/php-src#7089 * php/php-src@6780aaa * PHP 8.1 | MigrationGuide/New constants: add missing constants [5] While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357 While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well... Refs: * php/php-src#6922 * php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381 Co-authored-by: jrfnl <[email protected]> Closes GH-1449.

* PHP 8.1 | MigrationGuide/New constants: add missing constants [1] > * Added CURLOPT_DOH_URL option > * Added certificate blob options when for libcurl >= 7.71.0: > > CURLOPT_ISSUERCERT_BLOB > CURLOPT_PROXY_ISSUERCERT > CURLOPT_PROXY_ISSUERCERT_BLOB > CURLOPT_PROXY_SSLCERT_BLOB > CURLOPT_PROXY_SSLKEY_BLOB > CURLOPT_SSLCERT_BLOB > CURLOPT_SSLKEY_BLOB Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229 * php/php-src#6612 * php/php-src@3dad63b * php/php-src#7194 * php/php-src@b11785c * PHP 8.1 | MigrationGuide/New constants: add missing constants [2] > GD: > * Avif support is now available through the `imagecreatefromavif()` and > `imageavif()` functions, if libgd has been built with avif support. While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration... Refs: * https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247 * php/php-src#7026 * php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373 > GD: > * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as > quality. This constant is only defined, if a libgd is used which supports > lossless WebP encoding. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571 * php/php-src#7348 * php/php-src@eb6c9eb * PHP 8.1 | MigrationGuide/New constants: add missing constants [3] > Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix * php/php-src#6608 * php/php-src@ebca8de * PHP 8.1 | MigrationGuide/New constants: add missing constants [4] Refs: * https://wiki.php.net/rfc/readonly_properties_v2 * php/php-src#7089 * php/php-src@6780aaa * PHP 8.1 | MigrationGuide/New constants: add missing constants [5] While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well... Refs: * php/php-src#6868 * php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357 While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well... Refs: * php/php-src#6922 * php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381 Co-authored-by: jrfnl <[email protected]> Closes phpGH-1449.

Lossless conversion for webp

924c41a

Propagating lossless conversion from libgd to our bundled gd. Changing "quantization" to "quality" as in libgd. Adding test.

krakjoe added Feature Waiting on Review labels Aug 8, 2021

andypost reviewed Aug 9, 2021

View reviewed changes

ext/gd/tests/webp_basic.phpt Show resolved Hide resolved

nikic reviewed Aug 10, 2021

View reviewed changes

ext/gd/tests/webp_basic.phpt Outdated Show resolved Hide resolved

nikic removed the Waiting on Review label Aug 11, 2021

Created IMG_WEBP_LOSSLESS constant

337afb4

nikic approved these changes Aug 12, 2021

View reviewed changes

cmb69 reviewed Aug 12, 2021

View reviewed changes

Guard against older versions of gd that lack gdWebpLossless

eab1110

cmb69 approved these changes Aug 12, 2021

View reviewed changes

cmb69 closed this in eb6c9eb Aug 12, 2021

jrfnl mentioned this pull request Mar 9, 2022

PHP 8.1 | NewConstants: account for PHP 8.1 changes PHPCompatibility/PHPCompatibility#1324

Merged

jrfnl mentioned this pull request Mar 11, 2022

PHP 8.1 | MigrationGuide/New constants: add missing constants php/doc-en#1449

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lossless conversion for webp#7348

Lossless conversion for webp#7348
morsssss wants to merge 3 commits intophp:masterfrom
morsssss:webp-lossless

morsssss commented Aug 6, 2021

Uh oh!

Uh oh!

morsssss commented Aug 10, 2021

Uh oh!

Uh oh!

nikic left a comment

Uh oh!

cmb69 Aug 12, 2021

Uh oh!

morsssss Aug 12, 2021

Uh oh!

cmb69 Aug 12, 2021

Uh oh!

morsssss Aug 12, 2021

Uh oh!

cmb69 Aug 12, 2021

Uh oh!

cmb69 left a comment

Uh oh!

morsssss commented Aug 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		/* constant for webp encoding */
		REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS \| CONST_PERSISTENT);

Conversation

morsssss commented Aug 6, 2021

Uh oh!

Uh oh!

morsssss commented Aug 10, 2021

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

cmb69 Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

morsssss Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

cmb69 Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

morsssss Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

cmb69 Aug 12, 2021

Choose a reason for hiding this comment

Uh oh!

cmb69 left a comment

Choose a reason for hiding this comment

Uh oh!

morsssss commented Aug 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants