Refactor AImage/AImageReader
1/ Use BufferItemConsumer instead of CpuConsumer for AImageReader.
2/ Delay lock image in AImage until the first time getPlaneXXX functions
get called.
3/ Add libmedia_jni as dependency so that we can reuse existing code
from android_media_Utils.h
Bug: 35114769
Test: Ran the following CTS test case from CtsCameraTestCases:
NativeCameraDeviceTest, NativeCameraManagerTest,
NativeImageReaderTest, NativeStillCaptureTest
Change-Id: Ia8dc451ea873e8290592deacc7f8d40360382f86
diff --git a/media/ndk/NdkImageReader.cpp b/media/ndk/NdkImageReader.cpp
index ab3829e..e580dae 100644
--- a/media/ndk/NdkImageReader.cpp
+++ b/media/ndk/NdkImageReader.cpp
@@ -24,7 +24,9 @@
#include <cutils/atomic.h>
#include <utils/Log.h>
+#include <android_media_Utils.h>
#include <android_runtime/android_view_Surface.h>
+#include <android_runtime/android_hardware_HardwareBuffer.h>
using namespace android;
@@ -36,6 +38,7 @@
}
}
+const int32_t AImageReader::kDefaultUsage = AHARDWAREBUFFER_USAGE0_CPU_READ_OFTEN;
const char* AImageReader::kCallbackFpKey = "Callback";
const char* AImageReader::kContextKey = "Context";
@@ -151,10 +154,18 @@
}
}
-AImageReader::AImageReader(int32_t width, int32_t height, int32_t format, int32_t maxImages) :
- mWidth(width), mHeight(height), mFormat(format), mMaxImages(maxImages),
- mNumPlanes(getNumPlanesForFormat(format)),
- mFrameListener(new FrameListener(this)) {}
+AImageReader::AImageReader(int32_t width,
+ int32_t height,
+ int32_t format,
+ uint64_t usage,
+ int32_t maxImages)
+ : mWidth(width),
+ mHeight(height),
+ mFormat(format),
+ mUsage(usage),
+ mMaxImages(maxImages),
+ mNumPlanes(getNumPlanesForFormat(format)),
+ mFrameListener(new FrameListener(this)) {}
media_status_t
AImageReader::init() {
@@ -162,42 +173,44 @@
mHalFormat = android_view_Surface_mapPublicFormatToHalFormat(publicFormat);
mHalDataSpace = android_view_Surface_mapPublicFormatToHalDataspace(publicFormat);
+ uint64_t producerUsage;
+ uint64_t consumerUsage;
+ android_hardware_HardwareBuffer_convertToGrallocUsageBits(
+ &producerUsage, &consumerUsage, mUsage, 0);
+
sp<IGraphicBufferProducer> gbProducer;
sp<IGraphicBufferConsumer> gbConsumer;
BufferQueue::createBufferQueue(&gbProducer, &gbConsumer);
- sp<CpuConsumer> cpuConsumer;
- String8 consumerName = String8::format("ImageReader-%dx%df%xm%d-%d-%d",
- mWidth, mHeight, mFormat, mMaxImages, getpid(),
- createProcessUniqueId());
+ String8 consumerName = String8::format(
+ "ImageReader-%dx%df%xu%" PRIu64 "m%d-%d-%d", mWidth, mHeight, mFormat, mUsage,
+ mMaxImages, getpid(), createProcessUniqueId());
- cpuConsumer = new CpuConsumer(gbConsumer, mMaxImages, /*controlledByApp*/true);
- if (cpuConsumer == nullptr) {
- ALOGE("Failed to allocate CpuConsumer");
+ mBufferItemConsumer =
+ new BufferItemConsumer(gbConsumer, consumerUsage, mMaxImages, /*controlledByApp*/ true);
+ if (mBufferItemConsumer == nullptr) {
+ ALOGE("Failed to allocate BufferItemConsumer");
return AMEDIA_ERROR_UNKNOWN;
}
- mCpuConsumer = cpuConsumer;
- mCpuConsumer->setName(consumerName);
mProducer = gbProducer;
-
- sp<ConsumerBase> consumer = cpuConsumer;
- consumer->setFrameAvailableListener(mFrameListener);
+ mBufferItemConsumer->setName(consumerName);
+ mBufferItemConsumer->setFrameAvailableListener(mFrameListener);
status_t res;
- res = cpuConsumer->setDefaultBufferSize(mWidth, mHeight);
+ res = mBufferItemConsumer->setDefaultBufferSize(mWidth, mHeight);
if (res != OK) {
- ALOGE("Failed to set CpuConsumer buffer size");
+ ALOGE("Failed to set BufferItemConsumer buffer size");
return AMEDIA_ERROR_UNKNOWN;
}
- res = cpuConsumer->setDefaultBufferFormat(mHalFormat);
+ res = mBufferItemConsumer->setDefaultBufferFormat(mHalFormat);
if (res != OK) {
- ALOGE("Failed to set CpuConsumer buffer format");
+ ALOGE("Failed to set BufferItemConsumer buffer format");
return AMEDIA_ERROR_UNKNOWN;
}
- res = cpuConsumer->setDefaultBufferDataSpace(mHalDataSpace);
+ res = mBufferItemConsumer->setDefaultBufferDataSpace(mHalDataSpace);
if (res != OK) {
- ALOGE("Failed to set CpuConsumer buffer dataSpace");
+ ALOGE("Failed to set BufferItemConsumer buffer dataSpace");
return AMEDIA_ERROR_UNKNOWN;
}
@@ -209,7 +222,7 @@
mWindow = static_cast<ANativeWindow*>(mSurface.get());
for (int i = 0; i < mMaxImages; i++) {
- CpuConsumer::LockedBuffer* buffer = new CpuConsumer::LockedBuffer;
+ BufferItem* buffer = new BufferItem;
mBuffers.push_back(buffer);
}
@@ -248,133 +261,136 @@
image->close();
}
- // Delete LockedBuffers
+ // Delete Buffer Items
for (auto it = mBuffers.begin();
it != mBuffers.end(); it++) {
delete *it;
}
- if (mCpuConsumer != nullptr) {
- mCpuConsumer->abandon();
- mCpuConsumer->setFrameAvailableListener(nullptr);
+ if (mBufferItemConsumer != nullptr) {
+ mBufferItemConsumer->abandon();
+ mBufferItemConsumer->setFrameAvailableListener(nullptr);
}
}
media_status_t
-AImageReader::acquireCpuConsumerImageLocked(/*out*/AImage** image) {
+AImageReader::acquireImageLocked(/*out*/AImage** image) {
*image = nullptr;
- CpuConsumer::LockedBuffer* buffer = getLockedBufferLocked();
+ BufferItem* buffer = getBufferItemLocked();
if (buffer == nullptr) {
ALOGW("Unable to acquire a lockedBuffer, very likely client tries to lock more than"
" maxImages buffers");
return AMEDIA_IMGREADER_MAX_IMAGES_ACQUIRED;
}
- status_t res = mCpuConsumer->lockNextBuffer(buffer);
+ status_t res = mBufferItemConsumer->acquireBuffer(buffer, 0);
if (res != NO_ERROR) {
- returnLockedBufferLocked(buffer);
- if (res != BAD_VALUE /*no buffers*/) {
- if (res == NOT_ENOUGH_DATA) {
+ returnBufferItemLocked(buffer);
+ if (res != BufferQueue::NO_BUFFER_AVAILABLE) {
+ if (res == INVALID_OPERATION) {
return AMEDIA_IMGREADER_MAX_IMAGES_ACQUIRED;
} else {
- ALOGE("%s Fail to lockNextBuffer with error: %d ",
- __FUNCTION__, res);
+ ALOGE("%s: Acquire image failed with some unknown error: %s (%d)",
+ __FUNCTION__, strerror(-res), res);
return AMEDIA_ERROR_UNKNOWN;
}
}
return AMEDIA_IMGREADER_NO_BUFFER_AVAILABLE;
}
- if (buffer->flexFormat == HAL_PIXEL_FORMAT_YCrCb_420_SP) {
- ALOGE("NV21 format is not supported by AImageReader");
- return AMEDIA_ERROR_UNSUPPORTED;
- }
+ const int bufferWidth = getBufferWidth(buffer);
+ const int bufferHeight = getBufferHeight(buffer);
+ const int bufferFmt = buffer->mGraphicBuffer->getPixelFormat();
- // Check if the left-top corner of the crop rect is origin, we currently assume this point is
- // zero, will revist this once this assumption turns out problematic.
- Point lt = buffer->crop.leftTop();
- if (lt.x != 0 || lt.y != 0) {
- ALOGE("crop left top corner [%d, %d] need to be at origin", lt.x, lt.y);
- return AMEDIA_ERROR_UNKNOWN;
- }
+ const int readerWidth = mWidth;
+ const int readerHeight = mHeight;
+ const int readerFmt = mHalFormat;
- // Check if the producer buffer configurations match what ImageReader configured.
- int outputWidth = getBufferWidth(buffer);
- int outputHeight = getBufferHeight(buffer);
-
- int readerFmt = mHalFormat;
- int readerWidth = mWidth;
- int readerHeight = mHeight;
-
- if ((buffer->format != HAL_PIXEL_FORMAT_BLOB) && (readerFmt != HAL_PIXEL_FORMAT_BLOB) &&
- (readerWidth != outputWidth || readerHeight != outputHeight)) {
- ALOGW("%s: Producer buffer size: %dx%d, doesn't match AImageReader configured size: %dx%d",
- __FUNCTION__, outputWidth, outputHeight, readerWidth, readerHeight);
- }
-
- int bufFmt = buffer->format;
- if (readerFmt == HAL_PIXEL_FORMAT_YCbCr_420_888) {
- bufFmt = buffer->flexFormat;
- }
-
- if (readerFmt != bufFmt) {
- if (readerFmt == HAL_PIXEL_FORMAT_YCbCr_420_888 && (bufFmt ==
- HAL_PIXEL_FORMAT_YCrCb_420_SP || bufFmt == HAL_PIXEL_FORMAT_YV12)) {
- // Special casing for when producer switches to a format compatible with flexible YUV
- // (HAL_PIXEL_FORMAT_YCbCr_420_888).
- mHalFormat = bufFmt;
- ALOGD("%s: Overriding buffer format YUV_420_888 to %x.", __FUNCTION__, bufFmt);
- } else {
- // Return the buffer to the queue.
- mCpuConsumer->unlockBuffer(*buffer);
- returnLockedBufferLocked(buffer);
-
- ALOGE("Producer output buffer format: 0x%x, ImageReader configured format: 0x%x",
- buffer->format, readerFmt);
-
+ // Check if the producer buffer configurations match what AImageReader configured. Add some
+ // extra checks for non-opaque formats.
+ if (!isFormatOpaque(readerFmt)) {
+ // Check if the left-top corner of the crop rect is origin, we currently assume this point
+ // is zero, will revisit this once this assumption turns out problematic.
+ Point lt = buffer->mCrop.leftTop();
+ if (lt.x != 0 || lt.y != 0) {
+ ALOGE("Crop left top corner [%d, %d] not at origin", lt.x, lt.y);
return AMEDIA_ERROR_UNKNOWN;
}
+
+ // Check if the producer buffer configurations match what ImageReader configured.
+ if ((bufferFmt != HAL_PIXEL_FORMAT_BLOB) && (readerFmt != HAL_PIXEL_FORMAT_BLOB) &&
+ (readerWidth != bufferWidth || readerHeight != bufferHeight)) {
+ ALOGW("%s: Buffer size: %dx%d, doesn't match AImageReader configured size: %dx%d",
+ __FUNCTION__, bufferWidth, bufferHeight, readerWidth, readerHeight);
+ }
+
+ if (readerFmt != bufferFmt) {
+ if (readerFmt == HAL_PIXEL_FORMAT_YCbCr_420_888 && isPossiblyYUV(bufferFmt)) {
+ // Special casing for when producer switches to a format compatible with flexible
+ // YUV.
+ mHalFormat = bufferFmt;
+ ALOGD("%s: Overriding buffer format YUV_420_888 to 0x%x.", __FUNCTION__, bufferFmt);
+ } else {
+ // Return the buffer to the queue. No need to provide fence, as this buffer wasn't
+ // used anywhere yet.
+ mBufferItemConsumer->releaseBuffer(*buffer);
+ returnBufferItemLocked(buffer);
+
+ ALOGE("%s: Output buffer format: 0x%x, ImageReader configured format: 0x%x",
+ __FUNCTION__, bufferFmt, readerFmt);
+
+ return AMEDIA_ERROR_UNKNOWN;
+ }
+ }
}
if (mHalFormat == HAL_PIXEL_FORMAT_BLOB) {
- *image = new AImage(this, mFormat, buffer, buffer->timestamp,
+ *image = new AImage(this, mFormat, mUsage, buffer, buffer->mTimestamp,
readerWidth, readerHeight, mNumPlanes);
} else {
- *image = new AImage(this, mFormat, buffer, buffer->timestamp,
- outputWidth, outputHeight, mNumPlanes);
+ *image = new AImage(this, mFormat, mUsage, buffer, buffer->mTimestamp,
+ bufferWidth, bufferHeight, mNumPlanes);
}
mAcquiredImages.push_back(*image);
return AMEDIA_OK;
}
-CpuConsumer::LockedBuffer*
-AImageReader::getLockedBufferLocked() {
+BufferItem*
+AImageReader::getBufferItemLocked() {
if (mBuffers.empty()) {
return nullptr;
}
- // Return a LockedBuffer pointer and remove it from the list
+ // Return a BufferItem pointer and remove it from the list
auto it = mBuffers.begin();
- CpuConsumer::LockedBuffer* buffer = *it;
+ BufferItem* buffer = *it;
mBuffers.erase(it);
return buffer;
}
void
-AImageReader::returnLockedBufferLocked(CpuConsumer::LockedBuffer* buffer) {
+AImageReader::returnBufferItemLocked(BufferItem* buffer) {
mBuffers.push_back(buffer);
}
void
AImageReader::releaseImageLocked(AImage* image) {
- CpuConsumer::LockedBuffer* buffer = image->mBuffer;
+ BufferItem* buffer = image->mBuffer;
if (buffer == nullptr) {
// This should not happen, but is not fatal
ALOGW("AImage %p has no buffer!", image);
return;
}
- mCpuConsumer->unlockBuffer(*buffer);
- returnLockedBufferLocked(buffer);
+ int fenceFd = -1;
+ media_status_t ret = image->unlockImageIfLocked(&fenceFd);
+ if (ret < 0) {
+ ALOGW("%s: AImage %p is cannot be unlocked.", __FUNCTION__, image);
+ return;
+ }
+
+ sp<Fence> releaseFence = fenceFd > 0 ? new Fence(fenceFd) : Fence::NO_FENCE;
+ mBufferItemConsumer->releaseBuffer(*buffer, releaseFence);
+ returnBufferItemLocked(buffer);
image->mBuffer = nullptr;
bool found = false;
@@ -395,29 +411,31 @@
}
int
-AImageReader::getBufferWidth(CpuConsumer::LockedBuffer* buffer) {
- if (buffer == nullptr) return -1;
+AImageReader::getBufferWidth(BufferItem* buffer) {
+ if (buffer == NULL) return -1;
- if (!buffer->crop.isEmpty()) {
- return buffer->crop.getWidth();
+ if (!buffer->mCrop.isEmpty()) {
+ return buffer->mCrop.getWidth();
}
- return buffer->width;
+
+ return buffer->mGraphicBuffer->getWidth();
}
int
-AImageReader::getBufferHeight(CpuConsumer::LockedBuffer* buffer) {
- if (buffer == nullptr) return -1;
+AImageReader::getBufferHeight(BufferItem* buffer) {
+ if (buffer == NULL) return -1;
- if (!buffer->crop.isEmpty()) {
- return buffer->crop.getHeight();
+ if (!buffer->mCrop.isEmpty()) {
+ return buffer->mCrop.getHeight();
}
- return buffer->height;
+
+ return buffer->mGraphicBuffer->getHeight();
}
media_status_t
AImageReader::acquireNextImage(/*out*/AImage** image) {
Mutex::Autolock _l(mLock);
- return acquireCpuConsumerImageLocked(image);
+ return acquireImageLocked(image);
}
media_status_t
@@ -429,12 +447,12 @@
*image = nullptr;
AImage* prevImage = nullptr;
AImage* nextImage = nullptr;
- media_status_t ret = acquireCpuConsumerImageLocked(&prevImage);
+ media_status_t ret = acquireImageLocked(&prevImage);
if (prevImage == nullptr) {
return ret;
}
for (;;) {
- ret = acquireCpuConsumerImageLocked(&nextImage);
+ ret = acquireImageLocked(&nextImage);
if (nextImage == nullptr) {
*image = prevImage;
return AMEDIA_OK;
@@ -464,6 +482,12 @@
return AMEDIA_ERROR_INVALID_PARAMETER;
}
+ if (maxImages > BufferQueueDefs::NUM_BUFFER_SLOTS) {
+ ALOGE("%s: max outstanding image count (%d) cannot be larget than %d.",
+ __FUNCTION__, maxImages, BufferQueueDefs::NUM_BUFFER_SLOTS);
+ return AMEDIA_ERROR_INVALID_PARAMETER;
+ }
+
if (!AImageReader::isSupportedFormat(format)) {
ALOGE("%s: format %d is not supported by AImageReader",
__FUNCTION__, format);
@@ -475,8 +499,10 @@
return AMEDIA_ERROR_INVALID_PARAMETER;
}
- //*reader = new AImageReader(width, height, format, maxImages);
- AImageReader* tmpReader = new AImageReader(width, height, format, maxImages);
+ // Set consumer usage to AHARDWAREBUFFER_USAGE0_CPU_READ_OFTEN by default so that
+ // AImageReader_new behaves as if it's backed by CpuConsumer.
+ AImageReader* tmpReader = new AImageReader(
+ width, height, format, AImageReader::kDefaultUsage, maxImages);
if (tmpReader == nullptr) {
ALOGE("%s: AImageReader allocation failed", __FUNCTION__);
return AMEDIA_ERROR_UNKNOWN;
@@ -565,7 +591,7 @@
media_status_t AImageReader_acquireNextImage(AImageReader* reader, /*out*/AImage** image) {
ALOGV("%s", __FUNCTION__);
if (reader == nullptr || image == nullptr) {
- ALOGE("%s: invalid argument. reader %p, maxImages %p",
+ ALOGE("%s: invalid argument. reader %p, image %p",
__FUNCTION__, reader, image);
return AMEDIA_ERROR_INVALID_PARAMETER;
}
@@ -576,7 +602,7 @@
media_status_t AImageReader_acquireLatestImage(AImageReader* reader, /*out*/AImage** image) {
ALOGV("%s", __FUNCTION__);
if (reader == nullptr || image == nullptr) {
- ALOGE("%s: invalid argument. reader %p, maxImages %p",
+ ALOGE("%s: invalid argument. reader %p, image %p",
__FUNCTION__, reader, image);
return AMEDIA_ERROR_INVALID_PARAMETER;
}