Skip to content

[camera_android_camerax] Fix the rotation of captured videos and streamed images #9637

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,6 @@ class AndroidCameraCameraX extends CameraPlatform {
@visibleForTesting
bool captureOrientationLocked = false;

/// Whether or not the default rotation for [UseCase]s needs to be set
/// manually because the capture orientation was previously locked.
///
/// Currently, CameraX provides no way to unset target rotations for
/// [UseCase]s, so once they are set and unset, this plugin must start setting
/// the default orientation manually.
///
/// See https://developer.android.com/reference/androidx/camera/core/ImageCapture#setTargetRotation(int)
/// for an example on how setting target rotations for [UseCase]s works.
bool shouldSetDefaultRotation = false;

/// Error code indicating that an exposure offset value failed to be set.
static const String setExposureOffsetFailedErrorCode =
'setExposureOffsetFailed';
Expand Down Expand Up @@ -390,17 +379,18 @@ class AndroidCameraCameraX extends CameraPlatform {
);

// Configure ImageCapture instance.
_initialDefaultDisplayRotation =
await deviceOrientationManager.getDefaultDisplayRotation();

Choose a reason for hiding this comment

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

medium

Consider extracting deviceOrientationManager.getDefaultDisplayRotation() to a local variable before calling proxy.newImageCapture. This avoids calling the platform multiple times, which could be inefficient.

final int defaultDisplayRotation = await deviceOrientationManager.getDefaultDisplayRotation();

imageCapture = proxy.newImageCapture(
resolutionSelector: presetResolutionSelector,
/* use CameraX default target rotation */ targetRotation:
await deviceOrientationManager.getDefaultDisplayRotation(),
targetRotation: _initialDefaultDisplayRotation,
);

// Configure ImageAnalysis instance.
// Defaults to YUV_420_888 image format.
imageAnalysis = proxy.newImageAnalysis(
resolutionSelector: presetResolutionSelector,
/* use CameraX default target rotation */ targetRotation: null,
targetRotation: _initialDefaultDisplayRotation,
);

// Configure VideoCapture and Recorder instances.
Expand Down Expand Up @@ -437,8 +427,6 @@ class AndroidCameraCameraX extends CameraPlatform {
_initialDeviceOrientation = _deserializeDeviceOrientation(
await deviceOrientationManager.getUiOrientation(),
);
_initialDefaultDisplayRotation =
await deviceOrientationManager.getDefaultDisplayRotation();

return flutterSurfaceTextureId;
}
Expand Down Expand Up @@ -552,10 +540,7 @@ class AndroidCameraCameraX extends CameraPlatform {
int cameraId,
DeviceOrientation orientation,
) async {
// Flag that (1) default rotation for UseCases will need to be set manually
// if orientation is ever unlocked and (2) the capture orientation is locked
// and should not be changed until unlocked.
shouldSetDefaultRotation = true;
// Flag that the capture orientation is locked and should not be changed until unlocked.
captureOrientationLocked = true;

// Get target rotation based on locked orientation.
Expand Down Expand Up @@ -1104,7 +1089,7 @@ class AndroidCameraCameraX extends CameraPlatform {

// Set target rotation to default CameraX rotation only if capture
// orientation not locked.
if (!captureOrientationLocked && shouldSetDefaultRotation) {
if (!captureOrientationLocked) {
await videoCapture!.setTargetRotation(
await deviceOrientationManager.getDefaultDisplayRotation(),
Comment on lines +1092 to 1094

Choose a reason for hiding this comment

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

medium

Instead of calling deviceOrientationManager.getDefaultDisplayRotation() directly within setTargetRotation, consider storing the value in a variable and reusing it. This avoids redundant calls to the platform, improving efficiency.

final rotation = await deviceOrientationManager.getDefaultDisplayRotation();
await videoCapture!.setTargetRotation(
  rotation,
);

);
Expand Down Expand Up @@ -1253,7 +1238,7 @@ class AndroidCameraCameraX extends CameraPlatform {

// Set target rotation to default CameraX rotation only if capture
// orientation not locked.
if (!captureOrientationLocked && shouldSetDefaultRotation) {
if (!captureOrientationLocked) {
await imageAnalysis!.setTargetRotation(
await deviceOrientationManager.getDefaultDisplayRotation(),
Comment on lines +1241 to 1243

Choose a reason for hiding this comment

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

medium

Instead of calling deviceOrientationManager.getDefaultDisplayRotation() directly within setTargetRotation, consider storing the value in a variable and reusing it. This avoids redundant calls to the platform, improving efficiency.

final rotation = await deviceOrientationManager.getDefaultDisplayRotation();
      await imageAnalysis!.setTargetRotation(
        rotation,
      );

);
Expand Down