-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Remove image.copy()
op from exif_transpose()
#5574
Conversation
This version of the function avoids image copy and a dictionary build for the majority use case of missing exif tags / no action needed on call. As implemented in YOLOv5 in ultralytics/yolov5#3852
I count 3 changes here
It seems to me like the extra code complexity isn't worth the very minor speed increase.
As #5546 explains in full, this means that while original image hasn't been transposed, its orientation tag is gone now. from PIL import Image
im = Image.open("hopper.jpg")
exif = im.getexif()
orientation_before = exif.get(0x0112)
im.exif_transpose()
orientation_after = exif.get(0x0112) The
Because from PIL import Image
im = Image.open("hopper.jpg")
im2 = im.exif_transpose() sometimes, For example, afterwards, if I rotate im2.rotate(90) I don't know whether or not |
CI is failing. I think this appears to be due to the CI itself expecting copies of images returned from the function rather than a problem in the function. EDIT: Thanks for the comments @radarhere. I'll have to think through my changes again it seems. |
The specific failure seen in AppVeyor at the moment isn't because this isn't |
@radarhere thanks for the detailed analysis. I understand the need for continuity of behavior within the PIL package, so in this case it seems we want slightly different behaviors, as in our case we want to prioritize speed, so to use the official function we'd probably want to have an if-statement outside the function to determine if it required transposing first, which would duplicate the code inside the function. I think probably we'll implement our updated version in YOLOv5 with a comment referencing the source, and then I'll close this PR. |
@radarhere PR is closed. Thank you for your feedback! |
In Pillow 10.0.0, we added an |
This version of the function avoids image copy and a dictionary build for the majority use case of missing exif tags / no action needed on call.
As implemented in YOLOv5 in ultralytics/yolov5#3852