Merge "media: improve ImageReader/Writer native memory management"
diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java
index 2164eec..66a174c 100644
--- a/media/java/android/media/ImageReader.java
+++ b/media/java/android/media/ImageReader.java
@@ -17,10 +17,10 @@
package android.media;
import android.graphics.ImageFormat;
-import android.graphics.PixelFormat;
import android.os.Handler;
import android.os.Looper;
import android.os.Message;
+import android.util.Log;
import android.view.Surface;
import dalvik.system.VMRuntime;
@@ -29,6 +29,8 @@
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.NioUtils;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
/**
@@ -145,7 +147,7 @@
"NV21 format is not supported");
}
- mNumPlanes = getNumPlanesFromFormat();
+ mNumPlanes = ImageUtils.getNumPlanesForFormat(mFormat);
nativeInit(new WeakReference<ImageReader>(this), width, height, format, maxImages);
@@ -323,21 +325,27 @@
* @see #ACQUIRE_SUCCESS
*/
private int acquireNextSurfaceImage(SurfaceImage si) {
+ synchronized (mCloseLock) {
+ int status = nativeImageSetup(si);
- int status = nativeImageSetup(si);
+ switch (status) {
+ case ACQUIRE_SUCCESS:
+ si.createSurfacePlanes();
+ si.mIsImageValid = true;
+ case ACQUIRE_NO_BUFS:
+ case ACQUIRE_MAX_IMAGES:
+ break;
+ default:
+ throw new AssertionError("Unknown nativeImageSetup return code " + status);
+ }
- switch (status) {
- case ACQUIRE_SUCCESS:
- si.createSurfacePlanes();
- si.mIsImageValid = true;
- case ACQUIRE_NO_BUFS:
- case ACQUIRE_MAX_IMAGES:
- break;
- default:
- throw new AssertionError("Unknown nativeImageSetup return code " + status);
+ // Only keep track the successfully acquired image, as the native buffer is only mapped
+ // for such case.
+ if (status == ACQUIRE_SUCCESS) {
+ mAcquiredImages.add(si);
+ }
+ return status;
}
-
- return status;
}
/**
@@ -398,7 +406,11 @@
"This image was not produced by an ImageReader");
}
SurfaceImage si = (SurfaceImage) i;
- if (si.getReader() != this) {
+ if (si.mIsImageValid == false) {
+ return;
+ }
+
+ if (si.getReader() != this || !mAcquiredImages.contains(i)) {
throw new IllegalArgumentException(
"This image was not produced by this ImageReader");
}
@@ -406,6 +418,7 @@
si.clearSurfacePlanes();
nativeReleaseImage(i);
si.mIsImageValid = false;
+ mAcquiredImages.remove(i);
}
/**
@@ -475,7 +488,24 @@
public void close() {
setOnImageAvailableListener(null, null);
if (mSurface != null) mSurface.release();
- nativeClose();
+
+ /**
+ * Close all outstanding acquired images before closing the ImageReader. It is a good
+ * practice to close all the images as soon as it is not used to reduce system instantaneous
+ * memory pressure. CopyOnWrite list will use a copy of current list content. For the images
+ * being closed by other thread (e.g., GC thread), doubling the close call is harmless. For
+ * the image being acquired by other threads, mCloseLock is used to synchronize close and
+ * acquire operations.
+ */
+ synchronized (mCloseLock) {
+ for (Image image : mAcquiredImages) {
+ image.close();
+ }
+ mAcquiredImages.clear();
+
+ nativeClose();
+ }
+
if (mEstimatedNativeAllocBytes > 0) {
VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);
mEstimatedNativeAllocBytes = 0;
@@ -542,45 +572,6 @@
si.setDetached(true);
}
- /**
- * Only a subset of the formats defined in
- * {@link android.graphics.ImageFormat ImageFormat} and
- * {@link android.graphics.PixelFormat PixelFormat} are supported by
- * ImageReader. When reading RGB data from a surface, the formats defined in
- * {@link android.graphics.PixelFormat PixelFormat} can be used, when
- * reading YUV, JPEG or raw sensor data (for example, from camera or video
- * decoder), formats from {@link android.graphics.ImageFormat ImageFormat}
- * are used.
- */
- private int getNumPlanesFromFormat() {
- switch (mFormat) {
- case ImageFormat.YV12:
- case ImageFormat.YUV_420_888:
- case ImageFormat.NV21:
- return 3;
- case ImageFormat.NV16:
- return 2;
- case PixelFormat.RGB_565:
- case PixelFormat.RGBA_8888:
- case PixelFormat.RGBX_8888:
- case PixelFormat.RGB_888:
- case ImageFormat.JPEG:
- case ImageFormat.YUY2:
- case ImageFormat.Y8:
- case ImageFormat.Y16:
- case ImageFormat.RAW_SENSOR:
- case ImageFormat.RAW10:
- case ImageFormat.DEPTH16:
- case ImageFormat.DEPTH_POINT_CLOUD:
- return 1;
- case ImageFormat.PRIVATE:
- return 0;
- default:
- throw new UnsupportedOperationException(
- String.format("Invalid format specified %d", mFormat));
- }
- }
-
private boolean isImageOwnedbyMe(Image image) {
if (!(image instanceof SurfaceImage)) {
return false;
@@ -612,7 +603,6 @@
}
}
-
private final int mWidth;
private final int mHeight;
private final int mFormat;
@@ -622,8 +612,12 @@
private int mEstimatedNativeAllocBytes;
private final Object mListenerLock = new Object();
+ private final Object mCloseLock = new Object();
private OnImageAvailableListener mListener;
private ListenerHandler mListenerHandler;
+ // Keep track of the successfully acquired Images. This need to be thread safe as the images
+ // could be closed by different threads (e.g., application thread and GC thread).
+ private List<Image> mAcquiredImages = new CopyOnWriteArrayList<Image>();
/**
* This field is used by native code, do not access or modify.
@@ -657,9 +651,7 @@
@Override
public void close() {
- if (mIsImageValid) {
- ImageReader.this.releaseImage(this);
- }
+ ImageReader.this.releaseImage(this);
}
public ImageReader getReader() {
diff --git a/media/java/android/media/ImageUtils.java b/media/java/android/media/ImageUtils.java
index ba3949a..eefd69c 100644
--- a/media/java/android/media/ImageUtils.java
+++ b/media/java/android/media/ImageUtils.java
@@ -58,6 +58,9 @@
case ImageFormat.Y16:
case ImageFormat.RAW_SENSOR:
case ImageFormat.RAW10:
+ case ImageFormat.RAW12:
+ case ImageFormat.DEPTH16:
+ case ImageFormat.DEPTH_POINT_CLOUD:
return 1;
case ImageFormat.PRIVATE:
return 0;
diff --git a/media/java/android/media/ImageWriter.java b/media/java/android/media/ImageWriter.java
index 9fb3286..851d436 100644
--- a/media/java/android/media/ImageWriter.java
+++ b/media/java/android/media/ImageWriter.java
@@ -18,17 +18,21 @@
import android.graphics.ImageFormat;
import android.graphics.Rect;
+import android.hardware.camera2.utils.SurfaceUtils;
import android.os.Handler;
import android.os.Looper;
import android.os.Message;
+import android.util.Size;
import android.view.Surface;
+import dalvik.system.VMRuntime;
+
import java.lang.ref.WeakReference;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.NioUtils;
-import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
/**
* <p>
@@ -83,8 +87,10 @@
private int mWriterFormat;
private final int mMaxImages;
- // Keep track of the currently dequeued Image.
- private List<Image> mDequeuedImages = new ArrayList<Image>();
+ // Keep track of the currently dequeued Image. This need to be thread safe as the images
+ // could be closed by different threads (e.g., application thread and GC thread).
+ private List<Image> mDequeuedImages = new CopyOnWriteArrayList<Image>();
+ private int mEstimatedNativeAllocBytes;
/**
* <p>
@@ -128,6 +134,16 @@
// Note that the underlying BufferQueue is working in synchronous mode
// to avoid dropping any buffers.
mNativeContext = nativeInit(new WeakReference<ImageWriter>(this), surface, maxImages);
+
+ // Estimate the native buffer allocation size and register it so it gets accounted for
+ // during GC. Note that this doesn't include the buffers required by the buffer queue
+ // itself and the buffers requested by the producer.
+ Size surfSize = SurfaceUtils.getSurfaceSize(surface);
+ int format = SurfaceUtils.getSurfaceFormat(surface);
+ mEstimatedNativeAllocBytes =
+ ImageUtils.getEstimatedNativeAllocBytes(surfSize.getWidth(),surfSize.getHeight(),
+ format, maxImages);
+ VMRuntime.getRuntime().registerNativeAllocation(mEstimatedNativeAllocBytes);
}
/**
@@ -432,6 +448,11 @@
mDequeuedImages.clear();
nativeClose(mNativeContext);
mNativeContext = 0;
+
+ if (mEstimatedNativeAllocBytes > 0) {
+ VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);
+ mEstimatedNativeAllocBytes = 0;
+ }
}
@Override
@@ -569,9 +590,8 @@
}
WriterSurfaceImage wi = (WriterSurfaceImage) image;
-
if (!wi.mIsImageValid) {
- throw new IllegalStateException("Image is invalid");
+ return;
}
/**
diff --git a/media/jni/android_media_ImageReader.cpp b/media/jni/android_media_ImageReader.cpp
index 3ffdb17..9a53186 100644
--- a/media/jni/android_media_ImageReader.cpp
+++ b/media/jni/android_media_ImageReader.cpp
@@ -933,7 +933,7 @@
CpuConsumer* consumer = ctx->getCpuConsumer();
CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, image);
if (!buffer) {
- ALOGW("Image already released!!!");
+ // Release an already closed image is harmless.
return;
}
consumer->unlockBuffer(*buffer);
@@ -1078,7 +1078,8 @@
ALOGV("%s:", __FUNCTION__);
JNIImageReaderContext* ctx = ImageReader_getContext(env, thiz);
if (ctx == NULL) {
- jniThrowRuntimeException(env, "ImageReaderContext is not initialized");
+ jniThrowException(env, "java/lang/IllegalStateException",
+ "ImageReader is not initialized or was already closed");
return -1;
}
diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp
index f92a8ef..f50da85 100644
--- a/media/jni/android_media_ImageWriter.cpp
+++ b/media/jni/android_media_ImageWriter.cpp
@@ -403,8 +403,7 @@
ALOGV("%s", __FUNCTION__);
JNIImageWriterContext* const ctx = reinterpret_cast<JNIImageWriterContext *>(nativeCtx);
if (ctx == NULL || thiz == NULL) {
- jniThrowException(env, "java/lang/IllegalStateException",
- "ImageWriterContext is not initialized");
+ ALOGW("ImageWriter#close called before Image#close, consider calling Image#close first");
return;
}
@@ -414,8 +413,7 @@
int fenceFd = -1;
Image_getNativeContext(env, image, &buffer, &fenceFd);
if (buffer == NULL) {
- jniThrowException(env, "java/lang/IllegalStateException",
- "Image is not initialized");
+ // Cancel an already cancelled image is harmless.
return;
}