Merge "camera2: Fix native ImageReader test segfaults." into lmp-dev

rubenbrunk · Android (Google) Code Review · commit 7b28c66a6cc1 · 2014-09-27T02:16:30.000Z
diff --git a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
@@ -207,11 +207,18 @@ public void onPictureTaken(byte[] data, Camera camera) {
                         Log.i(TAG, "Producing jpeg buffer...");
 
                         int totalSize = data.length + LegacyCameraDevice.nativeGetJpegFooterSize();
-                        totalSize += ((totalSize - 1) & ~0x3) + 4; // align to next octonibble
-
+                        totalSize = (totalSize + 3) & ~0x3; // round up to nearest octonibble
+
+                        if (USE_BLOB_FORMAT_OVERRIDE) {
+                            // Override to RGBA_8888 format.
+                            LegacyCameraDevice.setSurfaceFormat(s,
+                                    LegacyMetadataMapper.HAL_PIXEL_FORMAT_RGBA_8888);
+                            // divide by 4 if using RGBA format (width is in pixels, not bytes).
+                            totalSize >>= 2;
+                        }
                         LegacyCameraDevice.setSurfaceDimens(s, totalSize, /*height*/1);
                         LegacyCameraDevice.setNextTimestamp(s, timestamp);
-                        LegacyCameraDevice.produceFrame(s, data, data.length, /*height*/1,
+                        LegacyCameraDevice.produceFrame(s, data, totalSize, /*height*/1,
                                 CameraMetadataNative.NATIVE_JPEG_FORMAT);
                     }
                 } catch (LegacyExceptionUtils.BufferQueueAbandonedException e) {
diff --git a/core/jni/android_hardware_camera2_legacy_LegacyCameraDevice.cpp b/core/jni/android_hardware_camera2_legacy_LegacyCameraDevice.cpp
@@ -52,15 +52,15 @@ using namespace android;
  * Convert from RGB 888 to Y'CbCr using the conversion specified in ITU-R BT.601 for
  * digital RGB with K_b = 0.114, and K_r = 0.299.
  */
-static void rgbToYuv420(uint8_t* rgbBuf, int32_t width, int32_t height, uint8_t* yPlane,
+static void rgbToYuv420(uint8_t* rgbBuf, size_t width, size_t height, uint8_t* yPlane,
         uint8_t* uPlane, uint8_t* vPlane, size_t chromaStep, size_t yStride, size_t chromaStride) {
     uint8_t R, G, B;
     size_t index = 0;
 
-    int32_t cStrideDiff = chromaStride - width;
+    size_t cStrideDiff = chromaStride - width;
 
-    for (int32_t j = 0; j < height; j++) {
-        for (int32_t i = 0; i < width; i++) {
+    for (size_t j = 0; j < height; j++) {
+        for (size_t i = 0; i < width; i++) {
             R = rgbBuf[index++];
             G = rgbBuf[index++];
             B = rgbBuf[index++];
@@ -83,7 +83,7 @@ static void rgbToYuv420(uint8_t* rgbBuf, int32_t width, int32_t height, uint8_t*
     }
 }
 
-static void rgbToYuv420(uint8_t* rgbBuf, int32_t width, int32_t height, android_ycbcr* ycbcr) {
+static void rgbToYuv420(uint8_t* rgbBuf, size_t width, size_t height, android_ycbcr* ycbcr) {
     size_t cStep = ycbcr->chroma_step;
     size_t cStride = ycbcr->cstride;
     size_t yStride = ycbcr->ystride;
@@ -157,51 +157,83 @@ static status_t configureSurface(const sp<ANativeWindow>& anw,
  */
 static status_t produceFrame(const sp<ANativeWindow>& anw,
                              uint8_t* pixelBuffer,
-                             int32_t width, // Width of the pixelBuffer
-                             int32_t height, // Height of the pixelBuffer
+                             int32_t bufWidth, // Width of the pixelBuffer
+                             int32_t bufHeight, // Height of the pixelBuffer
                              int32_t pixelFmt, // Format of the pixelBuffer
                              int32_t bufSize) {
     ATRACE_CALL();
     status_t err = NO_ERROR;
     ANativeWindowBuffer* anb;
     ALOGV("%s: Dequeue buffer from %p %dx%d (fmt=%x, size=%x)",
-            __FUNCTION__, anw.get(), width, height, pixelFmt, bufSize);
+            __FUNCTION__, anw.get(), bufWidth, bufHeight, pixelFmt, bufSize);
 
     if (anw == 0) {
         ALOGE("%s: anw must not be NULL", __FUNCTION__);
         return BAD_VALUE;
     } else if (pixelBuffer == NULL) {
         ALOGE("%s: pixelBuffer must not be NULL", __FUNCTION__);
         return BAD_VALUE;
-    } else if (width < 0) {
+    } else if (bufWidth < 0) {
         ALOGE("%s: width must be non-negative", __FUNCTION__);
         return BAD_VALUE;
-    } else if (height < 0) {
+    } else if (bufHeight < 0) {
         ALOGE("%s: height must be non-negative", __FUNCTION__);
         return BAD_VALUE;
     } else if (bufSize < 0) {
         ALOGE("%s: bufSize must be non-negative", __FUNCTION__);
         return BAD_VALUE;
     }
 
-    if (width < 0 || height < 0 || bufSize < 0) {
-        ALOGE("%s: Illegal argument, negative dimension passed to produceFrame", __FUNCTION__);
-        return BAD_VALUE;
-    }
+    size_t width = static_cast<size_t>(bufWidth);
+    size_t height = static_cast<size_t>(bufHeight);
+    size_t bufferLength = static_cast<size_t>(bufSize);
 
     // TODO: Switch to using Surface::lock and Surface::unlockAndPost
     err = native_window_dequeue_buffer_and_wait(anw.get(), &anb);
     if (err != NO_ERROR) return err;
 
-    // TODO: check anb is large enough to store the results
-
     sp<GraphicBuffer> buf(new GraphicBuffer(anb, /*keepOwnership*/false));
+    uint32_t gBufWidth = buf->getWidth();
+    uint32_t gBufHeight = buf->getHeight();
+    if (gBufWidth != width || gBufHeight != height) {
+        ALOGE("%s: Received gralloc buffer with bad dimensions %" PRIu32 "x%" PRIu32
+                ", expecting dimensions %zu x %zu",  __FUNCTION__, gBufWidth, gBufHeight,
+                width, height);
+        return BAD_VALUE;
+    }
+
+    int32_t bufFmt = 0;
+    err = anw->query(anw.get(), NATIVE_WINDOW_FORMAT, &bufFmt);
+    if (err != NO_ERROR) {
+        ALOGE("%s: Error while querying surface pixel format %s (%d).", __FUNCTION__,
+                strerror(-err), err);
+        return err;
+    }
+
+    uint64_t tmpSize = width * height;
+    if (bufFmt != pixelFmt) {
+        if (bufFmt == HAL_PIXEL_FORMAT_RGBA_8888 && pixelFmt == HAL_PIXEL_FORMAT_BLOB) {
+            ALOGV("%s: Using BLOB to RGBA format override.", __FUNCTION__);
+            tmpSize *= 4;
+        } else {
+            ALOGW("%s: Format mismatch in produceFrame: expecting format %#" PRIx32
+                    ", but received buffer with format %#" PRIx32, __FUNCTION__, pixelFmt, bufFmt);
+        }
+    }
+
+    if (tmpSize > SIZE_MAX) {
+        ALOGE("%s: Overflow calculating size, buffer with dimens %zu x %zu is absurdly large...",
+                __FUNCTION__, width, height);
+        return BAD_VALUE;
+    }
+
+    size_t totalSizeBytes = tmpSize;
 
     switch(pixelFmt) {
         case HAL_PIXEL_FORMAT_YCrCb_420_SP: {
-            if (bufSize < width * height * 4) {
-                ALOGE("%s: PixelBuffer size %" PRId32 " too small for given dimensions",
-                        __FUNCTION__, bufSize);
+            if (bufferLength < totalSizeBytes) {
+                ALOGE("%s: PixelBuffer size %zu too small for given dimensions",
+                        __FUNCTION__, bufferLength);
                 return BAD_VALUE;
             }
             uint8_t* img = NULL;
@@ -221,14 +253,14 @@ static status_t produceFrame(const sp<ANativeWindow>& anw,
             break;
         }
         case HAL_PIXEL_FORMAT_YV12: {
-            if (bufSize < width * height * 4) {
-                ALOGE("%s: PixelBuffer size %" PRId32 " too small for given dimensions",
-                        __FUNCTION__, bufSize);
+            if (bufferLength < totalSizeBytes) {
+                ALOGE("%s: PixelBuffer size %zu too small for given dimensions",
+                        __FUNCTION__, bufferLength);
                 return BAD_VALUE;
             }
 
             if ((width & 1) || (height & 1)) {
-                ALOGE("%s: Dimens %dx%d are not divisible by 2.", __FUNCTION__, width, height);
+                ALOGE("%s: Dimens %zu x %zu are not divisible by 2.", __FUNCTION__, width, height);
                 return BAD_VALUE;
             }
 
@@ -258,9 +290,9 @@ static status_t produceFrame(const sp<ANativeWindow>& anw,
         case HAL_PIXEL_FORMAT_YCbCr_420_888: {
             // Software writes with YCbCr_420_888 format are unsupported
             // by the gralloc module for now
-            if (bufSize < width * height * 4) {
-                ALOGE("%s: PixelBuffer size %" PRId32 " too small for given dimensions",
-                        __FUNCTION__, bufSize);
+            if (bufferLength < totalSizeBytes) {
+                ALOGE("%s: PixelBuffer size %zu too small for given dimensions",
+                        __FUNCTION__, bufferLength);
                 return BAD_VALUE;
             }
             android_ycbcr ycbcr = android_ycbcr();
@@ -276,34 +308,35 @@ static status_t produceFrame(const sp<ANativeWindow>& anw,
             break;
         }
         case HAL_PIXEL_FORMAT_BLOB: {
-            if (bufSize != width || height != 1) {
-                ALOGE("%s: Incorrect pixelBuffer size: %" PRId32, __FUNCTION__, bufSize);
-                return BAD_VALUE;
-            }
             int8_t* img = NULL;
             struct camera3_jpeg_blob footer = {
                 jpeg_blob_id: CAMERA3_JPEG_BLOB_ID,
                 jpeg_size: (uint32_t)width
             };
 
-            size_t totalSize = static_cast<size_t>(width) + sizeof(footer);
-            size_t padding = ((totalSize - 1) & ~0x3) + 4; // align to next octonibble
-            totalSize += padding;
-            if (anb->width != totalSize) {
-                ALOGE("%s: gralloc buffer wrong size to hold jpeg, failed to produce buffer.");
+            size_t totalJpegSize = bufferLength + sizeof(footer);
+            totalJpegSize = (totalJpegSize + 3) & ~0x3; // round up to nearest octonibble
+
+            if (height != 1) {
+                ALOGE("%s: Invalid height set for JPEG buffer output, %zu", __FUNCTION__, height);
+                return BAD_VALUE;
+            }
+
+            if (totalJpegSize > totalSizeBytes) {
+                ALOGE("%s: Pixel buffer needs size %zu, cannot fit in gralloc buffer of size %zu",
+                        __FUNCTION__, totalJpegSize, totalSizeBytes);
                 return BAD_VALUE;
             }
 
-            ALOGV("%s: Lock buffer from %p for write", __FUNCTION__, anw.get());
             err = buf->lock(GRALLOC_USAGE_SW_WRITE_OFTEN, (void**)(&img));
             if (err != NO_ERROR) {
                 ALOGE("%s: Failed to lock buffer, error %s (%d).", __FUNCTION__, strerror(-err),
                         err);
                 return err;
             }
-            memcpy(img, pixelBuffer, width);
-            memset(img + width, 0, padding);
-            memcpy(img + totalSize - sizeof(footer), &footer, sizeof(footer));
+
+            memcpy(img, pixelBuffer, bufferLength);
+            memcpy(img + totalSizeBytes - sizeof(footer), &footer, sizeof(footer));
             break;
         }
         default: {
diff --git a/media/jni/android_media_ImageReader.cpp b/media/jni/android_media_ImageReader.cpp
@@ -286,13 +286,17 @@ static int Image_getPixelFormat(JNIEnv* env, int format)
     return format;
 }
 
-static uint32_t Image_getJpegSize(CpuConsumer::LockedBuffer* buffer)
+static uint32_t Image_getJpegSize(CpuConsumer::LockedBuffer* buffer, bool usingRGBAOverride)
 {
     ALOG_ASSERT(buffer != NULL, "Input buffer is NULL!!!");
     uint32_t size = 0;
     uint32_t width = buffer->width;
     uint8_t* jpegBuffer = buffer->data;
 
+    if (usingRGBAOverride) {
+        width *= 4;
+    }
+
     // First check for JPEG transport header at the end of the buffer
     uint8_t* header = jpegBuffer + (width - sizeof(struct camera3_jpeg_blob));
     struct camera3_jpeg_blob *blob = (struct camera3_jpeg_blob*)(header);
@@ -317,11 +321,15 @@ static uint32_t Image_getJpegSize(CpuConsumer::LockedBuffer* buffer)
     return size;
 }
 
+static bool usingRGBAToJpegOverride(int32_t bufferFormat, int32_t readerCtxFormat) {
+    return readerCtxFormat == HAL_PIXEL_FORMAT_BLOB && bufferFormat == HAL_PIXEL_FORMAT_RGBA_8888;
+}
+
 static int32_t applyFormatOverrides(int32_t bufferFormat, int32_t readerCtxFormat)
 {
     // Using HAL_PIXEL_FORMAT_RGBA_8888 gralloc buffers containing JPEGs to get around SW
     // write limitations for some platforms (b/17379185).
-    if (readerCtxFormat == HAL_PIXEL_FORMAT_BLOB && bufferFormat == HAL_PIXEL_FORMAT_RGBA_8888) {
+    if (usingRGBAToJpegOverride(bufferFormat, readerCtxFormat)) {
         return HAL_PIXEL_FORMAT_BLOB;
     }
     return bufferFormat;
@@ -345,6 +353,7 @@ static void Image_getLockedBufferInfo(JNIEnv* env, CpuConsumer::LockedBuffer* bu
     dataSize = ySize = cSize = cStride = 0;
     int32_t fmt = buffer->format;
 
+    bool usingRGBAOverride = usingRGBAToJpegOverride(fmt, readerFormat);
     fmt = applyFormatOverrides(fmt, readerFormat);
     switch (fmt) {
         case HAL_PIXEL_FORMAT_YCbCr_420_888:
@@ -416,7 +425,7 @@ static void Image_getLockedBufferInfo(JNIEnv* env, CpuConsumer::LockedBuffer* bu
             ALOG_ASSERT(buffer->height == 1, "JPEG should has height value %d", buffer->height);
 
             pData = buffer->data;
-            dataSize = Image_getJpegSize(buffer);
+            dataSize = Image_getJpegSize(buffer, usingRGBAOverride);
             break;
         case HAL_PIXEL_FORMAT_RAW_SENSOR:
             // Single plane 16bpp bayer data.
@@ -912,7 +921,7 @@ static jobject Image_getByteBuffer(JNIEnv* env, jobject thiz, int idx, int reade
     if (size > static_cast<uint32_t>(INT32_MAX)) {
         // Byte buffer have 'int capacity', so check the range
         jniThrowExceptionFmt(env, "java/lang/IllegalStateException",
-                "Size too large for bytebuffer capacity " PRIu32, size);
+                "Size too large for bytebuffer capacity %" PRIu32, size);
         return NULL;
     }
 
