fix: Improve Android resource efficiency/cleanup (use class members for CoroutineScope and FrameProcessorThread) (#335)

* fix: Use custom CoroutineScope

* fix: Use custom `CoroutineScope`

* Make `frameProcessorThread` and `coroutineScope` instance variables

* Update VisionCameraScheduler.java

* Remove `HybridData::resetNative()` calls

they're called by a Java GC destructor anyways.

* Update CameraViewManager.kt

* Update CameraView.kt

android/src/main/java/com/mrousavy/camera/CameraPackage.kt

@@ -11,6 +11,6 @@ class CameraPackage : ReactPackage {
   }
 
   override fun createViewManagers(reactContext: ReactApplicationContext): List<ViewManager<*, *>> {
-    return listOf(CameraViewManager())
+    return listOf(CameraViewManager(reactContext))
   }
 }

android/src/main/java/com/mrousavy/camera/CameraView.kt

@@ -27,6 +27,7 @@ import com.mrousavy.camera.utils.*
 import kotlinx.coroutines.*
 import kotlinx.coroutines.guava.await
 import java.lang.IllegalArgumentException
+import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
 import kotlin.math.max
 import kotlin.math.min
@@ -60,8 +61,16 @@ import kotlin.math.min
 // TODO: takePhoto() return with jsi::Value Image reference for faster capture
 
 @Suppress("KotlinJniMissingFunction") // I use fbjni, Android Studio is not smart enough to realize that.
-@SuppressLint("ClickableViewAccessibility") // suppresses the warning that the pinch to zoom gesture is not accessible
-class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
+@SuppressLint("ClickableViewAccessibility", "ViewConstructor")
+class CameraView(context: Context, private val frameProcessorThread: ExecutorService) : FrameLayout(context), LifecycleOwner {
+  companion object {
+    const val TAG = "CameraView"
+    const val TAG_PERF = "CameraView.performance"
+
+    private val propsThatRequireSessionReconfiguration = arrayListOf("cameraId", "format", "fps", "hdr", "lowLightBoost", "photo", "video", "enableFrameProcessor")
+    private val arrayListOfZoom = arrayListOf("zoom")
+  }
+
   // react properties
   // props that require reconfiguring
   var cameraId: String? = null // this is actually not a react prop directly, but the result of setting device={}
@@ -95,6 +104,7 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
   private val cameraExecutor = Executors.newSingleThreadExecutor()
   internal val takePhotoExecutor = Executors.newSingleThreadExecutor()
   internal val recordVideoExecutor = Executors.newSingleThreadExecutor()
+  private var coroutineScope = CoroutineScope(Dispatchers.Main)
 
   internal var camera: Camera? = null
   internal var imageCapture: ImageCapture? = null
@@ -172,7 +182,6 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
     scaleGestureDetector = ScaleGestureDetector(context, scaleGestureListener)
     touchEventListener = OnTouchListener { _, event -> return@OnTouchListener scaleGestureDetector.onTouchEvent(event) }
 
-
     hostLifecycleState = Lifecycle.State.INITIALIZED
     lifecycleRegistry = LifecycleRegistry(this)
     reactContext.addLifecycleEventListener(object : LifecycleEventListener {
@@ -206,10 +215,6 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
     }
   }
 
-  fun finalize() {
-    mHybridData.resetNative()
-  }
-
   private external fun initHybrid(): HybridData
   private external fun frameProcessorCallback(frame: ImageProxy)
 
@@ -252,8 +257,8 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
   fun update(changedProps: ArrayList<String>) = previewView.post {
     // TODO: Does this introduce too much overhead?
     //  I need to .post on the previewView because it might've not been initialized yet
-    //  I need to use GlobalScope.launch because of the suspend fun [configureSession]
-    GlobalScope.launch(Dispatchers.Main) {
+    //  I need to use CoroutineScope.launch because of the suspend fun [configureSession]
+    coroutineScope.launch {
       try {
         val shouldReconfigureSession = changedProps.containsAny(propsThatRequireSessionReconfiguration)
         val shouldReconfigureZoom = shouldReconfigureSession || changedProps.contains("zoom")
@@ -334,7 +339,7 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
       val imageAnalysisBuilder = ImageAnalysis.Builder()
         .setTargetRotation(rotation)
         .setBackpressureStrategy(ImageAnalysis.STRATEGY_KEEP_ONLY_LATEST)
-        .setBackgroundExecutor(CameraViewModule.FrameProcessorThread)
+        .setBackgroundExecutor(frameProcessorThread)
 
       if (format == null) {
         // let CameraX automatically find best resolution for the target aspect ratio
@@ -473,12 +478,4 @@ class CameraView(context: Context) : FrameLayout(context), LifecycleOwner {
     }
     return map
   }
-
-  companion object {
-    const val TAG = "CameraView"
-    const val TAG_PERF = "CameraView.performance"
-
-    private val propsThatRequireSessionReconfiguration = arrayListOf("cameraId", "format", "fps", "hdr", "lowLightBoost", "photo", "video", "enableFrameProcessor")
-    private val arrayListOfZoom = arrayListOf("zoom")
-  }
 }

android/src/main/java/com/mrousavy/camera/CameraViewManager.kt

@@ -1,18 +1,67 @@
 package com.mrousavy.camera
 
-import android.util.Log
+import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.common.MapBuilder
 import com.facebook.react.uimanager.SimpleViewManager
 import com.facebook.react.uimanager.ThemedReactContext
 import com.facebook.react.uimanager.annotations.ReactProp
+import com.mrousavy.camera.frameprocessor.FrameProcessorRuntimeManager
+import java.util.concurrent.Executors
 
-class CameraViewManager : SimpleViewManager<CameraView>() {
-  private fun addChangedPropToTransaction(view: CameraView, changedProp: String) {
-    if (cameraViewTransactions[view] == null) {
-      cameraViewTransactions[view] = ArrayList()
+@Suppress("unused")
+class CameraViewManager(reactContext: ReactApplicationContext) : SimpleViewManager<CameraView>() {
+  private val frameProcessorThread = Executors.newSingleThreadExecutor()
+  private var frameProcessorManager: FrameProcessorRuntimeManager? = null
+
+  init {
+    if (frameProcessorManager == null) {
+      frameProcessorThread.execute {
+        frameProcessorManager = FrameProcessorRuntimeManager(reactContext, frameProcessorThread)
+
+        reactContext.runOnJSQueueThread {
+          frameProcessorManager!!.installJSIBindings()
+        }
+      }
     }
-    cameraViewTransactions[view]!!.add(changedProp)
+  }
+
+  private fun destroy() {
+    frameProcessorManager = null
+    frameProcessorThread.shutdown()
+  }
+
+
+  override fun onCatalystInstanceDestroy() {
+    super.onCatalystInstanceDestroy()
+    destroy()
+  }
+
+  override fun invalidate() {
+    super.invalidate()
+    destroy()
+  }
+
+  public override fun createViewInstance(context: ThemedReactContext): CameraView {
+    return CameraView(context, frameProcessorThread)
+  }
+
+  override fun onAfterUpdateTransaction(view: CameraView) {
+    super.onAfterUpdateTransaction(view)
+    val changedProps = cameraViewTransactions[view] ?: ArrayList()
+    view.update(changedProps)
+    cameraViewTransactions.remove(view)
+  }
+
+  override fun getExportedCustomDirectEventTypeConstants(): MutableMap<String, Any>? {
+    return MapBuilder.builder<String, Any>()
+      .put("cameraInitialized", MapBuilder.of("registrationName", "onInitialized"))
+      .put("cameraError", MapBuilder.of("registrationName", "onError"))
+      .build()
+  }
+
+  override fun getName(): String {
+    return TAG
   }
 
   @ReactProp(name = "cameraId")
@@ -78,6 +127,7 @@ class CameraViewManager : SimpleViewManager<CameraView>() {
     view.format = format
   }
 
+  // TODO: Change when TurboModules release.
   // We're treating -1 as "null" here, because when I make the fps parameter
   // of type "Int?" the react bridge throws an error.
   @ReactProp(name = "fps", defaultInt = -1)
@@ -144,36 +194,16 @@ class CameraViewManager : SimpleViewManager<CameraView>() {
     view.enableZoomGesture = enableZoomGesture
   }
 
-  override fun onAfterUpdateTransaction(view: CameraView) {
-    super.onAfterUpdateTransaction(view)
-    val changedProps = cameraViewTransactions[view] ?: ArrayList()
-    view.update(changedProps)
-    cameraViewTransactions.remove(view)
-  }
-
-  public override fun createViewInstance(context: ThemedReactContext): CameraView {
-    return CameraView(context)
-  }
-
-  override fun getExportedCustomDirectEventTypeConstants(): MutableMap<String, Any>? {
-    return MapBuilder.builder<String, Any>()
-      .put("cameraInitialized", MapBuilder.of("registrationName", "onInitialized"))
-      .put("cameraError", MapBuilder.of("registrationName", "onError"))
-      .build()
-  }
-
-  override fun onDropViewInstance(view: CameraView) {
-    Log.d(TAG, "onDropViewInstance() called!")
-    super.onDropViewInstance(view)
-  }
-
-  override fun getName(): String {
-    return TAG
-  }
-
   companion object {
     const val TAG = "CameraView"
 
     val cameraViewTransactions: HashMap<CameraView, ArrayList<String>> = HashMap()
+
+    private fun addChangedPropToTransaction(view: CameraView, changedProp: String) {
+      if (cameraViewTransactions[view] == null) {
+        cameraViewTransactions[view] = ArrayList()
+      }
+      cameraViewTransactions[view]!!.add(changedProp)
+    }
   }
 }

android/src/main/java/com/mrousavy/camera/CameraViewModule.kt

@@ -15,19 +15,16 @@ import androidx.core.content.ContextCompat
 import com.facebook.react.bridge.*
 import com.facebook.react.modules.core.PermissionAwareActivity
 import com.facebook.react.modules.core.PermissionListener
-import com.mrousavy.camera.frameprocessor.FrameProcessorRuntimeManager
 import com.mrousavy.camera.parsers.*
 import com.mrousavy.camera.utils.*
-import java.util.concurrent.ExecutorService
-import java.util.concurrent.Executors
 import kotlinx.coroutines.*
 import kotlinx.coroutines.guava.await
 
+@Suppress("unused")
 class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBaseJavaModule(reactContext) {
   companion object {
     const val TAG = "CameraView"
     var RequestCode = 10
-    val FrameProcessorThread: ExecutorService = Executors.newSingleThreadExecutor()
 
     fun parsePermissionStatus(status: Int): String {
       return when (status) {
@@ -38,24 +35,22 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
     }
   }
 
-  private var frameProcessorManager: FrameProcessorRuntimeManager? = null
+  private val coroutineScope = CoroutineScope(Dispatchers.Default) // TODO: or Dispatchers.Main?
 
-  override fun initialize() {
-    super.initialize()
-    if (frameProcessorManager == null) {
-      FrameProcessorThread.execute {
-        frameProcessorManager = FrameProcessorRuntimeManager(reactApplicationContext)
-        reactApplicationContext.runOnJSQueueThread {
-          frameProcessorManager!!.installJSIBindings()
-        }
-      }
+  private fun cleanup() {
+    if (coroutineScope.isActive) {
+      coroutineScope.cancel("CameraViewModule has been destroyed.")
     }
   }
 
   override fun onCatalystInstanceDestroy() {
     super.onCatalystInstanceDestroy()
-    frameProcessorManager?.destroy()
-    frameProcessorManager = null
+    cleanup()
+  }
+
+  override fun invalidate() {
+    super.invalidate()
+    cleanup()
   }
 
   override fun getName(): String {
@@ -66,7 +61,7 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
 
   @ReactMethod
   fun takePhoto(viewTag: Int, options: ReadableMap, promise: Promise) {
-    GlobalScope.launch(Dispatchers.Main) {
+    coroutineScope.launch {
       withPromise(promise) {
         val view = findCameraView(viewTag)
         view.takePhoto(options)
@@ -74,9 +69,10 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
     }
   }
 
+  @Suppress("unused")
   @ReactMethod
   fun takeSnapshot(viewTag: Int, options: ReadableMap, promise: Promise) {
-    GlobalScope.launch(Dispatchers.Main) {
+    coroutineScope.launch {
       withPromise(promise) {
         val view = findCameraView(viewTag)
         view.takeSnapshot(options)
@@ -87,7 +83,7 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
   // TODO: startRecording() cannot be awaited, because I can't have a Promise and a onRecordedCallback in the same function. Hopefully TurboModules allows that
   @ReactMethod
   fun startRecording(viewTag: Int, options: ReadableMap, onRecordCallback: Callback) {
-    GlobalScope.launch(Dispatchers.Main) {
+    coroutineScope.launch {
       val view = findCameraView(viewTag)
       try {
         view.startRecording(options, onRecordCallback)
@@ -112,7 +108,7 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
 
   @ReactMethod
   fun focus(viewTag: Int, point: ReadableMap, promise: Promise) {
-    GlobalScope.launch(Dispatchers.Main) {
+    coroutineScope.launch {
       withPromise(promise) {
         val view = findCameraView(viewTag)
         view.focus(point)
@@ -126,7 +122,7 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
   @ReactMethod
   fun getAvailableCameraDevices(promise: Promise) {
     val startTime = System.currentTimeMillis()
-    GlobalScope.launch(Dispatchers.Main) {
+    coroutineScope.launch {
       withPromise(promise) {
         val extensionsManager = ExtensionsManager.getInstance(reactApplicationContext).await()
         val cameraProvider = ProcessCameraProvider.getInstance(reactApplicationContext).await()

android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorPlugin.java

@@ -39,14 +39,6 @@ public abstract class FrameProcessorPlugin {
         mHybridData = initHybrid(name);
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        super.finalize();
-        if (mHybridData != null) {
-            mHybridData.resetNative();
-        }
-    }
-
     private native @NonNull HybridData initHybrid(@NonNull String name);
 
     /**

android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorRuntimeManager.kt

@@ -10,9 +10,10 @@ import com.mrousavy.camera.CameraView
 import com.mrousavy.camera.ViewNotFoundError
 import com.swmansion.reanimated.Scheduler
 import java.lang.ref.WeakReference
+import java.util.concurrent.ExecutorService
 
 @Suppress("KotlinJniMissingFunction") // I use fbjni, Android Studio is not smart enough to realize that.
-class FrameProcessorRuntimeManager(context: ReactApplicationContext) {
+class FrameProcessorRuntimeManager(context: ReactApplicationContext, frameProcessorThread: ExecutorService) {
   companion object {
     const val TAG = "FrameProcessorRuntime"
     val Plugins: ArrayList<FrameProcessorPlugin> = ArrayList()
@@ -30,7 +31,7 @@ class FrameProcessorRuntimeManager(context: ReactApplicationContext) {
 
   init {
     val holder = context.catalystInstance.jsCallInvokerHolder as CallInvokerHolderImpl
-    mScheduler = VisionCameraScheduler()
+    mScheduler = VisionCameraScheduler(frameProcessorThread)
     mContext = WeakReference(context)
     mHybridData = initHybrid(context.javaScriptContextHolder.get(), holder, mScheduler)
     initializeRuntime()
@@ -42,10 +43,7 @@ class FrameProcessorRuntimeManager(context: ReactApplicationContext) {
     Log.i(TAG, "Successfully installed ${Plugins.count()} Frame Processor Plugins!")
   }
 
-  fun destroy() {
-    mHybridData.resetNative()
-  }
-
+  @Suppress("unused")
   @DoNotStrip
   @Keep
   fun findCameraViewById(viewId: Int): CameraView {

android/src/main/java/com/mrousavy/camera/frameprocessor/VisionCameraScheduler.java

@@ -2,28 +2,26 @@ package com.mrousavy.camera.frameprocessor;
 
 import com.facebook.jni.HybridData;
 import com.facebook.proguard.annotations.DoNotStrip;
-import com.mrousavy.camera.CameraViewModule;
+import java.util.concurrent.ExecutorService;
 
 @SuppressWarnings("JavaJniMissingFunction") // using fbjni here
 public class VisionCameraScheduler {
+    @SuppressWarnings({"unused", "FieldCanBeLocal"})
     @DoNotStrip
     private final HybridData mHybridData;
+    private final ExecutorService frameProcessorThread;
 
-    public VisionCameraScheduler() {
+    public VisionCameraScheduler(ExecutorService frameProcessorThread) {
+        this.frameProcessorThread = frameProcessorThread;
         mHybridData = initHybrid();
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        mHybridData.resetNative();
-        super.finalize();
-    }
-
     private native HybridData initHybrid();
     private native void triggerUI();
 
+    @SuppressWarnings("unused")
     @DoNotStrip
     private void scheduleTrigger() {
-        CameraViewModule.Companion.getFrameProcessorThread().submit(this::triggerUI);
+        frameProcessorThread.submit(this::triggerUI);
     }
 }