Correctly apply EXIF orientation to images #1666
Conversation
Workaround for the PIL bug python-pillow/Pillow#3973 - the exif orientation metadata is used to correctly transpose a PIL image when reading an image. The current approach silences any exceptions here, leading to strange behavior, such as issue facebookresearch#933. This is indeed a PIL issue, but the fix here performs the same function that is necessary to correctly orient images. The only differences between the committed function `apply_exif_orientation` and the PIL `exif_transpose` is that a copy of the image is not generated if no transpose is needed, and that the exif data is not updated in the transposed image. As `read_image` returns a NumPy ndarray, this information is discarded, anyway.
|
Hi @craymichael! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
facebook-github-bot
added
the
CLA Signed
|
Thanks for the workaround. From the original issue, it seems the bug has been fixed in Pillow>=7.0 already. In that case it would be better to just require Pillow>=7.0 as a dependency. |
|
I would agree with you - however, that issue certainly is not closed, nor does the current fix address this issue, unfortunately. A quick MWE of the error: Python 3.7.7 (default, Mar 29 2020, 18:40:05)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from PIL import Image, ImageOps
>>> with open('IMG_5370.jpg', 'rb') as f:
... img = Image.open(f)
... img = ImageOps.exif_transpose(img)
...
/.../venv/lib/python3.7/site-packages/PIL/TiffImagePlugin.py:590: UserWarning: Metadata Warning, tag 282 had too many entries: 2, expected 1
% (tag, len(values))
/.../venv/lib/python3.7/site-packages/PIL/TiffImagePlugin.py:590: UserWarning: Metadata Warning, tag 283 had too many entries: 2, expected 1
% (tag, len(values))
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
File "/.../venv/lib/python3.7/site-packages/PIL/ImageOps.py", line 549, in exif_transpose
transposed_image.info["exif"] = exif.tobytes()
File "/.../venv/lib/python3.7/site-packages/PIL/Image.py", line 3292, in tobytes
return b"Exif\x00\x00" + head + ifd.tobytes(offset)
File "/.../venv/lib/python3.7/site-packages/PIL/TiffImagePlugin.py", line 808, in tobytes
data = self._write_dispatch[typ](self, *values)
TypeError: write_undefined() takes 2 positional arguments but 5 were given
>>> import PIL
>>> PIL.__version__
'7.1.2'This in current state would ignore the exif orientation value of this image (which is |
detectron2/data/detection_utils.py
Outdated
| @@ -97,6 +100,51 @@ def convert_image_to_rgb(image, format): | |||
| return image | |||
|
|
|||
|
|
|||
| def apply_exif_orientation(image): | |||
There was a problem hiding this comment.
prefix it by _ since we don't expect it to be used outside of this file
detectron2/data/detection_utils.py
Outdated
| https://github.com/wkentaro/labelme/blob/v4.5.4/labelme/utils/image.py#L59 | ||
| https://github.com/python-pillow/Pillow/blob/7.1.2/src/PIL/ImageOps.py#L527 | ||
|
|
||
| The Pillow source raises errors with various methods, especially `tobytes` |
There was a problem hiding this comment.
it seems this line stay with L107-L109 that explains the bug
There was a problem hiding this comment.
Right, I'll clean this part up then and push soon
There was a problem hiding this comment.
@ppwwyyxx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Can you also report this upstream as well, so there is a chance it gets fixed in Pillow? We obviously don't want to keep a workaround forever. |
|
I definitely agree with that. I just looked into this further. The Pillow PR python-pillow/Pillow#4637 appears to fix this issue. I just tested it with the image that gave me the error with Pillow 7.1.2 successfully using the dev version of Pillow (7.2.0.dev0). It looks like 7.2.0 won't be released until July 1st, 2020 python-pillow/Pillow#4658, however. The original Pillow issue is still open, and I am unsure if there are other cases here that may result in raised exceptions with the exif_transpose function. |
Summary: Workaround for the PIL bug python-pillow/Pillow#3973 - the exif orientation metadata is used to correctly transpose a PIL image when reading an image. The current approach silences any exceptions here, leading to strange behavior, such as issue #933. This is indeed a PIL issue, but the fix here performs the same function that is necessary to correctly orient images. The only differences between the committed function `apply_exif_orientation` and the PIL `exif_transpose` is that a copy of the image is not generated if no transpose is needed, and that the exif data is not updated in the transposed image. As `read_image` returns a NumPy ndarray, this information is discarded, anyway. Pull Request resolved: #1666 Differential Revision: D22238978 Pulled By: ppwwyyxx fbshipit-source-id: 497c97b99fbfcf9857cbdf7516df10da4b42278d
Workaround for the PIL bug python-pillow/Pillow#3973 - the exif orientation metadata is used to correctly transpose a PIL image when reading an image. The current approach silences any exceptions here, leading to strange behavior, such as issue #933. This is indeed a PIL issue, but the fix here performs the same function that is necessary to correctly orient images. The only differences between the committed function
apply_exif_orientationand the PILexif_transposeis that a copy of the image is not generated if no transpose is needed, and that the exif data is not updated in the transposed image. Asread_imagereturns a NumPy ndarray, this information is discarded, anyway.