From 07ad459c84b565ea216854a64e726a16c5824640 Mon Sep 17 00:00:00 2001
From: Eino-Ville Talvala <etalvala@google.com>
Date: Tue, 2 May 2017 12:44:05 -0700
Subject: [PATCH] ImageWriter: Attach non-opaque Images as well.

With all the work under the hood of ImageReader/Writer in the last few
releases, this is much more straightforward to enable.

Only trick is to ensure that the Image detached from the ImageReader
correctly clears out its planes, if present.

Bug: 19962027
Test: Camera CTS still passes on angler, sailfish, ryu
Change-Id: Ic8a1f3e2f45cf15971d76e308a5af25eb2fd66f1
---
 media/java/android/media/ImageReader.java |  2 ++
 media/java/android/media/ImageWriter.java | 30 ++++++-----------------
 media/jni/android_media_ImageWriter.cpp   | 20 +++++----------
 3 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java
index ccdf5ae7fbf58..c78c99f7a228f 100644
--- a/media/java/android/media/ImageReader.java
+++ b/media/java/android/media/ImageReader.java
@@ -705,6 +705,8 @@ public class ImageReader implements AutoCloseable {
         }
 
         nativeDetachImage(image);
+        si.clearSurfacePlanes();
+        si.mPlanes = null;
         si.setDetached(true);
     }
 
diff --git a/media/java/android/media/ImageWriter.java b/media/java/android/media/ImageWriter.java
index 349c9cb948602..2b7309f189775 100644
--- a/media/java/android/media/ImageWriter.java
+++ b/media/java/android/media/ImageWriter.java
@@ -359,28 +359,14 @@ public class ImageWriter implements AutoCloseable {
             }
 
             ImageReader prevOwner = (ImageReader) image.getOwner();
-            // Only do the image attach for PRIVATE format images for now. Do the image
-            // copy for other formats. TODO: use attach for other formats to
-            // improve the performance, and fall back to copy when attach/detach
-            // fails. Right now, detach is guaranteed to fail as the buffer is
-            // locked when ImageReader#acquireNextImage is called. See bug 19962027.
-            if (image.getFormat() == ImageFormat.PRIVATE) {
-                prevOwner.detachImage(image);
-                attachAndQueueInputImage(image);
-                // This clears the native reference held by the original owner.
-                // When this Image is detached later by this ImageWriter, the
-                // native memory won't be leaked.
-                image.close();
-                return;
-            } else {
-                Image inputImage = dequeueInputImage();
-                inputImage.setTimestamp(image.getTimestamp());
-                inputImage.setCropRect(image.getCropRect());
-                ImageUtils.imageCopy(image, inputImage);
-                image.close();
-                image = inputImage;
-                ownedByMe = true;
-            }
+
+            prevOwner.detachImage(image);
+            attachAndQueueInputImage(image);
+            // This clears the native reference held by the original owner.
+            // When this Image is detached later by this ImageWriter, the
+            // native memory won't be leaked.
+            image.close();
+            return;
         }
 
         Rect crop = image.getCropRect();
diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp
index ed5fbcfcd5b30..b5ea632544b94 100644
--- a/media/jni/android_media_ImageWriter.cpp
+++ b/media/jni/android_media_ImageWriter.cpp
@@ -499,30 +499,23 @@ static jint ImageWriter_attachAndQueueImage(JNIEnv* env, jobject thiz, jlong nat
 
     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;
-    }
-
-    if (!isFormatOpaque(ctx->getBufferFormat())) {
+    if (isFormatOpaque(imageFormat) != isFormatOpaque(ctx->getBufferFormat())) {
         jniThrowException(env, "java/lang/IllegalStateException",
-                "Trying to attach an opaque image into a non-opaque ImageWriter");
+                "Trying to attach an opaque image into a non-opaque ImageWriter, or vice versa");
         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) {
+    BufferItem* buffer = reinterpret_cast<BufferItem*>(nativeBuffer);
+    if (buffer == 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());
+    res = surface->attachBuffer(buffer->mGraphicBuffer.get());
     if (res != OK) {
         ALOGE("Attach image failed: %s (%d)", strerror(-res), res);
         switch (res) {
@@ -559,7 +552,7 @@ static jint ImageWriter_attachAndQueueImage(JNIEnv* env, jobject thiz, jlong nat
     }
 
     // Step 3. Queue Image.
-    res = anw->queueBuffer(anw.get(), opaqueBuffer->mGraphicBuffer.get(), /*fenceFd*/
+    res = anw->queueBuffer(anw.get(), buffer->mGraphicBuffer.get(), /*fenceFd*/
             -1);
     if (res != OK) {
         ALOGE("%s: Queue buffer failed: %s (%d)", __FUNCTION__, strerror(-res), res);
@@ -817,4 +810,3 @@ int register_android_media_ImageWriter(JNIEnv *env) {
 
     return (ret1 || ret2);
 }
-