-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[camera_android] Use WeakReference to prevent startImageStream OOM error when main thread hangs (flutter#166533) #9571
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
Changes from 11 commits
3a5ef0f
71e9414
f9db089
ee845f4
97e98f2
bf15d0b
d498fb7
cc61ef8
229ccd4
cad3010
6ed20c9
79deae1
511e27f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,11 +9,14 @@ | |
| import android.media.ImageReader; | ||
| import android.os.Handler; | ||
| import android.os.Looper; | ||
| import android.util.Log; | ||
| import android.view.Surface; | ||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.VisibleForTesting; | ||
| import io.flutter.plugin.common.EventChannel; | ||
| import io.flutter.plugins.camera.types.CameraCaptureProperties; | ||
| import java.lang.ref.WeakReference; | ||
| import java.nio.ByteBuffer; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
|
|
@@ -22,6 +25,7 @@ | |
|
|
||
| // Wraps an ImageReader to allow for testing of the image handler. | ||
| public class ImageStreamReader { | ||
| private static final String TAG = "ImageStreamReader"; | ||
|
|
||
| /** | ||
| * The image format we are going to send back to dart. Usually it's the same as streamImageFormat | ||
|
|
@@ -33,6 +37,16 @@ public class ImageStreamReader { | |
| private final ImageReader imageReader; | ||
| private final ImageStreamReaderUtils imageStreamReaderUtils; | ||
|
|
||
| @VisibleForTesting(otherwise = VisibleForTesting.NONE) | ||
| @Nullable | ||
| public Handler handler; | ||
|
|
||
| /** | ||
| * This hard reference is required so frames don't get randomly dropped before reaching the main | ||
| * looper. | ||
| */ | ||
| private Map<String, Object> latestImageBufferHardReference = null; | ||
|
|
||
| /** | ||
| * Creates a new instance of the {@link ImageStreamReader}. | ||
| * | ||
|
|
@@ -95,40 +109,67 @@ public void onImageAvailable( | |
| @NonNull Image image, | ||
| @NonNull CameraCaptureProperties captureProps, | ||
| @NonNull EventChannel.EventSink imageStreamSink) { | ||
| try { | ||
| Map<String, Object> imageBuffer = new HashMap<>(); | ||
| Map<String, Object> imageBuffer = new HashMap<>(); | ||
|
|
||
| imageBuffer.put("width", image.getWidth()); | ||
| imageBuffer.put("height", image.getHeight()); | ||
| try { | ||
| // Get plane data ready | ||
| if (dartImageFormat == ImageFormat.NV21) { | ||
| imageBuffer.put("planes", parsePlanesForNv21(image)); | ||
| } else { | ||
| imageBuffer.put("planes", parsePlanesForYuvOrJpeg(image)); | ||
| } | ||
|
|
||
| imageBuffer.put("width", image.getWidth()); | ||
| imageBuffer.put("height", image.getHeight()); | ||
| imageBuffer.put("format", dartImageFormat); | ||
| imageBuffer.put("lensAperture", captureProps.getLastLensAperture()); | ||
| imageBuffer.put("sensorExposureTime", captureProps.getLastSensorExposureTime()); | ||
| Integer sensorSensitivity = captureProps.getLastSensorSensitivity(); | ||
| imageBuffer.put( | ||
| "sensorSensitivity", sensorSensitivity == null ? null : (double) sensorSensitivity); | ||
|
|
||
| final Handler handler = new Handler(Looper.getMainLooper()); | ||
| handler.post(() -> imageStreamSink.success(imageBuffer)); | ||
| image.close(); | ||
|
|
||
| } catch (IllegalStateException e) { | ||
| // Handle "buffer is inaccessible" errors that can happen on some devices from ImageStreamReaderUtils.yuv420ThreePlanesToNV21() | ||
| final Handler handler = new Handler(Looper.getMainLooper()); | ||
| // Handle "buffer is inaccessible" errors that can happen on some devices from | ||
| // ImageStreamReaderUtils.yuv420ThreePlanesToNV21() | ||
| final Handler handler = | ||
| this.handler != null ? this.handler : new Handler(Looper.getMainLooper()); | ||
| handler.post( | ||
| () -> | ||
| imageStreamSink.error( | ||
| "IllegalStateException", | ||
| "Caught IllegalStateException: " + e.getMessage(), | ||
| null)); | ||
| } finally { | ||
| image.close(); | ||
| } | ||
|
|
||
| imageBuffer.put("format", dartImageFormat); | ||
| imageBuffer.put("lensAperture", captureProps.getLastLensAperture()); | ||
| imageBuffer.put("sensorExposureTime", captureProps.getLastSensorExposureTime()); | ||
| Integer sensorSensitivity = captureProps.getLastSensorSensitivity(); | ||
| imageBuffer.put( | ||
| "sensorSensitivity", sensorSensitivity == null ? null : (double) sensorSensitivity); | ||
|
|
||
| final Handler handler = | ||
| this.handler != null ? this.handler : new Handler(Looper.getMainLooper()); | ||
|
|
||
| // Keep a hard reference to the latest frame, so it isn't dropped before it reaches the main | ||
| // looper | ||
| latestImageBufferHardReference = imageBuffer; | ||
|
|
||
| boolean postResult = | ||
| handler.post( | ||
| new Runnable() { | ||
| // a public field is required in order to test this code | ||
| @VisibleForTesting public WeakReference<Map<String, Object>> weakImageBuffer; | ||
|
|
||
| public Runnable withImageBuffer(Map<String, Object> imageBuffer) { | ||
| weakImageBuffer = new WeakReference<>(imageBuffer); | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| public void run() { | ||
| final Map<String, Object> imageBuffer = weakImageBuffer.get(); | ||
| if (imageBuffer == null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is correct but can you add a comment here about why garbage collected image buffer is neither and error or success?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope the comment I added is sufficient? |
||
| Log.d(TAG, "Image buffer was dropped by garbage collector."); | ||
| return; | ||
| } | ||
| imageStreamSink.success(imageBuffer); | ||
| } | ||
| }.withImageBuffer(imageBuffer)); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.camera.media; | ||
|
|
||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import android.media.Image; | ||
| import java.nio.ByteBuffer; | ||
|
|
||
| public class ImageStreamReaderTestUtils { | ||
| public static Image getImage(int imageWidth, int imageHeight, int padding, int imageFormat) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add java doc for this method |
||
| int rowStride = imageWidth + padding; | ||
|
|
||
| int ySize = (rowStride * imageHeight) - padding; | ||
| int uSize = (ySize / 2) - (padding / 2); | ||
| int vSize = uSize; | ||
|
|
||
| // Mock YUV image | ||
| Image mockImage = mock(Image.class); | ||
| when(mockImage.getWidth()).thenReturn(imageWidth); | ||
| when(mockImage.getHeight()).thenReturn(imageHeight); | ||
| when(mockImage.getFormat()).thenReturn(imageFormat); | ||
|
|
||
| // Mock planes. YUV images have 3 planes (Y, U, V). | ||
| Image.Plane planeY = mock(Image.Plane.class); | ||
| Image.Plane planeU = mock(Image.Plane.class); | ||
| Image.Plane planeV = mock(Image.Plane.class); | ||
|
|
||
| // Y plane is width*height | ||
| // Row stride is generally == width but when there is padding it will | ||
| // be larger. | ||
| // Here we are adding 256 padding. | ||
| when(planeY.getBuffer()).thenReturn(ByteBuffer.allocate(ySize)); | ||
| when(planeY.getRowStride()).thenReturn(rowStride); | ||
| when(planeY.getPixelStride()).thenReturn(1); | ||
|
|
||
| // U and V planes are always the same sizes/values. | ||
| // https://developer.android.com/reference/android/graphics/ImageFormat#YUV_420_888 | ||
| when(planeU.getBuffer()).thenReturn(ByteBuffer.allocate(uSize)); | ||
| when(planeV.getBuffer()).thenReturn(ByteBuffer.allocate(vSize)); | ||
| when(planeU.getRowStride()).thenReturn(rowStride); | ||
| when(planeV.getRowStride()).thenReturn(rowStride); | ||
| when(planeU.getPixelStride()).thenReturn(2); | ||
| when(planeV.getPixelStride()).thenReturn(2); | ||
|
|
||
| // Add planes to image | ||
| Image.Plane[] planes = {planeY, planeU, planeV}; | ||
| when(mockImage.getPlanes()).thenReturn(planes); | ||
|
|
||
| return mockImage; | ||
| } | ||
| } | ||
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.
nit: This comment can be removed it does not add anything over the @visibleForTesing annotation.