diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java index 2164eec48705b..66a174c35f5ce 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.lang.ref.WeakReference; 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 @@ public class ImageReader implements AutoCloseable { "NV21 format is not supported"); } - mNumPlanes = getNumPlanesFromFormat(); + mNumPlanes = ImageUtils.getNumPlanesForFormat(mFormat); nativeInit(new WeakReference(this), width, height, format, maxImages); @@ -323,21 +325,27 @@ public class ImageReader implements AutoCloseable { * @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 @@ public class ImageReader implements AutoCloseable { "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 @@ public class ImageReader implements AutoCloseable { si.clearSurfacePlanes(); nativeReleaseImage(i); si.mIsImageValid = false; + mAcquiredImages.remove(i); } /** @@ -475,7 +488,24 @@ public class ImageReader implements AutoCloseable { 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 @@ public class ImageReader implements AutoCloseable { 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 @@ public class ImageReader implements AutoCloseable { } } - private final int mWidth; private final int mHeight; private final int mFormat; @@ -622,8 +612,12 @@ public class ImageReader implements AutoCloseable { 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 mAcquiredImages = new CopyOnWriteArrayList(); /** * This field is used by native code, do not access or modify. @@ -657,9 +651,7 @@ public class ImageReader implements AutoCloseable { @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 ba3949a64e235..eefd69c4cb3c2 100644 --- a/media/java/android/media/ImageUtils.java +++ b/media/java/android/media/ImageUtils.java @@ -58,6 +58,9 @@ class ImageUtils { 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 9fb328627612d..851d436744588 100644 --- a/media/java/android/media/ImageWriter.java +++ b/media/java/android/media/ImageWriter.java @@ -18,17 +18,21 @@ package android.media; 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; /** *

@@ -83,8 +87,10 @@ public class ImageWriter implements AutoCloseable { private int mWriterFormat; private final int mMaxImages; - // Keep track of the currently dequeued Image. - private List mDequeuedImages = new ArrayList(); + // 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 mDequeuedImages = new CopyOnWriteArrayList(); + private int mEstimatedNativeAllocBytes; /** *

@@ -128,6 +134,16 @@ public class ImageWriter implements AutoCloseable { // Note that the underlying BufferQueue is working in synchronous mode // to avoid dropping any buffers. mNativeContext = nativeInit(new WeakReference(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 @@ public class ImageWriter implements AutoCloseable { mDequeuedImages.clear(); nativeClose(mNativeContext); mNativeContext = 0; + + if (mEstimatedNativeAllocBytes > 0) { + VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes); + mEstimatedNativeAllocBytes = 0; + } } @Override @@ -569,9 +590,8 @@ public class ImageWriter implements AutoCloseable { } 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 3ffdb17f4ec0f..9a53186365e18 100644 --- a/media/jni/android_media_ImageReader.cpp +++ b/media/jni/android_media_ImageReader.cpp @@ -933,7 +933,7 @@ static void ImageReader_imageRelease(JNIEnv* env, jobject thiz, jobject image) 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 @@ static jint ImageReader_imageSetup(JNIEnv* env, jobject thiz, jobject image) { 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 f92a8eff3919d..f50da8547af21 100644 --- a/media/jni/android_media_ImageWriter.cpp +++ b/media/jni/android_media_ImageWriter.cpp @@ -403,8 +403,7 @@ static void ImageWriter_cancelImage(JNIEnv* env, jobject thiz, jlong nativeCtx, ALOGV("%s", __FUNCTION__); JNIImageWriterContext* const ctx = reinterpret_cast(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 @@ static void ImageWriter_cancelImage(JNIEnv* env, jobject thiz, jlong nativeCtx, 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; }