Skip to content

Fixed conversion of AVIF image rotation property to EXIF orientation #8866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 4, 2025

Conversation

fdintino
Copy link
Contributor

@fdintino fdintino commented Apr 3, 2025

This implements a fix for the issue identified in AOMediaCodec/libavif#2727 and fixed in AOMediaCodec/libavif#2729. The code in Pillow that converts irot and imir properties to EXIF orientation was repurposed from libavif, so it suffers the same bug.

This implements a fix for the issue identified in
AOMediaCodec/libavif#2727 and fixed in AOMediaCodec/libavif#2729. The
code to convert irot and imir properties to EXIF orientation when
decoding AVIF images in Pillow was repurposed from libavif, so it
suffers the same bug.
@radarhere
Copy link
Member

Would it be possible to add a test?

@fdintino
Copy link
Contributor Author

fdintino commented Apr 3, 2025

Oh, of course. Yes, I'll add one.

@@ -327,6 +327,7 @@ def test_exif_save(
if orientation == 1:
assert "exif" not in reloaded.info
else:
assert reloaded.getexif()[274] == orientation
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion fails before this change for orientations 6 and 8.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can include this new assertion if you like, but actually, the next line

assert reloaded.info["exif"] == exif_data

already fails with the new parameters. This is because the next line is checking the full EXIF data, not just the orientation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize they're redundant, but it was a bit obscure identifying the mismatch with the bytes, so I thought this more specific assertion might be more diagnostic of failures

@radarhere radarhere changed the title fix: conversion of AVIF image rotation property to EXIF orientation Fixed conversion of AVIF image rotation property to EXIF orientation Apr 3, 2025
@hugovk hugovk merged commit 9f654ff into python-pillow:main Apr 4, 2025
72 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants