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

Incorrect image transformation for few EXIF orientations #34

Closed
davpsh opened this issue Jan 11, 2021 · 11 comments
Closed

Incorrect image transformation for few EXIF orientations #34

davpsh opened this issue Jan 11, 2021 · 11 comments

Comments

@davpsh
Copy link

davpsh commented Jan 11, 2021

The problem is reproduced on any version of the module.

I tested auto-rotation using an image with all possible EXIF orientations and found that some of them are not handled (transformed) correctly.

Here is a list of EXIF orientations from my tests with handle results and original image without EXIF orientation:

ID Orientation Image Correctly transformed
1 Horizontal (normal) orientation-1 ✔️
2 Mirror horizontal orientation-2 ✔️
3 Rotate 180 orientation-3 ✔️
4 Mirror vertical orientation-4 ✔️
5 Mirror horizontal and rotate 270 CW orientation-5 ✔️
6 Rotate 90 CW orientation-6
7 Mirror horizontal and rotate 90 CW orientation-7 ✔️
8 Rotate 270 CW orientation-8
@davpsh
Copy link
Author

davpsh commented Feb 18, 2021

After some research, I found that the problem is more complex than few incorrect transformations.

Presented data are invalid and should be ignored.
I'll try to describe problem detailed as soon as possible.

I'll close referenced PR that doesn't fixes problem.

@ericnograles
Copy link
Owner

Thanks @davpsh -- if you have sample images that you can share, I can help troubleshoot this

@mei23
Copy link

mei23 commented Apr 30, 2021

Maybe the images published below are useful.

https://github.com/recurser/exif-orientation-examples
Landscape_[0-8].jpg

In my test, 2-8 seems to give wrong results.

@otaviosoares
Copy link

I fixed it here otaviosoares@31c6a93

It's a very poor implementation tho. I wanted to open a PR but I didn't have the time.

@tamaina
Copy link

tamaina commented Feb 15, 2022

Is there any progress? Or should I fork it?

@tamaina
Copy link

tamaina commented Feb 16, 2022

@ericnograles
Copy link
Owner

Will take a look at these, just bumped the library to the latest ExifReader so I'm wondering if the old one had some idiosyncracies.

@ericnograles
Copy link
Owner

@davpsh Looks like it may have been the older version of ExifReader we were using. I see the autoRotate transformations working (used your samples above):

90 CW

image

270 CW

image

If you grab the latest from NPM (v2.3.0) it should reflect these changes. lmk if you continue to see any issues, thanks!

@ericnograles
Copy link
Owner

ericnograles commented Sep 18, 2022

Correction, latest is v2.3.2

@ericnograles
Copy link
Owner

ericnograles commented Sep 25, 2022

Update: It appears we may not need ExifReader anymore, all browsers should now auto rotate images based on their EXIF. Will be testing a branch without ExifReader so we can really pare down the size of this library https://aws.amazon.com/blogs/machine-learning/how-an-important-change-in-web-standards-impacts-your-image-annotation-jobs/

The above was "fixed" because orientation from ExifReader would never resolve (it's now promise-y when you directly load a file) and it would never pass in the orientation value. And because my browser automatically rotates based on EXIF above, it should always resolve.

TL;DR: It appears web standards have saved us, we may not need ExifReader anymore.

ericnograles added a commit that referenced this issue Sep 25, 2022
* #34 removes ExifReader
- See https://aws.amazon.com/blogs/machine-learning/how-an-important-change-in-web-standards-impacts-your-image-annotation-jobs/
- Browsers now rotate images based on exif by default

* readme.md update

* updated dist for next release

Co-authored-by: Eric "Danger" Nograles <[email protected]>
@ericnograles
Copy link
Owner

Tested across all browsers, all engines autoRotate by default, exifreader and related logic removed.

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

No branches or pull requests

5 participants