Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Fix all capture timeout, autofocus, flash, exposure, and device crash issues #3651

Closed
wants to merge 144 commits into from
Closed

Conversation

acoutts
Copy link
Contributor

@acoutts acoutts commented Mar 1, 2021

This PR is a full rework of the android plugin. There were a number of issues before, including crashes on some legacy devices and some features just didn't work. I found that the camera API wasn't quite implemented correctly, and it wasn't transitioning between the states / modes as it should have. This led to instabilities on some devices.

I've tested this PR on the following devices and every feature in the plugin now works as expected:

  • Pixel 4
  • LG k50s
  • Samsung Galaxy S7
  • Sony XZ (F8331)

Summary of major changes / improvements:

  • Added comments throughout the plugin detailing what each part is doing.
  • Fixed capture timeout on devices, related to auto exposure / focus.
  • Added correct handling for starting/stopping auto exposure, to not crash on cameras with unsupported modes.
  • Added correct handling to stop/start autofocus when switching between focus lock and autofocus. Tapping the lock button now triggers a one-time autofocus of the current point/scene, and then locks it until later unlocked.
  • Added support for NV21 image streams in Android for more efficient use with Firebase ML Vision- no need to concatenate the YUV420 planes anymore, just feed the one plane from NV21 directly to ML Vision.
  • Fixed flickering issue on Sony front facing cameras when capturing photos and changing AE mode.
  • Moved capture callbacks / image processing to run on a background handler/thread.
  • Instead of managing the camera state in PictureCaptureRequest, we now have a new CameraState as well as the PictureCaptureRequestState. The PictureCaptureRequestState handles timing out captures while the CameraState tracks the state of the camera as it's processing the current request. This made it easier to keep track of how the camera is moving between states during capture.

Testing this PR

In order to use this PR before the update to camera and camera_platform_interface are published, you need to include the following in your pubspec.yaml:

  camera:
    git:
      url: https://github.com/bottlepay/plugins.git
      path: packages/camera/camera
      ref: android-rework

Future improvements

There is still a delay when capturing images on some devices. I believe it's because we are blocking the UI thread and freezing flutter, so the image preview freezes. I think what we can do to improve this is when calling take picture from android, we need to return a completer future so we can immediately call the result function to unblock flutter, and then once the image is processed we can complete it to return the image path back to dart.
The same could be applied to starting/stopping video capture too.

Related issues:

Pre-launch Checklist

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@Bringoff
Copy link

Bringoff commented Apr 13, 2021

I've connected this branch to our project and played with it a little. Looks promising, can't wait to see this in main repo. Unfortunately, for now I'll have to make a fork of this fork :) There are several problems described in this comment. Although we don't use front camera and don't record video, so most of them are not critical for us. Still, we do observe major freezing when flash is enabled. Also, our app freezes forever when we call await cameraController.initialize() while camera device is busy. I guess, we should get an exception, but this doesn't happen and I see WaitingInMainSignalCatcherLoop warning in logs. Looks like exception is thrown and handled in native code, but don't passed to Flutter through channel.

So, I guess, because of all the separating-in-multiple-PRs work for now I'll have to fork this branch and do some workarounds myself if you don't mind.

@IanDarwin
Copy link

This solved my problems on the Teracube 2e phone (Android 10).
No regressions on various Android (10,11) devices.
No regressions on iPhone (iOS 14.4.2)
Unfortunately it does break on Android 5, as I think was noted earlier. The preview is solid black and the
"Take Picture" button does nothing. Android 5 is NOT a show stopper for me; might be for others.
[Uploading Z220-camera-android5.log…](Console log attached in case it helps.)

@Bringoff
Copy link

@IanDarwin had the same problem. Turned out that was because we used another library to check if flash is available, that was likely implemented via Camera 1 API internally. Everything worked well on newer devices, but Android 5 devices often doesn't support Camera 2 API used in this library and switch to legacy mode, so we got an error about camera being locked by lower API. Works fine after removing that library usages.

@IanDarwin
Copy link

... had the same problem. Turned out that was because we used another library...

Thanks @Bringoff but not using any other camera-related APIs.

@davidpanic
Copy link

davidpanic commented Apr 22, 2021

If someone else is looking to use this right now before it's merged, use this in your pubspec:

  camera:
    git:
      url: https://github.com/bottlepay/plugins.git
      ref: android-rework
      path: packages/camera/camera

Just be warned, this is really BLEEDING EDGE and uses a branch that is being worked on and may break at any time as far as I'm aware! (unless @acoutts or someone that worked on this confirms it is fine to use)

@carman247
Copy link
Contributor

carman247 commented Apr 30, 2021

If someone else is looking to use this right now before it's merged, use this in your pubspec:

  camera:
    git:
      url: https://github.com/bottlepay/plugins.git
      ref: android-rework
      path: packages/camera/camera

Just be warned, this is really BLEEDING EDGE and uses a branch that is being worked on and may break at any time as far as I'm aware! (unless @acoutts or someone that worked on this confirms it is fine to use)

I've been using this for a while (during testing of a new build) but intermittently getting crashes when taking photos on Android .. Haven't noticed if the same on iOS.

I'll try and get a proper crash report added but not sure if it's relevant to this issue.

@wisnuwiry
Copy link

Conflict @acoutts

@stuartmorgan
Copy link
Contributor

Conflict @acoutts

The conflict is with the first of the incremental patches that are being extracted; there's not a lot of point in resolving them until all five have landed.

@google-cla
Copy link

google-cla bot commented May 25, 2021

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.

stuartmorgan pushed a commit that referenced this pull request May 27, 2021
)

Adds exposure related features containing the implementation to handle auto exposure, exposure lock, exposure offset and exposure point via the Android Camera2 API.

This is the third 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).
@carman247
Copy link
Contributor

Any news on this?

I'm desperate to release an update for an app but cannot use the current compatible versions or this PR because I'm still getting crashes that just say wrote stack traces to tombstoned and I'm unable to get any more info.

@stuartmorgan
Copy link
Contributor

Any news on this?

Individual pieces continue to be extracted, reviewed, and landed.

I'm desperate to release an update for an app but cannot use the current compatible versions or this PR

If you have a crash even when using this PR then this isn't going to be relevant to your use case; you should file an issue with details.

@BeMacized
Copy link
Contributor

Closing this, as the final changes from this PR have finally been merged with #4059. Many thanks to those who worked on this!

@BeMacized BeMacized closed this Aug 24, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
…utter#3797)

Adds exposure related features containing the implementation to handle auto exposure, exposure lock, exposure offset and exposure point via the Android Camera2 API.

This is the third PR in a series of pull-requests which will gradually introduce changes from PR flutter#3651, making it easier to review the code (as discussed with @stuartmorgan).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.