Fix ImageReader#newInstace with usage

Need to actually respect the given usage or the API doesn't
work. Shift the default to be done Java-side where it's clear
if usage was user-provided or not. Remove the incorrect
validations and assertions around usages.

Add missing HardwareBuffer#isSupported to let users
check if a usage & format combination is supported.

Change-Id: I70bfe6e6c41e78425a38af6b17d421f0e80e818b
Fixes: 123423319
Test: HardwareBuffer CTS tests

5 files changed, 71 insertions, 47 deletions

diff --git a/api/current.txt b/api/current.txt
index 0ea7ecc8d6b1..0d81f7b956aa 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -16145,6 +16145,7 @@ package android.hardware {
     method public long getUsage();
     method public int getWidth();
     method public boolean isClosed();
+    method public static boolean isSupported(int, int, int, int, long);
     method public void writeToParcel(android.os.Parcel, int);
     field public static final int BLOB = 33; // 0x21
     field public static final android.os.Parcelable.Creator<android.hardware.HardwareBuffer> CREATOR;
diff --git a/core/java/android/hardware/HardwareBuffer.java b/core/java/android/hardware/HardwareBuffer.java
index d2c0e7be5900..5d4928cd4ab3 100644
--- a/core/java/android/hardware/HardwareBuffer.java
+++ b/core/java/android/hardware/HardwareBuffer.java
@@ -182,6 +182,38 @@ public final class HardwareBuffer implements Parcelable, AutoCloseable {
     }
 
     /**
+     * Queries whether the given buffer description is supported by the system. If this returns
+     * true, then the allocation may succeed until resource exhaustion occurs. If this returns
+     * false then this combination will never succeed.
+     *
+     * @param width The width in pixels of the buffer
+     * @param height The height in pixels of the buffer
+     * @param format The @Format of each pixel
+     * @param layers The number of layers in the buffer
+     * @param usage The @Usage flags describing how the buffer will be used
+     * @return True if the combination is supported, false otherwise.
+     */
+    public static boolean isSupported(int width, int height, @Format int format, int layers,
+            @Usage long usage) {
+        if (!HardwareBuffer.isSupportedFormat(format)) {
+            throw new IllegalArgumentException("Invalid pixel format " + format);
+        }
+        if (width <= 0) {
+            throw new IllegalArgumentException("Invalid width " + width);
+        }
+        if (height <= 0) {
+            throw new IllegalArgumentException("Invalid height " + height);
+        }
+        if (layers <= 0) {
+            throw new IllegalArgumentException("Invalid layer count " + layers);
+        }
+        if (format == BLOB && height != 1) {
+            throw new IllegalArgumentException("Height must be 1 when using the BLOB format");
+        }
+        return nIsSupported(width, height, format, layers, usage);
+    }
+
+    /**
      * Private use only. See {@link #create(int, int, int, int, long)}. May also be
      * called from JNI using an already allocated native <code>HardwareBuffer</code>.
      */
@@ -386,4 +418,6 @@ public final class HardwareBuffer implements Parcelable, AutoCloseable {
     private static native int nGetLayers(long nativeObject);
     @FastNative
     private static native long nGetUsage(long nativeObject);
+    private static native boolean nIsSupported(int width, int height, int format, int layers,
+            long usage);
 }
diff --git a/core/jni/android_hardware_HardwareBuffer.cpp b/core/jni/android_hardware_HardwareBuffer.cpp
index 8174a4184c83..2d1fec819e97 100644
--- a/core/jni/android_hardware_HardwareBuffer.cpp
+++ b/core/jni/android_hardware_HardwareBuffer.cpp
@@ -104,6 +104,21 @@ static jlong android_hardware_HardwareBuffer_getNativeFinalizer(JNIEnv* env, job
     return static_cast<jlong>(reinterpret_cast<uintptr_t>(&destroyWrapper));
 }
 
+static jboolean android_hardware_HardwareBuffer_isSupported(JNIEnv* env, jobject clazz,
+        jint width, jint height, jint format, jint layers, jlong usage) {
+
+    AHardwareBuffer_Desc desc;
+    desc.width = width;
+    desc.height = height;
+    desc.format = format;
+    desc.layers = layers;
+    desc.usage = usage;
+    desc.stride = 0;
+    desc.rfu0 = 0;
+    desc.rfu1 = 0;
+    return AHardwareBuffer_isSupported(&desc);
+}
+
 //----------------------------------------------------------------------------
 // Accessors
 // ----------------------------------------------------------------------------
@@ -234,6 +249,8 @@ static const JNINativeMethod gMethods[] = {
             (void*) android_hardware_HardwareBuffer_write },
     { "nReadHardwareBufferFromParcel", "(Landroid/os/Parcel;)J",
             (void*) android_hardware_HardwareBuffer_read },
+    { "nIsSupported",  "(IIIIJ)Z",
+            (void*) android_hardware_HardwareBuffer_isSupported },
 
     // --------------- @FastNative ----------------------
     { "nGetWidth", "(J)I",      (void*) android_hardware_HardwareBuffer_getWidth },
diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java
index 9ac147b59ee5..a4818a367697 100644
--- a/media/java/android/media/ImageReader.java
+++ b/media/java/android/media/ImageReader.java
@@ -17,12 +17,10 @@
 package android.media;
 
 import android.graphics.ImageFormat;
-import android.graphics.PixelFormat;
 import android.hardware.HardwareBuffer;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
-import android.util.Log;
 import android.view.Surface;
 
 import dalvik.system.VMRuntime;
@@ -74,12 +72,6 @@ public class ImageReader implements AutoCloseable {
     private static final int ACQUIRE_MAX_IMAGES = 2;
 
     /**
-     * Invalid consumer buffer usage flag. This usage flag will be ignored
-     * by the {@code ImageReader} instance is constructed with this value.
-     */
-    private static final long BUFFER_USAGE_UNKNOWN = 0;
-
-    /**
      * <p>
      * Create a new reader for images of the desired size and format.
      * </p>
@@ -129,7 +121,10 @@ public class ImageReader implements AutoCloseable {
      * @see Image
      */
     public static ImageReader newInstance(int width, int height, int format, int maxImages) {
-        return new ImageReader(width, height, format, maxImages, BUFFER_USAGE_UNKNOWN);
+        // If the format is private don't default to USAGE_CPU_READ_OFTEN since it may not
+        // work, and is inscrutable anyway
+        return new ImageReader(width, height, format, maxImages,
+                format == ImageFormat.PRIVATE ? 0 : HardwareBuffer.USAGE_CPU_READ_OFTEN);
     }
 
     /**
@@ -207,18 +202,23 @@ public class ImageReader implements AutoCloseable {
      *            obtained by the user, one of them has to be released before a new Image will
      *            become available for access through {@link #acquireLatestImage()} or
      *            {@link #acquireNextImage()}. Must be greater than 0.
-     * @param usage The intended usage of the images produced by this ImageReader. It needs
-     *            to be one of the Usage defined by {@link HardwareBuffer}, or an
-     *            {@link IllegalArgumentException} will be thrown.
+     * @param usage The intended usage of the images produced by this ImageReader. See the usages
+     *              on {@link HardwareBuffer} for a list of valid usage bits. See also
+     *              {@link HardwareBuffer#isSupported(int, int, int, int, long)} for checking
+     *              if a combination is supported. If it's not supported this will throw
+     *              an {@link IllegalArgumentException}.
      * @see Image
      * @see HardwareBuffer
      */
     public static ImageReader newInstance(int width, int height, int format, int maxImages,
             long usage) {
-        if (!isFormatUsageCombinationAllowed(format, usage)) {
-            throw new IllegalArgumentException("Format usage combination is not supported:"
-                    + " format = " + format + ", usage = " + usage);
-        }
+        // TODO: Check this - can't do it just yet because format support is different
+        // Unify formats! The only reliable way to validate usage is to just try it and see.
+
+//        if (!HardwareBuffer.isSupported(width, height, format, 1, usage)) {
+//            throw new IllegalArgumentException("The given format=" + Integer.toHexString(format)
+//                + " & usage=" + Long.toHexString(usage) + " is not supported");
+//        }
         return new ImageReader(width, height, format, maxImages, usage);
     }
 
@@ -716,19 +716,6 @@ public class ImageReader implements AutoCloseable {
         return si.getReader() == this;
     }
 
-    private static boolean isFormatUsageCombinationAllowed(int format, long usage) {
-        if (!ImageFormat.isPublicFormat(format) && !PixelFormat.isPublicFormat(format)) {
-            return false;
-        }
-
-        // Valid usage needs to be provided.
-        if (usage == BUFFER_USAGE_UNKNOWN) {
-            return false;
-        }
-
-        return true;
-    }
-
     /**
      * Called from Native code when an Event happens.
      *
diff --git a/media/jni/android_media_ImageReader.cpp b/media/jni/android_media_ImageReader.cpp
index a45aa90f5f19..88671ecd27fc 100644
--- a/media/jni/android_media_ImageReader.cpp
+++ b/media/jni/android_media_ImageReader.cpp
@@ -379,24 +379,9 @@ static void ImageReader_init(JNIEnv* env, jobject thiz, jobject weakThiz, jint w
     String8 consumerName = String8::format("ImageReader-%dx%df%xm%d-%d-%d",
             width, height, format, maxImages, getpid(),
             createProcessUniqueId());
-    uint32_t consumerUsage = GRALLOC_USAGE_SW_READ_OFTEN;
-    bool needUsageOverride = ndkUsage != CONSUMER_BUFFER_USAGE_UNKNOWN;
-    uint64_t outProducerUsage = 0;
-    uint64_t outConsumerUsage = 0;
-    android_hardware_HardwareBuffer_convertToGrallocUsageBits(&outProducerUsage, &outConsumerUsage,
-            ndkUsage, 0);
-
-    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.
-        consumerUsage = GRALLOC_USAGE_SW_READ_NEVER;
-        if (needUsageOverride) {
-            consumerUsage = android_convertGralloc1To0Usage(0, outConsumerUsage);
-        }
-    } else if (needUsageOverride) {
-        ALOGW("Consumer usage override for non-opaque format is not implemented yet, "
-                "ignore the provided usage from the application");
-    }
+    uint64_t consumerUsage =
+            android_hardware_HardwareBuffer_convertToGrallocUsageBits(ndkUsage);
+
     bufferConsumer = new BufferItemConsumer(gbConsumer, consumerUsage, maxImages,
             /*controlledByApp*/true);
     if (bufferConsumer == nullptr) {