Removed ld64 flag

[radarhere]

Suggestions for python-pillow#5201

Since you're already adding EXIF data to the saved file with avifImageSetMetadataExif, it would seem unnecessary to also use exif_orientation_to_irot_imir?

I've removed get_modern_cmake, since python-pillow#8497 added python3 -m pip install cmake instead, and the goal of that PR was to try and avoid using brew, which get_modern_cmake does.

I saw that _avif.AvifCodecVersions() was only ever called from the test suite, so I've removed that.

[radarhere]

@fdintino I don't know if this many commits at once is overwhelming. Feel free to let me know if I should stop until you have a chance to look it over, or if you would prefer bigger PRs than more smaller PRs. Feel free to question my suggestions as well.

[fdintino]

I'll make comments on the individual changes, but as for EXIF: according to the AVIF spec, decoders are meant to ignore EXIF orientation and only use the irot and imir transform flags. This page has two example images, one with EXIF orientation and the other with irot and imir. If you are using a recent version of Safari, Chrome, or Firefox, it should look like this:

We are still setting EXIF metadata because we don't want to lose all of the other EXIF tags that might be present.

[fdintino]

I'm a little wary of removing this. It was a workaround for fdintino/pillow-avif-plugin#59. I wrote this comment elsewhere summarizing my findings:

image-rs/image#2108 (comment)

[...] I encountered an illegal instruction issue with libavif that I ultimately diagnosed as a bug in Apple's new(ish) static linker. Stepping through the executable in lldb you could see 4 bytes inserted into the middle of a subroutine that were invalid assembly (so, not even a valid but unsupported ARM intrinsic). If those 4 bytes were removed, the subroutine ran correctly. I noticed that the issue went away if I compiled with -O2; my best guess is that the bug occurs in functions with both inline assembly and C code that clang optimizes with ARM intrinsics (I imagine these are necessary but not sufficient conditions, or else people would be encountering many more corrupt executables). I was able to reliably avoid the issue if I compiled with the old ld64 linker, by adding -ld64 to LDFLAGS.

The test test_aom_optimizations served as a regression test of sorts, and it's no longer failing on macOS ARM. So perhaps the linker bug has since been fixed and we no longer need this flag.

[fdintino]

Everything other than the irot/imir removal and the -ld64 flag removal looks good to me. I'm fine removing the -ld64 flag for now, since I can no longer reproduce it. I'll ask the original bug reporter to try to reproduce it as well. If it recurs, we'll know how to fix it and we can raise it with the libaom project.

[radarhere]

It is a problem at the moment that the irot/imir isn't interpreted when opening an image, right? If the EXIF orientation is removed when saving, and changed into irot/imir, then when opening the image, Pillow doesn't know about the irot/imir, so the orientation is lost?

[fdintino]

This sort of change I think I would rather have in a separate pull request. You are aware that rav1e is only an encoder, and so if we don't include either aom or dav1d, we can't decode AVIF images?

[radarhere]

Ah, ok, I missed that libavif was installing the dependencies itself when using "LOCAL".

I've created #7 to use "LOCAL" for rav1e, except on manylinux2014 and aarch64.

[fdintino]

It is a problem at the moment that the irot/imir isn't interpreted when opening an image, right? If the EXIF orientation is removed when saving, and changed into irot/imir, then when opening the image, Pillow doesn't know about the irot/imir, so the orientation is lost?

Yes, this is an oversight. I've attempted to keep all of the same functionality as the avifenc and avifdec CLI applications bundled with libavif and I appear to have overlooked the bit that sets exif in avifdec. I'll add that in.