shssoichiro / oxipng

Multithreaded PNG optimizer written in Rust
MIT License
2.92k stars 125 forks source link

EXIF Orientation not supported #570

Open Yay295 opened 1 year ago

Yay295 commented 1 year ago

I created two images with ImageMagick and added the EXIF Orientation tag to both of them:

convert -size 64x128 canvas:#FFFF00 exif_rotated.png
exiftool -overwrite_original -n -Orientation=6 exif_rotated.png

convert -size 64x128 -delay 100 canvas:#FFFF00 canvas:#0000FF exif_rotated_a.png
exiftool -overwrite_original -n -Orientation=6 exif_rotated_a.png

exif_rotated exif_rotated_a After compressing them with oxipng, the EXIF tags have been stripped, and the images have not been rotated. Note that neither Firefox nor Chrome currently respect this tag, and Safari only sometimes does, so these images will not be displayed correctly in your browser. https://bugzilla.mozilla.org/show_bug.cgi?id=1627423 https://bugs.webkit.org/show_bug.cgi?id=210021

andrews05 commented 1 year ago

Hi @Yay295 Oxipng will not strip anything by default - this will only happen if you are using the --strip flag. If you don't want the EXIF tags to be stripped you'll need to either not use --strip or be more explicit about what chunks you want to strip/keep.

Yay295 commented 1 year ago

I was using --strip safe. I don't think EXIF orientation is a safe tag to strip.

andrews05 commented 1 year ago

EXIF is an interesting situation. It's only recently been adopted into the PNG spec (third edition, still not a final standard yet) and as you pointed out is not widely supported yet. EXIF can also contain a huge variety of different metadata and much of the time it is unimportant and perfectly safe to strip. Without adding full EXIF support to oxipng to read and understand the data, we can't know whether it might be "safe" or not. It's certainly a feature we could consider, but it's not a simple matter.

My advice for now would be to not use EXIF in pngs in the first place.

AlexTMjugador commented 1 year ago

Without adding full EXIF support to oxipng to read and understand the data, we can't know whether it might be "safe" or not. It's certainly a feature we could consider, but it's not a simple matter.

There's the exif crate for parsing EXIF tags which happens to mention EXIF orientation tags in its examples, and has only a single dependency. Its source package weighs 53.2 KiB, though, which makes me wonder "do we really want to include this rather large dependency to solve this use case?". But the building blocks are there, and it should not be too hard to do some research on which EXIF tags are not safe to strip.

ace-dent commented 1 year ago

Personal opinion: Any metadata that could have been 'baked' into the raw RGBA, but wasn't, can be stripped as part of 'lossless' optimization. i.e. removing gamma, color transforms, and now rotation. If you want to preserve metadata, don't use --strip. Or use a script to perform the transformation and then strip the metadata.

Yay295 commented 1 year ago

If you want to remove all metadata you can use --strip all. --strip safe exists specifically to not strip metadata like this though.

Yay295 commented 1 year ago

Theoretically, you could actually save space by adding an orientation tag if you were able to compress the image better by flipping/rotating it.

andrews05 commented 1 year ago

I think we would want more data on this before we can make a call on it. How and why EXIF is being used in pngs, what tags etc. Your report is helpful, but currently it’s the only data point we have.

Right now I don’t think it’s a big issue. If you need to preserve it, you can. If you strip it inadvertently, it’s easy enough to restore (unlike some other chunks).

Yay295 commented 1 year ago

Yeah, it's not a problem for me. I only noticed it because I added code to my scripts to check for it, so I'm already handling it. I just think it's something that oxipng should check for.