[camerax] Implement onCameraClosing #3419
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.camerax; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.camera.core.Camera; | ||
| import androidx.camera.core.CameraInfo; | ||
| import io.flutter.plugin.common.BinaryMessenger; | ||
| import io.flutter.plugins.camerax.GeneratedCameraXLibrary.CameraHostApi; | ||
| import java.util.Objects; | ||
|
|
||
| public class CameraHostApiImpl implements CameraHostApi { | ||
| private final BinaryMessenger binaryMessenger; | ||
| private final InstanceManager instanceManager; | ||
|
|
||
| public CameraHostApiImpl(BinaryMessenger binaryMessenger, InstanceManager instanceManager) { | ||
|
camsim99 marked this conversation as resolved.
Outdated
|
||
| this.binaryMessenger = binaryMessenger; | ||
| this.instanceManager = instanceManager; | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves the {@link CameraInfo} instance that contains information about the {@link Camera} | ||
| * instance with the specified identifier. | ||
| */ | ||
| @Override | ||
| public Long getCameraInfo(@NonNull Long identifier) { | ||
| Camera camera = (Camera) Objects.requireNonNull(instanceManager.getInstance(identifier)); | ||
| CameraInfo cameraInfo = camera.getCameraInfo(); | ||
|
|
||
| if (!instanceManager.containsInstance(cameraInfo)) { | ||
| CameraInfoFlutterApiImpl cameraInfoFlutterApiImpl = | ||
| new CameraInfoFlutterApiImpl(binaryMessenger, instanceManager); | ||
| cameraInfoFlutterApiImpl.create(cameraInfo, reply -> {}); | ||
|
camsim99 marked this conversation as resolved.
|
||
| } | ||
| return instanceManager.getIdentifierForStrongReference(cameraInfo); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.camerax; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.VisibleForTesting; | ||
| import androidx.camera.core.CameraState; | ||
| import io.flutter.plugin.common.BinaryMessenger; | ||
| import io.flutter.plugins.camerax.GeneratedCameraXLibrary.CameraStateErrorFlutterApi; | ||
|
|
||
| /** | ||
| * Flutter API implementation for {@link CameraStateError}. | ||
| * | ||
| * <p>This class may handle adding native instances that are attached to a Dart instance or passing | ||
| * arguments of callbacks methods to a Dart instance. | ||
| */ | ||
| public class CameraStateErrorFlutterApiWrapper { | ||
| private final BinaryMessenger binaryMessenger; | ||
| private final InstanceManager instanceManager; | ||
| private CameraStateErrorFlutterApi cameraStateErrorFlutterApi; | ||
|
|
||
| /** | ||
| * Constructs a {@link CameraStateErrorFlutterApiWrapper}. | ||
| * | ||
| * @param binaryMessenger used to communicate with Dart over asynchronous messages | ||
| * @param instanceManager maintains instances stored to communicate with attached Dart objects | ||
| */ | ||
| public CameraStateErrorFlutterApiWrapper( | ||
| @NonNull BinaryMessenger binaryMessenger, @NonNull InstanceManager instanceManager) { | ||
| this.binaryMessenger = binaryMessenger; | ||
| this.instanceManager = instanceManager; | ||
| cameraStateErrorFlutterApi = new CameraStateErrorFlutterApi(binaryMessenger); | ||
| } | ||
|
|
||
| /** | ||
| * Stores the {@link CameraStateError} instance and notifies Dart to create and store a new {@link | ||
| * CameraStateError} instance that is attached to this one. If {@code instance} has already been | ||
| * added, this method does nothing. | ||
| */ | ||
|
camsim99 marked this conversation as resolved.
|
||
| public void create( | ||
| @NonNull CameraState.StateError instance, | ||
| @NonNull Long code, | ||
| @NonNull String description, | ||
| @NonNull CameraStateErrorFlutterApi.Reply<Void> callback) { | ||
|
reidbaker marked this conversation as resolved.
|
||
| if (!instanceManager.containsInstance(instance)) { | ||
| cameraStateErrorFlutterApi.create( | ||
| instanceManager.addHostCreatedInstance(instance), code, description, callback); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sets the Flutter API used to send messages to Dart. | ||
| * | ||
| * <p>This is only visible for testing. | ||
| */ | ||
| @VisibleForTesting | ||
| void setApi(@NonNull CameraStateErrorFlutterApi api) { | ||
| this.cameraStateErrorFlutterApi = api; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.camerax; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.VisibleForTesting; | ||
| import androidx.camera.core.CameraState; | ||
| import io.flutter.plugin.common.BinaryMessenger; | ||
| import io.flutter.plugins.camerax.GeneratedCameraXLibrary.CameraStateFlutterApi; | ||
| import io.flutter.plugins.camerax.GeneratedCameraXLibrary.CameraStateType; | ||
| import io.flutter.plugins.camerax.GeneratedCameraXLibrary.CameraStateTypeData; | ||
|
|
||
| /** | ||
| * Flutter API implementation for {@link CameraState}. | ||
| * | ||
| * <p>This class may handle adding native instances that are attached to a Dart instance or passing | ||
| * arguments of callbacks methods to a Dart instance. | ||
| */ | ||
| public class CameraStateFlutterApiWrapper { | ||
| private final BinaryMessenger binaryMessenger; | ||
| private final InstanceManager instanceManager; | ||
| private CameraStateFlutterApi cameraStateFlutterApi; | ||
|
|
||
| /** | ||
| * Constructs a {@link CameraStateFlutterApiWrapper}. | ||
| * | ||
| * @param binaryMessenger used to communicate with Dart over asynchronous messages | ||
| * @param instanceManager maintains instances stored to communicate with attached Dart objects | ||
| */ | ||
| public CameraStateFlutterApiWrapper( | ||
| @NonNull BinaryMessenger binaryMessenger, @NonNull InstanceManager instanceManager) { | ||
| this.binaryMessenger = binaryMessenger; | ||
| this.instanceManager = instanceManager; | ||
| cameraStateFlutterApi = new CameraStateFlutterApi(binaryMessenger); | ||
| } | ||
|
|
||
| /** | ||
| * Stores the {@link CameraState} instance and notifies Dart to create and store a new {@link | ||
| * CameraState} instance that is attached to this one. If {@code instance} has already been added, | ||
| * this method does nothing. | ||
| */ | ||
| public void create( | ||
|
camsim99 marked this conversation as resolved.
|
||
| @NonNull CameraState instance, | ||
| @NonNull CameraState.Type type, | ||
| @Nullable CameraState.StateError error, | ||
| @NonNull CameraStateFlutterApi.Reply<Void> callback) { | ||
|
|
||
| // Convert CameraX CameraState.Type to CameraStateType that the Dart side understands. | ||
| CameraStateType cameraStateType = CameraStateType.CLOSED; | ||
|
camsim99 marked this conversation as resolved.
Outdated
camsim99 marked this conversation as resolved.
Outdated
|
||
| switch (type) { | ||
| case CLOSED: | ||
| cameraStateType = CameraStateType.CLOSED; | ||
| break; | ||
| case CLOSING: | ||
| cameraStateType = CameraStateType.CLOSING; | ||
| break; | ||
| case OPEN: | ||
| cameraStateType = CameraStateType.OPEN; | ||
| break; | ||
| case OPENING: | ||
| cameraStateType = CameraStateType.OPENING; | ||
| break; | ||
| case PENDING_OPEN: | ||
| cameraStateType = CameraStateType.PENDING_OPEN; | ||
| break; | ||
| } | ||
|
|
||
| if (error != null) { | ||
|
Contributor
There was a problem hiding this comment. Consider moving the error checking higher in the function since we don't need the conversion code to run if there is an error.
Contributor
Author
There was a problem hiding this comment. The error is separate from the type, so we'll still need it. I did, however, move the instance manager check higher. |
||
| // We need to create a CameraStateError if there is a problem with the current camera | ||
| // state to send to the Dart side. | ||
| new CameraStateErrorFlutterApiWrapper(binaryMessenger, instanceManager) | ||
| .create( | ||
| error, | ||
| Long.valueOf(error.getCode()), | ||
| getCameraStateErrorDescription(error), | ||
| reply -> {}); | ||
| } | ||
|
|
||
| if (!instanceManager.containsInstance(instance)) { | ||
| cameraStateFlutterApi.create( | ||
| instanceManager.addHostCreatedInstance(instance), | ||
| new CameraStateTypeData.Builder().setValue(cameraStateType).build(), | ||
| instanceManager.getIdentifierForStrongReference(error), | ||
| callback); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns an error message corresponding to the specified {@link CameraState.StateError}. | ||
| * | ||
| * <p>See https://developer.android.com/reference/androidx/camera/core/CameraState#constants_1 for | ||
| * more information on the different {@link CameraState.StateError} types. | ||
| */ | ||
| private String getCameraStateErrorDescription(@NonNull CameraState.StateError cameraStateError) { | ||
|
Contributor
There was a problem hiding this comment. Can you include who this error string is for? Ex for developers, can it be shown to users, can it be used for error handling etc? My guess is it is a Developer visible string and should not be used in error handling.
Contributor
Author
There was a problem hiding this comment. Moved this to Dart and specified this in the documentation. |
||
| final int cameraStateErrorCode = cameraStateError.getCode(); | ||
| final String cameraStateErrorDescription = cameraStateErrorCode + ": "; | ||
| switch (cameraStateErrorCode) { | ||
| case CameraState.ERROR_CAMERA_IN_USE: | ||
| return cameraStateErrorDescription | ||
|
Contributor
There was a problem hiding this comment. Given that all these values are cameraStateErrorCode + SOME_STRING, I suggest making this easier to read by setting a temporary string (
Contributor
Author
There was a problem hiding this comment. Agreed. Changed this on the Dart side, where I moved it. I decided to move it because this is not directly provided by CameraX and so I don't think we should be wrapping it like it is. |
||
| + "The camera was already in use, possibly by a higher-priority camera client."; | ||
| case CameraState.ERROR_MAX_CAMERAS_IN_USE: | ||
| return cameraStateErrorDescription | ||
| + "The limit number of open cameras has been reached, and more cameras cannot be opened until other instances are closed."; | ||
| case CameraState.ERROR_OTHER_RECOVERABLE_ERROR: | ||
| return cameraStateErrorDescription | ||
| + "The camera device has encountered a recoverable error. CameraX will attempt to recover from the error."; | ||
| case CameraState.ERROR_STREAM_CONFIG: | ||
| return cameraStateErrorDescription + "Configuring the camera has failed."; | ||
| case CameraState.ERROR_CAMERA_DISABLED: | ||
| return cameraStateErrorDescription | ||
| + "The camera device could not be opened due to a device policy. Thia may be caused by a client from a background process attempting to open the camera."; | ||
| case CameraState.ERROR_CAMERA_FATAL_ERROR: | ||
| return cameraStateErrorDescription | ||
| + "The camera was closed due to a fatal error. This may require the Android device be shut down and restarted to restore camera function or may indicate a persistent camera hardware problem."; | ||
| case CameraState.ERROR_DO_NOT_DISTURB_MODE_ENABLED: | ||
| return cameraStateErrorDescription | ||
| + "The camera could not be opened because 'Do Not Disturb' mode is enabled. Please disable this mode, and try opening the camera again."; | ||
| default: | ||
| return cameraStateErrorDescription + "There was an unspecified issue with the current camera state."; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sets the Flutter API used to send messages to Dart. | ||
| * | ||
| * <p>This is only visible for testing. | ||
|
Contributor
There was a problem hiding this comment. I think that this is redundant with the annotation. Consider removing or adding context for why test methods would want to set it or maybe why real apps would not.
Contributor
Author
There was a problem hiding this comment. This is generated, so maybe this can be feedback for the generator? @bparrishMines |
||
| */ | ||
| @VisibleForTesting | ||
| void setApi(@NonNull CameraStateFlutterApi api) { | ||
| this.cameraStateFlutterApi = api; | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems weird to me that this method takes in an
ActivityPluginBindingbut all the values that are updated referencepluginBindingandapplicationContextis used frompluginBindinginstead of the activity context that is available onactivityPluginBindingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems weird to me too. Definitely mistake on my end; fixing.