diff options
-rw-r--r-- | include/gui/BufferQueueProducer.h | 14 | ||||
-rw-r--r-- | libs/gui/BufferQueueProducer.cpp | 64 | ||||
-rw-r--r-- | libs/gui/tests/BufferQueue_test.cpp | 26 |
3 files changed, 84 insertions, 20 deletions
diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h index 835593fdac..645a07bafd 100644 --- a/include/gui/BufferQueueProducer.h +++ b/include/gui/BufferQueueProducer.h @@ -183,12 +183,24 @@ private: // 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 c48f23728c..56f1a0905c 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -184,12 +184,35 @@ status_t BufferQueueProducer::setAsyncMode(bool async) { 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 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller, 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 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller, 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 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller, 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 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot, 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 BufferQueueProducer::attachBuffer(int* outSlot, 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 a45a5be8e2..ac9af0796e 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -500,7 +500,7 @@ TEST_F(BufferQueueTest, TestSingleBufferMode) { 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 @@ TEST_F(BufferQueueTest, TestSingleBufferMode) { 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 @@ TEST_F(BufferQueueTest, TestTimeouts) { 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 |