Skip to content
Merged
Show file tree
Hide file tree
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
377 changes: 349 additions & 28 deletions packages/camera/camera/test/camera_preview_test.dart

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions packages/camera/camera_android_camerax/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.14

* Fixes incorrect camera preview rotation.

## 0.6.13

* Adds API support query for image streaming.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1441,9 +1441,6 @@ void requestCameraPermissions(
@NonNull
String getTempFilePath(@NonNull String prefix, @NonNull String suffix);

@NonNull
Boolean isPreviewPreTransformed();

/** The codec used by SystemServicesHostApi. */
static @NonNull MessageCodec<Object> getCodec() {
return SystemServicesHostApiCodec.INSTANCE;
Expand Down Expand Up @@ -1511,29 +1508,6 @@ public void error(Throwable error) {
channel.setMessageHandler(null);
}
}
{
BasicMessageChannel<Object> channel =
new BasicMessageChannel<>(
binaryMessenger,
"dev.flutter.pigeon.SystemServicesHostApi.isPreviewPreTransformed",
getCodec());
if (api != null) {
channel.setMessageHandler(
(message, reply) -> {
ArrayList<Object> wrapped = new ArrayList<Object>();
try {
Boolean output = api.isPreviewPreTransformed();
wrapped.add(0, output);
} catch (Throwable exception) {
ArrayList<Object> wrappedError = wrapError(exception);
wrapped = wrappedError;
}
reply.reply(wrapped);
});
} else {
channel.setMessageHandler(null);
}
}
}
}
/** Generated class from Pigeon that represents Flutter messages that can be called from Java. */
Expand Down Expand Up @@ -1761,6 +1735,9 @@ void create(

void setTargetRotation(@NonNull Long identifier, @NonNull Long rotation);

@NonNull
Boolean surfaceProducerHandlesCropAndRotation();

/** The codec used by PreviewHostApi. */
static @NonNull MessageCodec<Object> getCodec() {
return PreviewHostApiCodec.INSTANCE;
Expand Down Expand Up @@ -1898,6 +1875,29 @@ static void setup(@NonNull BinaryMessenger binaryMessenger, @Nullable PreviewHos
channel.setMessageHandler(null);
}
}
{
BasicMessageChannel<Object> channel =
new BasicMessageChannel<>(
binaryMessenger,
"dev.flutter.pigeon.PreviewHostApi.surfaceProducerHandlesCropAndRotation",
getCodec());
if (api != null) {
channel.setMessageHandler(
(message, reply) -> {
ArrayList<Object> wrapped = new ArrayList<Object>();
try {
Boolean output = api.surfaceProducerHandlesCropAndRotation();
wrapped.add(0, output);
} catch (Throwable exception) {
ArrayList<Object> wrappedError = wrapError(exception);
wrapped = wrappedError;
}
reply.reply(wrapped);
});
} else {
channel.setMessageHandler(null);
}
}
}
}
/** Generated interface from Pigeon that represents a handler of messages from Flutter. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ String getProvideSurfaceErrorDescription(int resultCode) {
public void releaseFlutterSurfaceTexture() {
if (flutterSurfaceProducer != null) {
flutterSurfaceProducer.release();
return;
}
throw new IllegalStateException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does our code catch IllegalStateException? if we dont where do we document for our users that they need to catch an exception?

Native code thrown exceptions I remember tripping up my previous team on a couple of occasions to the point where I think we made a rule that exceptions were not allowed to leave the plugin api and if they did it was considered a plugin bug.

I think our best guidance is here https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We throw exceptions like this throughout the camera_android_camerax implementation to signal that there may be an error in the plugin implementation to plugin authors, so this is not meant to be caught by users.

If you have greater concerns about this pattern, I suggest we file an issue and address this for the plugin as a whole in a follow up PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, Non blocking question. Do we catch this (or other) exception(s) or is it thrown to users code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't catch it. They're really just supposed to help debug.

"releaseFlutterSurfaceTexture() cannot be called if the flutterSurfaceProducer for the camera preview has not yet been initialized.");
}

/** Returns the resolution information for the specified {@link Preview}. */
Expand All @@ -179,6 +182,16 @@ public void setTargetRotation(@NonNull Long identifier, @NonNull Long rotation)
preview.setTargetRotation(rotation.intValue());
}

@NonNull
@Override
public Boolean surfaceProducerHandlesCropAndRotation() {
if (flutterSurfaceProducer != null) {
return flutterSurfaceProducer.handlesCropAndRotation();
}
throw new IllegalStateException(
"surfaceProducerHandlesCropAndRotation() cannot be called if the flutterSurfaceProducer for the camera preview has not yet been initialized.");
}

/** Retrieves the {@link Preview} instance associated with the specified {@code identifier}. */
private Preview getPreviewInstance(@NonNull Long identifier) {
return Objects.requireNonNull(instanceManager.getInstance(identifier));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import android.app.Activity;
import android.content.Context;
import android.os.Build;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
Expand Down Expand Up @@ -104,18 +103,4 @@ public String getTempFilePath(@NonNull String prefix, @NonNull String suffix) {
null);
}
}

/**
* Returns whether or not Impeller uses an {@code ImageReader} backend to provide a {@code
* Surface} to CameraX to build the preview. If it is backed by an {@code ImageReader}, then
* CameraX will not automatically apply the transformation needed to correct the preview.
*
* <p>This is determined by the engine, which approximately uses {@code SurfaceTexture}s on
* Android SDKs below 29.
*/
@Override
@NonNull
public Boolean isPreviewPreTransformed() {
return Build.VERSION.SDK_INT < 29;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,20 @@ public void setTargetRotation_makesCallToSetTargetRotation() {
verify(mockPreview).setTargetRotation(targetRotation);
}

@Test
public void
surfaceProducerHandlesCropAndRotation_returnsIfSurfaceProducerHandlesCropAndRotation() {
final PreviewHostApiImpl hostApi =
new PreviewHostApiImpl(mockBinaryMessenger, testInstanceManager, mockTextureRegistry);
final TextureRegistry.SurfaceProducer mockSurfaceProducer =
mock(TextureRegistry.SurfaceProducer.class);

hostApi.flutterSurfaceProducer = mockSurfaceProducer;
when(mockSurfaceProducer.handlesCropAndRotation()).thenReturn(true);

assertEquals(hostApi.surfaceProducerHandlesCropAndRotation(), true);
}

// TODO(bparrishMines): Replace with inline calls to onSurfaceCleanup once available on stable;
// see https://github.com/flutter/flutter/issues/16125. This separate method only exists to scope
// the suppression.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
package io.flutter.plugins.camerax;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
Expand All @@ -33,7 +31,6 @@
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
public class SystemServicesTest {
Expand Down Expand Up @@ -136,28 +133,4 @@ public void getTempFilePath_throwsRuntimeExceptionOnIOException() {

mockedStaticFile.close();
}

@Test
@Config(sdk = 28)
public void isPreviewPreTransformed_returnsTrueWhenRunningBelowSdk29() {
final SystemServicesHostApiImpl systemServicesHostApi =
new SystemServicesHostApiImpl(mockBinaryMessenger, mockInstanceManager, mockContext);
assertTrue(systemServicesHostApi.isPreviewPreTransformed());
}

@Test
@Config(sdk = 28)
public void isPreviewPreTransformed_returnsTrueWhenRunningSdk28() {
final SystemServicesHostApiImpl systemServicesHostApi =
new SystemServicesHostApiImpl(mockBinaryMessenger, mockInstanceManager, mockContext);
assertTrue(systemServicesHostApi.isPreviewPreTransformed());
}

@Test
@Config(sdk = 29)
public void isPreviewPreTransformed_returnsFalseWhenRunningAboveSdk28() {
final SystemServicesHostApiImpl systemServicesHostApi =
new SystemServicesHostApiImpl(mockBinaryMessenger, mockInstanceManager, mockContext);
assertFalse(systemServicesHostApi.isPreviewPreTransformed());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import 'recording.dart';
import 'resolution_filter.dart';
import 'resolution_selector.dart';
import 'resolution_strategy.dart';
import 'rotated_preview.dart';
import 'surface.dart';
import 'system_services.dart';
import 'use_case.dart';
Expand Down Expand Up @@ -238,12 +239,18 @@ class AndroidCameraCameraX extends CameraPlatform {
late bool cameraIsFrontFacing;

/// The camera sensor orientation.
///
/// This can change if the camera being used changes. Also, it is independent
/// of the device orientation or user interface orientation.
@visibleForTesting
late int sensorOrientation;
late double sensorOrientationDegrees;

/// Whether or not the Android surface producer automatically handles
/// correcting the rotation of camera previews for the device this plugin runs on.
late bool _handlesCropAndRotation;

/// Subscription for listening to changes in device orientation.
StreamSubscription<DeviceOrientationChangedEvent>?
_subscriptionForDeviceOrientationChanges;
/// The initial orientation of the device when the camera is created.
late DeviceOrientation _initialDeviceOrientation;

/// Returns list of all available cameras and their descriptions.
@override
Expand Down Expand Up @@ -380,10 +387,10 @@ class AndroidCameraCameraX extends CameraPlatform {

// Retrieve info required for correcting the rotation of the camera preview
// if necessary.

final Camera2CameraInfo camera2CameraInfo =
await proxy.getCamera2CameraInfo(cameraInfo!);
sensorOrientation = await proxy.getSensorOrientation(camera2CameraInfo);
sensorOrientationDegrees = cameraDescription.sensorOrientation.toDouble();
_handlesCropAndRotation =
await proxy.previewSurfaceProducerHandlesCropAndRotation(preview!);
_initialDeviceOrientation = await proxy.getUiOrientation();

return flutterSurfaceTextureId;
}
Expand Down Expand Up @@ -444,7 +451,6 @@ class AndroidCameraCameraX extends CameraPlatform {
await liveCameraState?.removeObservers();
processCameraProvider?.unbindAll();
await imageAnalysis?.clearAnalyzer();
await _subscriptionForDeviceOrientationChanges?.cancel();
}

/// The camera has been initialized.
Expand Down Expand Up @@ -836,7 +842,30 @@ class AndroidCameraCameraX extends CameraPlatform {
);
}

return Texture(textureId: cameraId);
final Widget preview = Texture(textureId: cameraId);

if (_handlesCropAndRotation) {
return preview;
}

final Stream<DeviceOrientation> deviceOrientationStream =
onDeviceOrientationChanged()
.map((DeviceOrientationChangedEvent e) => e.orientation);
if (cameraIsFrontFacing) {
return RotatedPreview.frontFacingCamera(
_initialDeviceOrientation,
deviceOrientationStream,
sensorOrientationDegrees: sensorOrientationDegrees,
child: preview,
);
} else {
return RotatedPreview.backFacingCamera(
_initialDeviceOrientation,
deviceOrientationStream,
sensorOrientationDegrees: sensorOrientationDegrees,
child: preview,
);
}
}

/// Captures an image and returns the file where it was saved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,33 +1003,6 @@ class SystemServicesHostApi {
return (replyList[0] as String?)!;
}
}

Future<bool> isPreviewPreTransformed() async {
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.SystemServicesHostApi.isPreviewPreTransformed',
codec,
binaryMessenger: _binaryMessenger);
final List<Object?>? replyList = await channel.send(null) as List<Object?>?;
if (replyList == null) {
throw PlatformException(
code: 'channel-error',
message: 'Unable to establish connection on channel.',
);
} else if (replyList.length > 1) {
throw PlatformException(
code: replyList[0]! as String,
message: replyList[1] as String?,
details: replyList[2],
);
} else if (replyList[0] == null) {
throw PlatformException(
code: 'null-error',
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (replyList[0] as bool?)!;
}
}
}

abstract class SystemServicesFlutterApi {
Expand Down Expand Up @@ -1356,6 +1329,33 @@ class PreviewHostApi {
return;
}
}

Future<bool> surfaceProducerHandlesCropAndRotation() async {
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.PreviewHostApi.surfaceProducerHandlesCropAndRotation',
codec,
binaryMessenger: _binaryMessenger);
final List<Object?>? replyList = await channel.send(null) as List<Object?>?;
if (replyList == null) {
throw PlatformException(
code: 'channel-error',
message: 'Unable to establish connection on channel.',
);
} else if (replyList.length > 1) {
throw PlatformException(
code: replyList[0]! as String,
message: replyList[1] as String?,
details: replyList[2],
);
} else if (replyList[0] == null) {
throw PlatformException(
code: 'null-error',
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (replyList[0] as bool?)!;
}
}
}

class VideoCaptureHostApi {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class CameraXProxy {
this.getCamera2CameraInfo = _getCamera2CameraInfo,
this.getUiOrientation = _getUiOrientation,
this.getSensorOrientation = _getSensorOrientation,
this.previewSurfaceProducerHandlesCropAndRotation =
_previewSurfaceProducerHandlesCropAndRotation,
});

/// Returns a [ProcessCameraProvider] instance.
Expand Down Expand Up @@ -200,6 +202,11 @@ class CameraXProxy {
Future<int> Function(Camera2CameraInfo camera2CameraInfo)
getSensorOrientation;

/// Returns whether or not the preview's surface producer handles correctly
/// rotating the camera preview automatically.
Future<bool> Function(Preview preview)
previewSurfaceProducerHandlesCropAndRotation;

static Future<ProcessCameraProvider> _getProcessCameraProvider() {
return ProcessCameraProvider.getInstance();
}
Expand Down Expand Up @@ -355,4 +362,9 @@ class CameraXProxy {
Camera2CameraInfo camera2CameraInfo) async {
return camera2CameraInfo.getSensorOrientation();
}

static Future<bool> _previewSurfaceProducerHandlesCropAndRotation(
Preview preview) async {
return preview.surfaceProducerHandlesCropAndRotation();
}
}
Loading