Merge "libgui: Fix attaching buffers without allocation"
diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h
index 835593f..645a07b 100644
--- a/include/gui/BufferQueueProducer.h
+++ b/include/gui/BufferQueueProducer.h
@@ -183,12 +183,24 @@
// This is required by the IBinder::DeathRecipient interface
virtual void binderDied(const wp<IBinder>& who);
+ // Returns the slot of the next free buffer if one is available or
+ // BufferQueueCore::INVALID_BUFFER_SLOT otherwise
+ int getFreeBufferLocked() const;
+
+ // Returns the next free slot if one less than or equal to maxBufferCount
+ // is available or BufferQueueCore::INVALID_BUFFER_SLOT otherwise
+ int getFreeSlotLocked(int maxBufferCount) const;
+
// waitForFreeSlotThenRelock finds the oldest slot in the FREE state. It may
// block if there are no available slots and we are not in non-blocking
// mode (producer and consumer controlled by the application). If it blocks,
// it will release mCore->mMutex while blocked so that other operations on
// the BufferQueue may succeed.
- status_t waitForFreeSlotThenRelock(const char* caller, int* found,
+ enum class FreeSlotCaller {
+ Dequeue,
+ Attach,
+ };
+ status_t waitForFreeSlotThenRelock(FreeSlotCaller caller, int* found,
status_t* returnFlags) const;
sp<BufferQueueCore> mCore;
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index c48f237..56f1a09 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -184,12 +184,35 @@
return NO_ERROR;
}
-status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
+int BufferQueueProducer::getFreeBufferLocked() const {
+ if (mCore->mFreeBuffers.empty()) {
+ return BufferQueueCore::INVALID_BUFFER_SLOT;
+ }
+ auto slot = mCore->mFreeBuffers.front();
+ mCore->mFreeBuffers.pop_front();
+ return slot;
+}
+
+int BufferQueueProducer::getFreeSlotLocked(int maxBufferCount) const {
+ if (mCore->mFreeSlots.empty()) {
+ return BufferQueueCore::INVALID_BUFFER_SLOT;
+ }
+ auto slot = *(mCore->mFreeSlots.begin());
+ if (slot < maxBufferCount) {
+ mCore->mFreeSlots.erase(slot);
+ return slot;
+ }
+ return BufferQueueCore::INVALID_BUFFER_SLOT;
+}
+
+status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller,
int* found, status_t* returnFlags) const {
+ auto callerString = (caller == FreeSlotCaller::Dequeue) ?
+ "dequeueBuffer" : "attachBuffer";
bool tryAgain = true;
while (tryAgain) {
if (mCore->mIsAbandoned) {
- BQ_LOGE("%s: BufferQueue has been abandoned", caller);
+ BQ_LOGE("%s: BufferQueue has been abandoned", callerString);
return NO_INIT;
}
@@ -221,7 +244,7 @@
if (mCore->mBufferHasBeenQueued &&
dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
BQ_LOGE("%s: attempting to exceed the max dequeued buffer count "
- "(%d)", caller, mCore->mMaxDequeuedBufferCount);
+ "(%d)", callerString, mCore->mMaxDequeuedBufferCount);
return INVALID_OPERATION;
}
@@ -234,7 +257,7 @@
bool tooManyBuffers = mCore->mQueue.size()
> static_cast<size_t>(maxBufferCount);
if (tooManyBuffers) {
- BQ_LOGV("%s: queue size is %zu, waiting", caller,
+ BQ_LOGV("%s: queue size is %zu, waiting", callerString,
mCore->mQueue.size());
} else {
// If in single buffer mode and a shared buffer exists, always
@@ -242,16 +265,23 @@
if (mCore->mSingleBufferMode && mCore->mSingleBufferSlot !=
BufferQueueCore::INVALID_BUFFER_SLOT) {
*found = mCore->mSingleBufferSlot;
- } else if (!mCore->mFreeBuffers.empty()) {
- auto slot = mCore->mFreeBuffers.begin();
- *found = *slot;
- mCore->mFreeBuffers.erase(slot);
- } else if (mCore->mAllowAllocation && !mCore->mFreeSlots.empty()) {
- auto slot = mCore->mFreeSlots.begin();
- // Only return free slots up to the max buffer count
- if (*slot < maxBufferCount) {
- *found = *slot;
- mCore->mFreeSlots.erase(slot);
+ } else {
+ if (caller == FreeSlotCaller::Dequeue) {
+ // If we're calling this from dequeue, prefer free buffers
+ auto slot = getFreeBufferLocked();
+ if (slot != BufferQueueCore::INVALID_BUFFER_SLOT) {
+ *found = slot;
+ } else if (mCore->mAllowAllocation) {
+ *found = getFreeSlotLocked(maxBufferCount);
+ }
+ } else {
+ // If we're calling this from attach, prefer free slots
+ auto slot = getFreeSlotLocked(maxBufferCount);
+ if (slot != BufferQueueCore::INVALID_BUFFER_SLOT) {
+ *found = slot;
+ } else {
+ *found = getFreeBufferLocked();
+ }
}
}
}
@@ -338,8 +368,8 @@
int found = BufferItem::INVALID_BUFFER_SLOT;
while (found == BufferItem::INVALID_BUFFER_SLOT) {
- status_t status = waitForFreeSlotThenRelock("dequeueBuffer", &found,
- &returnFlags);
+ status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue,
+ &found, &returnFlags);
if (status != NO_ERROR) {
return status;
}
@@ -614,7 +644,7 @@
status_t returnFlags = NO_ERROR;
int found;
- status_t status = waitForFreeSlotThenRelock("attachBuffer", &found,
+ status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, &found,
&returnFlags);
if (status != NO_ERROR) {
return status;
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index a45a5be..ac9af07 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -500,7 +500,7 @@
ASSERT_EQ(HAL_DATASPACE_UNKNOWN, item.mDataSpace);
ASSERT_EQ(Rect(0, 0, 1, 1), item.mCrop);
ASSERT_EQ(NATIVE_WINDOW_SCALING_MODE_FREEZE, item.mScalingMode);
- ASSERT_EQ(0, item.mTransform);
+ ASSERT_EQ(0u, item.mTransform);
ASSERT_EQ(Fence::NO_FENCE, item.mFence);
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
@@ -527,7 +527,7 @@
ASSERT_EQ(HAL_DATASPACE_UNKNOWN, item.mDataSpace);
ASSERT_EQ(Rect(0, 0, 1, 1), item.mCrop);
ASSERT_EQ(NATIVE_WINDOW_SCALING_MODE_FREEZE, item.mScalingMode);
- ASSERT_EQ(0, item.mTransform);
+ ASSERT_EQ(0u, item.mTransform);
ASSERT_EQ(Fence::NO_FENCE, item.mFence);
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
@@ -597,4 +597,26 @@
ASSERT_GE(systemTime() - startTime, TIMEOUT);
}
+TEST_F(BufferQueueTest, CanAttachWhileDisallowingAllocation) {
+ createBufferQueue();
+ sp<DummyConsumer> dc(new DummyConsumer);
+ ASSERT_EQ(OK, mConsumer->consumerConnect(dc, true));
+ IGraphicBufferProducer::QueueBufferOutput output;
+ ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener,
+ NATIVE_WINDOW_API_CPU, true, &output));
+
+ int slot = BufferQueue::INVALID_BUFFER_SLOT;
+ sp<Fence> sourceFence;
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+ mProducer->dequeueBuffer(&slot, &sourceFence, 0, 0, 0, 0));
+ sp<GraphicBuffer> buffer;
+ ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buffer));
+ ASSERT_EQ(OK, mProducer->detachBuffer(slot));
+
+ ASSERT_EQ(OK, mProducer->allowAllocation(false));
+
+ slot = BufferQueue::INVALID_BUFFER_SLOT;
+ ASSERT_EQ(OK, mProducer->attachBuffer(&slot, buffer));
+}
+
} // namespace android