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

feat: Safe atomic core/ lib with configure { config -> ... } API for Android #1996

Closed
wants to merge 48 commits into from

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Oct 13, 2023

What

Refactors the Android codebase to use a similar API to the iOS Core/ codebase, where every Camera option can be set within a single configure call.

cameraSession.configure { config ->
  config.fps = 60
  // ...other options
}

Internally, this properly locks the Mutex/Hardware so that only one Configure call can happen at a time, and then it diffs the configuration before vs after to figure out what actually needs to be reconfigured.

Benefits

  • This is fast, and much more safe than before as we now can't do anything wrong on the outside - for example previously we had the PreviewView asynchronously be created and destroyed, React props being updated, or the Camera being disconnected - all potentially at the same time causing collisions.
    This would fix errors like "Configuration contains unconfigured input/output Surfaces" or "Surface abandoned!"
  • This new API somewhat ensures atomicity in this approach, and is considered much more safe and stable than before.
  • We have much more room for improvements:
    • We can have new calls to configure override old calls if the old calls still haven't executed yet, making it faster in cases where the Hardware is a bit slower than the Software
    • We can have the configuration be dynamically computed under a lock, meaning we have a single source of truth. If configuration.isActive is true, the Camera is definitely running.
  • The core/ library ("VisionCamera") could be extracted and released as a native Java/Kotlin Android Camera library (or a Flutter library), it is not tightly bound to React Native anymore.

Things to test

  • Verify that the Session Setup works
  • Verify that updating props (like hdr or zoom) works
  • Verify that the PreviewView correctly constructs and tears down the session (see feat: Upgrade VisionCamera to V3 Expensify/App#28914)
    • Minimizing an app and opening it again (no black screen)
    • Navigating away and back correctly works (no black screen when SurfaceView gets destroyed)

Changes

Tested on

Related issues

@vercel
Copy link

vercel bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2023 10:21am

Base automatically changed from fix/lock-in-caller to main October 13, 2023 16:33
@mrousavy mrousavy changed the title feat: Android core/ lib with configure { config -> ... } API feat: Safe atomic core/ lib with configure { config -> ... } API for Android Oct 16, 2023
@mrousavy mrousavy added 🤖 android Issue affects the Android platform ✨ feature Proposes a new feature or enhancement labels Oct 16, 2023
@mrousavy
Copy link
Owner Author

Closing in favor of #2049 - new base

@mrousavy mrousavy closed this Oct 19, 2023
@mrousavy mrousavy deleted the feat/configure-lib-android branch October 19, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Issue affects the Android platform ✨ feature Proposes a new feature or enhancement
Projects
None yet
1 participant