Fix ImageWriter builder pattern APIs
- fix getFormat() mismatch by fix the way of reading dataspace value in
the android_media_ImageWriter#Image_getFormat function.
- fix getDataSpace() mismatch when the Image is dequeued from
ImageWriter.
- skip usage bit setup if image format is opaque.
Bug: 215717202
Bug: 215718355
Test: android.hardware.cts.DataSpaceTest,android.hardware.camera2.cts.ImageWriterTest,android.hardware.camera2.cts.ImageReaderTest
Change-Id: I375739322c3fdad2dc23be3558f4bd8cd396d8b6
diff --git a/media/java/android/media/ImageWriter.java b/media/java/android/media/ImageWriter.java
index 6168c22..a1aedf1 100644
--- a/media/java/android/media/ImageWriter.java
+++ b/media/java/android/media/ImageWriter.java
@@ -102,11 +102,10 @@
private int mWidth;
private int mHeight;
private final int mMaxImages;
- private @Usage long mUsage = HardwareBuffer.USAGE_CPU_WRITE_OFTEN;
+ private long mUsage = HardwareBuffer.USAGE_CPU_WRITE_OFTEN;
private @HardwareBuffer.Format int mHardwareBufferFormat;
private @NamedDataSpace long mDataSpace;
private boolean mUseLegacyImageFormat;
- private boolean mUseSurfaceImageFormatInfo;
// Field below is used by native code, do not access or modify.
private int mWriterFormat;
@@ -255,35 +254,38 @@
+ ", maxImages: " + maxImages);
}
- mUseSurfaceImageFormatInfo = useSurfaceImageFormatInfo;
mUseLegacyImageFormat = useLegacyImageFormat;
// Note that the underlying BufferQueue is working in synchronous mode
// to avoid dropping any buffers.
mNativeContext = nativeInit(new WeakReference<>(this), surface, maxImages, width, height,
useSurfaceImageFormatInfo, hardwareBufferFormat, dataSpace, usage);
+ // if useSurfaceImageFormatInfo is true, imageformat should be read from the surface.
if (useSurfaceImageFormatInfo) {
// nativeInit internally overrides UNKNOWN format. So does surface format query after
// nativeInit and before getEstimatedNativeAllocBytes().
imageFormat = SurfaceUtils.getSurfaceFormat(surface);
- // 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);
+ }
+
+ // 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);
@@ -307,7 +309,6 @@
private ImageWriter(Surface surface, int maxImages, boolean useSurfaceImageFormatInfo,
int imageFormat, int width, int height) {
mMaxImages = maxImages;
- // update hal format and dataspace only if image format is overridden by producer.
mHardwareBufferFormat = PublicFormatUtils.getHalFormat(imageFormat);
mDataSpace = PublicFormatUtils.getHalDataspace(imageFormat);
@@ -566,6 +567,9 @@
/**
* Get the ImageWriter usage flag.
*
+ * <p>It is not recommended to use this function if {@link Builder#setUsage} is not called.
+ * Invalid usage value will be returned if so.</p>
+ *
* @return The ImageWriter usage flag.
*/
public @Usage long getUsage() {
@@ -873,7 +877,7 @@
private int mHeight = -1;
private int mMaxImages = 1;
private int mImageFormat = ImageFormat.UNKNOWN;
- private @Usage long mUsage = HardwareBuffer.USAGE_CPU_WRITE_OFTEN;
+ private long mUsage = -1;
private @HardwareBuffer.Format int mHardwareBufferFormat = HardwareBuffer.RGBA_8888;
private @NamedDataSpace long mDataSpace = DataSpace.DATASPACE_UNKNOWN;
private boolean mUseSurfaceImageFormatInfo = true;
@@ -885,10 +889,19 @@
/**
* Constructs a new builder for {@link ImageWriter}.
*
+ * <p>Uses {@code surface} input parameter to retrieve image format, hal format
+ * and hal dataspace value for default. </p>
+ *
* @param surface The destination Surface this writer produces Image data into.
+ *
+ * @throws IllegalArgumentException if the surface is already abandoned.
*/
public Builder(@NonNull Surface surface) {
mSurface = surface;
+ // retrieve format from surface
+ mImageFormat = SurfaceUtils.getSurfaceFormat(surface);
+ mDataSpace = SurfaceUtils.getSurfaceDataspace(surface);
+ mHardwareBufferFormat = PublicFormatUtils.getHalFormat(mImageFormat);
}
/**
@@ -926,6 +939,8 @@
* @param imageFormat The format of the {@link ImageWriter}. It can be any valid specified
* by {@link ImageFormat} or {@link PixelFormat}.
* @return the Builder instance with customized image format.
+ *
+ * @throws IllegalArgumentException if {@code imageFormat} is invalid.
*/
@SuppressLint("MissingGetterMatchingBuilder")
public @NonNull Builder setImageFormat(@Format int imageFormat) {
@@ -985,12 +1000,16 @@
/**
* Set the usage flag of this ImageWriter.
- * Default value is {@link HardwareBuffer#USAGE_CPU_WRITE_OFTEN}.
+ *
+ * <p>If this function is not called, usage bit will be set
+ * to {@link HardwareBuffer#USAGE_CPU_WRITE_OFTEN} if the image format is not
+ * {@link ImageFormat#PRIVATE PRIVATE}.</p>
*
* @param usage The intended usage of the images produced by this ImageWriter.
* @return the Builder instance with customized usage flag.
*
* @see HardwareBuffer
+ * @see #getUsage
*/
public @NonNull Builder setUsage(@Usage long usage) {
mUsage = usage;
@@ -1022,6 +1041,7 @@
private int mHeight = -1;
private int mWidth = -1;
private int mFormat = -1;
+ private @NamedDataSpace long mDataSpace = DataSpace.DATASPACE_UNKNOWN;
// When this default timestamp is used, timestamp for the input Image
// will be generated automatically when queueInputBuffer is called.
private final long DEFAULT_TIMESTAMP = Long.MIN_VALUE;
@@ -1034,19 +1054,34 @@
mOwner = writer;
mWidth = writer.mWidth;
mHeight = writer.mHeight;
+ mDataSpace = writer.mDataSpace;
- if (!writer.mUseLegacyImageFormat) {
+ if (!mOwner.mUseLegacyImageFormat) {
mFormat = PublicFormatUtils.getPublicFormat(
- writer.mHardwareBufferFormat, writer.mDataSpace);
+ mOwner.mHardwareBufferFormat, mDataSpace);
}
}
@Override
+ public @NamedDataSpace long getDataSpace() {
+ throwISEIfImageIsInvalid();
+
+ return mDataSpace;
+ }
+
+ @Override
+ public void setDataSpace(@NamedDataSpace long dataSpace) {
+ throwISEIfImageIsInvalid();
+
+ mDataSpace = dataSpace;
+ }
+
+ @Override
public int getFormat() {
throwISEIfImageIsInvalid();
- if (mFormat == -1) {
- mFormat = nativeGetFormat();
+ if (mOwner.mUseLegacyImageFormat && mFormat == -1) {
+ mFormat = nativeGetFormat(mDataSpace);
}
return mFormat;
}
@@ -1114,7 +1149,8 @@
if (mPlanes == null) {
int numPlanes = ImageUtils.getNumPlanesForFormat(getFormat());
- mPlanes = nativeCreatePlanes(numPlanes, getOwner().getFormat());
+ mPlanes = nativeCreatePlanes(numPlanes, getOwner().getFormat(),
+ getOwner().getDataSpace());
}
return mPlanes.clone();
@@ -1222,13 +1258,14 @@
}
// Create the SurfacePlane object and fill the information
- private synchronized native SurfacePlane[] nativeCreatePlanes(int numPlanes, int writerFmt);
+ private synchronized native SurfacePlane[] nativeCreatePlanes(int numPlanes, int writerFmt,
+ long dataSpace);
private synchronized native int nativeGetWidth();
private synchronized native int nativeGetHeight();
- private synchronized native int nativeGetFormat();
+ private synchronized native int nativeGetFormat(long dataSpace);
private synchronized native HardwareBuffer nativeGetHardwareBuffer();
}
diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp
index 2e419a6..eca26dc 100644
--- a/media/jni/android_media_ImageWriter.cpp
+++ b/media/jni/android_media_ImageWriter.cpp
@@ -460,8 +460,6 @@
} else {
// Set consumer buffer format to user specified format
android_dataspace nativeDataspace = static_cast<android_dataspace>(dataSpace);
- int userFormat = static_cast<int>(mapHalFormatDataspaceToPublicFormat(
- hardwareBufferFormat, nativeDataspace));
res = native_window_set_buffers_format(anw.get(), hardwareBufferFormat);
if (res != OK) {
ALOGE("%s: Unable to configure consumer native buffer format to %#x",
@@ -478,20 +476,29 @@
return 0;
}
ctx->setBufferDataSpace(nativeDataspace);
- surfaceFormat = userFormat;
+ surfaceFormat = static_cast<int32_t>(mapHalFormatDataspaceToPublicFormat(
+ hardwareBufferFormat, nativeDataspace));
}
ctx->setBufferFormat(surfaceFormat);
env->SetIntField(thiz,
gImageWriterClassInfo.mWriterFormat, reinterpret_cast<jint>(surfaceFormat));
- res = native_window_set_usage(anw.get(), ndkUsage);
- if (res != OK) {
- ALOGE("%s: Configure usage %08x for format %08x failed: %s (%d)",
- __FUNCTION__, static_cast<unsigned int>(ndkUsage),
- surfaceFormat, strerror(-res), res);
- jniThrowRuntimeException(env, "Failed to SW_WRITE_OFTEN configure usage");
- return 0;
+ // ndkUsage == -1 means setUsage in ImageWriter class is not called.
+ // skip usage setting if setUsage in ImageWriter is not called and imageformat is opaque.
+ if (!(ndkUsage == -1 && isFormatOpaque(surfaceFormat))) {
+ if (ndkUsage == -1) {
+ ndkUsage = GRALLOC_USAGE_SW_WRITE_OFTEN;
+ }
+ res = native_window_set_usage(anw.get(), ndkUsage);
+ if (res != OK) {
+ ALOGE("%s: Configure usage %08x for format %08x failed: %s (%d)",
+ __FUNCTION__, static_cast<unsigned int>(ndkUsage),
+ surfaceFormat, strerror(-res), res);
+ jniThrowRuntimeException(env,
+ "Failed to SW_WRITE_OFTEN configure usage");
+ return 0;
+ }
}
int minUndequeuedBufferCount = 0;
@@ -952,7 +959,7 @@
return buffer->getHeight();
}
-static jint Image_getFormat(JNIEnv* env, jobject thiz) {
+static jint Image_getFormat(JNIEnv* env, jobject thiz, jlong dataSpace) {
ALOGV("%s", __FUNCTION__);
GraphicBuffer* buffer;
Image_getNativeContext(env, thiz, &buffer, NULL);
@@ -962,9 +969,9 @@
return 0;
}
- // ImageWriter doesn't support data space yet, assuming it is unknown.
PublicFormat publicFmt = mapHalFormatDataspaceToPublicFormat(buffer->getPixelFormat(),
- HAL_DATASPACE_UNKNOWN);
+ static_cast<android_dataspace>(dataSpace));
+
return static_cast<jint>(publicFmt);
}
@@ -1031,14 +1038,14 @@
}
static jobjectArray Image_createSurfacePlanes(JNIEnv* env, jobject thiz,
- int numPlanes, int writerFormat) {
+ int numPlanes, int writerFormat, long dataSpace) {
ALOGV("%s: create SurfacePlane array with size %d", __FUNCTION__, numPlanes);
int rowStride, pixelStride;
uint8_t *pData;
uint32_t dataSize;
jobject byteBuffer;
- int format = Image_getFormat(env, thiz);
+ int format = Image_getFormat(env, thiz, dataSpace);
if (isFormatOpaque(format) && numPlanes > 0) {
String8 msg;
msg.appendFormat("Format 0x%x is opaque, thus not writable, the number of planes (%d)"
@@ -1108,11 +1115,11 @@
};
static JNINativeMethod gImageMethods[] = {
- {"nativeCreatePlanes", "(II)[Landroid/media/ImageWriter$WriterSurfaceImage$SurfacePlane;",
+ {"nativeCreatePlanes", "(IIJ)[Landroid/media/ImageWriter$WriterSurfaceImage$SurfacePlane;",
(void*)Image_createSurfacePlanes },
{"nativeGetWidth", "()I", (void*)Image_getWidth },
{"nativeGetHeight", "()I", (void*)Image_getHeight },
- {"nativeGetFormat", "()I", (void*)Image_getFormat },
+ {"nativeGetFormat", "(J)I", (void*)Image_getFormat },
{"nativeGetHardwareBuffer", "()Landroid/hardware/HardwareBuffer;",
(void*)Image_getHardwareBuffer },
};