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 feature to set exif orientation #15

Closed
TonyBrobston opened this issue Jul 7, 2020 · 6 comments
Closed

Add feature to set exif orientation #15

TonyBrobston opened this issue Jul 7, 2020 · 6 comments

Comments

@TonyBrobston
Copy link
Contributor

TonyBrobston commented Jul 7, 2020

Hey @ginpei ,

I'm considering implementing this package into my client side image compression package (https://www.npmjs.com/package/jpegasus).

I wrote up some logic to read the orientation, then I use that orientation to determine which context transform to do, to flip/rotate an image to orientation 1. However I have no way to set the orientation to 1 after the flip/rotate.

Would you be interested in adding functionality to your package to be able to update the orientation on a File? If so, I'll ditch my logic for reading orientation and implement yours, then also use the feature to update the orientation.

It looks like Chrome and Firefox have added a feature where they will correct the orientation when an image is displayed in the browser. So for example if the orientation is 6, it will display in the browser as orientation 1. This creates a bug with my image compression package, so I may flip/rotate an image of orientation 6 to orientation 1; but if I don't update the orientation to 1 in that case, then Chrome/Firefox still assume the image is in orientation 6 and so Chrome/Firefox will do another flip/rotate. If that's not enough info, here's the issue on my package that I'm working on: TonyBrobston/jpegasus#151

I could potentially collaborate on the coding also. Let me know what you think.

Thanks,
Tony

@ginpei
Copy link
Owner

ginpei commented Jul 8, 2020

Hi @TonyBrobston ,

Thank you for this awesome proposal! I'm going to upgrade dep packages. Please merge or rebase and please go ahead.

@TonyBrobston
Copy link
Contributor Author

@ginpei It looks like there aren't any conflicts.

I took an initial stab at passing the test since I had a few minutes to spare. I need to refactor as I have code duplication and I also need to implement IOrientationInfo as an input option.

Thankfully all the coding you've already done did all the heavy lifting. I also need to figure out what to do when !isValidJpeg(view) and/or segmentOffset < 0. I may just wrap those if's around the remainder of the code, since I won't be able to perform a setUint16 without a valid DataView for example.

I'm also wondering what will happen if I try to set orientation on an image that doesn't have orientation. Seems the way I've passed the current test will work, for updating an image that has orientation already, but may it probably won't work for an image that doesn't; I'll write another test over that.

@ginpei
Copy link
Owner

ginpei commented Jul 9, 2020

For an image without orientation data, I suppose we don't have to support it and throw a not support error instead. There are another options to deal with JPEG/Exif things with many features. I would rather keep mine simple one.

@TonyBrobston
Copy link
Contributor Author

I would also like to propose changing to updateOrientation as this implies there already is an orientation to update. Whereas setOrientation implies we'll set it no matter what, which isn't the case.

@TonyBrobston
Copy link
Contributor Author

I went ahead and converted to getOrientation from your package. Once updateOrientation is added I'll implement that. Here's a reference incase you're interested in seeing it:
https://github.com/TonyBrobston/jpegasus/blob/8f75758be83d90e508f42db5d6e48489a981dd9d/src/services/canvasService.ts#L18-L43

@ginpei
Copy link
Owner

ginpei commented Jul 13, 2020

@ginpei ginpei closed this as completed Jul 13, 2020
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

2 participants