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

Add autorotate flag #51

Merged
merged 4 commits into from
Apr 11, 2019
Merged

Add autorotate flag #51

merged 4 commits into from
Apr 11, 2019

Conversation

andrews05
Copy link
Contributor

@andrews05 andrews05 commented Nov 28, 2017

This PR adds an -autorotate flag to automatically transform JPEGs according to their Exif orientation. The -autorotate flag requires -strip. If --strict is also provided, transformation will only be done if it is "perfect", otherwise untransformable blocks will be trimmed (very rare).

Disclaimer: I'm not a C programmer and don't know what I'm doing.
Keen to hear any feedback and happy to make any changes requested.

Fixes #47

@fhanau
Copy link
Owner

fhanau commented Nov 28, 2017

This looks good, I'll try to review it soon.

@andrews05
Copy link
Contributor Author

Have you had a chance to review this yet?

@andrews05
Copy link
Contributor Author

I've just merged from master so this is again free of any conflicts.
Rotation is important to my workflow so currently I'm still using jpegtran for jpegs (and manually figuring out the rotation needed) and ect for pngs only. Hoping this can get merged so I only have to use one tool :)

@fhanau
Copy link
Owner

fhanau commented Mar 24, 2019

The code looks good to me, but when testing it on a file I got a wrong, but different rotation. I was able to resolve this by just setting the Exif rotation byte to 0 (deleting the Exif chunk would also work). However, this approach would not work when a perfect transformation is not possible. Trying to do a perfect transformation should also be the default as a not perfect transformation would result in cut off pixels. I will try to modify the code such that the file is rotated correctly and the exif chunk is preserved when a perfect transformation is not possible.

Thanks for your work by the way, I was too busy with classes to put a lot work into this project for a while now.

@andrews05
Copy link
Contributor Author

Oops, perhaps some of my map values are wrong! Could you attach the file you tried? I'm sure I tested all the values, I was using this example set: https://github.com/recurser/exif-orientation-examples

My use case is for the web where orientation values are ignored by all browsers, so actually rotating the data is necessary and retaining the orientation flag is useless. (Plus, imperfect images with orientation values are extremely rare outside of test cases).
Do you think there's a better way to indicate that you always want to rotate even if it's imperfect, maybe -autorotate=always or something?

@PanisSupraOmnia
Copy link

I would suggest -autorotate=force, and mentioning the reason for this in the docs.

@andrews05
Copy link
Contributor Author

I've changed the behaviour to be perfect by default and to force rotation via -autorotate=force as suggested above.
Ran some tests again and still couldn't see a case where the rotation came out wrong, so still curious to see that file that caused problems for you.

@fhanau
Copy link
Owner

fhanau commented Apr 5, 2019

a
This file is an example of this behavior. It is rendered with a wrong rotation in the browser, but has the right rotation when viewed with other software. It gets the wrong rotation after running -autorotate

@fhanau
Copy link
Owner

fhanau commented Apr 5, 2019

I think the problem that caused the wrong rotation in my tests is that your code transforms the image data in such a way that the image is rotated correctly, but does not modify or remove the EXIF data which causes it to be rotated in a different way again.

@andrews05
Copy link
Contributor Author

Odd, it's not supposed to let you use -autorotate with also specifying -strip, which enforces the removal of the orientation flag.

@fhanau
Copy link
Owner

fhanau commented Apr 5, 2019

@andrews05
Copy link
Contributor Author

Yeah, is that not working for you? I just tried it on that image, it bails out if I don't specify -strip.

@fhanau
Copy link
Owner

fhanau commented Apr 5, 2019

What I meant is that the only way to enable auto rotation is '-autorotate(-strict) -strip', which changes the rotation from right to wrong when viewed with software that does not ignore rotation values.

@andrews05
Copy link
Contributor Author

andrews05 commented Apr 5, 2019

Sorry, I'm still a bit confused.
If I look at that image in Finder it interprets the orientation flag and the image reads left-to-right.
If I run ect -strip, the orientation flag is gone and it now reads bottom-to-top.
If I run ect -strip -autorotate, it complains that it can't rotate because it isn't perfect and the result is the same as above (this would be with -strict prior to my last commit).
If I run ect -strip -autorotate=force, it performs the rotation and strips the orientation flag, so it correctly reads left-to-right again.

What is the incorrect output you're seeing?

@tssajo
Copy link
Contributor

tssajo commented Apr 5, 2019

a
This file is an example of this behavior. It is rendered with a wrong rotation in the browser, but has the right rotation when viewed with other software. It gets the wrong rotation after running -autorotate

Guys, when I view this github page with Chrome browser on a Windows PC then the above image reads from bottom to top, the orientation is wrong. However when I viewed the exact same page with Chrome browser on my Android phone (I held the phone in portrait mode) then the image read from left to right (correct rotation). It's weird that the Chrome browser displays the same image differently depending on the device used. Maybe something is wrong with this image or its metadata.

@fhanau
Copy link
Owner

fhanau commented Apr 11, 2019

Ok, so I hadn't tested the new version. With this commit, autorotate=force seems to work properly, but using -autorotate on a file that can't be transformed perfectly causes it to have a changed rotation (I think that it shouldn't be like that as the autorotate flag indicates that a right rotation should be preserved). I think this can be fixed by just not stripping the EXIF data in that case.

@fhanau
Copy link
Owner

fhanau commented Apr 11, 2019

a
This file is an example of this behavior. It is rendered with a wrong rotation in the browser, but has the right rotation when viewed with other software. It gets the wrong rotation after running -autorotate

Guys, when I view this github page with Chrome browser on a Windows PC then the above image reads from bottom to top, the orientation is wrong. However when I viewed the exact same page with Chrome browser on my Android phone (I held the phone in portrait mode) then the image read from left to right (correct rotation). It's weird that the Chrome browser displays the same image differently depending on the device used. Maybe something is wrong with this image or its metadata.

I think the image is fine. For me it is rendered wrongly in both FF and Chrome on both Mac OS and Android, but rendered correctly when just viewing the image file.

@fhanau
Copy link
Owner

fhanau commented Apr 11, 2019

Ok, both versions of autorotate should lead to the right rotation now. Thank you for your work.

@fhanau fhanau merged commit 836fd6a into fhanau:master Apr 11, 2019
@andrews05
Copy link
Contributor Author

Awesome, thanks heaps for merging that :)

@andrews05 andrews05 deleted the rotation branch April 11, 2019 06:37
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

4 participants