Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an implementation of Camera for Android. #2353

Merged
merged 6 commits into from
Jan 31, 2024

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Jan 20, 2024

Implements the Camera API on Android.

Requires the use of beeware/briefcase#1610 and beeware/briefcase-android-gradle-template#83, because an updated version of AppCompat. This is imported indirectly by the Material libraries. It also requires callback hooks for resolving permission requests, and a FileProvider declaration.

As this uses an intent to launch the camera, it includes tests of invoking and getting results from an intent. As part of the PR, I've deprecated the intent_result method, on the basis that the API it exposes matches what I'd expect of a public API, rather than one that is useful internally to satisfy the needs of a public API. This new API is both documented and tested. Fixes #1798

There's one notable discrepancy with iOS/macOS cameras. Android doesn't appear to have an "unknown/unasked" state for permissions, so we can't differentiate between "You've never asked for permissions" and "You've denied permission" - they're both "no permission". There's a lot of discussion about the shouldShowPermissionRequestRationale method - however, this only appears to allow you to differentiate between a permission that was denied once, and a permission that was denied permanently.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@mhsmith
Copy link
Member

mhsmith commented Jan 29, 2024

we can't differentiate between "You've never asked for permissions" and "You've denied permission"

Related: I previously thought that we didn't need a permission to use the camera intent, because it's the camera app that's accessing the camera and not us. But apparently it's not that simple, because if you have the camera permission in the manifest, then it must be enabled even to use the camera intent, and it would be ridiculous for us to tell the user "if you want to use the camera, then DON'T include the camera permission".

"give me the photo you just took" seems like table stakes to me

Unfortunately it looks like the mechanism for passing intents and responses has a 1 MB limit, so that's why they have to pass the image using a file.

I haven't been able to work out a way that will return the actual photo, that doesn't also involve some truly mind boggling permission wrangling around defining a file provider

This is the first time I've looked into this area, and I agree it's very frustrating. And 80% of the answers online are useless because they were written before Android locked down the shared storage directories in 2019.

I don't see any simpler solution than to follow this example. Some notes:

  • There should be no need to request the EXTERNAL_STORAGE permissions. The reference to maxSdkVersion="28" is a typo – it should be 18.
  • The app will have to include a <provider> block in the AndroidManifest, and a <paths> file in res/xml, using particular names which are known to Toga. I suggest we just name them like "Toga" rather than "camera", so they can potentially be used for other things in the future.
  • Although the example uses getExternalFilesDir, it would be better to use getFilesDir to keep everything in the same place.

@freakboy3742
Copy link
Member Author

Unfortunately it looks like the mechanism for passing intents and responses has a 1 MB limit, so that's why they have to pass the image using a file.

That's at least a reasonable explanation... it's still annoying, though.

I don't see any simpler solution than to follow this example.

I've broadly followed this approach; with one notable deviation - I've used the cache dir, not the files dir. My reasoning is that the file itself should be a transient artefact - ideally it wouldn't hit the disk at all. If you do want to save the file as a permanent artefact, you can toga.Image.save() to a permanent location; at some point in the future, we can add a cross platform API to save to the photo library. By using the cache files directory, we get a physical location on disk; but we don't consume the end-user's storage space (at least, not permanently).

This also means we can create a general purpose "shared" file provider that any intent could use for cross-app communication. Based on what I understand of file providers, we might need to add additional path registrations (or add a briefcase customisation) to allow for sharing of regular files or external files; but I think we can cross that bridge when get get to it.

@mhsmith mhsmith merged commit 97ef075 into beeware:main Jan 31, 2024
35 checks passed
@freakboy3742 freakboy3742 deleted the android-camera branch January 31, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android intent_result is not documented
2 participants