Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Move copyPixelsFromBuffer to a background thread. #49414

Closed
wants to merge 1 commit into from
Closed

Move copyPixelsFromBuffer to a background thread. #49414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 70 additions & 45 deletions shell/platform/android/io/flutter/embedding/android/FlutterImageView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
import android.media.Image;
import android.media.Image.Plane;
import android.media.ImageReader;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.Looper;
import android.util.AttributeSet;
import android.view.Surface;
import android.view.View;
Expand All @@ -40,13 +43,18 @@
* onDraw}.
*/
@TargetApi(19)
public class FlutterImageView extends View implements RenderSurface {
public class FlutterImageView extends View
implements RenderSurface, ImageReader.OnImageAvailableListener {
private static final String TAG = "FlutterImageView";
private static final Handler sUiHandler = new Handler(Looper.getMainLooper());

@NonNull private ImageReader imageReader;
@Nullable private Image currentImage;
@NonNull private final Object bitmapLock = new Object();
// Read/write protected by |bitmapLock|
@Nullable private Bitmap currentBitmap;
private boolean isBitmapAvailableForRendering;
@Nullable private FlutterRenderer flutterRenderer;
@Nullable private static Handler sBackgroundHandler;

public ImageReader getImageReader() {
return imageReader;
Expand Down Expand Up @@ -93,6 +101,12 @@ public FlutterImageView(@NonNull Context context, @NonNull AttributeSet attrs) {

private void init() {
setAlpha(0.0f);
if (sBackgroundHandler == null) {
final HandlerThread handlerThread = new HandlerThread("OnImageAvailableHandler");
handlerThread.start();
sBackgroundHandler = new Handler(handlerThread.getLooper());
}
imageReader.setOnImageAvailableListener(this, sBackgroundHandler);
}

private static void logW(String format, Object... args) {
Expand All @@ -111,6 +125,7 @@ private static ImageReader createImageReader(int width, int height) {
logW("ImageReader height must be greater than 0, but given height=%d, set height=1", height);
height = 1;
}

if (android.os.Build.VERSION.SDK_INT >= 29) {
return ImageReader.newInstance(
width,
Expand Down Expand Up @@ -168,10 +183,8 @@ public void detachFromRenderer() {
// attached to the renderer again.
acquireLatestImage();
// Clear drawings.
currentBitmap = null;
setCurrentBitmap(null);

// Close and clear the current image if any.
closeCurrentImage();
invalidate();
isAttachedToFlutterRenderer = false;
if (kind == SurfaceKind.background) {
Expand Down Expand Up @@ -199,20 +212,11 @@ public boolean acquireLatestImage() {
if (!isAttachedToFlutterRenderer) {
return false;
}
// 1. `acquireLatestImage()` may return null if no new image is available.
// 2. There's no guarantee that `onDraw()` is called after `invalidate()`.
// For example, the device may not produce new frames if it's in sleep mode
// or some special Android devices so the calls to `invalidate()` queued up
// until the device produces a new frame.
// 3. While the engine will also stop producing frames, there is a race condition.
final Image newImage = imageReader.acquireLatestImage();
if (newImage != null) {
// Only close current image after acquiring valid new image
closeCurrentImage();
currentImage = newImage;

if (isBitmapAvailableForRendering) {
invalidate();
}
return newImage != null;
return currentBitmap != null;
}

/** Creates a new image reader with the provided size. */
Expand All @@ -224,12 +228,11 @@ public void resizeIfNeeded(int width, int height) {
return;
}

// Close resources.
closeCurrentImage();
// Close the current image reader, then create a new one with the new size.
// Image readers cannot be resized once created.
closeImageReader();
imageReader = createImageReader(width, height);
imageReader.setOnImageAvailableListener(this, sBackgroundHandler);
}

/**
Expand All @@ -242,52 +245,74 @@ public void closeImageReader() {
imageReader.close();
}

private void setCurrentBitmap(Bitmap bitmap) {
synchronized (bitmapLock) {
currentBitmap = bitmap;
isBitmapAvailableForRendering = bitmap != null;
}
}

@Override
protected void onDraw(Canvas canvas) {
super.onDraw(canvas);
if (currentImage != null) {
updateCurrentBitmap();
}
if (currentBitmap != null) {
canvas.drawBitmap(currentBitmap, 0, 0, null);
synchronized (bitmapLock) {
if (currentBitmap != null) {
canvas.drawBitmap(currentBitmap, 0, 0, null);
isBitmapAvailableForRendering = false;
}
}
}

private void closeCurrentImage() {
// Close and clear the current image if any.
if (currentImage != null) {
currentImage.close();
currentImage = null;
// This method is run on the |sBackgroundHandler|.
@Override
public void onImageAvailable(ImageReader reader) {
// 1. `acquireLatestImage` will discard any images in the queue up to the most recent
// one.
// 2. `acquireLatestImage()` may return null if no new image is available.
// 3. There's no guarantee that `onDraw()` is called after `invalidate()`.
// For example, the device may not produce new frames if it's in sleep mode
// or some special Android devices so the calls to `invalidate()` queued up
// until the device produces a new frame.
// 4. While the engine will also stop producing frames, there is a race condition.
try (final Image image = reader.acquireLatestImage()) {
if (image == null) {
return;
}

Bitmap bitmap = convertImageToBitmap(image);
if (bitmap != null) {
setCurrentBitmap(bitmap);
sUiHandler.postAtFrontOfQueue(() -> invalidate());
}
}
}

@TargetApi(29)
private void updateCurrentBitmap() {
@VisibleForTesting
/*package*/ Bitmap convertImageToBitmap(@NonNull Image image) {
Bitmap bitmap = null;
if (android.os.Build.VERSION.SDK_INT >= 29) {
final HardwareBuffer buffer = currentImage.getHardwareBuffer();
currentBitmap = Bitmap.wrapHardwareBuffer(buffer, ColorSpace.get(ColorSpace.Named.SRGB));
final HardwareBuffer buffer = image.getHardwareBuffer();
bitmap = Bitmap.wrapHardwareBuffer(buffer, ColorSpace.get(ColorSpace.Named.SRGB));
Comment on lines +294 to +295
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be super cheap right?

We're then paying the cost of synchronization/extra threading which will slow things down.

buffer.close();
} else {
final Plane[] imagePlanes = currentImage.getPlanes();
final Plane[] imagePlanes = image.getPlanes();
if (imagePlanes.length != 1) {
return;
return null;
}

final Plane imagePlane = imagePlanes[0];
final int desiredWidth = imagePlane.getRowStride() / imagePlane.getPixelStride();
final int desiredHeight = currentImage.getHeight();

if (currentBitmap == null
|| currentBitmap.getWidth() != desiredWidth
|| currentBitmap.getHeight() != desiredHeight) {
currentBitmap =
Bitmap.createBitmap(
desiredWidth, desiredHeight, android.graphics.Bitmap.Config.ARGB_8888);
}
final int desiredHeight = image.getHeight();

bitmap =
Bitmap.createBitmap(
desiredWidth, desiredHeight, android.graphics.Bitmap.Config.ARGB_8888);
ByteBuffer buffer = imagePlane.getBuffer();
buffer.rewind();
currentBitmap.copyPixelsFromBuffer(buffer);
bitmap.copyPixelsFromBuffer(buffer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only piece that would benefit things, but without a benchmark and/or tracing to show that this actually helps things in real applications we're just adding overhead for newer devices.

It might make sense to add some tracing around this call so we could look at existing benchmarks/applications and see how long it's really taking. I'm not 100% convinced that this will benefit too much from parallelization though, because Android devices tend to have slower memory access, and making more threads do more memory related work will probably be bad in some cases.

}

return bitmap;
}

@Override
Expand Down
57 changes: 57 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/android/FlutterImageViewTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package io.flutter.embedding.android;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

import android.annotation.TargetApi;
import android.content.Context;
import android.graphics.Bitmap;
import android.hardware.HardwareBuffer;
import android.media.Image;
import android.media.Image.Plane;
import android.media.ImageReader;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import io.flutter.embedding.engine.FlutterJNI;
import io.flutter.embedding.engine.renderer.FlutterRenderer;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.annotation.Config;

@Config(manifest = Config.NONE)
@RunWith(AndroidJUnit4.class)
@TargetApi(30)
public class FlutterImageViewTest {
private final Context ctx = ApplicationProvider.getApplicationContext();

@Test
public void acquireLatestImage() {
final ImageReader mockReader = mock(ImageReader.class);
final Image mockImage = mock(Image.class);
final HardwareBuffer mockHardwareBuffer = mock(HardwareBuffer.class);
final Bitmap mockBitmap = mock(Bitmap.class);
final FlutterImageView imageView =
spy(new FlutterImageView(ctx, mockReader, FlutterImageView.SurfaceKind.background));

when(mockReader.getMaxImages()).thenReturn(2);
when(mockReader.acquireLatestImage()).thenReturn(mockImage);
when(mockImage.getPlanes()).thenReturn(new Plane[0]);
when(mockImage.getHardwareBuffer()).thenReturn(mockHardwareBuffer);
when(mockHardwareBuffer.getUsage()).thenReturn(HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE);
when(imageView.convertImageToBitmap(mockImage)).thenReturn(mockBitmap);

final FlutterJNI jni = mock(FlutterJNI.class);
imageView.attachToRenderer(new FlutterRenderer(jni));
doNothing().when(imageView).invalidate();

assertFalse(imageView.acquireLatestImage());

// Simulate the next frame available.
imageView.onImageAvailable(mockReader);
assertTrue(imageView.acquireLatestImage());
Comment on lines +51 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this test does cover the new behavior, I'm not sure that we really want the new behavior.

}
}