[camera] Add implementations for `camera_platform_interface` package. #3302

BeMacized · 2020-12-03T16:38:08Z

Description

Updates the dart plugin and its native iOS and Android implementations to make use of the new platform interface that has been submitted in #3253

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

google-cla · 2020-12-03T16:39:15Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mvanbeusekom · 2020-12-03T16:48:59Z

@googlebot I consent.

google-cla · 2020-12-03T16:49:55Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-12-04T08:16:32Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-12-04T08:22:01Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

danielroek · 2020-12-04T09:32:33Z

@googlebot I consent.

ditman · 2020-12-08T00:50:16Z

(I've published camera_platform_interface, sorry for the delay!)

BeMacized · 2020-12-08T08:43:13Z

(I've published camera_platform_interface, sorry for the delay!)

Nice! The dependency should now be updated as well.

ditman · 2020-12-10T02:38:26Z

Hey @BeMacized @mvanbeusekom, can you edit this PR so I'm allowed to push changes to it (and its branch?) I have a couple of fixes for the issues outlined by the publishable check that I'd like to push! I'll also add some extra comments to the code :)

mvanbeusekom · 2020-12-10T07:49:26Z

Hey @BeMacized @mvanbeusekom, can you edit this PR so I'm allowed to push changes to it (and its branch?) I have a couple of fixes for the issues outlined by the publishable check that I'd like to push! I'll also add some extra comments to the code :)

Hi @ditman, I gave you write access to our fork of the plugins repo (https://github.com/baseflow/flutter-plugins) which should give you the required access to push changes to the branch (which will show up in the PR). You should have received an invite by email. Thanks for helping out!

ditman

Hey, I've checked this (except for the Java bits, maybe @bparrishMines can take a look at those), and I have a few comments. I don't think there's anything blocking. Some things are not very idiomatic. My only major concerns:

Reexporting the whole camera_platform_interface from camera. Please reexport only the things that are being removed from Camera. That way we can expose things on platform_interface that are useful to other platform authors, but that shouldn't be directly seen by the users of the plugin.
Do not import dart:io, that way the plugin will be marked as "WEB" compatible in pub.dev later!

ditman · 2020-12-10T02:43:55Z

packages/camera/camera/lib/camera.dart

-    }
-  }
-}
+export 'package:camera_platform_interface/camera_platform_interface.dart';


please, in this one, do show explicitly the entities that we want to show from the platform_interface; that way it's harder to accidentally "leak" API if we add a public API on the platform_interface that might not be suited for end-user consumption (but is required for plugin authors, for example).

I seem to have some problems with this one. Locally I have updated this statement to:

export 'package:camera_platform_interface/camera_platform_interface.dart' show CameraDescription, CameraException, CameraLensDirection, ResolutionPreset, XFile;

But now the code in the example/lib/main.dart won't compile anymore because it can't locate any of the types mentioned by the show statement. I made sure I imported the import package:camera/camera.dart Dart file but still not working.

I can't figure out what I am doing wrong here, probably overlooking something (it is getting late here). Maybe you have some pointers? Will give it a fresh look tomorrow morning.

It should work, maybe you need a flutter clean in the example app, or to "restart" the analyzer (or both?).

If you want, push whatever you have to the branch and I can take a look on my end (it's going to break some tests, but oh well!)

it was indeed my environment that got a bit confused, I corrected everything and it works without issues now. I guess I really needed to sleep on it ;)

packages/camera/camera/lib/src/camera_controller.dart

ditman · 2020-12-10T20:30:42Z

packages/camera/camera/lib/src/camera_controller.dart

+    }
+    _isDisposed = true;
+    super.dispose();
+    if (_creatingCompleter != null) {


This is the only usage of the _creatingCompleter, which seems to be only checking if the initalize method has been called (but not finished running). I've modified this by a FutureOr, it's probable this can be simplified even further...

ditman · 2020-12-10T20:46:49Z

packages/camera/camera/test/camera_test.dart

+          )));
+    });
+
+    test('startImageStream() calls CameraPlatform', () async {


Would it make sense to split the start/stop ImageStream tests to their own files, so we don't mix the PlatformInterface-based initialization and tests, with the MethodChannel ones?

(Also for later cleanup, it's easier, just deleting a file)

I agree, thanks for pointing it out. I moved the tests into a new file called camera_image_stream_test.dart.

google-cla · 2020-12-10T20:58:09Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

ditman · 2020-12-10T21:14:08Z

@googlebot I consent.

ditman · 2020-12-10T21:18:50Z

Hi @ditman, I gave you write access to our fork of the plugins repo

I have added a couple of commits above, I don't think I've broken anything (tests seemed to continue passing), but please verify those! They have to do with the initialization sequence and how it used completers a little bit weirdly.

(Also, when creating PRs, I think it's enough to mark the "allow maintainers to do changes to this PR" checkbox to enable me to push stuff to the branch, without having to give me access to the whole repo (but thanks anyway, Maurits!))

AlexV525 · 2020-12-17T08:20:24Z

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).

I just wondering why only the MINOR version has been increased?

mvanbeusekom · 2020-12-17T09:09:24Z

@AlexV525, @EvsioOn, @LaelLuo, @sunqihui222 it actually is a major version number. When a package is not considered stable (version 1.0.0) Dart considers the second digit the major number, the third digit as minor and a fourth digit (added as +n) as the patch number:

Although semantic versioning doesn’t promise any compatibility between versions prior to 1.0.0, the Dart community convention is to treat those versions semantically as well. The interpretation of each number is just shifted down one slot: going from 0.1.2 to 0.2.0 indicates a breaking change, going to 0.1.3 indicates a new feature, and going to 0.1.2+1 indicates a change that doesn’t affect the public API. For simplicity’s sake, avoid using + after the version reaches 1.0.0.

The quote above is copied from the article Package versioning | Dart, in particular the Semantic versions chapter. So in this case it is a major version update from 0.5.x+x to 0.6.0

AlexV525 · 2020-12-17T09:13:58Z

https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0 This might be against to the migration proposal. The package using >=0.x.y+z <2.0.0 directly suffered from breaking changes.

mvanbeusekom · 2020-12-17T09:25:18Z

Not really sure how to answer this in a satisfying manor ;). The Flutter plugin teams follows the versioning semantics as discussed in the Package versioning | Dart article. As such we bumped the second digit to indicate a major version change.

AlexV525 · 2020-12-17T11:29:44Z

Thanks for the response. I think I'll consult others for the conflict later, since they're obviously against each other at present.

…package. (flutter/plugins#3302)

…flutter#3302) * First suggestion for camera platform interface * Remove test coverage folder * Renamed onLatestImageAvailableHandler definition * Split CameraEvents into separate streams * Implemented & tested first parts of method channel implementation * Remove unused EventChannelMock class * Add missing unit tests * Added placeholders in default method channel implementation * Updated platform interface * Update packages/camera/camera_platform_interface/lib/src/platform_interface/camera_platform.dart Co-authored-by: Maurits van Beusekom <[email protected]> * Add unit test for availableCameras * Expand availableCameras unit test. Added unit test for takePicture. * Add unit test for startVideoRecording * Add unit test for prepareForVideoRecording * Add unit test for stopVideoRecording * Add unit test for pauseVideoRecording * Add unit test for buildView * WIP: Dart and Android implementation * Fix formatting * Have resolution stream replay last value on subscription. Replace stream_transform with rxdart. * Added reverse method channel to replace event channel. Updated initialise and takePicture implementations for android. WIP implementation for startVideoRecording * Fixed example app for Android. Removed isRecordingVideo and isStreamingImages from buildView method. * Added some first tests for camera/camera * More tests and some feedback * iOS implementation: Removed standard event channel. Added reverse method channel. Updated initialize method. Added resolution changed event. Updated error reporting to use new method channel. * Started splitting initialize method * Finish splitting up initialize for iOS * Update unit tests * Fix takePicture method on iOS * Split initialize method on Android * Fix video recording on iOS. Updated platform interface. * Update unit tests * Update error handling of video methods in iOS code. Make iOS code more consistent. * Updated startVideoRecording documentation * Make sure file is returned by stopVideoRecording * Use correct event-type after initializing * Fix DartMessenger unit-tests * Change cast * Fix formatting * Fixed tests, formatting and analysis warnings * Added missing license to Dart files * Updated CHANGELOG and version * Added additional unit-tests to platform_interface * Added more tests * Formatted code * Re-added the CameraPreview widget * Use import/export instead of part implementation * fixed formatting * Resolved additional feedback * Update dependency to git repo * Depend on pub.dev for camera_platform_interface * Fix JAVA formatting * Fix changelog * Make sure camera package can be published * Assert when stream methods are called from wrong platform * Add dev_dependency on plugin_platform_interface package, required by tests. * Remove pedantic requirement from initialize() method. Remove unnecessary completers. * Remove dependency on dart:io * Restrict exposed types from platform interface * Moved test for image stream in separate file * Fixed formatting issue * Fix deprecation warning * Apply feedback from bparrishMines * Fix formatting issues * Removed redundant podspec files * Removed redundant ios files * Handle SecurityException Co-authored-by: Maurits van Beusekom <[email protected]> Co-authored-by: Maurits van Beusekom <[email protected]> Co-authored-by: daniel <[email protected]> Co-authored-by: David Iglesias Teixeira <[email protected]>

BeMacized requested review from bparrishMines and cyanglaz as code owners December 3, 2020 16:38

google-cla bot added the cla: no label Dec 3, 2020

mvanbeusekom requested a review from ditman December 3, 2020 16:49

mvanbeusekom mentioned this pull request Dec 3, 2020

[camera] Add camera_platform_interface package #3253

Merged

22 tasks

cyanglaz added the feature label Dec 3, 2020

google-cla bot added cla: yes and removed cla: no labels Dec 4, 2020

mvanbeusekom force-pushed the camera_federated_implementation branch from 906720a to 4778ad0 Compare December 8, 2020 13:00

BeMacized mentioned this pull request Dec 10, 2020

[camera] Flash functionality for Android and iOS #3314

Merged

13 tasks

mvanbeusekom mentioned this pull request Dec 10, 2020

[camera] Zoom functionality for Android and iOS #3315

Merged

13 tasks

ditman reviewed Dec 10, 2020

View reviewed changes

google-cla bot added cla: no and removed cla: yes labels Dec 10, 2020

google-cla bot added cla: yes and removed cla: no labels Dec 10, 2020

mvanbeusekom force-pushed the camera_federated_implementation branch from 080812a to 915cb1f Compare December 11, 2020 10:26

engine-flutter-autoroll mentioned this pull request Dec 17, 2020

Roll Plugins from 7d392476ac20 to b2e9ca5c5299 (18 revisions) flutter/flutter#72543

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 17, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

5f15bce

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 18, 2020

Roll Plugins from 7d392476ac20 to 41b448633350 (19 revisions) flutter/flutter#72567

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 18, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

c576cea

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 18, 2020

Roll Plugins from 7d392476ac20 to 73eeb30c9819 (20 revisions) flutter/flutter#72574

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 18, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

122a047

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 18, 2020

Roll Plugins from 7d392476ac20 to b6af039a969b (21 revisions) flutter/flutter#72621

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 18, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

7e6a7ac

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 18, 2020

Roll Plugins from 7d392476ac20 to b1ab21a63152 (22 revisions) flutter/flutter#72626

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 18, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

c539431

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 18, 2020

Roll Plugins from 7d392476ac20 to debfbecb53a5 (23 revisions) flutter/flutter#72631

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 18, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

ee1a1e4

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 19, 2020

Roll Plugins from 7d392476ac20 to 22422af71d79 (24 revisions) flutter/flutter#72644

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 19, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

9a4d22e

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 19, 2020

Roll Plugins from 7d392476ac20 to 0c6ed0418bde (25 revisions) flutter/flutter#72647

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 19, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

7788e75

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 19, 2020

Roll Plugins from 7d392476ac20 to b289e1a4fb1b (26 revisions) flutter/flutter#72661

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 19, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

ad02f18

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 20, 2020

Roll Plugins from 7d392476ac20 to b4466e7c7fea (27 revisions) flutter/flutter#72667

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 20, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

a595360

…package. (flutter/plugins#3302)

engine-flutter-autoroll mentioned this pull request Dec 29, 2020

Roll Plugins from 0251017cc973 to 65d041fd9adc (31 revisions) flutter/flutter#73071

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 29, 2020

0b158bd [camera] Add implementations for camera_platform_interface …

b447cd8

…package. (flutter/plugins#3302)

mvanbeusekom deleted the camera_federated_implementation branch September 21, 2021 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[camera] Add implementations for `camera_platform_interface` package. #3302

[camera] Add implementations for `camera_platform_interface` package. #3302

BeMacized commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

mvanbeusekom commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

google-cla bot commented Dec 4, 2020

google-cla bot commented Dec 4, 2020

danielroek commented Dec 4, 2020

ditman commented Dec 8, 2020

BeMacized commented Dec 8, 2020

ditman commented Dec 10, 2020

mvanbeusekom commented Dec 10, 2020

ditman left a comment

ditman Dec 10, 2020

mvanbeusekom Dec 10, 2020

ditman Dec 10, 2020

mvanbeusekom Dec 11, 2020 •

edited

Loading

ditman Dec 10, 2020

ditman Dec 10, 2020

ditman Dec 10, 2020

mvanbeusekom Dec 11, 2020

google-cla bot commented Dec 10, 2020

ditman commented Dec 10, 2020

ditman commented Dec 10, 2020

AlexV525 commented Dec 17, 2020

mvanbeusekom commented Dec 17, 2020 •

edited

Loading

AlexV525 commented Dec 17, 2020

mvanbeusekom commented Dec 17, 2020

AlexV525 commented Dec 17, 2020

[camera] Add implementations for camera_platform_interface package. #3302

[camera] Add implementations for camera_platform_interface package. #3302

Conversation

BeMacized commented Dec 3, 2020

Description

Checklist

Breaking Change

google-cla bot commented Dec 3, 2020

mvanbeusekom commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

google-cla bot commented Dec 4, 2020

google-cla bot commented Dec 4, 2020

danielroek commented Dec 4, 2020

ditman commented Dec 8, 2020

BeMacized commented Dec 8, 2020

ditman commented Dec 10, 2020

mvanbeusekom commented Dec 10, 2020

ditman left a comment

Choose a reason for hiding this comment

ditman Dec 10, 2020

Choose a reason for hiding this comment

mvanbeusekom Dec 10, 2020

Choose a reason for hiding this comment

ditman Dec 10, 2020

Choose a reason for hiding this comment

mvanbeusekom Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

ditman Dec 10, 2020

Choose a reason for hiding this comment

ditman Dec 10, 2020

Choose a reason for hiding this comment

ditman Dec 10, 2020

Choose a reason for hiding this comment

mvanbeusekom Dec 11, 2020

Choose a reason for hiding this comment

google-cla bot commented Dec 10, 2020

ditman commented Dec 10, 2020

ditman commented Dec 10, 2020

AlexV525 commented Dec 17, 2020

mvanbeusekom commented Dec 17, 2020 • edited Loading

AlexV525 commented Dec 17, 2020

mvanbeusekom commented Dec 17, 2020

AlexV525 commented Dec 17, 2020

[camera] Add implementations for `camera_platform_interface` package. #3302

[camera] Add implementations for `camera_platform_interface` package. #3302

mvanbeusekom Dec 11, 2020 •

edited

Loading

mvanbeusekom commented Dec 17, 2020 •

edited

Loading