From ff5a8b8900eaab5ad079a26a3a55e0cf486046ca Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Wed, 25 Aug 2021 11:33:57 +0200 Subject: [PATCH] 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 --- .../java/com/mrousavy/camera/CameraPackage.kt | 2 +- .../java/com/mrousavy/camera/CameraView.kt | 33 +++---- .../com/mrousavy/camera/CameraViewManager.kt | 96 ++++++++++++------- .../com/mrousavy/camera/CameraViewModule.kt | 38 ++++---- .../frameprocessor/FrameProcessorPlugin.java | 8 -- .../FrameProcessorRuntimeManager.kt | 10 +- .../frameprocessor/VisionCameraScheduler.java | 16 ++-- 7 files changed, 107 insertions(+), 96 deletions(-) diff --git a/android/src/main/java/com/mrousavy/camera/CameraPackage.kt b/android/src/main/java/com/mrousavy/camera/CameraPackage.kt index 9e8e03f..dbbd00f 100644 --- a/android/src/main/java/com/mrousavy/camera/CameraPackage.kt +++ b/android/src/main/java/com/mrousavy/camera/CameraPackage.kt @@ -11,6 +11,6 @@ class CameraPackage : ReactPackage { } override fun createViewManagers(reactContext: ReactApplicationContext): List> { - return listOf(CameraViewManager()) + return listOf(CameraViewManager(reactContext)) } } diff --git a/android/src/main/java/com/mrousavy/camera/CameraView.kt b/android/src/main/java/com/mrousavy/camera/CameraView.kt index 13b5629..e126a63 100644 --- a/android/src/main/java/com/mrousavy/camera/CameraView.kt +++ b/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) = 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") - } } diff --git a/android/src/main/java/com/mrousavy/camera/CameraViewManager.kt b/android/src/main/java/com/mrousavy/camera/CameraViewManager.kt index 6f3dc6b..87d6f0f 100644 --- a/android/src/main/java/com/mrousavy/camera/CameraViewManager.kt +++ b/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() { - private fun addChangedPropToTransaction(view: CameraView, changedProp: String) { - if (cameraViewTransactions[view] == null) { - cameraViewTransactions[view] = ArrayList() +@Suppress("unused") +class CameraViewManager(reactContext: ReactApplicationContext) : SimpleViewManager() { + 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? { + return MapBuilder.builder() + .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() { 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() { 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? { - return MapBuilder.builder() - .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> = HashMap() + + private fun addChangedPropToTransaction(view: CameraView, changedProp: String) { + if (cameraViewTransactions[view] == null) { + cameraViewTransactions[view] = ArrayList() + } + cameraViewTransactions[view]!!.add(changedProp) + } } } diff --git a/android/src/main/java/com/mrousavy/camera/CameraViewModule.kt b/android/src/main/java/com/mrousavy/camera/CameraViewModule.kt index e51da7c..0062a2d 100644 --- a/android/src/main/java/com/mrousavy/camera/CameraViewModule.kt +++ b/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() diff --git a/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorPlugin.java b/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorPlugin.java index 166a7f6..7b9bc5f 100644 --- a/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorPlugin.java +++ b/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); /** diff --git a/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorRuntimeManager.kt b/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorRuntimeManager.kt index b7d9e30..990af78 100644 --- a/android/src/main/java/com/mrousavy/camera/frameprocessor/FrameProcessorRuntimeManager.kt +++ b/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 = 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 { diff --git a/android/src/main/java/com/mrousavy/camera/frameprocessor/VisionCameraScheduler.java b/android/src/main/java/com/mrousavy/camera/frameprocessor/VisionCameraScheduler.java index c88faa2..4a96564 100644 --- a/android/src/main/java/com/mrousavy/camera/frameprocessor/VisionCameraScheduler.java +++ b/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); } }