[camera] android-rework part 4: Android flash and zoom features by mvanbeusekom · Pull Request #3798 · flutter/plugins

mvanbeusekom · 2021-04-08T13:36:26Z

Adds flash and zoom features containing the implementation to handle flash and zoom via the Android Camera2 API.

This is the forth PR in a series of pull-requests which will gradually introduce changes from PR #3651, making it easier to review the code (as discussed with @stuartmorgan).

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

google-cla · 2021-04-08T13:36: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.

acoutts · 2021-04-08T14:49:00Z

@googlebot I consent.

Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com>

…ash_zoom_features

...mera/camera/android/src/main/java/io/flutter/plugins/camera/features/flash/FlashFeature.java

stuartmorgan-g · 2021-04-30T03:37:03Z

...ra/camera/android/src/main/java/io/flutter/plugins/camera/features/zoomlevel/CameraZoom.java

+import androidx.annotation.Nullable;
+import androidx.core.math.MathUtils;
+
+public final class CameraZoom {


Javadoc for the class itself and its public members.

Done.

I have renamed this class to ZoomUtils and made it package private. Also I simplified the class moving all state into the ZoomLevelFeature class where it belongs.

This reduces method chains and makes the whole design a lot simpler.

stuartmorgan-g · 2021-04-30T03:38:41Z

...ra/camera/android/src/main/java/io/flutter/plugins/camera/features/zoomlevel/CameraZoom.java

+    }
+
+    this.maxZoom =
+        ((maxZoom == null) || (maxZoom < DEFAULT_ZOOM_FACTOR)) ? DEFAULT_ZOOM_FACTOR : maxZoom;


Why does the default change the max, rather than the max changing the default?

This statement is part of the constructor where one can pass in a parameter specifying the maximum zoom level. However if the specified maximum zoom level if not specified or below the minimum zoom level (which is named incorrectly and something I will fix), the maximum zoom level will be set equal to the minimum zoom level.

stuartmorgan-g · 2021-04-30T03:40:01Z

...ra/camera/android/src/main/java/io/flutter/plugins/camera/features/zoomlevel/CameraZoom.java

+      return null;
+    }
+
+    final float newZoom = MathUtils.clamp(zoom, DEFAULT_ZOOM_FACTOR, this.maxZoom);


Why is the default a minimum?

The constant DEFAULT_ZOOM_FACTOR is named incorrect and should have been named MIN_ZOOM_LEVEL. This is something I will correct.

stuartmorgan-g · 2021-04-30T03:40:52Z

...era/android/src/main/java/io/flutter/plugins/camera/features/zoomlevel/ZoomLevelFeature.java

+import io.flutter.plugins.camera.CameraProperties;
+import io.flutter.plugins.camera.features.CameraFeature;
+
+/** Exposure offset makes the image brighter or darker. */


This looks like copypasta from another class.

Fixed and documented the other public members in this class.

…ash_zoom_features

mvanbeusekom · 2021-05-19T13:41:02Z

@stuartmorgan I have gone through your feedback and applied the necessary changes. I would appreciate it if you could have another look.

stuartmorgan-g

LGTM with one comment nit.

stuartmorgan-g · 2021-05-25T17:42:40Z

...era/android/src/main/java/io/flutter/plugins/camera/features/zoomlevel/ZoomLevelFeature.java

+  }
+
+  /**
+   * Gets the maximum supported zoom level.


The docs on these two methods are swapped.

Excellent catch, I swapped the documentation around so they match the correct method.

…es (flutter/plugins#3798)

…es (flutter/plugins#3798) (#83364)

…ter#3798) * Base classes to support Android camera features Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com> * Fixed formatting * Applied feedback from PR * Added Android Flash and Zoom features Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com> * Use mockito-inline * Fix formatting issue * Processed feedback on pull request. * Fixed formatting * Fixed formatting * Swap docs to match correct methods Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com>

mvanbeusekom requested a review from stuartmorgan-g April 8, 2021 13:36

mvanbeusekom requested a review from bparrishMines as a code owner April 8, 2021 13:36

google-cla bot added the cla: no label Apr 8, 2021

github-actions bot added p: camera platform-android labels Apr 8, 2021

google-cla bot added cla: yes and removed cla: no labels Apr 8, 2021

mvanbeusekom and others added 6 commits April 20, 2021 09:35

Base classes to support Android camera features

2b7aa9b

Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com>

Fixed formatting

f780742

Applied feedback from PR

76bc5bd

Added Android Flash and Zoom features

547f58a

Co-authored-by: Andrew Coutts <andrewjohncoutts@gmail.com>

Use mockito-inline

215fe53

Fix formatting issue

3402d95

mvanbeusekom force-pushed the camera-android/flash_zoom_features branch from 6868a50 to 3402d95 Compare April 20, 2021 11:48

Merge remote-tracking branch 'upstream/master' into camera-android/fl…

6fcaa5f

…ash_zoom_features

stuartmorgan-g suggested changes Apr 30, 2021

View reviewed changes

mvanbeusekom added 3 commits May 19, 2021 11:21

Merge remote-tracking branch 'upstream/master' into camera-android/fl…

37d06c8

…ash_zoom_features

Processed feedback on pull request.

fab71a5

Fixed formatting

788bbf9

mvanbeusekom requested a review from stuartmorgan-g May 19, 2021 13:41

Fixed formatting

a87045a

stuartmorgan-g approved these changes May 25, 2021

View reviewed changes

Swap docs to match correct methods

64e2e98

mvanbeusekom merged commit 2576d6b into flutter:master May 25, 2021

mvanbeusekom deleted the camera-android/flash_zoom_features branch May 25, 2021 19:03

engine-flutter-autoroll mentioned this pull request May 25, 2021

Roll Plugins from 3b0f89725425 to 2576d6b8bb9d (1 revision) flutter/flutter#83364

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 25, 2021

2576d6b [camera] android-rework part 4: Android flash and zoom featur…

259da0c

…es (flutter/plugins#3798)

fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request May 25, 2021

2576d6b [camera] android-rework part 4: Android flash and zoom featur…

e58f416

…es (flutter/plugins#3798) (#83364)

Conversation

mvanbeusekom commented Apr 8, 2021

Pre-launch Checklist

Uh oh!

google-cla bot commented Apr 8, 2021

Uh oh!

acoutts commented Apr 8, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvanbeusekom May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvanbeusekom commented May 19, 2021

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mvanbeusekom May 19, 2021 •

edited

Loading