image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.86k stars 597 forks source link

Expose zune-jpeg's Exif chunk extraction #2291

Closed Shnatsel closed 1 week ago

Shnatsel commented 1 month ago

Carbon copy of the ICC chunk handling, but for Exif.

Required for wondermagick so that we could rotate the JPEGs correctly based on metadata.

In the long term we probably want to integrate kamadak-exif, since it doesn't have any unsafe code and seems to be in quite a good shape. But this PR seems like a simple and uncontroversial start.

Shnatsel commented 1 month ago

The single failing CI job seems to be unrelated to this change.

Shnatsel commented 1 month ago

In theory PNG and WebP images can have Exif metadata too. Do you want me to make this a method on ImageDecoder with a default impl that just returns None?

Update: I went ahead and moved it to the Decoder trait with a default impl to preserve semver compatibility. The API is much nicer to use that way.

Shnatsel commented 1 month ago

Oh god, that actually doesn't work because Rust always calls the default impl on an impl Trait, not the specialized one. So this way we always get None on ImageReader.into_decoder().

The workaround is to make the method required and to NOT provide a default impl, but that's semver-breaking because the trait is public and can be implemented by third-party crates.

This means ICC chunk extraction has also been broken the entire time...

Shnatsel commented 1 month ago

nvm I figured it out, this works and is ready for review

Shnatsel commented 1 week ago

Nice! I'll try to add that later, but I'd appreciate a merge of this as-is so that the PR doesn't stall.