From 49afba7d80a545131e3b6c1e989e56a4a2e581bd Mon Sep 17 00:00:00 2001 From: Sally Qi Date: Tue, 30 Aug 2022 22:46:57 -0700 Subject: [PATCH] Fix throw exception when using HardwareBuffer.BLOB format to create ImageReader/ImageWriter object. - Internally, we should deal with HardwareBuffer.BLOB format according to surface's dataSpace. Directly passing BLOB format to getEstimatedNativeAllocBytes function will throw exception because BLOB format is not handled inside. Instead, use their corresponding imageFormat to avoid such failure in this case. Bug: 244268240 Test: atest android.hardware.camera2.cts.ImageReaderTest, atest android.hardware.camera2.cts.ImageWriterTest, atest android.hardware.cts.DataSpaceTest Change-Id: I3f18b9d3550f88be82cde75372637a3ae528b5ec --- core/api/current.txt | 1 + core/java/android/hardware/DataSpace.java | 16 +++++ media/java/android/media/ImageReader.java | 13 ++-- media/java/android/media/ImageWriter.java | 72 ++++++++++++----------- 4 files changed, 65 insertions(+), 37 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index 96517004b9f2b..b3dfd61aa9713 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -16892,6 +16892,7 @@ package android.hardware { field public static final int DATASPACE_DEPTH = 4096; // 0x1000 field public static final int DATASPACE_DISPLAY_P3 = 143261696; // 0x88a0000 field public static final int DATASPACE_DYNAMIC_DEPTH = 4098; // 0x1002 + field public static final int DATASPACE_HEIF = 4100; // 0x1004 field public static final int DATASPACE_JFIF = 146931712; // 0x8c20000 field public static final int DATASPACE_SCRGB = 411107328; // 0x18810000 field public static final int DATASPACE_SCRGB_LINEAR = 406913024; // 0x18410000 diff --git a/core/java/android/hardware/DataSpace.java b/core/java/android/hardware/DataSpace.java index 01ed1326074a5..6c42776cca5ee 100644 --- a/core/java/android/hardware/DataSpace.java +++ b/core/java/android/hardware/DataSpace.java @@ -407,6 +407,22 @@ public final class DataSpace { */ public static final int DATASPACE_DYNAMIC_DEPTH = 4098; + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(flag = true, value = { + DATASPACE_HEIF, + }) + public @interface DataSpaceFileFormat {}; + + /** + * High Efficiency Image File Format (HEIF). + * + *

This value is valid with {@link android.hardware.HardwareBuffer#BLOB HardwareBuffer.BLOB} + * format. The combination is an HEIC image encoded by HEIC or HEVC encoder according to + * ISO/IEC 23008-12.

+ */ + public static final int DATASPACE_HEIF = 4100; + /** @hide */ @Retention(RetentionPolicy.SOURCE) @IntDef(flag = true, value = { diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java index fde7afd214209..ca906d2cd6d1d 100644 --- a/media/java/android/media/ImageReader.java +++ b/media/java/android/media/ImageReader.java @@ -342,9 +342,14 @@ public class ImageReader implements AutoCloseable { // Only include memory for 1 buffer, since actually accounting for the memory used is // complex, and 1 buffer is enough for the VM to treat the ImageReader as being of some // size. - mEstimatedNativeAllocBytes = ImageUtils.getEstimatedNativeAllocBytes( - width, height, useLegacyImageFormat ? imageFormat : hardwareBufferFormat, - /*buffer count*/ 1); + if (hardwareBufferFormat == HardwareBuffer.BLOB) { + mEstimatedNativeAllocBytes = ImageUtils.getEstimatedNativeAllocBytes( + width, height, imageFormat, /*buffer count*/ 1); + } else { + mEstimatedNativeAllocBytes = ImageUtils.getEstimatedNativeAllocBytes( + width, height, useLegacyImageFormat ? imageFormat : hardwareBufferFormat, + /*buffer count*/ 1); + } VMRuntime.getRuntime().registerNativeAllocation(mEstimatedNativeAllocBytes); } @@ -370,7 +375,6 @@ public class ImageReader implements AutoCloseable { MultiResolutionImageReader parent, int hardwareBufferFormat, int dataSpace) { mWidth = width; mHeight = height; - mFormat = ImageFormat.UNKNOWN; // set default image format value as UNKNOWN mUsage = usage; mMaxImages = maxImages; mParent = parent; @@ -378,6 +382,7 @@ public class ImageReader implements AutoCloseable { mDataSpace = dataSpace; mUseLegacyImageFormat = false; mNumPlanes = ImageUtils.getNumPlanesForHardwareBufferFormat(mHardwareBufferFormat); + mFormat = PublicFormatUtils.getPublicFormat(hardwareBufferFormat, dataSpace); initializeImageReader(width, height, mFormat, maxImages, usage, hardwareBufferFormat, dataSpace, mUseLegacyImageFormat); diff --git a/media/java/android/media/ImageWriter.java b/media/java/android/media/ImageWriter.java index 9f52bf18f4e37..259c1386aff4e 100644 --- a/media/java/android/media/ImageWriter.java +++ b/media/java/android/media/ImageWriter.java @@ -29,7 +29,6 @@ import android.hardware.DataSpace.NamedDataSpace; import android.hardware.HardwareBuffer; import android.hardware.HardwareBuffer.Usage; import android.hardware.SyncFence; -import android.hardware.camera2.params.StreamConfigurationMap; import android.hardware.camera2.utils.SurfaceUtils; import android.os.Handler; import android.os.Looper; @@ -266,31 +265,11 @@ public class ImageWriter implements AutoCloseable { // nativeInit internally overrides UNKNOWN format. So does surface format query after // nativeInit and before getEstimatedNativeAllocBytes(). imageFormat = SurfaceUtils.getSurfaceFormat(surface); - mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); - mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + mDataSpace = dataSpace = PublicFormatUtils.getHalDataspace(dataSpace); + mHardwareBufferFormat = + hardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); } - // Several public formats use the same native HAL_PIXEL_FORMAT_BLOB. The native - // allocation estimation sequence depends on the public formats values. To avoid - // possible errors, convert where necessary. - if (imageFormat == StreamConfigurationMap.HAL_PIXEL_FORMAT_BLOB) { - int surfaceDataspace = SurfaceUtils.getSurfaceDataspace(surface); - switch (surfaceDataspace) { - case StreamConfigurationMap.HAL_DATASPACE_DEPTH: - imageFormat = ImageFormat.DEPTH_POINT_CLOUD; - break; - case StreamConfigurationMap.HAL_DATASPACE_DYNAMIC_DEPTH: - imageFormat = ImageFormat.DEPTH_JPEG; - break; - case StreamConfigurationMap.HAL_DATASPACE_HEIF: - imageFormat = ImageFormat.HEIC; - break; - default: - imageFormat = ImageFormat.JPEG; - } - mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); - mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); - } // 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. @@ -301,17 +280,44 @@ public class ImageWriter implements AutoCloseable { mWidth = width == -1 ? surfSize.getWidth() : width; mHeight = height == -1 ? surfSize.getHeight() : height; - mEstimatedNativeAllocBytes = - ImageUtils.getEstimatedNativeAllocBytes(mWidth, mHeight, - useLegacyImageFormat ? imageFormat : hardwareBufferFormat, /*buffer count*/ 1); + if (hardwareBufferFormat == HardwareBuffer.BLOB) { + // TODO(b/246344817): remove mWriterFormat and mDataSpace re-set here after fixing. + mWriterFormat = imageFormat = getImageFormatByBLOB(dataSpace); + mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + + mEstimatedNativeAllocBytes = ImageUtils.getEstimatedNativeAllocBytes(mWidth, mHeight, + imageFormat, /*buffer count*/ 1); + } else { + mEstimatedNativeAllocBytes = + ImageUtils.getEstimatedNativeAllocBytes(mWidth, mHeight, + useLegacyImageFormat ? imageFormat : hardwareBufferFormat, /*buffer count*/ 1); + } VMRuntime.getRuntime().registerNativeAllocation(mEstimatedNativeAllocBytes); } + // Several public formats use the same native HAL_PIXEL_FORMAT_BLOB. The native + // allocation estimation sequence depends on the public formats values. To avoid + // possible errors, convert where necessary. + private int getImageFormatByBLOB(int dataspace) { + switch (dataspace) { + case DataSpace.DATASPACE_DEPTH: + return ImageFormat.DEPTH_POINT_CLOUD; + case DataSpace.DATASPACE_DYNAMIC_DEPTH: + return ImageFormat.DEPTH_JPEG; + case DataSpace.DATASPACE_HEIF: + return ImageFormat.HEIC; + default: + return ImageFormat.JPEG; + } + } + private ImageWriter(Surface surface, int maxImages, boolean useSurfaceImageFormatInfo, int imageFormat, int width, int height) { mMaxImages = maxImages; - mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); - mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + if (!useSurfaceImageFormatInfo) { + mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); + mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + } initializeImageWriter(surface, maxImages, useSurfaceImageFormatInfo, true, imageFormat, mHardwareBufferFormat, mDataSpace, width, height, mUsage); @@ -321,8 +327,10 @@ public class ImageWriter implements AutoCloseable { int imageFormat, int width, int height, long usage) { mMaxImages = maxImages; mUsage = usage; - mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); - mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + if (!useSurfaceImageFormatInfo) { + mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); + mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); + } initializeImageWriter(surface, maxImages, useSurfaceImageFormatInfo, true, imageFormat, mHardwareBufferFormat, mDataSpace, width, height, usage); @@ -337,8 +345,6 @@ public class ImageWriter implements AutoCloseable { // and retrieve corresponding hardwareBufferFormat and dataSpace here. if (useSurfaceImageFormatInfo) { imageFormat = ImageFormat.UNKNOWN; - mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat); - mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat); } else { imageFormat = PublicFormatUtils.getPublicFormat(hardwareBufferFormat, dataSpace); mHardwareBufferFormat = hardwareBufferFormat;