fix: switch width and height of target resolution if portrait for Android #833

xulihang · 2022-02-17T08:50:01Z

What

This PR fixes the incorrect resolution setting for Android.

Changes

switch height and width of target resolution if portrait for Android

Tested on

Sharp S2, Android 8

Related issues

#770
#325
#281

…roid

mrousavy · 2022-02-17T10:08:27Z

What happens when the user rotates his phone after the Camera has been initialized?

xulihang · 2022-02-17T10:13:26Z

In my case, the camera still works in the specified resolution.

xulihang · 2022-02-17T11:38:02Z

In the docs, it says that the resolution Size should be expressed in the coordinate frame after rotating the supported sizes by the target rotation: https://developer.android.com/reference/kotlin/androidx/camera/core/ImageAnalysis.Builder#setTargetResolution(android.util.Size)

ldstein · 2022-02-18T07:27:42Z

FWIW I've been using a hack which shares similarities with what @xulihang proposes.

https://gist.github.com/ldstein/bf8b88fabc27617dca891aa3be58f9d7

Seemed to get me out of trouble for the limited devices I have on hand.

I noticed in this PR, Surface.ROTATION_0 is being used to determine if the width and height should be reversed. This works for devices whose natural orientation is potrait but may have the opposite effect for devices which are landscape natural orientation (I say "may" because I have no such devices on hand to test).

An alternative would be to reference context.resources.configuration.orientation as this will inform us explicitly if the device is currently in Landscape or Portrait.

What happens when the user rotates his phone after the Camera has been initialized?

As @xulihang mentions, it appears to work. Maybe under the hood it's already flipping it around when targetRotation is set during updateOrientation()?

I do wonder however if imageAnalysis.targetRotation and preview.targetRotation should both be set to inputRotation to keep them 1:1, but maybe that's a different topic.

xulihang · 2022-02-18T08:49:53Z

This works for devices whose natural orientation is potrait but may have the opposite effect for devices which are landscape natural orientation

In the docs: https://developer.android.com/training/camera2/camera-preview-large-screens#camera-orientation, it says that the camera sensor’s natural orientation is landscape.

ldstein · 2022-02-18T10:41:15Z

In the above link it states:

The Android Compatibility Definition specifies that a camera image sensor “MUST be oriented so that the long dimension of the camera aligns with the screen’s long dimension.”

If you have an Android tablet which is naturally landscape orientated, this logic I think will incorrectly set the wrong resolution:

// Asume:
// Device Rotation: 0
// Device Screen Size: 1024x768
// Camera Sensor Orientation: Landscape (same as screen)
// format.videoSize.width:1024
// format.videoSize.height: 768

if (getDisplay().rotation == Surface.ROTATION_0) {
          val newWidth = format.videoSize.height // 768
          val newHeight = format.videoSize.width // 1024
          targetPreviewResolution = Size(newWidth, newHeight) // resolution is incorrectly set to portrait

I think that's why my original hack fixed the resolution for phones, but did not work on @viljark 's TB-X505L tablet.

format.videoSize appears to always be reported in landscape, so I think we only need to set a portrait targetResolution when the device is in portrait:

// Asume:
// Device Rotation: 0
// Device Screen Size: 1024x768
// Camera Sensor Orientation: Landscape (same as screen)
// format.videoSize.width:1024
// format.videoSize.height: 768

val videoSize = when (context.resources.configuration.orientation) {
    Configuration.ORIENTATION_PORTRAIT -> Size(format.videoSize.height, format.videoSize.width) // resolution is set to portrait
    else -> format.videoSize // resolution is set to landscape
}

Having said all this, I'm no expert, so happy to be proven wrong :)

xulihang · 2022-02-18T11:27:52Z

Using orientation seems a better solution. I also find a page on detecting the natural orientation: http://www.java2s.com/example/android/user-interface/get-device-natural-orientation.html

xulihang · 2022-02-21T05:34:05Z

I've updated the code to use orientation.

mrousavy · 2022-02-21T08:19:02Z

android/src/main/java/com/mrousavy/camera/CameraView.kt

@@ -410,9 +411,15 @@ class CameraView(context: Context, private val frameProcessorThread: ExecutorSer
        // User has selected a custom format={}. Use that
        val format = DeviceFormat(format!!)
        Log.i(TAG, "Using custom format - photo: ${format.photoSize}, video: ${format.videoSize} @ $fps FPS")
-        previewBuilder.setTargetResolution(format.videoSize)
+        val videoSize:Size
+        if (context.resources.configuration.orientation == Configuration.ORIENTATION_PORTRAIT) {


Hey, thanks again for the PR.

Unfortunately I am still not quite sure if I understand this correctly, doesn't this only check the "locked" Orientation in AndroidManifest? What if the user is holding his phone in portrait, then going to landscape? And then going back to portrait? Is it still the correct resolution?

If I read the docs correctly, context.resources.configuration.orientation returns the device's current orientation.

Okay, but can we use outputOrientation instead of context.resources.configuration.orientation here? I already detect orientation in this class

outputRotation is referencing context.displayRotation. That tells us how many degrees the device is rotated from it's natural position but I believe we cannot assume zero rotation is portrait.

context.resources.configuration.orientation differs as it tells us explicitly whether the device is currently being viewed in portrait or landscape.

Similarly, there maybe an issue here in the outputRotation getter. Portrait would only equal Surface.ROTATION_0 for naturally-portrait devices:

private val outputRotation: Int get() { if (orientation != null) { // user is overriding output orientation return when (orientation!!) { "portrait" -> Surface.ROTATION_0 "landscapeRight" -> Surface.ROTATION_90 "portraitUpsideDown" -> Surface.ROTATION_180 "landscapeLeft" -> Surface.ROTATION_270

Also not 100% sure on this, but maybe imageAnalysis.targetRotation should be the same as preview.targetRotation ? Keeps the analysis results 1:1 with the preview (when drawing overlays, for example).

I can see the use of the 'orientation' prop to affect photo / video output, but maybe should be independent to modifying the frame processor rotation.

I cannot find outputOrientation. There is orientation which is a string and outputRotation which is an integer.

Sorry, typo on my end. I fixed up my earlier post. I meant outputRotation

Okay guys thanks for the insights. I will test this some time this week and then merge the PR if everything works in every use-case :)

Would be helpful if anyone could help me with testing, with various devices and various orientations (e.g. start with landscape or portrait, then rotate)

@mrousavy @xulihang What are your views on this PR, is it ready to get merged or ...?

Use `ImageProxy.imageInfo.rotationDegrees` to apply the appropriate rotation to the image in order to align the orientation of the image with the target orientation. Potential alternative to mrousavy#833

bekatd · 2022-05-27T16:23:25Z

Camera formats and everything related to it are are completely wrong.

width and heights are swapped,
unable to set desired formats,
also photoWidth and photoHeight of useCameraFormat hook are totally wrong, since the width and height of frame in frameprocessor plugin is for example 640x640 while current cameraformat is 2000x4000 :|
also camera feed strangely zooms on different formats
also if you log all available formats and look at the format returned by default, you will discover that it can't be found in the available formats :|
also on every different device everything works differently and wrongly :|

how all of this is even possible :|

mrousavy · 2023-09-30T13:40:42Z

Hey, thanks for your PR and sorry for the long radio silence here! width and height are swapped because Camera sensors are in 90deg orientation. Orientation is now tracked in this issue: #1891

xulihang added 2 commits February 17, 2022 16:46

fix: switch width and height of target resolution if portrait for And…

412dc2d

…roid

add missing import

a5789f9

use orientation instead of rotation

ee6eef2

mrousavy reviewed Feb 21, 2022

View reviewed changes

mrousavy mentioned this pull request Feb 21, 2022

🐛 takePhoto always returns asset with landscape orientation on iOS #818

Closed

4 tasks

mrousavy closed this Sep 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: switch width and height of target resolution if portrait for Android #833

fix: switch width and height of target resolution if portrait for Android #833

xulihang commented Feb 17, 2022

mrousavy commented Feb 17, 2022

xulihang commented Feb 17, 2022

xulihang commented Feb 17, 2022

ldstein commented Feb 18, 2022 •

edited

Loading

xulihang commented Feb 18, 2022

ldstein commented Feb 18, 2022 •

edited

Loading

xulihang commented Feb 18, 2022

xulihang commented Feb 21, 2022

mrousavy Feb 21, 2022

xulihang Feb 21, 2022

mrousavy Feb 21, 2022

ldstein Feb 21, 2022 •

edited

Loading

xulihang Feb 21, 2022

ldstein Feb 21, 2022

mrousavy Feb 21, 2022

siddharth-kt Mar 31, 2022 •

edited

Loading

bekatd commented May 27, 2022

mrousavy commented Sep 30, 2023

fix: switch width and height of target resolution if portrait for Android #833

fix: switch width and height of target resolution if portrait for Android #833

Conversation

xulihang commented Feb 17, 2022

What

Changes

Tested on

Related issues

mrousavy commented Feb 17, 2022

xulihang commented Feb 17, 2022

xulihang commented Feb 17, 2022

ldstein commented Feb 18, 2022 • edited Loading

xulihang commented Feb 18, 2022

ldstein commented Feb 18, 2022 • edited Loading

xulihang commented Feb 18, 2022

xulihang commented Feb 21, 2022

mrousavy Feb 21, 2022

Choose a reason for hiding this comment

xulihang Feb 21, 2022

Choose a reason for hiding this comment

mrousavy Feb 21, 2022

Choose a reason for hiding this comment

ldstein Feb 21, 2022 • edited Loading

Choose a reason for hiding this comment

xulihang Feb 21, 2022

Choose a reason for hiding this comment

ldstein Feb 21, 2022

Choose a reason for hiding this comment

mrousavy Feb 21, 2022

Choose a reason for hiding this comment

siddharth-kt Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

bekatd commented May 27, 2022

mrousavy commented Sep 30, 2023

ldstein commented Feb 18, 2022 •

edited

Loading

ldstein commented Feb 18, 2022 •

edited

Loading

ldstein Feb 21, 2022 •

edited

Loading

siddharth-kt Mar 31, 2022 •

edited

Loading