fix: fix barcode scanner leak on Android #1139
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,19 +13,20 @@ import android.os.Looper | |
| import android.util.Size | ||
| import android.view.Surface | ||
| import android.view.WindowManager | ||
| import androidx.annotation.VisibleForTesting | ||
| import androidx.camera.core.Camera | ||
| import androidx.camera.core.CameraSelector | ||
| import androidx.camera.core.ExperimentalGetImage | ||
| import androidx.camera.core.ImageAnalysis | ||
| import androidx.camera.core.ImageProxy | ||
| import androidx.camera.core.Preview | ||
| import androidx.camera.core.TorchState | ||
| import androidx.camera.core.resolutionselector.ResolutionSelector | ||
| import androidx.camera.core.resolutionselector.ResolutionStrategy | ||
| import androidx.camera.lifecycle.ProcessCameraProvider | ||
| import androidx.core.content.ContextCompat | ||
| import androidx.lifecycle.LifecycleOwner | ||
| import com.google.mlkit.vision.barcode.BarcodeScanner | ||
| import com.google.mlkit.vision.barcode.BarcodeScannerOptions | ||
| import com.google.mlkit.vision.barcode.BarcodeScanning | ||
| import com.google.mlkit.vision.barcode.common.Barcode | ||
|
|
@@ -37,20 +38,20 @@ import io.flutter.view.TextureRegistry | |
| import java.io.ByteArrayOutputStream | ||
| import kotlin.math.roundToInt | ||
|
|
||
| class MobileScanner( | ||
| private val activity: Activity, | ||
| private val textureRegistry: TextureRegistry, | ||
| private val mobileScannerCallback: MobileScannerCallback, | ||
| private val mobileScannerErrorCallback: MobileScannerErrorCallback, | ||
| private val barcodeScannerFactory: (options: BarcodeScannerOptions?) -> BarcodeScanner = ::defaultBarcodeScannerFactory, | ||
|
There was a problem hiding this comment. This new callback is mainly for tests. I'm adding a new test soon that needs this, in a follow up PR for the rounding bug #1134 |
||
| ) { | ||
|
|
||
| /// Internal variables | ||
| private var cameraProvider: ProcessCameraProvider? = null | ||
| private var camera: Camera? = null | ||
| private var preview: Preview? = null | ||
| private var textureEntry: TextureRegistry.SurfaceTextureEntry? = null | ||
| private var scanner: BarcodeScanner? = null | ||
|
There was a problem hiding this comment. This client was never disposed |
||
| private var lastScanned: List<String?>? = null | ||
| private var scannerTimeout = false | ||
| private var displayListener: DisplayManager.DisplayListener? = null | ||
|
|
@@ -61,6 +62,15 @@ class MobileScanner( | |
| private var detectionTimeout: Long = 250 | ||
| private var returnImage = false | ||
|
|
||
| companion object { | ||
| /** | ||
| * Create a barcode scanner from the given options. | ||
| */ | ||
| fun defaultBarcodeScannerFactory(options: BarcodeScannerOptions?) : BarcodeScanner { | ||
|
There was a problem hiding this comment. Default for the barcode scanner creator function |
||
| return if (options == null) BarcodeScanning.getClient() else BarcodeScanning.getClient(options) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * callback for the camera. Every frame is passed through this function. | ||
| */ | ||
|
|
@@ -76,76 +86,75 @@ class MobileScanner( | |
| scannerTimeout = true | ||
| } | ||
|
|
||
| scanner?.let { | ||
|
There was a problem hiding this comment. Scanner is nullable now, so used a let block |
||
| it.process(inputImage).addOnSuccessListener { barcodes -> | ||
| if (detectionSpeed == DetectionSpeed.NO_DUPLICATES) { | ||
| val newScannedBarcodes = barcodes.mapNotNull { | ||
|
There was a problem hiding this comment. Formatting of mapNotNull to use braces |
||
| barcode -> barcode.rawValue | ||
| }.sorted() | ||
|
|
||
| if (newScannedBarcodes == lastScanned) { | ||
| // New scanned is duplicate, returning | ||
| return@addOnSuccessListener | ||
| } | ||
| if (newScannedBarcodes.isNotEmpty()) { | ||
|
There was a problem hiding this comment. Same formatting change here |
||
| lastScanned = newScannedBarcodes | ||
| } | ||
| } | ||
|
|
||
| val barcodeMap: MutableList<Map<String, Any?>> = mutableListOf() | ||
|
|
||
| for (barcode in barcodes) { | ||
| if (scanWindow == null) { | ||
|
There was a problem hiding this comment. Inverting the check here made more sense |
||
| barcodeMap.add(barcode.data) | ||
| continue | ||
| } | ||
|
|
||
| if (isBarcodeInScanWindow(scanWindow!!, barcode, imageProxy)) { | ||
|
There was a problem hiding this comment. Also invert the check here, which drops a redundant continue statement |
||
| barcodeMap.add(barcode.data) | ||
| } | ||
| } | ||
|
|
||
| if (barcodeMap.isEmpty()) { | ||
|
There was a problem hiding this comment. Early return 2x |
||
| return@addOnSuccessListener | ||
| } | ||
|
|
||
| if (!returnImage) { | ||
| mobileScannerCallback( | ||
| barcodeMap, | ||
| null, | ||
| null, | ||
| null | ||
| ) | ||
| return@addOnSuccessListener | ||
| } | ||
|
|
||
| val bitmap = Bitmap.createBitmap(mediaImage.width, mediaImage.height, Bitmap.Config.ARGB_8888) | ||
| val imageFormat = YuvToRgbConverter(activity.applicationContext) | ||
|
|
||
| imageFormat.yuvToRgb(mediaImage, bitmap) | ||
|
|
||
| val bmResult = rotateBitmap(bitmap, camera?.cameraInfo?.sensorRotationDegrees?.toFloat() ?: 90f) | ||
|
|
||
| val stream = ByteArrayOutputStream() | ||
| bmResult.compress(Bitmap.CompressFormat.PNG, 100, stream) | ||
| val byteArray = stream.toByteArray() | ||
| val bmWidth = bmResult.width | ||
| val bmHeight = bmResult.height | ||
| bmResult.recycle() | ||
|
|
||
| mobileScannerCallback( | ||
| barcodeMap, | ||
| byteArray, | ||
| bmWidth, | ||
| bmHeight | ||
| ) | ||
| }.addOnFailureListener { e -> | ||
| mobileScannerErrorCallback( | ||
| e.localizedMessage ?: e.toString() | ||
| ) | ||
| }.addOnCompleteListener { imageProxy.close() } | ||
| } | ||
|
|
||
| if (detectionSpeed == DetectionSpeed.NORMAL) { | ||
| // Set timer and continue | ||
|
|
@@ -161,7 +170,6 @@ class MobileScanner( | |
| return Bitmap.createBitmap(bitmap, 0, 0, bitmap.width, bitmap.height, matrix, true) | ||
| } | ||
|
|
||
| // scales the scanWindow to the provided inputImage and checks if that scaled | ||
| // scanWindow contains the barcode | ||
| private fun isBarcodeInScanWindow( | ||
|
|
@@ -240,11 +248,7 @@ class MobileScanner( | |
| } | ||
|
|
||
| lastScanned = null | ||
| scanner = barcodeScannerFactory(barcodeScannerOptions) | ||
|
|
||
| val cameraProviderFuture = ProcessCameraProvider.getInstance(activity) | ||
| val executor = ContextCompat.getMainExecutor(activity) | ||
|
|
@@ -408,14 +412,27 @@ class MobileScanner( | |
| } | ||
|
|
||
| val owner = activity as LifecycleOwner | ||
| // Release the camera observers first. | ||
| camera?.cameraInfo?.let { | ||
|
There was a problem hiding this comment. This was a bug, we didn't remove all observers. But we are listening to torch & zoom |
||
| it.torchState.removeObservers(owner) | ||
| it.zoomState.removeObservers(owner) | ||
| it.cameraState.removeObservers(owner) | ||
| } | ||
| // Unbind the camera use cases, the preview is a use case. | ||
| // The camera will be closed when the last use case is unbound. | ||
| cameraProvider?.unbindAll() | ||
| cameraProvider = null | ||
|
There was a problem hiding this comment. The following is just reflowing code so that the relevant things are together. |
||
| camera = null | ||
| preview = null | ||
|
|
||
| // Release the texture for the preview. | ||
| textureEntry?.release() | ||
| textureEntry = null | ||
|
|
||
| // Release the scanner. | ||
| scanner?.close() | ||
|
There was a problem hiding this comment. This is new, we release the scanner when the users stops the camera. No need to keep it around when the preview / camera is gone. |
||
| scanner = null | ||
| lastScanned = null | ||
|
There was a problem hiding this comment. Setting |
||
| } | ||
|
|
||
| private fun isStopped() = camera == null && preview == null | ||
|
|
@@ -439,22 +456,29 @@ class MobileScanner( | |
| /** | ||
| * Analyze a single image. | ||
| */ | ||
| fun analyzeImage( | ||
|
There was a problem hiding this comment. Analyze image now takes configurable options and uses a short lived scanner. Using the short lived scanner is part of the fix for the retain cycle thing and allowing options is part of the fix for analyze image allowing all formats. |
||
| image: Uri, | ||
| scannerOptions: BarcodeScannerOptions?, | ||
| onSuccess: AnalyzerSuccessCallback, | ||
| onError: AnalyzerErrorCallback) { | ||
| val inputImage = InputImage.fromFilePath(activity, image) | ||
|
|
||
| // Use a short lived scanner instance, which is closed when the analysis is done. | ||
| val barcodeScanner: BarcodeScanner = barcodeScannerFactory(scannerOptions) | ||
|
|
||
| barcodeScanner.process(inputImage).addOnSuccessListener { barcodes -> | ||
| val barcodeMap = barcodes.map { barcode -> barcode.data } | ||
|
|
||
| if (barcodeMap.isEmpty()) { | ||
| onSuccess(null) | ||
| } else { | ||
| onSuccess(barcodeMap) | ||
| } | ||
| }.addOnFailureListener { e -> | ||
| onError(e.localizedMessage ?: e.toString()) | ||
| }.addOnCompleteListener { | ||
| barcodeScanner.close() | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -474,4 +498,14 @@ class MobileScanner( | |
| camera?.cameraControl?.setZoomRatio(1f) | ||
| } | ||
|
|
||
| /** | ||
| * Dispose of this scanner instance. | ||
| */ | ||
| fun dispose() { | ||
| if (isStopped()) { | ||
| return | ||
| } | ||
|
|
||
| stop() // Defer to the stop method, which disposes all resources anyway. | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import