Skip to content
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

raw input: fix LibRaw flip to Exif orientation conversion #3847

Merged
merged 3 commits into from
May 20, 2023

Conversation

mugulmd
Copy link
Contributor

@mugulmd mugulmd commented May 19, 2023

Description

When reading a raw image while having a user_flip set to 1, we set the orientation Exif tag to LibRaw's flip value (so that LibRaw doesn't rotate and/or mirror the buffer but the image still gets displayed correctly).

However LibRaw's convention for flip values differ from Exif orientation values: https://www.libraw.org/comment/5176#comment-5176.

In this PR we fix this issue by converting LibRaw flip values to Exif orientation values. This conversion is based on:

Tests

Tested on:

  • CR2 images with flip values 1, 5 and 6
  • ARW images with flip values 1 and 5
    (unfortunately I could not find any image covering other flip values).

Since the conversion is [1, 2, 3, 4, 5, 6, 7, 8] -> [1, 2, 4, 3, 5, 8, 6, 7], the only test that came out with different results before and after the change is the one with flip value 6:
in that case, the image orientation got corrected by the fix.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM, pending that fix I suggested to satisfy the formatting rules.

Thank for tracking this down!

@lgritz lgritz merged commit 5ca9ffe into AcademySoftwareFoundation:master May 20, 2023
19 of 20 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request May 20, 2023
…wareFoundation#3847)

When reading a raw image while having a `user_flip` set to 1, we set the
orientation Exif tag to LibRaw's flip value (so that LibRaw doesn't
rotate and/or mirror the buffer but the image still gets displayed
correctly).

However LibRaw's convention for flip values differ from Exif orientation
values: https://www.libraw.org/comment/5176#comment-5176.

In this PR we fix this issue by converting LibRaw flip values to Exif
orientation values. This conversion is based on:
- https://www.libraw.org/comment/5178#comment-5178 (conversion proposed
by LibRaw's maintainer)
-
https://github.com/LibRaw/LibRaw/blob/6fffd414bfda63dfef2276ae07f7ca36660b8888/src/write/file_write.cpp#L22
(LibRaw's function for applying the flip).

Tested on:
- CR2 images with flip values 1, 5 and 6
- ARW images with flip values 1 and 5
(unfortunately I could not find any image covering other flip values).

Since the conversion is `[1, 2, 3, 4, 5, 6, 7, 8] -> [1, 2, 4, 3, 5, 8,
6, 7]`, the only test that came out with different results before and
after the change is the one with flip value 6:
in that case, the image orientation got corrected by the fix.
lgritz pushed a commit that referenced this pull request May 31, 2023
When reading a raw image, setting the `user_flip` to 1 meant ignoring
the `flip` value (to avoid buffer rotation from LibRaw).

However the value 1 for the `user_flip` is **not neutral**: values lie
between 0 and 7 (see
https://www.libraw.org/docs/API-datastruct-eng.html#libraw_image_sizes_t),
and 1 corresponds to a horizontal mirror (in my understanding of the
source code).

The proposed fix is to use 0 as `user_flip` value for ignoring the
`flip` value.

This modification invalidates the changes made in this PR:
#3847.
Therefore, flip to orientation conversion has been adapted to match
exactly LibRaw's documentation, which handles only rotations (+90, +180,
-90 degrees).

The previous PR was only tested on images of landscape and human faces,
hence the mirroring issue remained unnoticed.
When reading images containing text however the problem is clearly
visible.
This PR fixes this mirroring issue on the following 4 orientations: 0,
+90, +180 and -90 degrees rotation.
Other orientations could not be tested as they don't seem to ever appear
on raw images datasets.
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Jun 1, 2023
When reading a raw image, setting the `user_flip` to 1 meant ignoring
the `flip` value (to avoid buffer rotation from LibRaw).

However the value 1 for the `user_flip` is **not neutral**: values lie
between 0 and 7 (see
https://www.libraw.org/docs/API-datastruct-eng.html#libraw_image_sizes_t),
and 1 corresponds to a horizontal mirror (in my understanding of the
source code).

The proposed fix is to use 0 as `user_flip` value for ignoring the
`flip` value.

This modification invalidates the changes made in this PR:
AcademySoftwareFoundation#3847.
Therefore, flip to orientation conversion has been adapted to match
exactly LibRaw's documentation, which handles only rotations (+90, +180,
-90 degrees).

The previous PR was only tested on images of landscape and human faces,
hence the mirroring issue remained unnoticed.
When reading images containing text however the problem is clearly
visible.
This PR fixes this mirroring issue on the following 4 orientations: 0,
+90, +180 and -90 degrees rotation.
Other orientations could not be tested as they don't seem to ever appear
on raw images datasets.
birsoyo pushed a commit to birsoyo/oiio that referenced this pull request Jun 8, 2023
…wareFoundation#3847)

When reading a raw image while having a `user_flip` set to 1, we set the
orientation Exif tag to LibRaw's flip value (so that LibRaw doesn't
rotate and/or mirror the buffer but the image still gets displayed
correctly).

However LibRaw's convention for flip values differ from Exif orientation
values: https://www.libraw.org/comment/5176#comment-5176.

In this PR we fix this issue by converting LibRaw flip values to Exif
orientation values. This conversion is based on:
- https://www.libraw.org/comment/5178#comment-5178 (conversion proposed
by LibRaw's maintainer)
-
https://github.com/LibRaw/LibRaw/blob/6fffd414bfda63dfef2276ae07f7ca36660b8888/src/write/file_write.cpp#L22
(LibRaw's function for applying the flip).

Tested on:
- CR2 images with flip values 1, 5 and 6
- ARW images with flip values 1 and 5
(unfortunately I could not find any image covering other flip values).

Since the conversion is `[1, 2, 3, 4, 5, 6, 7, 8] -> [1, 2, 4, 3, 5, 8,
6, 7]`, the only test that came out with different results before and
after the change is the one with flip value 6:
in that case, the image orientation got corrected by the fix.
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.

None yet

2 participants