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

Switch to using CameraX's camera-viewfinder-compose module. #58

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

donovanfm
Copy link
Contributor

This updates to the new CameraX camera-viewfinder-compose module.

Note: this module hasn't landed in a CameraX version yet, so we have to depend on the SNAPSHOT version. A future PR will fix this when the module reaches an alpha release.

The prototype of camera-viewfinder-compose was copied under /app/src/main/java/com/google/android/samples/socialite/ui/camera/viewfinder/, which is also removed in this PR.

@donovanfm donovanfm requested a review from yaraki April 17, 2024 21:24
@calren
Copy link
Contributor

calren commented Apr 18, 2024

@SigmanZero @madebymozart Hoping to get this reviewed and merged today. Thanks!

Will be creating a separate branch for demo purposes after this is merged.

@SigmanZero
Copy link
Contributor

SigmanZero commented Apr 18, 2024

Some observations from testing on a Pixel Fold:

  • The preview has a brief cut-to-black when switching between portrait and landscape, not sure if this is WAI
  • Only in photo mode, switching the view to the front camera first shows a bright white preview before fading into the right image (see screen recording: https://github.com/android/socialite/assets/15023904/1302ce58-8d33-46fa-878f-6974023be335)
  • Might not be related to this PR, but I'm not able to capture a picture with the front camera - I'm getting a "Photo capture failed" toast
    • Other photo/video capture combinations seem to be okay
  • Also not sure if this is specifically related to this PR, but the camera looks to be capturing more than what the preview shows:
    Preview: 5wqT9FyQuXuP6xo
    Captured image: As2whccx8VwVQRj

The goal of setting the target rotation on the ImageCapture and VideoCapture use cases can be accomplished without restarting the preview, which cause a visual glitch on the preview stream.
@donovanfm
Copy link
Contributor Author

Thanks for the feedback, @SigmanZero!

  1. Good catch. I checked the main branch, and it didn't go black, but it the Preview did freeze on rotation. And I have great news! I realized I don't have to restart the Preview stream in order to set the rotation on the final image/video. So I updated that code and now the Preview is smooth when you change the orientation. Yay!
  2. It looks like the Auto Exposure hasn't converged yet. I tested on a Pixel 7 Pro and a Pixel Fold, and I didn't see the effect as strongly as you pointed out. Perhaps it's something to do with the light in your testing environment. I highly doubt this PR introduced that issue though.
  3. I could not reproduce this on a Samsung Galaxy S23, a Pixel 7 Pro, or a Pixel Fold. Can you restart the app and retry? Or send me a stack trace?
  4. This is only happening on the Pixel Fold. Other devices show the correct Preview for the final output. I opened Preview is cropped on Pixel Fold #62 for this since it's happening on the main branch as well. Since it's an existing issue, I don't think it should block this PR.

@SigmanZero
Copy link
Contributor

Thank you! I think this PR LGTM now - I'll open a separate issue for item 3, I was able to repro on my Pixel Fold and got a screen recording + bug report, but I'm able to capture the front-facing picture on a Galaxy S24 Ultra.

@donovanfm donovanfm merged commit d45834b into main Apr 24, 2024
2 checks passed
@donovanfm donovanfm deleted the camera-viewfinder-compose branch April 24, 2024 18:53
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.

3 participants