ImageReader/Writer: implement opaque format operations

Implement attach/detach for image reader and writer.

Bug: 19872821
Change-Id: Ib45a054c6be0b56b370fa8d709b47b0298ba5ea7

2 files changed, 409 insertions, 104 deletions

diff --git a/media/jni/android_media_ImageReader.cpp b/media/jni/android_media_ImageReader.cpp
index 708c083..043e20b 100644
--- a/media/jni/android_media_ImageReader.cpp
+++ b/media/jni/android_media_ImageReader.cpp
@@ -24,6 +24,7 @@
 #include <cstdio>
 
 #include <gui/CpuConsumer.h>
+#include <gui/BufferItemConsumer.h>
 #include <gui/Surface.h>
 #include <camera3.h>
 
@@ -39,7 +40,7 @@
 #define ALIGN(x, mask) ( ((x) + (mask) - 1) & ~((mask) - 1) )
 
 #define ANDROID_MEDIA_IMAGEREADER_CTX_JNI_ID       "mNativeContext"
-#define ANDROID_MEDIA_SURFACEIMAGE_BUFFER_JNI_ID   "mLockedBuffer"
+#define ANDROID_MEDIA_SURFACEIMAGE_BUFFER_JNI_ID   "mNativeBuffer"
 #define ANDROID_MEDIA_SURFACEIMAGE_TS_JNI_ID       "mTimestamp"
 
 // ----------------------------------------------------------------------------
@@ -62,7 +63,7 @@ static struct {
 } gImageReaderClassInfo;
 
 static struct {
-    jfieldID mLockedBuffer;
+    jfieldID mNativeBuffer;
     jfieldID mTimestamp;
 } gSurfaceImageClassInfo;
 
@@ -73,7 +74,7 @@ static struct {
 
 // ----------------------------------------------------------------------------
 
-class JNIImageReaderContext : public CpuConsumer::FrameAvailableListener
+class JNIImageReaderContext : public ConsumerBase::FrameAvailableListener
 {
 public:
     JNIImageReaderContext(JNIEnv* env, jobject weakThiz, jclass clazz, int maxImages);
@@ -83,12 +84,19 @@ public:
     virtual void onFrameAvailable(const BufferItem& item);
 
     CpuConsumer::LockedBuffer* getLockedBuffer();
-
     void returnLockedBuffer(CpuConsumer::LockedBuffer* buffer);
 
+    BufferItem* getOpaqueBuffer();
+    void returnOpaqueBuffer(BufferItem* buffer);
+
     void setCpuConsumer(const sp<CpuConsumer>& consumer) { mConsumer = consumer; }
     CpuConsumer* getCpuConsumer() { return mConsumer.get(); }
 
+    void setOpaqueConsumer(const sp<BufferItemConsumer>& consumer) { mOpaqueConsumer = consumer; }
+    BufferItemConsumer* getOpaqueConsumer() { return mOpaqueConsumer.get(); }
+    // This is the only opaque format exposed in the ImageFormat public API.
+    bool isOpaque() { return mFormat == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED; }
+
     void setProducer(const sp<IGraphicBufferProducer>& producer) { mProducer = producer; }
     IGraphicBufferProducer* getProducer() { return mProducer.get(); }
 
@@ -109,7 +117,9 @@ private:
     static void detachJNI();
 
     List<CpuConsumer::LockedBuffer*> mBuffers;
+    List<BufferItem*> mOpaqueBuffers;
     sp<CpuConsumer> mConsumer;
+    sp<BufferItemConsumer> mOpaqueConsumer;
     sp<IGraphicBufferProducer> mProducer;
     jobject mWeakThiz;
     jclass mClazz;
@@ -125,7 +135,9 @@ JNIImageReaderContext::JNIImageReaderContext(JNIEnv* env,
     mClazz((jclass)env->NewGlobalRef(clazz)) {
     for (int i = 0; i < maxImages; i++) {
         CpuConsumer::LockedBuffer *buffer = new CpuConsumer::LockedBuffer;
+        BufferItem* opaqueBuffer = new BufferItem;
         mBuffers.push_back(buffer);
+        mOpaqueBuffers.push_back(opaqueBuffer);
     }
 }
 
@@ -169,6 +181,21 @@ void JNIImageReaderContext::returnLockedBuffer(CpuConsumer::LockedBuffer* buffer
     mBuffers.push_back(buffer);
 }
 
+BufferItem* JNIImageReaderContext::getOpaqueBuffer() {
+    if (mOpaqueBuffers.empty()) {
+        return NULL;
+    }
+    // Return an opaque buffer pointer and remove it from the list
+    List<BufferItem*>::iterator it = mOpaqueBuffers.begin();
+    BufferItem* buffer = *it;
+    mOpaqueBuffers.erase(it);
+    return buffer;
+}
+
+void JNIImageReaderContext::returnOpaqueBuffer(BufferItem* buffer) {
+    mOpaqueBuffers.push_back(buffer);
+}
+
 JNIImageReaderContext::~JNIImageReaderContext() {
     bool needsDetach = false;
     JNIEnv* env = getJNIEnv(&needsDetach);
@@ -187,8 +214,20 @@ JNIImageReaderContext::~JNIImageReaderContext() {
             it != mBuffers.end(); it++) {
         delete *it;
     }
+
+    // Delete opaque buffers
+    for (List<BufferItem *>::iterator it = mOpaqueBuffers.begin();
+            it != mOpaqueBuffers.end(); it++) {
+        delete *it;
+    }
+
     mBuffers.clear();
-    mConsumer.clear();
+    if (mConsumer != 0) {
+        mConsumer.clear();
+    }
+    if (mOpaqueConsumer != 0) {
+        mOpaqueConsumer.clear();
+    }
 }
 
 void JNIImageReaderContext::onFrameAvailable(const BufferItem& /*item*/)
@@ -210,6 +249,11 @@ void JNIImageReaderContext::onFrameAvailable(const BufferItem& /*item*/)
 
 extern "C" {
 
+static bool isFormatOpaque(int format) {
+    // Only treat IMPLEMENTATION_DEFINED as an opaque format for now.
+    return format == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED;
+}
+
 static JNIImageReaderContext* ImageReader_getContext(JNIEnv* env, jobject thiz)
 {
     JNIImageReaderContext *ctx;
@@ -226,6 +270,13 @@ static CpuConsumer* ImageReader_getCpuConsumer(JNIEnv* env, jobject thiz)
         jniThrowRuntimeException(env, "ImageReaderContext is not initialized");
         return NULL;
     }
+
+    if (ctx->isOpaque()) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Opaque ImageReader doesn't support this method");
+        return NULL;
+    }
+
     return ctx->getCpuConsumer();
 }
 
@@ -237,6 +288,7 @@ static IGraphicBufferProducer* ImageReader_getProducer(JNIEnv* env, jobject thiz
         jniThrowRuntimeException(env, "ImageReaderContext is not initialized");
         return NULL;
     }
+
     return ctx->getProducer();
 }
 
@@ -258,13 +310,19 @@ static void ImageReader_setNativeContext(JNIEnv* env,
 static CpuConsumer::LockedBuffer* Image_getLockedBuffer(JNIEnv* env, jobject image)
 {
     return reinterpret_cast<CpuConsumer::LockedBuffer*>(
-            env->GetLongField(image, gSurfaceImageClassInfo.mLockedBuffer));
+            env->GetLongField(image, gSurfaceImageClassInfo.mNativeBuffer));
 }
 
 static void Image_setBuffer(JNIEnv* env, jobject thiz,
         const CpuConsumer::LockedBuffer* buffer)
 {
-    env->SetLongField(thiz, gSurfaceImageClassInfo.mLockedBuffer, reinterpret_cast<jlong>(buffer));
+    env->SetLongField(thiz, gSurfaceImageClassInfo.mNativeBuffer, reinterpret_cast<jlong>(buffer));
+}
+
+static void Image_setOpaqueBuffer(JNIEnv* env, jobject thiz,
+        const BufferItem* buffer)
+{
+    env->SetLongField(thiz, gSurfaceImageClassInfo.mNativeBuffer, reinterpret_cast<jlong>(buffer));
 }
 
 static uint32_t Image_getJpegSize(CpuConsumer::LockedBuffer* buffer, bool usingRGBAOverride)
@@ -633,6 +691,52 @@ static int Image_getBufferHeight(CpuConsumer::LockedBuffer* buffer) {
     return buffer->height;
 }
 
+// --------------------------Methods for opaque Image and ImageReader----------
+
+static BufferItemConsumer* ImageReader_getOpaqueConsumer(JNIEnv* env, jobject thiz)
+{
+    ALOGV("%s:", __FUNCTION__);
+    JNIImageReaderContext* const ctx = ImageReader_getContext(env, thiz);
+    if (ctx == NULL) {
+        jniThrowRuntimeException(env, "ImageReaderContext is not initialized");
+        return NULL;
+    }
+
+    if (!ctx->isOpaque()) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Non-opaque ImageReader doesn't support this method");
+    }
+
+    return ctx->getOpaqueConsumer();
+}
+
+static BufferItem* Image_getOpaqueBuffer(JNIEnv* env, jobject image)
+{
+    return reinterpret_cast<BufferItem*>(
+            env->GetLongField(image, gSurfaceImageClassInfo.mNativeBuffer));
+}
+
+static int Image_getOpaqueBufferWidth(BufferItem* buffer) {
+    if (buffer == NULL) return -1;
+
+    if (!buffer->mCrop.isEmpty()) {
+        return buffer->mCrop.getWidth();
+    }
+    return buffer->mGraphicBuffer->getWidth();
+}
+
+static int Image_getOpaqueBufferHeight(BufferItem* buffer) {
+    if (buffer == NULL) return -1;
+
+    if (!buffer->mCrop.isEmpty()) {
+        return buffer->mCrop.getHeight();
+    }
+
+    return buffer->mGraphicBuffer->getHeight();
+}
+
+
+
 // ----------------------------------------------------------------------------
 
 static void ImageReader_classInit(JNIEnv* env, jclass clazz)
@@ -642,9 +746,9 @@ static void ImageReader_classInit(JNIEnv* env, jclass clazz)
     jclass imageClazz = env->FindClass("android/media/ImageReader$SurfaceImage");
     LOG_ALWAYS_FATAL_IF(imageClazz == NULL,
                         "can't find android/graphics/ImageReader$SurfaceImage");
-    gSurfaceImageClassInfo.mLockedBuffer = env->GetFieldID(
+    gSurfaceImageClassInfo.mNativeBuffer = env->GetFieldID(
             imageClazz, ANDROID_MEDIA_SURFACEIMAGE_BUFFER_JNI_ID, "J");
-    LOG_ALWAYS_FATAL_IF(gSurfaceImageClassInfo.mLockedBuffer == NULL,
+    LOG_ALWAYS_FATAL_IF(gSurfaceImageClassInfo.mNativeBuffer == NULL,
                         "can't find android/graphics/ImageReader.%s",
                         ANDROID_MEDIA_SURFACEIMAGE_BUFFER_JNI_ID);
 
@@ -691,24 +795,42 @@ static void ImageReader_init(JNIEnv* env, jobject thiz, jobject weakThiz,
     nativeDataspace = android_view_Surface_mapPublicFormatToHalDataspace(
         publicFormat);
 
-    sp<IGraphicBufferProducer> gbProducer;
-    sp<IGraphicBufferConsumer> gbConsumer;
-    BufferQueue::createBufferQueue(&gbProducer, &gbConsumer);
-    sp<CpuConsumer> consumer = new CpuConsumer(gbConsumer, maxImages,
-                                               /*controlledByApp*/true);
-    // TODO: throw dvm exOutOfMemoryError?
-    if (consumer == NULL) {
-        jniThrowRuntimeException(env, "Failed to allocate native CpuConsumer");
-        return;
-    }
-
     jclass clazz = env->GetObjectClass(thiz);
     if (clazz == NULL) {
         jniThrowRuntimeException(env, "Can't find android/graphics/ImageReader");
         return;
     }
     sp<JNIImageReaderContext> ctx(new JNIImageReaderContext(env, weakThiz, clazz, maxImages));
-    ctx->setCpuConsumer(consumer);
+
+    sp<IGraphicBufferProducer> gbProducer;
+    sp<IGraphicBufferConsumer> gbConsumer;
+    BufferQueue::createBufferQueue(&gbProducer, &gbConsumer);
+    sp<ConsumerBase> consumer;
+    sp<CpuConsumer> cpuConsumer;
+    sp<BufferItemConsumer> opaqueConsumer;
+    if (isFormatOpaque(nativeFormat)) {
+        // Use the SW_READ_NEVER usage to tell producer that this format is not for preview or video
+        // encoding. The only possibility will be ZSL output.
+        opaqueConsumer =
+                new BufferItemConsumer(gbConsumer, GRALLOC_USAGE_SW_READ_NEVER, maxImages,
+                        /*controlledByApp*/true);
+        if (opaqueConsumer == NULL) {
+            jniThrowRuntimeException(env, "Failed to allocate native opaque consumer");
+            return;
+        }
+        ctx->setOpaqueConsumer(opaqueConsumer);
+        consumer = opaqueConsumer;
+    } else {
+        cpuConsumer = new CpuConsumer(gbConsumer, maxImages, /*controlledByApp*/true);
+        // TODO: throw dvm exOutOfMemoryError?
+        if (cpuConsumer == NULL) {
+            jniThrowRuntimeException(env, "Failed to allocate native CpuConsumer");
+            return;
+        }
+        ctx->setCpuConsumer(cpuConsumer);
+        consumer = cpuConsumer;
+    }
+
     ctx->setProducer(gbProducer);
     consumer->setFrameAvailableListener(ctx);
     ImageReader_setNativeContext(env, thiz, ctx);
@@ -718,23 +840,42 @@ static void ImageReader_init(JNIEnv* env, jobject thiz, jobject weakThiz,
     ctx->setBufferHeight(height);
 
     // Set the width/height/format/dataspace to the CpuConsumer
-    res = consumer->setDefaultBufferSize(width, height);
-    if (res != OK) {
-        jniThrowException(env, "java/lang/IllegalStateException",
-                          "Failed to set CpuConsumer buffer size");
-        return;
-    }
-    res = consumer->setDefaultBufferFormat(nativeFormat);
-    if (res != OK) {
-        jniThrowException(env, "java/lang/IllegalStateException",
-                          "Failed to set CpuConsumer buffer format");
-    }
-    res = consumer->setDefaultBufferDataSpace(nativeDataspace);
-    if (res != OK) {
-        jniThrowException(env, "java/lang/IllegalStateException",
-                          "Failed to set CpuConsumer buffer dataSpace");
+    // TODO: below code can be simplified once b/19977701 is fixed.
+    if (isFormatOpaque(nativeFormat)) {
+        res = opaqueConsumer->setDefaultBufferSize(width, height);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set opaque consumer buffer size");
+            return;
+        }
+        res = opaqueConsumer->setDefaultBufferFormat(nativeFormat);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set opaque consumer buffer format");
+        }
+        res = opaqueConsumer->setDefaultBufferDataSpace(nativeDataspace);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set opaque consumer buffer dataSpace");
+        }
+    } else {
+        res = cpuConsumer->setDefaultBufferSize(width, height);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set CpuConsumer buffer size");
+            return;
+        }
+        res = cpuConsumer->setDefaultBufferFormat(nativeFormat);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set CpuConsumer buffer format");
+        }
+        res = cpuConsumer->setDefaultBufferDataSpace(nativeDataspace);
+        if (res != OK) {
+            jniThrowException(env, "java/lang/IllegalStateException",
+                              "Failed to set CpuConsumer buffer dataSpace");
+        }
     }
-
 }
 
 static void ImageReader_close(JNIEnv* env, jobject thiz)
@@ -747,7 +888,13 @@ static void ImageReader_close(JNIEnv* env, jobject thiz)
         return;
     }
 
-    CpuConsumer* consumer = ImageReader_getCpuConsumer(env, thiz);
+    ConsumerBase* consumer = NULL;
+    if (ctx->isOpaque()) {
+        consumer = ImageReader_getOpaqueConsumer(env, thiz);
+    } else {
+        consumer = ImageReader_getCpuConsumer(env, thiz);
+    }
+
     if (consumer != NULL) {
         consumer->abandon();
         consumer->setFrameAvailableListener(NULL);
@@ -764,27 +911,66 @@ static void ImageReader_imageRelease(JNIEnv* env, jobject thiz, jobject image)
         return;
     }
 
-    CpuConsumer* consumer = ctx->getCpuConsumer();
-    CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, image);
-    if (!buffer) {
-        ALOGW("Image already released!!!");
-        return;
+    if (ctx->isOpaque()) {
+        BufferItemConsumer* opaqueConsumer = ctx->getOpaqueConsumer();
+        BufferItem* opaqueBuffer = Image_getOpaqueBuffer(env, image);
+        opaqueConsumer->releaseBuffer(*opaqueBuffer); // Not using fence for now.
+        Image_setOpaqueBuffer(env, image, NULL);
+        ctx->returnOpaqueBuffer(opaqueBuffer);
+        ALOGV("%s: Opaque Image has been released", __FUNCTION__);
+    } else {
+        CpuConsumer* consumer = ctx->getCpuConsumer();
+        CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, image);
+        if (!buffer) {
+            ALOGW("Image already released!!!");
+            return;
+        }
+        consumer->unlockBuffer(*buffer);
+        Image_setBuffer(env, image, NULL);
+        ctx->returnLockedBuffer(buffer);
+        ALOGV("%s: Image (format: 0x%x) has been released", __FUNCTION__, ctx->getBufferFormat());
     }
-    consumer->unlockBuffer(*buffer);
-    Image_setBuffer(env, image, NULL);
-    ctx->returnLockedBuffer(buffer);
 }
 
-static jint ImageReader_imageSetup(JNIEnv* env, jobject thiz,
-                                             jobject image)
-{
+static jint ImageReader_opaqueImageSetup(JNIEnv* env, JNIImageReaderContext* ctx, jobject image) {
     ALOGV("%s:", __FUNCTION__);
-    JNIImageReaderContext* ctx = ImageReader_getContext(env, thiz);
-    if (ctx == NULL) {
+    if (ctx == NULL || !ctx->isOpaque()) {
         jniThrowRuntimeException(env, "ImageReaderContext is not initialized");
         return -1;
     }
 
+    BufferItemConsumer* opaqueConsumer = ctx->getOpaqueConsumer();
+    BufferItem* buffer = ctx->getOpaqueBuffer();
+    if (buffer == NULL) {
+        ALOGW("Unable to acquire a buffer item, very likely client tried to acquire more than"
+            " maxImages buffers");
+        return ACQUIRE_MAX_IMAGES;
+    }
+
+    status_t res = opaqueConsumer->acquireBuffer(buffer, 0);
+    if (res != OK) {
+        ctx->returnOpaqueBuffer(buffer);
+        if (res == INVALID_OPERATION) {
+            // Max number of images were already acquired.
+            ALOGE("%s: Max number of buffers allowed are already acquired : %s (%d)",
+                    __FUNCTION__, strerror(-res), res);
+            return ACQUIRE_MAX_IMAGES;
+        } else {
+            ALOGE("%s: Acquire image failed with error: %s (%d)",
+                    __FUNCTION__, strerror(-res), res);
+            return ACQUIRE_NO_BUFFERS;
+        }
+    }
+
+    // Set SurfaceImage instance member variables
+    Image_setOpaqueBuffer(env, image, buffer);
+    env->SetLongField(image, gSurfaceImageClassInfo.mTimestamp,
+            static_cast<jlong>(buffer->mTimestamp));
+
+    return ACQUIRE_SUCCESS;
+}
+
+static jint ImageReader_lockedImageSetup(JNIEnv* env, JNIImageReaderContext* ctx, jobject image) {
     CpuConsumer* consumer = ctx->getCpuConsumer();
     CpuConsumer::LockedBuffer* buffer = ctx->getLockedBuffer();
     if (buffer == NULL) {
@@ -877,23 +1063,55 @@ static jint ImageReader_imageSetup(JNIEnv* env, jobject thiz,
     return ACQUIRE_SUCCESS;
 }
 
-static void ImageReader_detachImage(JNIEnv* env, jobject thiz, jobject image) {
+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");
+        return -1;
+    }
+
+    if (ctx->isOpaque()) {
+        return ImageReader_opaqueImageSetup(env, ctx, image);
+    } else {
+        return ImageReader_lockedImageSetup(env, ctx, image);
+    }
+}
+
+static jint ImageReader_detachImage(JNIEnv* env, jobject thiz, jobject image) {
     ALOGV("%s:", __FUNCTION__);
     JNIImageReaderContext* ctx = ImageReader_getContext(env, thiz);
     if (ctx == NULL) {
         jniThrowException(env, "java/lang/IllegalStateException", "ImageReader was already closed");
-        return;
+        return -1;
     }
 
-    // CpuConsumer* consumer = ctx->getCpuConsumer();
-    CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, image);
-    if (!buffer) {
-        ALOGW("Image already released!!!");
-        return;
+    status_t res = OK;
+    if (!ctx->isOpaque()) {
+        // TODO: Non-Opaque format detach is not implemented yet.
+        jniThrowRuntimeException(env,
+                "nativeDetachImage is not implemented yet for non-opaque format !!!");
+        return -1;
     }
 
-    // TODO: need implement
-    jniThrowRuntimeException(env, "nativeDetachImage is not implemented yet!!!");
+    BufferItemConsumer* opaqueConsumer = ctx->getOpaqueConsumer();
+    BufferItem* opaqueBuffer = Image_getOpaqueBuffer(env, image);
+    if (!opaqueBuffer) {
+        ALOGE(
+                "Opaque Image already released and can not be detached from ImageReader!!!");
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Opaque Image detach from ImageReader failed: buffer was already released");
+        return -1;
+    }
+
+    res = opaqueConsumer->detachBuffer(opaqueBuffer->mSlot);
+    if (res != OK) {
+        ALOGE("Opaque Image detach failed: %s (%d)!!!", strerror(-res), res);
+        jniThrowRuntimeException(env,
+                "nativeDetachImage failed for opaque image!!!");
+        return res;
+    }
+    return OK;
 }
 
 static jobject ImageReader_getSurface(JNIEnv* env, jobject thiz)
@@ -914,8 +1132,15 @@ static jobject Image_createSurfacePlane(JNIEnv* env, jobject thiz, int idx, int
 {
     int rowStride, pixelStride;
     PublicFormat publicReaderFormat = static_cast<PublicFormat>(readerFormat);
+    int halReaderFormat = android_view_Surface_mapPublicFormatToHalFormat(
+        publicReaderFormat);
 
     ALOGV("%s: buffer index: %d", __FUNCTION__, idx);
+    if (isFormatOpaque(halReaderFormat)) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Opaque images from Opaque ImageReader do not have any planes");
+        return NULL;
+    }
 
     CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
 
@@ -924,9 +1149,6 @@ static jobject Image_createSurfacePlane(JNIEnv* env, jobject thiz, int idx, int
         jniThrowException(env, "java/lang/IllegalStateException", "Image was released");
     }
 
-    int halReaderFormat = android_view_Surface_mapPublicFormatToHalFormat(
-        publicReaderFormat);
-
     rowStride = Image_imageGetRowStride(env, buffer, idx, halReaderFormat);
     pixelStride = Image_imageGetPixelStride(env, buffer, idx, halReaderFormat);
 
@@ -942,18 +1164,23 @@ static jobject Image_getByteBuffer(JNIEnv* env, jobject thiz, int idx, int reade
     uint32_t size = 0;
     jobject byteBuffer;
     PublicFormat readerPublicFormat = static_cast<PublicFormat>(readerFormat);
+    int readerHalFormat = android_view_Surface_mapPublicFormatToHalFormat(
+            readerPublicFormat);
 
     ALOGV("%s: buffer index: %d", __FUNCTION__, idx);
 
+    if (isFormatOpaque(readerHalFormat)) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Opaque images from Opaque ImageReader do not have any plane");
+        return NULL;
+    }
+
     CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
 
     if (buffer == NULL) {
         jniThrowException(env, "java/lang/IllegalStateException", "Image was released");
     }
 
-    int readerHalFormat = android_view_Surface_mapPublicFormatToHalFormat(
-            readerPublicFormat);
-
     // Create byteBuffer from native buffer
     Image_getLockedBufferInfo(env, buffer, idx, &base, &size, readerHalFormat);
 
@@ -973,19 +1200,28 @@ static jobject Image_getByteBuffer(JNIEnv* env, jobject thiz, int idx, int reade
     return byteBuffer;
 }
 
-static jint Image_getWidth(JNIEnv* env, jobject thiz)
+static jint Image_getWidth(JNIEnv* env, jobject thiz, jint format)
 {
-    CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
-    return Image_getBufferWidth(buffer);
+    if (isFormatOpaque(format)) {
+        BufferItem* opaqueBuffer = Image_getOpaqueBuffer(env, thiz);
+        return Image_getOpaqueBufferWidth(opaqueBuffer);
+    } else {
+        CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
+        return Image_getBufferWidth(buffer);
+    }
 }
 
-static jint Image_getHeight(JNIEnv* env, jobject thiz)
+static jint Image_getHeight(JNIEnv* env, jobject thiz, jint format)
 {
-    CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
-    return Image_getBufferHeight(buffer);
+    if (isFormatOpaque(format)) {
+        BufferItem* opaqueBuffer = Image_getOpaqueBuffer(env, thiz);
+        return Image_getOpaqueBufferHeight(opaqueBuffer);
+    } else {
+        CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, thiz);
+        return Image_getBufferHeight(buffer);
+    }
 }
 
-
 } // extern "C"
 
 // ----------------------------------------------------------------------------
@@ -997,15 +1233,15 @@ static JNINativeMethod gImageReaderMethods[] = {
     {"nativeReleaseImage",     "(Landroid/media/Image;)V",   (void*)ImageReader_imageRelease },
     {"nativeImageSetup",       "(Landroid/media/Image;)I",   (void*)ImageReader_imageSetup },
     {"nativeGetSurface",       "()Landroid/view/Surface;",   (void*)ImageReader_getSurface },
-    {"nativeDetachImage",      "(Landroid/media/Image;)V",   (void*)ImageReader_detachImage },
+    {"nativeDetachImage",      "(Landroid/media/Image;)I",   (void*)ImageReader_detachImage },
 };
 
 static JNINativeMethod gImageMethods[] = {
     {"nativeImageGetBuffer",   "(II)Ljava/nio/ByteBuffer;",   (void*)Image_getByteBuffer },
     {"nativeCreatePlane",      "(II)Landroid/media/ImageReader$SurfaceImage$SurfacePlane;",
                                                               (void*)Image_createSurfacePlane },
-    {"nativeGetWidth",         "()I",                         (void*)Image_getWidth },
-    {"nativeGetHeight",        "()I",                         (void*)Image_getHeight },
+    {"nativeGetWidth",         "(I)I",                         (void*)Image_getWidth },
+    {"nativeGetHeight",        "(I)I",                         (void*)Image_getHeight },
 };
 
 int register_android_media_ImageReader(JNIEnv *env) {
diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp
index d10df3e9..d2c614e 100644
--- a/media/jni/android_media_ImageWriter.cpp
+++ b/media/jni/android_media_ImageWriter.cpp
@@ -74,8 +74,8 @@ public:
     // has returned a buffer and it is ready for ImageWriter to dequeue.
     virtual void onBufferReleased();
 
-    void setProducer(const sp<ANativeWindow>& producer) { mProducer = producer; }
-    ANativeWindow* getProducer() { return mProducer.get(); }
+    void setProducer(const sp<Surface>& producer) { mProducer = producer; }
+    Surface* getProducer() { return mProducer.get(); }
 
     void setBufferFormat(int format) { mFormat = format; }
     int getBufferFormat() { return mFormat; }
@@ -90,7 +90,7 @@ private:
     static JNIEnv* getJNIEnv(bool* needsDetach);
     static void detachJNI();
 
-    sp<ANativeWindow> mProducer;
+    sp<Surface> mProducer;
     jobject mWeakThiz;
     jclass mClazz;
     int mFormat;
@@ -155,10 +155,21 @@ void JNIImageWriterContext::onBufferReleased() {
     bool needsDetach = false;
     JNIEnv* env = getJNIEnv(&needsDetach);
     if (env != NULL) {
+        // Detach the buffer every time when a buffer consumption is done,
+        // need let this callback give a BufferItem, then only detach if it was attached to this
+        // Writer. Do the detach unconditionally for opaque format now. see b/19977520
+        if (mFormat == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED) {
+            sp<Fence> fence;
+            ANativeWindowBuffer* buffer;
+            ALOGV("%s: One buffer is detached", __FUNCTION__);
+            mProducer->detachNextBuffer(&buffer, &fence);
+        }
+
         env->CallStaticVoidMethod(mClazz, gImageWriterClassInfo.postEventFromNative, mWeakThiz);
     } else {
         ALOGW("onBufferReleased event will not posted");
     }
+
     if (needsDetach) {
         detachJNI();
     }
@@ -170,13 +181,13 @@ extern "C" {
 
 // -------------------------------Private method declarations--------------
 
-static bool isWritable(int format);
 static bool isPossiblyYUV(PixelFormat format);
 static void Image_setNativeContext(JNIEnv* env, jobject thiz,
         sp<GraphicBuffer> buffer, int fenceFd);
 static void Image_getNativeContext(JNIEnv* env, jobject thiz,
         GraphicBuffer** buffer, int* fenceFd);
 static void Image_unlockIfLocked(JNIEnv* env, jobject thiz);
+static bool isFormatOpaque(int format);
 
 // --------------------------ImageWriter methods---------------------------------------
 
@@ -278,7 +289,7 @@ static jlong ImageWriter_init(JNIEnv* env, jobject thiz, jobject weakThiz, jobje
     env->SetIntField(thiz, gImageWriterClassInfo.mWriterFormat, reinterpret_cast<jint>(format));
 
 
-    if (isWritable(format)) {
+    if (!isFormatOpaque(format)) {
         res = native_window_set_usage(anw.get(), GRALLOC_USAGE_SW_WRITE_OFTEN);
         if (res != OK) {
             ALOGE("%s: Configure usage %08x for format %08x failed: %s (%d)",
@@ -461,31 +472,87 @@ static void ImageWriter_queueImage(JNIEnv* env, jobject thiz, jlong nativeCtx, j
         return;
     }
 
+    // Clear the image native context: end of this image's lifecycle in public API.
     Image_setNativeContext(env, image, NULL, -1);
 }
 
-static void ImageWriter_attachImage(JNIEnv* env, jobject thiz, jlong nativeCtx, jobject image) {
+static jint ImageWriter_attachAndQueueImage(JNIEnv* env, jobject thiz, jlong nativeCtx,
+        jlong nativeBuffer, jint imageFormat, jlong timestampNs, jint left, jint top,
+        jint right, jint bottom) {
     ALOGV("%s", __FUNCTION__);
     JNIImageWriterContext* const ctx = reinterpret_cast<JNIImageWriterContext *>(nativeCtx);
     if (ctx == NULL || thiz == NULL) {
         jniThrowException(env, "java/lang/IllegalStateException",
                 "ImageWriterContext is not initialized");
-        return;
+        return -1;
     }
 
-    sp<ANativeWindow> anw = ctx->getProducer();
+    sp<Surface> surface = ctx->getProducer();
+    status_t res = OK;
+    if (!isFormatOpaque(imageFormat)) {
+        // TODO: need implement, see b/19962027
+        jniThrowRuntimeException(env,
+                "nativeAttachImage for non-opaque image is not implement yet!!!");
+        return -1;
+    }
 
-    GraphicBuffer *buffer = NULL;
-    int fenceFd = -1;
-    Image_getNativeContext(env, image, &buffer, &fenceFd);
-    if (buffer == NULL) {
+    if (!isFormatOpaque(ctx->getBufferFormat())) {
         jniThrowException(env, "java/lang/IllegalStateException",
-                "Image is not initialized");
-        return;
+                "Trying to attach an opaque image into a non-opaque ImageWriter");
+        return -1;
+    }
+
+    // Image is guaranteed to be from ImageReader at this point, so it is safe to
+    // cast to BufferItem pointer.
+    BufferItem* opaqueBuffer = reinterpret_cast<BufferItem*>(nativeBuffer);
+    if (opaqueBuffer == NULL) {
+        jniThrowException(env, "java/lang/IllegalStateException",
+                "Image is not initialized or already closed");
+        return -1;
+    }
+
+    // Step 1. Attach Image
+    res = surface->attachBuffer(opaqueBuffer->mGraphicBuffer.get());
+    if (res != OK) {
+        // TODO: handle different error case separately.
+        ALOGE("Attach image failed: %s (%d)", strerror(-res), res);
+        jniThrowRuntimeException(env, "nativeAttachImage failed!!!");
+        return res;
     }
+    sp < ANativeWindow > anw = surface;
 
-    // TODO: need implement
-    jniThrowRuntimeException(env, "nativeAttachImage is not implement yet!!!");
+    // Step 2. Set timestamp and crop. Note that we do not need unlock the image because
+    // it was not locked.
+    ALOGV("timestamp to be queued: %" PRId64, timestampNs);
+    res = native_window_set_buffers_timestamp(anw.get(), timestampNs);
+    if (res != OK) {
+        jniThrowRuntimeException(env, "Set timestamp failed");
+        return res;
+    }
+
+    android_native_rect_t cropRect;
+    cropRect.left = left;
+    cropRect.top = top;
+    cropRect.right = right;
+    cropRect.bottom = bottom;
+    res = native_window_set_crop(anw.get(), &cropRect);
+    if (res != OK) {
+        jniThrowRuntimeException(env, "Set crop rect failed");
+        return res;
+    }
+
+    // Step 3. Queue Image.
+    res = anw->queueBuffer(anw.get(), opaqueBuffer->mGraphicBuffer.get(), /*fenceFd*/
+            -1);
+    if (res != OK) {
+        jniThrowRuntimeException(env, "Queue input buffer failed");
+        return res;
+    }
+
+    // Do not set the image native context. Since it would overwrite the existing native context
+    // of the image that is from ImageReader, the subsequent image close will run into issues.
+
+    return res;
 }
 
 // --------------------------Image methods---------------------------------------
@@ -534,10 +601,13 @@ static void Image_unlockIfLocked(JNIEnv* env, jobject thiz) {
 
     // Is locked?
     bool isLocked = false;
-    jobject planes = env->GetObjectField(thiz, gSurfaceImageClassInfo.mPlanes);
+    jobject planes = NULL;
+    if (!isFormatOpaque(buffer->getPixelFormat())) {
+        planes = env->GetObjectField(thiz, gSurfaceImageClassInfo.mPlanes);
+    }
     isLocked = (planes != NULL);
     if (isLocked) {
-        // no need to use fence here, as we it will be consumed by either concel or queue buffer.
+        // no need to use fence here, as we it will be consumed by either cancel or queue buffer.
         status_t res = buffer->unlock();
         if (res != OK) {
             jniThrowRuntimeException(env, "unlock buffer failed");
@@ -900,7 +970,7 @@ static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz,
     jobject byteBuffer;
 
     int format = Image_getFormat(env, thiz);
-    if (!isWritable(format) && numPlanes > 0) {
+    if (isFormatOpaque(format) && numPlanes > 0) {
         String8 msg;
         msg.appendFormat("Format 0x%x is opaque, thus not writable, the number of planes (%d)"
                 " must be 0", format, numPlanes);
@@ -915,6 +985,9 @@ static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz,
                 " probably out of memory");
         return NULL;
     }
+    if (isFormatOpaque(format)) {
+        return surfacePlanes;
+    }
 
     // Buildup buffer info: rowStride, pixelStride and byteBuffers.
     LockedImage lockedImg = LockedImage();
@@ -943,13 +1016,9 @@ static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz,
 
 // -------------------------------Private convenience methods--------------------
 
-// Check if buffer with this format is writable. Generally speaking, the opaque formats
-// like IMPLEMENTATION_DEFINED is not writable, as the actual buffer formats and layouts
-// are unknown to frameworks.
-static bool isWritable(int format) {
-    // Assume all other formats are writable.
-    return !(format == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED ||
-            format == HAL_PIXEL_FORMAT_RAW_OPAQUE);
+static bool isFormatOpaque(int format) {
+    // Only treat IMPLEMENTATION_DEFINED as an opaque format for now.
+    return format == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED;
 }
 
 static bool isPossiblyYUV(PixelFormat format) {
@@ -986,8 +1055,8 @@ static JNINativeMethod gImageWriterMethods[] = {
     {"nativeClassInit",         "()V",                        (void*)ImageWriter_classInit },
     {"nativeInit",              "(Ljava/lang/Object;Landroid/view/Surface;I)J",
                                                               (void*)ImageWriter_init },
-    {"nativeClose",              "(J)V",                       (void*)ImageWriter_close },
-    {"nativeAttachImage",       "(JLandroid/media/Image;)V",  (void*)ImageWriter_attachImage },
+    {"nativeClose",              "(J)V",                      (void*)ImageWriter_close },
+    {"nativeAttachAndQueueImage", "(JJIJIIII)I",          (void*)ImageWriter_attachAndQueueImage },
     {"nativeDequeueInputImage", "(JLandroid/media/Image;)V",  (void*)ImageWriter_dequeueImage },
     {"nativeQueueInputImage",   "(JLandroid/media/Image;JIIII)V",  (void*)ImageWriter_queueImage },
     {"cancelImage",             "(JLandroid/media/Image;)V",   (void*)ImageWriter_cancelImage },