Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Conversation

UKDeveloper99
Copy link
Contributor

@UKDeveloper99 UKDeveloper99 commented Feb 11, 2021

Description of Change

This should fix issue with incorrectly rotated media capture on some Android devices. As this article implies. The JPEG_ORIENTATION flag in CaptureRequest is effectively ignored in some devices and the Android docs also imply it will have varying effects across devices.

See Android docs
There are two points about the implementation I would like to elaborate on. Firstly the removal or the JPEG_ORIENTATION flag. I wasn't able to reliably determine (in code) across various devices (some with rotation issue some not) if the addition of the flag had rotated the capture data or modified the Exif info. I don't believe this flag is needed at all now. As we now calculate the correct rotation upon capture. If the JPEG_ORIENTATION flag has modified the capture data by rotation. Then we will be applying another rotation on top of that. If it hasn't then only our rotation will be applied. Either way I can't see an easy way to determine that. Setting any rotation value for this flag on certain devices will have no effect.
So with those assumptions, if the only purpose now of the JPEG_ORIENTATION flag is to modify the Exif orientation info. Then we can remove it and set the Exif information using the ExifInterface class when the SaveToFile implementation has been completed.

Secondly, the correct point to apply the capture rotation. I tried several methods of applying the rotation to the capture data (byte array) all with sub optimal performance. E.g. ByteArray -> Bitmap -> CreateBitmapRotateWithMatrix -> ByteArray. Also copying the pixel data to a ByteBuffer and looking at methods to rotate a Jpeg byte array. With the Forms side needing the data in ByteArray format I believe I have provided the best solution by passing the rotation in degrees with the MediaCaptureEventArgs. This allows the ImageView to handle the rotation in a performant way. Citing a post from StackOverflow. It is not the job of the camera view to change the orientation of the capture data (camera2 api) and I believe this to be true.

No samples or test changes required.

Bugs Fixed

#751

API Changes

None

Added:

  • double MediaCapturedEventArgs.Rotation { get; }
  • int CameraFragment.GetRotationCompensation();

Changed:

  • void CameraFragment.OnPhoto(object sender, Tuple<string, byte[]> tuple) => void CameraFragment.OnPhoto(object sender, Tuple<string, byte[], int> tuple)

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

@ghost
Copy link

ghost commented Feb 11, 2021

CLA assistant check
All CLA requirements met.

@jfversluis
Copy link
Member

Wowza! Thanks for this @UKDeveloper99 !

Let me try to understand what this does... The only thing this does is report back what the rotation is, right? So then you should rotate yourself?

@jfversluis jfversluis added a/CameraView 📸 bug Something isn't working. Breaky break. p/android This issue impacts Android api-change 🚨 labels Feb 11, 2021
@UKDeveloper99
Copy link
Contributor Author

UKDeveloper99 commented Feb 11, 2021

@jfversluis no problem, don't thank me too much, I'll end up getting carried away and adding more features lol.
Yes so it uses a Google recommended (recent/current) func to get the correct rotation for sensor and device orientation then passes that along with the Byte data which the user can use to set a rotation on their image view. Shouldn't effect iOS. It seemed not feasible to rotate the image data in the camera fragment.

I've tested it with all the devices I have here some with bad rotation some without and it corrects the rotation on all of them. For front and rear cam.

@jfversluis
Copy link
Member

I think we can use some help in the CameraView area so no worries, just make sure to coordinate before taking on too much! :)

I'll have a more in-depth look at this as soon as I have the time.

Appreciate the effort!

@UKDeveloper99
Copy link
Contributor Author

@jfversluis well I'm relying on this fix in my current app so I thought I'd lend a hand. Glad I could be helpful. How long do you think that might be?

Sure, how would I go about coordinating, do you have a Slack of some sorts or something?

@jfversluis
Copy link
Member

How would one go about using this to show the image correctly? I don't think we have anything for that in Forms today, right?

We have a Discord! The DotNetEvolution one, you should be able to join here https://aka.ms/dotnet-discord and find us in the XamarinCommunityToolkit channel :)

@UKDeveloper99
Copy link
Contributor Author

@jfversluis Could you elaborate what you mean by that sorry? I added the usage in to the camera sample page.
Thanks I'll join up now.

Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small nits. Overall I think we should give this a try! Again, thanks so much!

sessionBuilder.AddTarget(photoReader.Surface);
sessionBuilder.Set(CaptureRequest.FlashMode, (int)flashMode);
sessionBuilder.Set(CaptureRequest.JpegOrientation, GetJpegOrientation());
/*sessionBuilder.Set(CaptureRequest.JpegOrientation, GetJpegOrientation());*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why keep this around? Let's get it out if it doesn't work!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well someone might have an idea on how to determine if the capture data has been rotated or just applied to the Exif data. I tried to extract the Exif information without success. But yea I don't think it particularly matters. Hopefully it resolves the issue for the people who have run in to it. Happy I could assist!

var rotation = GetRotationCompensation();

// See TODO on CameraView.SavePhotoToFile
// Insert Exif information to jpeg file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a TODO? Do we know how?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's a TODO - it should be fairly simple using the ExifInterface class, which you just provide a File path to and then you can set the Exif data accordingly as far as I'm aware. E.g. pass the calculated orientation to the Orientation tag. I might have a look at doing something with the SavePhotoToFile functionality if no one is looking at it. Coordinating with yourselves of course.

@jfversluis jfversluis added this to the v1.0.3 milestone Feb 16, 2021
@jfversluis jfversluis merged commit 52db518 into xamarin:main Feb 16, 2021
@UKDeveloper99 UKDeveloper99 deleted the camera-media-rotation-android-issue branch February 16, 2021 10:03
inforithmics pushed a commit to inforithmics/XamarinCommunityToolkit that referenced this pull request Feb 19, 2021
* xamarin#751

* Update camera view sample.

Co-authored-by: Mark Silver <[email protected]>
Co-authored-by: Gerald Versluis <[email protected]>
@AmirMahdiNassiri
Copy link

Hi guys. Thanks for all your efforts!

Is there a reason preventing us from using the new GetRotationCompensation function for mediaRecorder.SetOrientationHint as well?

I mean, my front camera on a Samsung Note 10 was capturing videos in this direction ← and I'm aiming for a vertical portrait by the front camera. But when I use the resulting value of the new GetRotationCompensation for mediaRecorder.SetOrientationHint as well, everything looks fine!

@matteovelati
Copy link

@jfversluis no problem, don't thank me too much, I'll end up getting carried away and adding more features lol.
Yes so it uses a Google recommended (recent/current) func to get the correct rotation for sensor and device orientation then passes that along with the Byte data which the user can use to set a rotation on their image view. Shouldn't effect iOS. It seemed not feasible to rotate the image data in the camera fragment.

I've tested it with all the devices I have here some with bad rotation some without and it corrects the rotation on all of them. For front and rear cam.

How do you rotate the byte array?

@UKDeveloper99
Copy link
Contributor Author

@jfversluis no problem, don't thank me too much, I'll end up getting carried away and adding more features lol.
Yes so it uses a Google recommended (recent/current) func to get the correct rotation for sensor and device orientation then passes that along with the Byte data which the user can use to set a rotation on their image view. Shouldn't effect iOS. It seemed not feasible to rotate the image data in the camera fragment.
I've tested it with all the devices I have here some with bad rotation some without and it corrects the rotation on all of them. For front and rear cam.

How do you rotate the byte array?

Load the byte array in to a UI component such as UIImageView or ImageView and supply the rotation to the UI component.

@matteovelati
Copy link

@jfversluis no problem, don't thank me too much, I'll end up getting carried away and adding more features lol.
Yes so it uses a Google recommended (recent/current) func to get the correct rotation for sensor and device orientation then passes that along with the Byte data which the user can use to set a rotation on their image view. Shouldn't effect iOS. It seemed not feasible to rotate the image data in the camera fragment.
I've tested it with all the devices I have here some with bad rotation some without and it corrects the rotation on all of them. For front and rear cam.

How do you rotate the byte array?

Load the byte array in to a UI component such as UIImageView or ImageView and supply the rotation to the UI component.

Yeah but that's only a UI rotation. I'm interested in rotating the image by 90 degrees in order to store it in my online database. Everything I found on the web (SO) is outdated or not working with Xamarin.Forms. Do you have any suggestions?
Thanks

@UKDeveloper99
Copy link
Contributor Author

Can't you just store it in your database as it is with the rotation and then when you come to display it then rotate it in the UI. I did have a look down that path but it also depends on image format (jpg etc). And a few other factors. It's really complex I think you should avoid doing that if you can. What's the purpose of rotating it before it goes in to the online database?

@matteovelati
Copy link

I need to perform OCR on server side for all the images coming from my cross-platform application, and the mismatch in rotation between pictures taken with Android or iOS phones is a problem for me.

Actually it is a bug that the same framework behaves differently on different platforms, isn't it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/CameraView 📸 api-change 🚨 bug Something isn't working. Breaky break. p/android This issue impacts Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants