Let GDToolkit support AVIF image format

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased

it's RTBC IMO but needs summary update and CR, probably release note as well

Added a simple CR and RN snippet https://www.drupal.org/node/3348348 →

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased and applied suggestion for strict comparison

FYI, it seem there are problems with the diff.

$ git ac https://git.drupalcode.org/project/drupal/-/merge_requests/936.diff
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7846    0  7846    0     0  20701      0 --:--:-- --:--:-- --:--:-- 20701
error: missing binary patch data for 'core/tests/fixtures/files/img-test.avif'
error: binary patch does not apply to 'core/tests/fixtures/files/img-test.avif'
error: core/tests/fixtures/files/img-test.avif: patch does not apply

@quietone that seems like a gitlab issue. The .patch URL (instead of .diff) shows the binary content of the avif file.

@mstrelan, thanks. I forgot about the .patch version

I think we can add support independently, but both this and the existing webp support make ✨ Add fallback format support to responsive images Needs work more important so that sites can actually use it. Adding as a related issue.

Does the GD toolkit already handle whether the webserver has support for AVIF - i.e. does it only show in the list of options if it's actually going to work?

The check was added in #3116611: Add a requirements check for GD support of allowed image types →

but not every place in core using the filtered list of formats, probably there was an issue about add general requirements check

There's a set, hope it addresses #116 points

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

There's binary file in patch, so to apply patch it needs to download it and apply using git apply 936.patch or use patch -p1 -i 936.diff (it should be downloaded https://git.drupalcode.org/project/drupal/-/merge_requests/936.diff using .diff instead of .patch)

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@catch re #116 - yes the toolkit will tell if AVIF is available, but that check is unfortunately not enough. In order to have FULL support, PHP needs to be compiled with an AVIF codec, and that’s non-trivial, see #100 and #103; besides, there’s no way to detect automatically if the codec is installed, see #104.

Thanks @mondrake I had read that earlier in the issue but had forgotten about it by now - I've added both of those to the issue summary. Also thanks to @andypost for opening them and working on them!

I wonder a bit if we should do some kind of kludgy support detection by catching the error in the meantime until those issues are fixed (or at least until AVIF is more widely available?), not sure where to put that though and if it's expensive we might have to cache the results or something.

Meantime animated AVIS added to latest Firefox https://www.mozilla.org/en-US/firefox/113.0/releasenotes/

Given the bleeding edge support (php 8.2) for AVIF, and complexity in it being compiled into PHP, it occurred to me that there should be an option to work around those limitations in contrib space. Alas, there is no conversion image style effect feature that I could find, so I opened up a feature request in what seemed like the most logical place to put it.

✨ Add an AVIF conversion image styles effect Active

Sadly that module doesn't seem like it has been getting a lot of love, use, or maintenance for a few years now.

Can MR be updated for 11.x

Also open thread mentions needing a follow up if that could be created also before the new branch is made.

Reading wrong commit forget the follow-up part.

@andypost done, I guess this needs a rebase now

Rebased

Changes look good and all tests green.

Unfortunately there’s no solution for #124 yet, and IMHO getting this in as it is now would be very fragile.

NW for #124.

NOTE: the Imagemagick toolkit is a working alternative (as long as the Imagemagick binary can support AVIF itself).

Rebased and sqashed commits into 2 for 11.x branch

Filed upgrade of PHP 8.3 CI image to latest Debian stable, AVIF is shipped now with libaom dependency fixed

https://git.drupalcode.org/project/drupalci_environments/-/merge_request...

Updated is with https://jakearchibald.com/2020/avif-has-landed/ which perfectly explains benefits

I think we can add to reports if no AVIF support available so people can read logs to get why imagecreatefromavif() or imageavif() are not found to solve #124

I think we can add to reports if no AVIF support available so people can read logs to get why imagecreatefromavif() or imageavif() are not found to solve #124

Unfortunately these functions still exist, even if avif is not supported. I did find the following code (found in https://bugs.php.net/bug.php?id=81217) would trigger a warning:

$ php -r 'var_dump(imageavif(imagecreatetruecolor(8, 8), "/tmp/test"));'
PHP Warning:  imageavif(): AVIF image support has been disabled
 in Command line code on line 1

I've tried to test for this in the status report using a pattern I found in \Drupal\Core\Render\ElementInfoManager::buildInfo. This works in my very limited test of two environments, one supporting AVIF, one not.

Rebased

I needed that for AVIF to work with imagemagick, but probably that is needed for GD as well: 📌 ExtensionMimeTypeGuesser doesn't support .avif Active

I put some caching around @mstrelan solution + added some info in status report if AVIF write support is broken.

This fits #124 I think so back to NR.

IMHO the CR should be expanded with some hints on how to fix the lack of the codec or of the binding of libavif. That’s above me though, help appreciated.

Not sure correct status, but #141 mentions change record updates. Which is technically apart of the review process.

php/php-src just committed Image format detection false positives with external GD #12019. This will likely only hit PHP 8.4.

Core's CI images are not yet using external GD https://git.drupalcode.org/project/drupalci_environments/-/blob/producti...

--with-external-gd Use external libgd

Not sure CR should elaborate how specific distros can install codec if it missing, it's unrelated to core

Brought up in #needs-review-queue-initative in slack. Was mentioned by @catch that

detection logic is still in progress

and that parts will need the release manager review.

Wonder if anyone has any update for the previous comment?

IMHO the CR should be expanded with some hints on how to fix the lack of the codec or of the binding of libavif.

The avif contrib module leverages the lib_avif dependency, maybe the CR could point people to the issue I linked above if they're not able to affect how PHP is compiled with avif support enabled on their hosting environment. ✨ Add an AVIF conversion image styles effect Active

The code here looks good to me.

The needs release manager review tag was added in 9.x era when we were discussing min PHP requirements.

I don't think we need it anymore - especially as we're into 11.x territory where 8.2+ will be available.

I think the remaining tasks here are to update the CR with instructions for folks on how to add avif support to their version of PHP

Tagging as NW for that.

Great work here folks

We are in Drupal 11.1-land now. Which will require PHP 8.3. I've updated the related CR to include libavif installation instructions. Back to NR.

Merged 11.x and removed PHP 8.2 related workaround

I simplified check to prevent checking avif writes if no readonly support

Moving to NW for open questions on MR.

All threads resolved and a green pipeline 🥳

LGTM. All concerns addressed.