Add draft support to retrieve Exif from HEIF file

[benstonezhang]

Test passed on HEIC file retrieve from iPhoneSE3 and some pictures get from internet.

The attachments are patch files for version 8.1 and 8.2
php_8.1_exif_heif.patch
php_8.2_exif_heif.patch

[jorgsowa]

Hey, @benstonezhang!
You have some tests failing in the pipeline from the extension you modified. Also, it would be cool to always include the tests for your changes to the PRs. In this case, remember about the license of the HEIF file you would upload.

[benstonezhang]

@jorgsowa Is there any test about exif I can use as reference? I'm not family with php.

[benstonezhang]

@jorgsowa I added one test case for heif, please check. The image is just taken by my phone's camera.

[bukka]

Thanks for the PR! I will need to do a bit more research but initially it looks good except few smaller things that I commented on.

[jorgsowa]

I think the last missing piece is to add the entry to the NEWS file.

[bukka]

Well this still needs a proper review. I have got it on my TODO list. It might take couple of weeks though.

[benstonezhang]

seems I need to submit it

[bukka]

I had quick look throught and it will still need some testing.

That said I'm actually wondering how useful this really is. There's actually related PR to add HEIF support for ext/gd: #14504 . Last comment from @cmb69 questioned the usefulness of HEIF support in GD. I'm actually wondering how much in use this is. I just shared some photos from my iphone (15 pro) and it gets always converted either to gif or jpg so it doesn't like new iPhones make it easy to get the images in the HEIF. Could you clarify the use case for this please?

[bukka]

is it supposed to be always little endian for heif?

[bukka]

CS: missing spaces before and after = and <

[benstonezhang]

I had quick look throught and it will still need some testing.

That said I'm actually wondering how useful this really is. There's actually related PR to add HEIF support for ext/gd: #14504 . Last comment from @cmb69 questioned the usefulness of HEIF support in GD. I'm actually wondering how much in use this is. I just shared some photos from my iphone (15 pro) and it gets always converted either to gif or jpg so it doesn't like new iPhones make it easy to get the images in the HEIF. Could you clarify the use case for this please?

Please check you iphone "Settings -> Photos -> TRANSFER TO MAC OR PC -> Keep Originals". If you set is as "Automatic", all the photos will convert to JPG.

[adamsilverstein]

I had quick look throught and it will still need some testing.

That said I'm actually wondering how useful this really is. There's actually related PR to add HEIF support for ext/gd: #14504 . Last comment from @cmb69 questioned the usefulness of HEIF support in GD. I'm actually wondering how much in use this is. I just shared some photos from my iphone (15 pro) and it gets always converted either to gif or jpg so it doesn't like new iPhones make it easy to get the images in the HEIF. Could you clarify the use case for this please?

Dropping in to provide a use case for this...

As a media component maintainer for the WordPress project, I can add that we regularly get support requests from users who have HEIC image files on their computer and try to upload them to their WordPress site. I understand that iPhones can be set to copy files over as JPEG (and that might even be the default?) - nonetheless, HEIC images are widespread already and people do try to upload them to use on their sites.

Until recently, WordPress would just throw an error when users uploaded these images. In this ticket we added support for converting these uploaded files into JPEG or WebP images, as long as the system has Imagick installed with HEIC support. If PHP's GD supported HEIF directly, we would able to provide this feature for those users as well..

[cmb69]

Note that there are actually three not necessarily related features regarding HEIF support for php-src:

1. support for reading/writing HEIF images in ext/gd
2. support for getimagesize() and friends (part of ext/standard)
3. support for Exif information in HEIF images (part of ext/exif)

(1) and (2) would already be addressed by #14504, so let's focus on (3) here, which this PR about.

Now, I'm not convinced that adding support for HEIF to ext/exif is a good idea. ext/exif is not well maintained (there is not even a code owner), and I doubt that a userland implementation would be noticeably slower than a C implementation, although an implementation in C is certainly more verbose (and as such harder to understand), and might more easily be prone to typical C issues (memory management etc.) As such I would prefer an Exif library implemented in userland. It seems, though, that the more popular userland packages either use ext/exif, or are dormant/abandoned.

From quickly looking at the implementation, it doesn't seem to be terribly complex, so maybe adding this to ext/exif is not the worst idea, but I've also noticed that apparently there is some support for getimagesize(). In my opinion, this should be separate PR.

Anyhow, @benstonezhang, are you still interested in pursuing this PR?

[cmb69]

FWIW: this feature has been requested as https://bugs.php.net/77757.

[benstonezhang]

@cmb69 Yes, I had use it almost 1 year. By the way, the decent routine to handle AVIF (heif just one file format follow avif spec) maybe go through ext/standard/libavifinfo. But I found it too heavy and use lots of resource, so the current implement is light and just for heif. I can update the patch if needed.

[benstonezhang]

Note that there are actually three not necessarily related features regarding HEIF support for php-src:

1. support for reading/writing HEIF images in ext/gd

2. support for getimagesize() and friends (part of ext/standard)

3. support for Exif information in HEIF images (part of ext/exif)

(1) and (2) would already be addressed by #14504, so let's focus on (3) here, which this PR about.

Now, I'm not convinced that adding support for HEIF to ext/exif is a good idea. ext/exif is not well maintained (there is not even a code owner), and I doubt that a userland implementation would be noticeably slower than a C implementation, although an implementation in C is certainly more verbose (and as such harder to understand), and might more easily be prone to typical C issues (memory management etc.) As such I would prefer an Exif library implemented in userland. It seems, though, that the more popular userland packages either use ext/exif, or are dormant/abandoned.

From quickly looking at the implementation, it doesn't seem to be terribly complex, so maybe adding this to ext/exif is not the worst idea, but I've also noticed that apparently there is some support for getimagesize(). In my opinion, this should be separate PR.

Anyhow, @benstonezhang, are you still interested in pursuing this PR?

This PR is only for exif within heif handing, and can do nothing else. So I prefer keep them in one PR. The patch code is rebased on latest master branch.