-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[camerax] Implements setFocusPoint, setExposurePoint, setExposureOffset
#6059
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
Changes from 1 commit
b6729ac
14d1024
975806b
a7181b2
ea5f15f
5fe2976
e41a825
9736287
38e88a8
2843626
35ebf13
c2f75ec
b8526e1
928b4cd
3527d9e
1d4d37e
35ab369
0de865a
bb13a35
1a601a4
2c3c9ab
6652171
a08eed6
c35fa15
fac3ff6
077ee37
ff3fc39
fc60e77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,8 @@ import 'dart:math' show Point; | |
|
|
||
| import 'package:async/async.dart'; | ||
| import 'package:camera_platform_interface/camera_platform_interface.dart'; | ||
| import 'package:flutter/services.dart' show DeviceOrientation; | ||
| import 'package:flutter/services.dart' | ||
| show DeviceOrientation, PlatformException; | ||
| import 'package:flutter/widgets.dart'; | ||
| import 'package:stream_transform/stream_transform.dart'; | ||
|
|
||
|
|
@@ -72,6 +73,9 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| @visibleForTesting | ||
| CameraInfo? cameraInfo; | ||
|
|
||
| /// The [CameraControl] instance that corresponds to the [camera] instance. | ||
| CameraControl? cameraControl; | ||
camsim99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Decided to save this whenever the related |
||
| /// The [LiveData] of the [CameraState] that represents the state of the | ||
| /// [camera] instance. | ||
| LiveData<CameraState>? liveCameraState; | ||
|
|
@@ -498,27 +502,25 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| /// Returns the (rounded) offset value that was set. | ||
| @override | ||
| Future<double> setExposureOffset(int cameraId, double offset) async { | ||
| // TODO(camsim99): cache camera + related values if possible. | ||
| final double minOffset = await getMinExposureOffset(cameraId); | ||
| final double maxOffset = await getMaxExposureOffset(cameraId); | ||
|
|
||
| if (offset < minOffset || offset > maxOffset) { | ||
| throw CameraException('TODO(camsim99)', 'TODO(camsim99)'); | ||
| } | ||
|
|
||
| final double exposureOffsetStepSize = | ||
| (await cameraInfo!.getExposureState()).exposureCompensationStep; | ||
| if (exposureOffsetStepSize == 0) { | ||
| throw CameraException( | ||
| 'TODO(camsim99)', 'Exposure compensation not supported'); | ||
| throw CameraException(exposureCompensationNotSupported, | ||
| 'Exposure compensation not supported'); | ||
| } | ||
|
|
||
| final int roundedExposureCompensationIndex = | ||
| (offset / exposureOffsetStepSize).round(); | ||
| final CameraControl cameraControl = await camera!.getCameraControl(); | ||
|
|
||
| await cameraControl | ||
| .setExposureCompensationIndex(roundedExposureCompensationIndex); | ||
| try { | ||
| await cameraControl! | ||
| .setExposureCompensationIndex(roundedExposureCompensationIndex); | ||
| } on PlatformException catch (e) { | ||
| throw CameraException( | ||
| 'setExposureOffsetFailed', | ||
| e.message ?? | ||
| 'Setting the camera exposure compensation index failed.'); | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking for feedback on throwing an exception here and forwarding the exception from
as per its documentation. Another option is explicitly checking in this method, but was worried that such an asynchronous call could impact performance.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bparrishMines no pressure on a review, but if you have time to take a look, would appreciate your feedback on this decision!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with throwing an exception here since a user is explicitly warned about it in the app-facing package. Although this does highlight the need for a way to check if a feature is supported without using a try/catch for every method. Something we will consider when the camera platform interface is updated. |
||
| return roundedExposureCompensationIndex * exposureOffsetStepSize; | ||
| } | ||
|
|
||
|
|
@@ -575,8 +577,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| /// Throws a `CameraException` when an illegal zoom level is supplied. | ||
| @override | ||
| Future<void> setZoomLevel(int cameraId, double zoom) async { | ||
| final CameraControl cameraControl = await camera!.getCameraControl(); | ||
| await cameraControl.setZoomRatio(zoom); | ||
| await cameraControl!.setZoomRatio(zoom); | ||
| } | ||
|
|
||
| /// The ui orientation changed. | ||
|
|
@@ -663,8 +664,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| CameraControl? cameraControl; | ||
| // Turn off torch mode if it is enabled and not being redundantly set. | ||
| if (mode != FlashMode.torch && torchEnabled) { | ||
| cameraControl = await camera!.getCameraControl(); | ||
| await cameraControl.enableTorch(false); | ||
| await cameraControl!.enableTorch(false); | ||
| torchEnabled = false; | ||
| } | ||
|
|
||
|
|
@@ -681,8 +681,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| // Torch mode enabled already. | ||
| return; | ||
| } | ||
| cameraControl = await camera!.getCameraControl(); | ||
| await cameraControl.enableTorch(true); | ||
| await cameraControl!.enableTorch(true); | ||
| torchEnabled = true; | ||
| } | ||
| } | ||
|
|
@@ -754,7 +753,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| if (videoOutputPath == null) { | ||
| // Stop the current active recording as we will be unable to complete it | ||
| // in this error case. | ||
| unawaited(recording!.close()); | ||
| await recording!.close(); | ||
| recording = null; | ||
| pendingRecording = null; | ||
| throw CameraException( | ||
|
|
@@ -763,7 +762,7 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| 'while reporting success. The platform should always ' | ||
| 'return a valid path or report an error.'); | ||
| } | ||
| unawaited(recording!.close()); | ||
| await recording!.close(); | ||
| recording = null; | ||
| pendingRecording = null; | ||
| return XFile(videoOutputPath!); | ||
|
|
@@ -905,14 +904,15 @@ class AndroidCameraCameraX extends CameraPlatform { | |
|
|
||
| // Methods concerning camera state: | ||
|
|
||
| /// Updates [cameraInfo] to the information corresponding to [camera] and | ||
| /// adds observers to the [LiveData] of the [CameraState] of the current | ||
| /// [camera], saved as [liveCameraState]. | ||
| /// Updates [cameraInfo] and [cameraControl] to the information corresponding | ||
| /// to [camera] and adds observers to the [LiveData] of the [CameraState] of | ||
| /// the current [camera], saved as [liveCameraState]. | ||
| /// | ||
| /// If a previous [liveCameraState] was stored, existing observers are | ||
| /// removed, as well. | ||
| Future<void> _updateCameraInfoAndLiveCameraState(int cameraId) async { | ||
| cameraInfo = await camera!.getCameraInfo(); | ||
| cameraControl = await camera!.getCameraControl(); | ||
| await liveCameraState?.removeObservers(); | ||
| liveCameraState = await cameraInfo!.getCameraState(); | ||
| await liveCameraState!.observe(_createCameraClosingObserver(cameraId)); | ||
|
|
@@ -1054,50 +1054,83 @@ class AndroidCameraCameraX extends CameraPlatform { | |
| videoQuality: videoQuality, fallbackStrategy: fallbackStrategy); | ||
| } | ||
|
|
||
| /// TODO(camsim99) | ||
| // Methods for configuring auto-focus and auto-exposure: | ||
|
|
||
| /// Starts a foucs and metering action. | ||
camsim99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// | ||
| /// If [meteringPoint] is non-null, this action includes | ||
| /// * metering points and their modes previously added to | ||
| /// [currentFocusMeteringAction] that do not share a metering mode with | ||
| /// [meteringPoint] and | ||
| /// * [meteringPoint] with the specified [meteringMode]. | ||
| /// If [meteringPoint] is null, this action includes only metering points and | ||
| /// their modes previously added to [currentFocusMeteringAction] that do not | ||
| /// share a metering mode with [meteringPoint]. If there are no such metering | ||
| /// points, then any previously enabled focus and metering actions will be | ||
| /// canceled. | ||
| Future<void> _startFocusAndMeteringFor( | ||
| {required Point<double>? meteringPoint, | ||
gmackall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| required int? meteringMode}) async { | ||
| // TODO(camsim99): Consider caching cameraControl when camera created. | ||
| final CameraControl cameraControl = await camera!.getCameraControl(); | ||
| required int meteringMode}) async { | ||
| if (meteringPoint == null) { | ||
| // Try to clear any metering point from previous action with the specified | ||
| // meteringMode. | ||
| if (currentFocusMeteringAction == null) { | ||
| // Attempting to clear a metering point from a previous action, but no | ||
| // such action exists. | ||
| return; | ||
| } | ||
|
|
||
| // Remove metering point with specified meteringMode from current focus | ||
| // and metering action, as only one focus or exposure point may be set | ||
| // at once in this plugin. | ||
gmackall marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| final List<(MeteringPoint, int?)> newMeteringPointInfos = | ||
| currentFocusMeteringAction!.meteringPointInfos | ||
| .where(((MeteringPoint, int?) meteringPointInfo) => | ||
| // meteringPointInfo may technically include points without a | ||
| // mode specified, but this logic is safe because this plugin | ||
| // only explicitly uses FocusMeteringAction.flagAe or | ||
| // FocusMeteringAction.flagAf. | ||
| meteringPointInfo.$2 != meteringMode) | ||
| .toList(); | ||
|
|
||
| if (newMeteringPointInfos.isEmpty) { | ||
| await cameraControl.cancelFocusAndMetering(); | ||
| // If no other metering points were specified, cancel any previously | ||
| // started focus and metering actions. | ||
| await cameraControl!.cancelFocusAndMetering(); | ||
gmackall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return; | ||
| } | ||
|
|
||
| currentFocusMeteringAction = | ||
| FocusMeteringAction(meteringPointInfos: newMeteringPointInfos); | ||
|
|
||
| await cameraControl.startFocusAndMetering(currentFocusMeteringAction!); | ||
| return; | ||
| } else if (meteringPoint.x < 0 || | ||
| meteringPoint.x > 1 || | ||
| meteringPoint.y < 0 && meteringPoint.y > 1) { | ||
gmackall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| throw CameraException('TODO(camsim99)', 'TODO(camsim99)'); | ||
| } | ||
|
|
||
| List<(MeteringPoint, int?)> newMeteringPointInfos = | ||
| <(MeteringPoint, int?)>[]; | ||
| if (currentFocusMeteringAction != null) { | ||
| newMeteringPointInfos = currentFocusMeteringAction!.meteringPointInfos | ||
| .where(((MeteringPoint, int?) meteringPointInfo) => | ||
| meteringPointInfo.$2 != meteringMode) | ||
| .toList(); | ||
| throw CameraException('meteringPointInvalid', | ||
| 'The coordinates of a metering point for an auto-focus or auto-exposure action must be within (0,0) and (1,1).'); | ||
gmackall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } else { | ||
| // Add new metering point with specified meteringMode, which might | ||
| // involve replacing a metering point with the same specified meteringMode | ||
| // from the current focus and metering action. | ||
| List<(MeteringPoint, int?)> newMeteringPointInfos = | ||
| <(MeteringPoint, int?)>[]; | ||
|
|
||
| if (currentFocusMeteringAction != null) { | ||
| newMeteringPointInfos = currentFocusMeteringAction!.meteringPointInfos | ||
| .where(((MeteringPoint, int?) meteringPointInfo) => | ||
| // meteringPointInfo may technically include points without a | ||
| // mode specified, but this logic is safe because this plugin | ||
| // only explicitly uses FocusMeteringAction.flagAe or | ||
| // FocusMeteringAction.flagAf. | ||
| meteringPointInfo.$2 != meteringMode) | ||
| .toList(); | ||
| } | ||
| final MeteringPoint newMeteringPoint = | ||
| MeteringPoint(x: meteringPoint.x, y: meteringPoint.y); | ||
| newMeteringPointInfos.add((newMeteringPoint, meteringMode)); | ||
| currentFocusMeteringAction = | ||
| FocusMeteringAction(meteringPointInfos: newMeteringPointInfos); | ||
| } | ||
| final MeteringPoint newMeteringPoint = | ||
| MeteringPoint(x: meteringPoint!.x, y: meteringPoint.y); | ||
| newMeteringPointInfos.add((newMeteringPoint, meteringMode)); | ||
| currentFocusMeteringAction = | ||
| FocusMeteringAction(meteringPointInfos: newMeteringPointInfos); | ||
|
|
||
| await cameraControl.startFocusAndMetering(currentFocusMeteringAction!); | ||
| await cameraControl!.startFocusAndMetering(currentFocusMeteringAction!); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,10 +180,9 @@ class _CameraControlHostApiImpl extends CameraControlHostApi { | |
| } | ||
| return exposureCompensationIndex; | ||
| } on PlatformException catch (e) { | ||
| // TODO(camsim99): perhaps throw cameraexception here. | ||
| SystemServices.cameraErrorStreamController.add(e.message ?? | ||
| 'Setting the camera exposure compensation index failed.'); | ||
| return Future<int?>.value(); | ||
| rethrow; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking for feedback on this as per https://github.com/flutter/packages/pull/6059/files#r1481841622. |
||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.