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.

Sign up for GitHub

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
hugovk merged 2 commits into from
Apr 4, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af1d2a5
fix: conversion of AVIF image rotation property to EXIF orientation
fdintino Apr 3, 2025
f7fc268
add test
fdintino Apr 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Tests/test_file_avif.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def test_exif(self) -> None:
assert exif[274] == 3

@pytest.mark.parametrize("use_bytes", [True, False])
@pytest.mark.parametrize("orientation", [1, 2])
@pytest.mark.parametrize("orientation", [1, 2, 3, 4, 5, 6, 7, 8])
def test_exif_save(
self,
tmp_path: Path,
Expand All @@ -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

@fdintino fdintino Apr 3, 2025

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

@radarhere radarhere Apr 3, 2025

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

@fdintino fdintino Apr 3, 2025

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

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

def test_exif_without_orientation(self, tmp_path: Path) -> None:
Expand Down
4 changes: 2 additions & 2 deletions src/_avif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ irot_imir_to_exif_orientation(const avifImage *image) {
return axis ? 7 // 90 degrees anti-clockwise then swap left and right.
: 5; // 90 degrees anti-clockwise then swap top and bottom.
}
return 6; // 90 degrees anti-clockwise.
return 8; // 90 degrees anti-clockwise.
}
if (angle == 2) {
if (imir) {
Expand All @@ -75,7 +75,7 @@ irot_imir_to_exif_orientation(const avifImage *image) {
? 5 // 270 degrees anti-clockwise then swap left and right.
: 7; // 270 degrees anti-clockwise then swap top and bottom.
}
return 8; // 270 degrees anti-clockwise.
return 6; // 270 degrees anti-clockwise.
}
}
if (imir) {
Expand Down
Loading