From 6d72a0e7ff57ef1ba2b87257afcdeff4ec604ec2 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Thu, 11 Jul 2019 12:57:21 -0700 Subject: [PATCH 1/9] Re implement initialize and add documentation --- .../camera/lib/new/src/camera_controller.dart | 43 ++++++++++++++----- .../lib/new/src/common/camera_interface.dart | 7 ++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 4ae6b472a3a7..3318345162fd 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -12,14 +12,21 @@ import 'common/camera_interface.dart'; /// /// Use [CameraController.availableCameras] to get a list of available cameras. /// -/// This class is used as a simple interface that works for Android and iOS. +/// This class is used as a simple interface to control a camera on Android or +/// iOS. /// -/// When using iOS, simultaneously calling [start] on two [CameraController]s -/// will throw a [PlatformException]. +/// Only one instance of [CameraController] can be active at a time. If you call +/// [initialize] on a [CameraController] while another is active, the old +/// controller will be disposed before initializing the new controller. /// -/// When using Android, simultaneously calling [start] on two -/// [CameraController]s may throw a [PlatformException] depending on the -/// hardware resources of the device. +/// Example using [CameraController]: +/// +/// ```dart +/// final List cameras = async CameraController.availableCameras(); +/// final CameraController controller = CameraController(description: cameras[0]); +/// controller.initialize(); +/// controller.start(); +/// ``` class CameraController { /// Default constructor. /// @@ -28,7 +35,6 @@ class CameraController { /// /// This will choose the best [CameraConfigurator] for the current device. factory CameraController({@required CameraDescription description}) { - assert(description != null); return CameraController._( description: description, configurator: _createDefaultConfigurator(description), @@ -59,6 +65,9 @@ class CameraController { ); } + // Keep only one active instance of CameraController. + static CameraController _instance; + /// Details for the camera this controller accesses. final CameraDescription description; @@ -75,14 +84,28 @@ class CameraController { throw UnimplementedError('$defaultTargetPlatform not supported'); } - /// Begins the flow of data between the inputs and outputs connected the camera instance. + /// Initializes the camera on the device. + /// + /// You must call [dispose] when you are done using the camera, otherwise it + /// will remain locked and be unavailable to other applications. + Future initialize() { + if (_instance != this) _instance?.dispose(); + _instance = this; + + return configurator.initialize(); + } + + /// Begins the flow of data between the inputs and outputs connected to the camera instance. Future start() => configurator.start(); - /// Stops the flow of data between the inputs and outputs connected the camera instance. + /// Stops the flow of data between the inputs and outputs connected to the camera instance. Future stop() => configurator.stop(); /// Deallocate all resources and disables further use of the controller. - Future dispose() => configurator.dispose(); + Future dispose() { + _instance = null; + return configurator.dispose(); + } static CameraConfigurator _createDefaultConfigurator( CameraDescription description, diff --git a/packages/camera/lib/new/src/common/camera_interface.dart b/packages/camera/lib/new/src/common/camera_interface.dart index 8b5ca9d4e4a3..99ead09550c9 100644 --- a/packages/camera/lib/new/src/common/camera_interface.dart +++ b/packages/camera/lib/new/src/common/camera_interface.dart @@ -38,12 +38,15 @@ abstract class CameraConfigurator { /// You must call [addPreviewTexture] first or this will only return null. int get previewTextureId; - /// Begins the flow of data between the inputs and outputs connected the camera instance. + /// Initializes the camera on the device. + Future initialize(); + + /// Begins the flow of data between the inputs and outputs connected to the camera instance. /// /// This will start updating the texture with id: [previewTextureId]. Future start(); - /// Stops the flow of data between the inputs and outputs connected the camera instance. + /// Stops the flow of data between the inputs and outputs connected to the camera instance. Future stop(); /// Dispose all resources and disables further use of this configurator. From db577c8d6b55a5c62ea7a513150bba30059a50d8 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Thu, 11 Jul 2019 13:10:21 -0700 Subject: [PATCH 2/9] Check for this --- packages/camera/lib/new/src/camera_controller.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 3318345162fd..7e35c2450a66 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -89,9 +89,12 @@ class CameraController { /// You must call [dispose] when you are done using the camera, otherwise it /// will remain locked and be unavailable to other applications. Future initialize() { - if (_instance != this) _instance?.dispose(); - _instance = this; + if (_instance == this) { + return Future.value(); + } + _instance?.dispose(); + _instance = this; return configurator.initialize(); } From 879b925151842ec2cfa1ecd4ba01899a926c1661 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Thu, 11 Jul 2019 16:12:57 -0700 Subject: [PATCH 3/9] Add documentation to init method --- packages/camera/lib/new/src/camera_controller.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 7e35c2450a66..17e26a51318e 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -88,6 +88,10 @@ class CameraController { /// /// You must call [dispose] when you are done using the camera, otherwise it /// will remain locked and be unavailable to other applications. + /// + /// Only one instance of [CameraController] can be active at a time. If you + /// call [initialize] on a [CameraController] while another is active, the old + /// controller will be disposed before initializing the new controller. Future initialize() { if (_instance == this) { return Future.value(); From 0dc5840ae7e5f7d3e966a2d5ca205cb38711e59b Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:02:59 -0700 Subject: [PATCH 4/9] Use completer in initialize --- packages/camera/lib/new/src/camera_controller.dart | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 17e26a51318e..6f07b902fcdf 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -93,13 +93,21 @@ class CameraController { /// call [initialize] on a [CameraController] while another is active, the old /// controller will be disposed before initializing the new controller. Future initialize() { + final Completer completer = Completer(); + if (_instance == this) { return Future.value(); } - _instance?.dispose(); + if (_instance != null) { + _instance + .dispose() + .then((_) => configurator.initialize()) + .then((_) => completer.complete()); + } _instance = this; - return configurator.initialize(); + + return completer.future; } /// Begins the flow of data between the inputs and outputs connected to the camera instance. From f1d8abf9a4e814fb988319ba9ab195bb7a6b9c77 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:23:55 -0700 Subject: [PATCH 5/9] Check if disposed or initialized --- .../camera/lib/new/src/camera_controller.dart | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 6f07b902fcdf..746327bc689d 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -65,9 +65,14 @@ class CameraController { ); } + static const String _isNotInitializedMessage = 'Initialize was not called.'; + static const String _isDisposedMessage = 'This controller has been disposed.'; + // Keep only one active instance of CameraController. static CameraController _instance; + bool _isDisposed = false; + /// Details for the camera this controller accesses. final CameraDescription description; @@ -77,6 +82,8 @@ class CameraController { /// Api used by the [configurator]. final CameraApi api; + bool get isDisposed => _isDisposed; + /// Retrieves a list of available cameras for the current device. /// /// This will choose the best [CameraAPI] for the current device. @@ -93,12 +100,12 @@ class CameraController { /// call [initialize] on a [CameraController] while another is active, the old /// controller will be disposed before initializing the new controller. Future initialize() { - final Completer completer = Completer(); - if (_instance == this) { return Future.value(); } + final Completer completer = Completer(); + if (_instance != null) { _instance .dispose() @@ -111,14 +118,25 @@ class CameraController { } /// Begins the flow of data between the inputs and outputs connected to the camera instance. - Future start() => configurator.start(); + Future start() { + assert(!_isDisposed, _isDisposedMessage); + assert(_instance != this, _isNotInitializedMessage); + + return configurator.start(); + } /// Stops the flow of data between the inputs and outputs connected to the camera instance. - Future stop() => configurator.stop(); + Future stop() { + assert(!_isDisposed, _isDisposedMessage); + assert(_instance != this, _isNotInitializedMessage); + + return configurator.stop(); + } /// Deallocate all resources and disables further use of the controller. Future dispose() { _instance = null; + _isDisposed = true; return configurator.dispose(); } From ecaa84d03a218fadf5af5f8f19be55592d49b436 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:40:40 -0700 Subject: [PATCH 6/9] Check if disposed or initialized --- .../camera/lib/new/src/camera_controller.dart | 5 ++ packages/camera/test/camera_test.dart | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/packages/camera/lib/new/src/camera_controller.dart b/packages/camera/lib/new/src/camera_controller.dart index 746327bc689d..4296f39d7002 100644 --- a/packages/camera/lib/new/src/camera_controller.dart +++ b/packages/camera/lib/new/src/camera_controller.dart @@ -157,10 +157,15 @@ class CameraController { } static CameraApi _getCameraApi(CameraDescription description) { + return CameraApi.iOS; + + // TODO(bparrishMines): Uncomment this when platform specific code is added. + /* throw ArgumentError.value( description.runtimeType, 'description.runtimeType', 'Failed to get $CameraApi from', ); + */ } } diff --git a/packages/camera/test/camera_test.dart b/packages/camera/test/camera_test.dart index ae6c38f9b1f9..85c9d37e1b71 100644 --- a/packages/camera/test/camera_test.dart +++ b/packages/camera/test/camera_test.dart @@ -2,7 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:camera/new/camera.dart'; import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:camera/new/src/camera_testing.dart'; import 'package:camera/new/src/common/native_texture.dart'; @@ -33,6 +35,34 @@ void main() { CameraTesting.nextHandle = 0; }); + group('$CameraController', () { + test('Initializing a second controller closes the first', () { + final MockCameraDescription description = MockCameraDescription(); + final MockCameraConfigurator configurator = MockCameraConfigurator(); + + final CameraController controller1 = + CameraController.customConfigurator( + description: description, + configurator: configurator, + ); + + controller1.initialize(); + + final CameraController controller2 = + CameraController.customConfigurator( + description: description, + configurator: configurator, + ); + + controller2.initialize(); + + expect( + () => controller1.start(), + throwsA(const TypeMatcher()), + ); + }); + }); + group('$NativeTexture', () { test('allocate', () async { final NativeTexture texture = await NativeTexture.allocate(); @@ -48,3 +78,31 @@ void main() { }); }); } + +class MockCameraDescription extends CameraDescription { + @override + LensDirection get direction => LensDirection.unknown; + + @override + String get name => 'none'; +} + +class MockCameraConfigurator extends CameraConfigurator { + @override + Future addPreviewTexture() => Future.value(7); + + @override + Future dispose() => Future.value(); + + @override + Future initialize() => Future.value(); + + @override + int get previewTextureId => 7; + + @override + Future start() => Future.value(); + + @override + Future stop() => Future.value(); +} From 22aaae7591b3c96c9cfcf8214f0184ab924756e3 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:42:36 -0700 Subject: [PATCH 7/9] Check if disposed or initialized --- packages/camera/test/camera_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/camera/test/camera_test.dart b/packages/camera/test/camera_test.dart index 85c9d37e1b71..2bff739e8c46 100644 --- a/packages/camera/test/camera_test.dart +++ b/packages/camera/test/camera_test.dart @@ -36,7 +36,7 @@ void main() { }); group('$CameraController', () { - test('Initializing a second controller closes the first', () { + test('Initializing a second controller closes the first', () async { final MockCameraDescription description = MockCameraDescription(); final MockCameraConfigurator configurator = MockCameraConfigurator(); @@ -57,7 +57,7 @@ void main() { controller2.initialize(); expect( - () => controller1.start(), + () async => await controller1.start(), throwsA(const TypeMatcher()), ); }); From b535d46df3eb924ed6e196c0cbfebfa77551aafc Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:45:33 -0700 Subject: [PATCH 8/9] Tests --- packages/camera/test/camera_test.dart | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/camera/test/camera_test.dart b/packages/camera/test/camera_test.dart index 2bff739e8c46..13d017f826b1 100644 --- a/packages/camera/test/camera_test.dart +++ b/packages/camera/test/camera_test.dart @@ -36,7 +36,7 @@ void main() { }); group('$CameraController', () { - test('Initializing a second controller closes the first', () async { + test('Initializing a second controller closes the first', () { final MockCameraDescription description = MockCameraDescription(); final MockCameraConfigurator configurator = MockCameraConfigurator(); @@ -57,9 +57,16 @@ void main() { controller2.initialize(); expect( - () async => await controller1.start(), - throwsA(const TypeMatcher()), + () => controller1.start(), + throwsA(isInstanceOf()), ); + + expect( + () => controller1.stop(), + throwsA(isInstanceOf()), + ); + + expect(controller1.isDisposed, isTrue); }); }); From 99093a9d75d9cc0992f3fff1ead3f4aa9376b072 Mon Sep 17 00:00:00 2001 From: Maurice Parrish Date: Fri, 12 Jul 2019 12:56:28 -0700 Subject: [PATCH 9/9] unused import --- packages/camera/test/camera_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/camera/test/camera_test.dart b/packages/camera/test/camera_test.dart index 13d017f826b1..39b08ecd8020 100644 --- a/packages/camera/test/camera_test.dart +++ b/packages/camera/test/camera_test.dart @@ -4,7 +4,6 @@ import 'package:camera/new/camera.dart'; import 'package:flutter/services.dart'; -import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:camera/new/src/camera_testing.dart'; import 'package:camera/new/src/common/native_texture.dart';