Opened 4 years ago

Closed 4 years ago

Last modified 4 weeks ago

#34035 closed Cleanup/optimization (wontfix)

ImageField doesn't consider EXIF rotation when storing width and height

Reported by: Adam Owned by: nobody
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: Johannes Maron Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When providing a height_field and width_field for an ImageField, it stores the dimensions, but without consideration of EXIF rotation. I have attached files to test with. The files ending with 5 through 8 all have their width and height swapped.

Attachments (9)

EXIF Rotation 0.jpg (9.3 KB ) - added by Adam 4 years ago.
EXIF Rotation 1.jpg (9.1 KB ) - added by Adam 4 years ago.
EXIF Rotation 2.jpg (8.6 KB ) - added by Adam 4 years ago.
EXIF Rotation 3.jpg (7.8 KB ) - added by Adam 4 years ago.
EXIF Rotation 4.jpg (8.1 KB ) - added by Adam 4 years ago.
EXIF Rotation 5.jpg (11.9 KB ) - added by Adam 4 years ago.
EXIF Rotation 6.jpg (8.6 KB ) - added by Adam 4 years ago.
EXIF Rotation 7.jpg (11.6 KB ) - added by Adam 4 years ago.
EXIF Rotation 8.jpg (8.8 KB ) - added by Adam 4 years ago.

Download all attachments as: .zip

Change History (15)

by Adam, 4 years ago

Attachment: EXIF Rotation 0.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 1.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 2.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 3.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 4.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 5.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 6.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 7.jpg added

by Adam, 4 years ago

Attachment: EXIF Rotation 8.jpg added

comment:1 by Tim Graham, 4 years ago

Resolution: invalid
Status: newclosed

Django simply uses Pillow to retrieve the dimensions, so this should be reproduced without Django and then (if a valid issue) reported there. A quick search revealed a possibly related issue.

If my analysis is incorrect, please reopen this ticket with an explanation of where Django is at fault.

comment:2 by Adam, 4 years ago

Resolution: invalid
Status: closednew

Here is another Pillow issue that may be of interest. It seems that Pillow ignoring the EXIF orientation is by design. Pillow is used in various contexts, not just web. I think in other cases it may make sense to return the dimensions without considering the EXIF orientation. But the web defaults to considering the EXIF orientation (see this link). Since Django is for the web, I think it should consider the EXIF orientation. I was able to make a quick and dirty fix to django/core/files/images.py. I added the following right above where it says return p.image.size (source):

try:
    exif = p.image._getexif()
except (AttributeError, ValueError):
    pass
else:
    if exif is not None and exif.get(274, 0) > 4:
        return (p.image.size[1], p.image.size[0])
Last edited 4 years ago by Adam (previous) (diff)

comment:3 by David Sanders, 4 years ago

Adam I'd encourage you to start a discussion on the django-developers mailing list [1] seeing as this is a behaviour change. Your suggestion seems like it could be a helpful addition but many folks have probably already encoded exif rotation into their codebases and might get an unwelcome surprise if Django started doing it… there may be a bunch of other reasons that folks would rather it not behave this way? 🤔

That being said the triage team might simply accept it :)

[1] https://groups.google.com/g/django-developers

comment:4 by Mariusz Felisiak, 4 years ago

Resolution: wontfix
Status: newclosed
Type: BugCleanup/optimization

I agree with David that some discussion is needed because this change is backward incompatible. You can also subclass ImageField if the current behavior doesn't suit you.

Please follow the triaging guidelines with regards to wontfix tickets and take this to DevelopersMailingList to reach a wider audience and see what other think.

comment:5 by Johannes Maron, 4 weeks ago

Cc: Johannes Maron added
Component: Database layer (models, ORM)File uploads/storage
Easy pickings: set
Version: 3.2dev

I am taking the liberty to reopen this, since I believe there is a backwards-compatible way. Thus, invalidating the previous reason to close the issue.

Both the get_image_dimensions function and ImageField can default to the current behavior while providing an explicit argument to include the EXIF rotation.

This would be a fairly minor patch. In future Django versions, we could consider deprecating the default behavior and forcing users to make an educated decision.

At the very least, this deserves a little note in the docs.

Version 0, edited 4 weeks ago by Johannes Maron (next)

comment:6 by Tim Graham, 4 weeks ago

Please ​follow the triaging guidelines with regards to wontfix tickets and take this to DevelopersMailingList to reach a wider audience and see what other think of your proposal.

Note: See TracTickets for help on using tickets.
Back to Top