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

[camera] Return all possible cameras on Android #6091

Conversation

martingeorgiu
Copy link
Contributor

This PR is Android equivalent to #5636 which I did recently for iOS.

flutter/flutter#91247

Pre-launch Checklist

  • 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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@martingeorgiu martingeorgiu requested a review from camsim99 as a code owner July 11, 2022 14:40
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@martingeorgiu
Copy link
Contributor Author

Hi everyone, I intentionally did not add any tests for now, because I want to make sure, that this PR has any chances of merging. The way I solved the issue is that the .getCameraIdList() does not return all physical cameras and .getPhysicalCameraIds() on CameraCharacteristics does not help either (it works for some devices, but it miserably fails for others).

In the Android docs, there is information that non-removable cameras ids are integers from 0 (https://developer.android.com/reference/android/hardware/camera2/CameraManager.html#getCameraIdList()).

With this fact in mind, I've created this PR, which finally returns all possible cameras on my test device "Alcatel 1s", which was the hardest nut to crack out of all my Android test phones.

I'm fully aware that this is not an ideal solution, but with the current state of Camera2 Android API, AFAIK there is sadly no better solution. We were using this change in production for some time now and sometimes there is camera id to which .getCameraCharacteristics() does not throw Exception (so it's included in the final list), but when user tries to access it, it fails. We tackle this issue with "middleware" which remembers IDs which weren't in fact working and we won't display them to the user.

@martingeorgiu
Copy link
Contributor Author

Also, I made gradle version changes to the Android example to be able to build it with no strange problems (which I hope is not an issue in any way).

@stuartmorgan
Copy link
Contributor

.getPhysicalCameraIds() on CameraCharacteristics does not help either (it works for some devices, but it miserably fails for others

Can you elaborate on "miserably fails"? What does it do?

sometimes there is camera id to which .getCameraCharacteristics() does not throw Exception (so it's included in the final list), but when user tries to access it, it fails.

Based on the explanation in https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics#getPhysicalCameraIds() it sounds like that's expected behavior, and that what you're doing here is explicitly bypassing code that's meant to prevent that situation.

It's not clear to me why returning cameras that won't work is a desirable change.

@martingeorgiu
Copy link
Contributor Author

Hi @stuartmorgan,

I meant that the getCameraIdList() returns e.g. on "Alcatel 1s" 2 cameras. Index 0 (one camera from the back) and index 1 (one camera from the front). When I use getPhysicalCameraIds() on the one from the back, I get an empty array. But the device has 3 back cameras and when I access the index 2 and 3, I can use them with no real issue.

The problem is, that at least from what I've read and tried there is no official way to get these two additional indexes. So the final solution for me was to do a simple loop which tries to find all of them. (and the mechanism to check whether there is a camera for such index, was using the getCameraCharacteristics())

@stuartmorgan
Copy link
Contributor

But the device has 3 back cameras and when I access the index 2 and 3, I can use them with no real issue.

But you also said that on other devices these extra camera indexes fail when used. Given the docs I linked to, it sounds like what you are doing here is exposing cameras that are not actually supported for individual use, and if/when they do work it's basically an accident. Am I misunderstanding?

@lucasoskorep
Copy link
Contributor

@stuartmorgan @martingeorgiu https://tech.gc.com/every-camera-every-angle-on-android/ this article may be a great help explaining what is going on here. It seems that Flutter camera for now supports some physical camera setups properly, but it gets lost when using logical camera setups and some of the weirder physical camera setups.

For instance the pixel 6 has 2 physical back cameras and 1 logical back camera. The camera returned by the camera flutter plugin is logical, but the zoom instances are set incorrectly on it. The phone supports .7 to 7 zoom levels on its logical camera, but the camera plugin lists 1 as the minimum which stops the android API from swapping between the physical cameras built into the 1 logical camera returned.

Looking more into what a good solution for this would look like now without trying to open the physical cameras directly.

@lucasoskorep
Copy link
Contributor

@martingeorgiu let me know if this allows you to get around your issue using the multiple physical cameras combined into a single logical camera. #6150

@martingeorgiu
Copy link
Contributor Author

@lucasoskorep Sadly that did not help. The issues with devices where this plugin is not returning all cameras are afaik caused by an intentional restriction of the manufacturer (probably to promote their main camera app) and the cases, where this PR would help, are probably as @stuartmorgan said basically by accident.
So I'll close the PR as it is more or less a hack. If I would in the future found out about some better way, I will post a new PR:)

@lucasoskorep
Copy link
Contributor

@martingeorgiu oh yeah I see that now - seems my issue is for a different bug. Thanks for clarifying :)

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.

3 participants