Re: Re: Finishing AVIF support in getimagesize()

On 09.12.2021 at 20:59, Flávio Heleno wrote: > On Thu, Dec 9, 2021 at 4:46 PM Nikita Popov <nikita.ppv@gmail.com> wrote: > >> On Wed, Dec 8, 2021 at 12:41 PM Christoph M. Becker <cmbecker69@gmx.de> >> wrote: >> >>> On 01.12.2021 at 00:52, Ben Morss via internals wrote: >>> >>>> Earlier this year, I added support for AVIF images >>>> <https://github.com/libgd/libgd/pull/671> to >>>> libgd >>>> <https://github.com/libgd/libgd>. My ultimate >>>> goal was to bring >> support >>> for >>>> this new image format to PHP, so that the world's top CMSs could let >>> sites >>>> serve AVIFs. A few of you kindly guided me as I made my first >>> contributions >>>> to the PHP codebase, propagating libgd's new AVIF support >>>> <https://github.com/php/php-src/pull/7026> >>>> into PHP's bundled gd fork, >>>> and adding >>>> AVIF awareness >>>> <https://github.com/php/php-src/pull/7091> to non-gd >>>> functions like getimagesize() and imagecreatefromstring(). >>>> >>>> Unfortunately, when I worked on getimagesize(), AVIF experts advised >> that >>>> there was no compact, reliable way to determine the size of an AVIF >> image >>>> without relying on an external library like libavif. We decided >>>> >>>> <https://github.com/php/php-src/pull/5127#issuecomment-799825277o¾Ë >>>> Ï°A%Â\psfj8> that >>> it >>>> was useful to return the information that a given image was an AVIF, >> but >>>> simply to return 0 for the actual dimensions and other details. The >> user >>>> would simply need to decode the image to determine this information. >>>> >>>> Of course, users would really like >>>> >>>> <https://github.com/php/php-src/pull/5127#issuecomment-976241397> >>>> to >> use >>>> getimagesize() to return the actual size. So a colleague has kindly >>>> created standalone >>>> code >>>> <https://aomedia-review.googlesource.com/c/libavifinfo/+/148321> >>> (that >>>> doesn't depend on libavif) to do this, with the goal of adding it to >> PHP. >>>> >>>> The code comprises several hundred lines. But - it works, and it works >>> well. >>>> >>>> Would it be ok to submit a PR containing this code to add this >>>> functionality? Or does someone have a superior approach? >>> >>> Thanks for your and your colleague's work! It's highly appreciated. >>> >>> Anyhow, a respective PR[1] has been submitted now, and I'm in favor of >>> bundling libavifinfo. I'm not too concerned regarding the additional >>> size of the PHP binaries which would result by linking it in, but if >>> others are, we could still introduce a configuration option (e.g. >>> --with-libavifinfo). >>> >>> Thoughts? Objections to bundling libavifinfo at all? >>> >>> [1] <https://github.com/php/php-src/pull/7711> >>> >> >> Assuming no licensing concerns, bundling libavifinfo sounds reasonable. The >> library is small, our avif functionality is incomplete without it, and we >> can't expect this to be available as a system library at this time. >> Although I'm not a core contributor, my 2c is that even with libavifinfo > bundled, a configuration option is > very welcome, the same way we have for png, jpeg, xpm and webp. Whether AVIF support should be included for ext/gd is already configurable. This is solely about the getimagesize(fromstring) and image_type_to_* functions which are part of ext/std. We do not have configuration options for these for the other image formats either. -- Christoph M. Becker