[MEDIUM] Smartscan: Update to vision camera v3

[puneetlath]

Some important changes about V3:

• Fully rewritten codebase for Android (now uses the low-level Camera API "Camera2" in favor of the more limiting alternative "CameraX")
• Fully rewritten Frame Processors implementation (now uses react-native-worklets-core, FPs are disabled if Worklets is not installed; should fix a ton of build issues)
• Fully rewritten Camera Devices API (useCameraDevice(..); useCameraFormat(..) - always have a device, never null)
• Muuuuch faster (Camera device selection is now instant, and Camera is faster in general)
• See full changelog here: mrousavy/react-native-vision-camera@v2.16.2...v3.3.0
• Requires that we bump the min supported Android version and block ~7% of potential users 😬 -- please remove this if we can drop this requirement (@Julesssss )

[melvin-bot]

Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new.

[puneetlath]

FYI @mrousavy I created a new issue here, since this PR is linked to an issue for iPhone 15 crashing which is no longer happening.

[puneetlath]

@mrousavy what do you think are the next steps here? Are we still trying to make v3 happen?

[mrousavy]

Hey @puneetlath - yup I got a bunch of other stuff on my plate and the Camera is running fine for now - but I'll try to implement Flash capture soon. I just implemented a basic HAL flash trigger which works on some phones, but for a proper implementation we need a custom precapture sequence that enables the torch, then waits for AE/AF to adjust, then lock AE/AF to that value, turn off the torch again, then capture with a single flash burst, and then turn the torch off again, and return to auto-AE/AF. This is quite complex, so I didn't implement it yet. I'll try to find the time next week.

Once that's done we can update Expensify to VisionCamera V3 :)

[puneetlath]

Sounds good!

[Julesssss]

Before moving forward we need to discuss the Android mimimum SDk bump. Losing 5% of our potential users is not worth an improved camera IMO.

[mrousavy]

I think I could also lower the minSdk to 23 or even 21 if needed, I was mostly focused on the video capturing feature when I set it to 26. Should be max. 2 hour effort to make it work on SDK 23/21

[Julesssss]

That's great, we should definitely reduce to 23.

[roryabraham]

Looks like we're adding a vision-camera patch in https://github.com/Expensify/App/pull/31558/files, can we make sure we have an upstream issue and can work to remove the patch in vision-camera v3?

[mrousavy]

Yep - V3 already has a safe fix for this which crashes with a proper error message (only happening if running in bridge-less mode, I'm working on a workaround for that but bridge-less is a while to come).

Just backported the fix to V2.

[puneetlath]

Ok so what's the urgency on this? It sounds like the main benefit is that the camera is much faster? If so, should we include this in wave5 since it'll improve the camera speed for SmartScan? cc @dylanexpensify

[dylanexpensify]

@puneetlath agree with this being wave5!

[mrousavy]

Update from Slack:

Yes there are still blockers on my side - sorry about that.
If I remember correctly, those are:

• minSdkVersion should not be raised (working on this now; feat: Lower minSdkVersion from 26 to 23 mrousavy/react-native-vision-camera#2414)
• Flash doesn't fire correctly on Android (taking a look at this next week)

There are also some people saying that there are issues with the Preview View being stretched, although I haven't found a way to reproduce that yet. I do have a fix in mind which requires a bit of a refactor of the CameraSession.kt class, but I wanted to do this for a while now to make sure everything is stable and is better organized in code.

[mrousavy]

Regarding the minSdkVersion; quite a lot of code requires minSdk 26, for example the entire Video Pipeline (video + frameProcessor) would no longer work on SDK <26.

I think it is not a good idea to bring this upstream, but we should be able to roll with a patched version, since photo capture is not affected by this. I think that might work. Do you have so many users on SDK 23?

[Julesssss]

Do you have so many users on SDK 23?

Nope, very few of our users are running 6.0 and below, so SDK 23 is a much more reasonable min version than SDK 26👍

[mrousavy]

Oh I see - you are currently on SDK 21:

App/android/build.gradle

Line 6 in 2a03320

minSdkVersion = 21

VisionCamera requires SDK 26: https://github.com/mrousavy/react-native-vision-camera/blob/f400487a8d24711c9998418d746bbab06f54a690/package/android/build.gradle#L107

We thought that bumping the SDK from 23 to SDK 26 is a fair requirement for VisionCamera, since only around 3.3% of Android phones worldwide are on SDK 23, 24, or 25.

Afaik, according to Expensify's Google Analytics data for the past week, we have 0 users on SDK 25 or lower.

I think it'd be a fair requirement to bump the minSdk to 26, but it's your call.

I would probably not merge mrousavy/react-native-vision-camera#2414 into main, as fundamental pieces of VisionCamera will need to be wrapped with ifs and they will throw if the user is not on SDK 26 yet - e.g. video capture, frame processing, etc etc.
I'll think about it, but I'd find it weird if there is an error thrown on some phones that aren't on SDK 26 yet for video capture. Instead a hard requirement is much safer. Plus, the JNI HardwareBuffer APIs are available then, I need those.

[Julesssss]

Could you share the source for that table, please? I've been using Google's data, which shows a reduction in supported devices from 99.6 to 95.0 (Android Studio > New Project > Help Me Choose), but I'd be interested to check out an alternate source.

In Google Play, our users have a higher device version than the above suggests (1000 active users are on 25 and below), but this is because our existing app supports specific English-speaking countries. With the new app we want to extend our app reach as wide as possible for P2P IOUs and chat. That's why I'm hesitant to block a large amount of potential users for the sake of a few hour work (if that is still true).

[Julesssss]

Oh sorry, you're sharing the difference between 23 & 26, not 21 & 26. In that case the Google data shows a lower amount of blocked users, but I still believe it's significant:

[mrousavy]

Update: I've merged a ton of stability PRs for Android recently, and will now tackle the blackscreen issues (task 1). I'll probably write a custom CameraCaptureSession wrapper, which will also make it easier to implement flash then (task 2).

As for the minSDK discussion - we can probably patch it in Expensify to have it working for minSDK 23.

[puneetlath]

Sounds like a good plan to me. Shall I assign you this issue?

[mrousavy]

Sure! 👍

[puneetlath]

Hey @mrousavy. Checking in on this. How's it going?

[mrousavy]

Hey! Made some great progress on the Android side, got focus() implemented, got flash implemented, fixed many black screen errors and partially fixed preview stretching. Will take a look at that tomorrow to hopefully finalize everything there, hoping to get a Expensify PR up by end of next week?

[puneetlath]

Next week sounds great, thanks for the update.

[mrousavy]

update: I just got a prototype running on my end that works on API 21, so we don’t need to drop the minSdk requirement! 💪

[puneetlath]

@mrousavy still on pace for Friday?

[mrousavy]

@puneetlath not sure, we are currently working on a new release and I did encounter an issue on Android. iOS works fine. See #37483

[puneetlath]

I believe this is no longer relevant since we are upgrading to v4. I'm going to close this out, but feel free to comment or reopen if I'm mistaken!