[camera] android-rework part 2: Android auto focus feature #3796
Conversation
|
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 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 ℹ️ Googlers: Go here for more info. |
|
@googlebot I consent. |
Co-authored-by: Andrew Coutts <[email protected]>
Co-authored-by: Andrew Coutts <[email protected]>
mvanbeusekom
force-pushed
the
camera-android/auto_focus_feature
branch
from
6e38197
to
87c3989
Compare
| private FocusMode currentSetting = FocusMode.auto; | ||
|
|
||
| // When we switch recording modes we re-create this feature with | ||
| // the appropriate setting here. |
There was a problem hiding this comment.
I have no idea who/what the "we"s in this sentence refer to. It doesn't even feel like they are referring to the same thing; setting a private field would be done internally to the class, but recreating the feature would be done from outside the class. Please use clear subjects in the comment instead.
"we" is almost always problematic in (non-TODO) comments, as this ambiguity is extremely common; I advise against using it in comments.
...era/android/src/main/java/io/flutter/plugins/camera/features/autofocus/AutoFocusFeature.java
Show resolved
Hide resolved
| // Check if fixed focal length lens. If LENS_INFO_MINIMUM_FOCUS_DISTANCE=0, then this is fixed. | ||
| // Can be null on some devices. | ||
| final Float minFocus = cameraProperties.getLensInfoMinimumFocusDistance(); | ||
| // final Float maxFocus = cameraCharacteristics.get(CameraCharacteristics.LENS_INFO_HYPERFOCAL_DISTANCE); |
There was a problem hiding this comment.
Why is there commented out code?
| isFixedLength = true; | ||
| } else { | ||
| isFixedLength = minFocus == 0; | ||
| } |
There was a problem hiding this comment.
Consider a ternary so the assignment and declaration are the same statement: boolean isFixedLength = minFocus == null ? true : (minFocus == 0);
|
|
||
| switch (currentSetting) { | ||
| case locked: | ||
| /** If we're locking AF we should do a one-time focus, then set the AF to idle */ |
There was a problem hiding this comment.
s/If we're locking AF we should do/When locking AF do/
Although it's not entirely clear to me what this comment is describing; is the line that is being commented actually setting auto-focus to idle?
mvanbeusekom
force-pushed
the
camera-android/auto_focus_feature
branch
from
5bd292b
to
f2d4f90
Compare
* master: (79 commits) Fix grammatical error in contributing guide (flutter#3217) [google_sign_in_web] fix README typos. [tool] combine run and runAndExitOnError (flutter#3827) [camera] android-rework part 2: Android auto focus feature (flutter#3796) [in_app_purchase_platform_interface] Added additional fields to ProductDetails (flutter#3826) Move all null safety packages' min dart sdk to 2.12.0 (flutter#3822) [path_provider_*] code cleanup: sort directives (flutter#3823) [in_app_purchase] Implementation of platform interface (flutter#3781) [google_sign_in] Add todo WRT correctly setting X-Goog-AuthUser header (flutter#3819) [tools] fix version check command not working for new packages (flutter#3818) [camera] android-rework part 1: Base classes to support Android Camera features (flutter#3795) fix MD (flutter#3815) Path provider windows crash fix (flutter#3814) [local_auth] docs update (flutter#3103) Update PULL_REQUEST_TEMPLATE.md (flutter#3801) [quick_actions] handle cold start on iOS correctly (flutter#3811) Replace path_provider_linux widget tests with simple unit tests (flutter#3812) [sensors] format dart code based on the new dart formatter (flutter#3809) [google_sign_in] Fix "pick account" on iOS (flutter#3805) [image_picker_platform_interface] Added pickMultiImage (flutter#3782) ...
* upstream/master: (383 commits) Add implement and registerWith method to plugins (flutter#3833) Update third_party license checking (flutter#3844) [tool] add `all` and `dry-run` flags to publish-plugin command (flutter#3776) Re-add bin/ to flutter_plugin_tools (flutter#3839) switch from using 'tuneup' to analyze to 'dart analyze' (flutter#3837) [in_app_purchase] Federated iOS implementation (flutter#3832) Prep the tools for publishing (flutter#3836) [in_app_purchase] Make PurchaseDetails.status mandatory (flutter#3831) Fix grammatical error in contributing guide (flutter#3217) [google_sign_in_web] fix README typos. [tool] combine run and runAndExitOnError (flutter#3827) [camera] android-rework part 2: Android auto focus feature (flutter#3796) [in_app_purchase_platform_interface] Added additional fields to ProductDetails (flutter#3826) Move all null safety packages' min dart sdk to 2.12.0 (flutter#3822) [path_provider_*] code cleanup: sort directives (flutter#3823) [in_app_purchase] Implementation of platform interface (flutter#3781) [google_sign_in] Add todo WRT correctly setting X-Goog-AuthUser header (flutter#3819) [tools] fix version check command not working for new packages (flutter#3818) [camera] android-rework part 1: Base classes to support Android Camera features (flutter#3795) fix MD (flutter#3815) ...
Adds the
AutoFocusFeaturecontaining the implementation to handle auto focus via the Android Camera2 API.This is the second 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
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.