strukturag / libheif

libheif is an HEIF and AVIF file format decoder and encoder.
Other
1.63k stars 294 forks source link

heif_convert should remove Orientation fields from EXIF #680

Closed farindk closed 1 year ago

farindk commented 1 year ago

The Orientation field in the EXIF is ignored for HEIF images, but it is used for JPEG images. This has to be considered when converting HEIC to JPEG. Currently, heif_convert does this by ignoring the HEIF transformations during decoding the image and keeping the EXIF Orientation. However, this does not work when the EXIF Orientation is not in sync with the irot box. For example, image 43.heic in #677 has an EXIF Orientation set to "90 CW", while there is no irot box. This leads to a wrong orientation in the resulting JPEG.

We have to remove the EXIF Orientation field during the conversion. However, this will likely require a new dependency on some EXIF processing library like libexif.

kmilos commented 1 year ago

Instead of removing, may I suggest resetting to default value of 1 (top left)? It should have the same effect.

It might be easier to manage without an external library (number of bytes in the blob stays the same).

y-guyon commented 1 year ago

Here is a related discussion about Exif orientation import in libavif from Jpeg/PNG to irot/imir: https://github.com/AOMediaCodec/libavif/pull/1099#discussion_r970928340

Even though MIAF says There should be no image transformations expressed by Exif, the Exif specification says the default value 1 (top-left) applies if there is no explicit orientation data in the Exif metadata payload. So stripping it has the same result as setting it to 1. I think @kmilos's suggestion is simpler between stripping and setting it to 1.

libavif does not yet export Exif metadata from AVIF to Jpeg so the issue was not yet encountered there.

farindk commented 1 year ago

I have implemented it such that the EXIF Orientation is changed to 1 if there was an Orientation set. If no Orientation was set in the original file, no EXIF Orientation field is added. The main reason for this was that it's easier to implement as we would have to rewrite the whole EXIF IFD table when we add a value.

kmilos commented 1 year ago

Yep, that's the way to go, 1 is implied anyway if not present.