BQ: Flexible resizing
- Allow the producer to call setMaxDequeuedBufferCount and the
consumer to call setMaxAcquiredBufferCount when buffers are
currently dequeued/acquired as long as the new value is not less
than the number of dequeued/acquired buffers.
Bug 22768206
Change-Id: I599a4027a6ae9cb0a1c0d5ec60cb5e65b86a345b
diff --git a/include/gui/IGraphicBufferConsumer.h b/include/gui/IGraphicBufferConsumer.h
index ebce7fb..e983c16 100644
--- a/include/gui/IGraphicBufferConsumer.h
+++ b/include/gui/IGraphicBufferConsumer.h
@@ -206,17 +206,26 @@
virtual status_t setMaxBufferCount(int bufferCount) = 0;
// setMaxAcquiredBufferCount sets the maximum number of buffers that can
- // be acquired by the consumer at one time (default 1). This call will
- // fail if a producer is connected to the BufferQueue.
+ // be acquired by the consumer at one time (default 1). If this method
+ // succeeds, any new buffer slots will be both unallocated and owned by the
+ // BufferQueue object (i.e. they are not owned by the producer or consumer).
+ // Calling this may also cause some buffer slots to be emptied.
+ //
+ // This function should not be called with a value of maxAcquiredBuffers
+ // that is less than the number of currently acquired buffer slots. Doing so
+ // will result in a BAD_VALUE error.
//
// maxAcquiredBuffers must be (inclusive) between 1 and
// MAX_MAX_ACQUIRED_BUFFERS. It also cannot cause the maxBufferCount value
// to be exceeded.
//
// Return of a value other than NO_ERROR means an error has occurred:
+ // * NO_INIT - the buffer queue has been abandoned
// * BAD_VALUE - one of the below conditions occurred:
// * maxAcquiredBuffers was out of range (see above).
// * failure to adjust the number of available slots.
+ // * client would have more than the requested number of
+ // acquired buffers after this call
// * INVALID_OPERATION - attempting to call this after a producer connected.
virtual status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers) = 0;
diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h
index 1f4c8ac..265728f 100644
--- a/include/gui/IGraphicBufferProducer.h
+++ b/include/gui/IGraphicBufferProducer.h
@@ -82,15 +82,16 @@
virtual status_t requestBuffer(int slot, sp<GraphicBuffer>* buf) = 0;
// setMaxDequeuedBufferCount sets the maximum number of buffers that can be
- // dequeued by the producer at one time. If this method succeeds, buffer
- // slots will be both unallocated and owned by the BufferQueue object (i.e.
- // they are not owned by the producer or consumer). Calling this will also
- // cause all buffer slots to be emptied. If the caller is caching the
+ // dequeued by the producer at one time. If this method succeeds, any new
+ // buffer slots will be both unallocated and owned by the BufferQueue object
+ // (i.e. they are not owned by the producer or consumer). Calling this may
+ // also cause some buffer slots to be emptied. If the caller is caching the
// contents of the buffer slots, it should empty that cache after calling
// this method.
//
- // This function should not be called when there are any currently dequeued
- // buffer slots. Doing so will result in a BAD_VALUE error.
+ // This function should not be called with a value of maxDequeuedBuffers
+ // that is less than the number of currently dequeued buffer slots. Doing so
+ // will result in a BAD_VALUE error.
//
// The buffer count should be at least 1 (inclusive), but at most
// (NUM_BUFFER_SLOTS - the minimum undequeued buffer count) (exclusive). The
@@ -100,9 +101,10 @@
// Return of a value other than NO_ERROR means an error has occurred:
// * NO_INIT - the buffer queue has been abandoned.
// * BAD_VALUE - one of the below conditions occurred:
- // * bufferCount was out of range (see above)
- // * client has too many buffers dequeued
- // * this call would cause the maxBufferCount value to be exceeded
+ // * bufferCount was out of range (see above).
+ // * client would have more than the requested number of dequeued
+ // buffers after this call.
+ // * this call would cause the maxBufferCount value to be exceeded.
// * failure to adjust the number of available slots.
virtual status_t setMaxDequeuedBufferCount(int maxDequeuedBuffers) = 0;
diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 9c1bbe4..92285e5 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -611,35 +611,59 @@
return BAD_VALUE;
}
- Mutex::Autolock lock(mCore->mMutex);
+ sp<IConsumerListener> listener;
+ { // Autolock scope
+ Mutex::Autolock lock(mCore->mMutex);
+ mCore->waitWhileAllocatingLocked();
- if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
- BQ_LOGE("setMaxAcquiredBufferCount: producer is already connected");
- return INVALID_OPERATION;
+ if (mCore->mIsAbandoned) {
+ BQ_LOGE("setMaxAcquiredBufferCount: consumer is abandoned");
+ return NO_INIT;
+ }
+
+ // The new maxAcquiredBuffers count should not be violated by the number
+ // of currently acquired buffers
+ int acquiredCount = 0;
+ for (int slot : mCore->mActiveBuffers) {
+ if (mSlots[slot].mBufferState.isAcquired()) {
+ acquiredCount++;
+ }
+ }
+ if (acquiredCount > maxAcquiredBuffers) {
+ BQ_LOGE("setMaxAcquiredBufferCount: the requested maxAcquiredBuffer"
+ "count (%d) exceeds the current acquired buffer count (%d)",
+ maxAcquiredBuffers, acquiredCount);
+ return BAD_VALUE;
+ }
+
+ if ((maxAcquiredBuffers + mCore->mMaxDequeuedBufferCount +
+ (mCore->mAsyncMode || mCore->mDequeueBufferCannotBlock ? 1 : 0))
+ > mCore->mMaxBufferCount) {
+ BQ_LOGE("setMaxAcquiredBufferCount: %d acquired buffers would "
+ "exceed the maxBufferCount (%d) (maxDequeued %d async %d)",
+ maxAcquiredBuffers, mCore->mMaxBufferCount,
+ mCore->mMaxDequeuedBufferCount, mCore->mAsyncMode ||
+ mCore->mDequeueBufferCannotBlock);
+ return BAD_VALUE;
+ }
+
+ int delta = maxAcquiredBuffers - mCore->mMaxAcquiredBufferCount;
+ if (!mCore->adjustAvailableSlotsLocked(delta)) {
+ return BAD_VALUE;
+ }
+
+ BQ_LOGV("setMaxAcquiredBufferCount: %d", maxAcquiredBuffers);
+ mCore->mMaxAcquiredBufferCount = maxAcquiredBuffers;
+ VALIDATE_CONSISTENCY();
+ if (delta < 0) {
+ listener = mCore->mConsumerListener;
+ }
+ }
+ // Call back without lock held
+ if (listener != NULL) {
+ listener->onBuffersReleased();
}
- if ((maxAcquiredBuffers + mCore->mMaxDequeuedBufferCount +
- (mCore->mAsyncMode || mCore->mDequeueBufferCannotBlock ? 1 : 0)) >
- mCore->mMaxBufferCount) {
- BQ_LOGE("setMaxAcquiredBufferCount: %d acquired buffers would exceed "
- "the maxBufferCount (%d) (maxDequeued %d async %d)",
- maxAcquiredBuffers, mCore->mMaxBufferCount,
- mCore->mMaxDequeuedBufferCount, mCore->mAsyncMode ||
- mCore->mDequeueBufferCannotBlock);
- return BAD_VALUE;
- }
-
- if (!mCore->adjustAvailableSlotsLocked(
- maxAcquiredBuffers - mCore->mMaxAcquiredBufferCount)) {
- BQ_LOGE("setMaxAcquiredBufferCount: BufferQueue failed to adjust the "
- "number of available slots. Delta = %d",
- maxAcquiredBuffers - mCore->mMaxAcquiredBufferCount);
- return BAD_VALUE;
- }
-
- BQ_LOGV("setMaxAcquiredBufferCount: %d", maxAcquiredBuffers);
- mCore->mMaxAcquiredBufferCount = maxAcquiredBuffers;
- VALIDATE_CONSISTENCY();
return NO_ERROR;
}
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index 5c80694..f785db0 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -227,6 +227,10 @@
bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) {
if (delta >= 0) {
+ // If we're going to fail, do so before modifying anything
+ if (delta > static_cast<int>(mUnusedSlots.size())) {
+ return false;
+ }
while (delta > 0) {
if (mUnusedSlots.empty()) {
return false;
@@ -237,6 +241,11 @@
delta--;
}
} else {
+ // If we're going to fail, do so before modifying anything
+ if (-delta > static_cast<int>(mFreeSlots.size() +
+ mFreeBuffers.size())) {
+ return false;
+ }
while (delta < 0) {
if (!mFreeSlots.empty()) {
auto slot = mFreeSlots.begin();
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 3c06899..9d42464 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -101,13 +101,20 @@
return NO_INIT;
}
- // There must be no dequeued buffers when changing the buffer count.
+ // The new maxDequeuedBuffer count should not be violated by the number
+ // of currently dequeued buffers
+ int dequeuedCount = 0;
for (int s : mCore->mActiveBuffers) {
if (mSlots[s].mBufferState.isDequeued()) {
- BQ_LOGE("setMaxDequeuedBufferCount: buffer owned by producer");
- return BAD_VALUE;
+ dequeuedCount++;
}
}
+ if (dequeuedCount > maxDequeuedBuffers) {
+ BQ_LOGE("setMaxDequeuedBufferCount: the requested maxDequeuedBuffer"
+ "count (%d) exceeds the current dequeued buffer count (%d)",
+ maxDequeuedBuffers, dequeuedCount);
+ return BAD_VALUE;
+ }
int bufferCount = mCore->getMinUndequeuedBufferCountLocked();
bufferCount += maxDequeuedBuffers;
@@ -134,21 +141,16 @@
return BAD_VALUE;
}
- // Here we are guaranteed that the producer doesn't have any dequeued
- // buffers and will release all of its buffer references. We don't
- // clear the queue, however, so that currently queued buffers still
- // get displayed.
- if (!mCore->adjustAvailableSlotsLocked(
- maxDequeuedBuffers - mCore->mMaxDequeuedBufferCount)) {
- BQ_LOGE("setMaxDequeuedBufferCount: BufferQueue failed to adjust "
- "the number of available slots. Delta = %d",
- maxDequeuedBuffers - mCore->mMaxDequeuedBufferCount);
+ int delta = maxDequeuedBuffers - mCore->mMaxDequeuedBufferCount;
+ if (!mCore->adjustAvailableSlotsLocked(delta)) {
return BAD_VALUE;
}
mCore->mMaxDequeuedBufferCount = maxDequeuedBuffers;
VALIDATE_CONSISTENCY();
+ if (delta < 0) {
+ listener = mCore->mConsumerListener;
+ }
mCore->mDequeueCondition.broadcast();
- listener = mCore->mConsumerListener;
} // Autolock scope
// Call back without lock held
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index ac9af07..f4c47ed 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -179,6 +179,14 @@
sp<DummyConsumer> dc(new DummyConsumer);
mConsumer->consumerConnect(dc, false);
+ EXPECT_EQ(OK, mConsumer->setMaxBufferCount(10));
+ EXPECT_EQ(BAD_VALUE, mConsumer->setMaxAcquiredBufferCount(10));
+
+ IGraphicBufferProducer::QueueBufferOutput qbo;
+ mProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
+ &qbo);
+ mProducer->setMaxDequeuedBufferCount(3);
+
int minBufferCount;
ASSERT_NO_FATAL_FAILURE(GetMinUndequeuedBufferCount(&minBufferCount));
EXPECT_EQ(BAD_VALUE, mConsumer->setMaxAcquiredBufferCount(
@@ -190,8 +198,24 @@
BufferQueue::MAX_MAX_ACQUIRED_BUFFERS+1));
EXPECT_EQ(BAD_VALUE, mConsumer->setMaxAcquiredBufferCount(100));
- EXPECT_EQ(OK, mConsumer->setMaxBufferCount(5));
- EXPECT_EQ(BAD_VALUE, mConsumer->setMaxAcquiredBufferCount(5));
+ int slot;
+ sp<Fence> fence;
+ sp<GraphicBuffer> buf;
+ IGraphicBufferProducer::QueueBufferInput qbi(0, false,
+ HAL_DATASPACE_UNKNOWN, Rect(0, 0, 1, 1),
+ NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE);
+ BufferItem item;
+ EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(3));
+ for (int i = 0; i < 3; i++) {
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+ mProducer->dequeueBuffer(&slot, &fence, 1, 1, 0,
+ GRALLOC_USAGE_SW_READ_OFTEN));
+ ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buf));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, qbi, &qbo));
+ ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+ }
+
+ EXPECT_EQ(BAD_VALUE, mConsumer->setMaxAcquiredBufferCount(2));
}
TEST_F(BufferQueueTest, SetMaxAcquiredBufferCountWithLegalValues_Succeeds) {
@@ -199,12 +223,44 @@
sp<DummyConsumer> dc(new DummyConsumer);
mConsumer->consumerConnect(dc, false);
+ IGraphicBufferProducer::QueueBufferOutput qbo;
+ mProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
+ &qbo);
+ mProducer->setMaxDequeuedBufferCount(2);
+
int minBufferCount;
ASSERT_NO_FATAL_FAILURE(GetMinUndequeuedBufferCount(&minBufferCount));
EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(1));
EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(2));
EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(minBufferCount));
+
+ int slot;
+ sp<Fence> fence;
+ sp<GraphicBuffer> buf;
+ IGraphicBufferProducer::QueueBufferInput qbi(0, false,
+ HAL_DATASPACE_UNKNOWN, Rect(0, 0, 1, 1),
+ NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE);
+ BufferItem item;
+
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+ mProducer->dequeueBuffer(&slot, &fence, 1, 1, 0,
+ GRALLOC_USAGE_SW_READ_OFTEN));
+ ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buf));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, qbi, &qbo));
+ ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+
+ EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(3));
+
+ for (int i = 0; i < 2; i++) {
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+ mProducer->dequeueBuffer(&slot, &fence, 1, 1, 0,
+ GRALLOC_USAGE_SW_READ_OFTEN));
+ ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buf));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, qbi, &qbo));
+ ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+ }
+
EXPECT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(
BufferQueue::MAX_MAX_ACQUIRED_BUFFERS));
}
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index 882b14c..45b6463 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -502,31 +502,30 @@
ASSERT_OK(mProducer->setMaxDequeuedBufferCount(minBuffers))
<< "bufferCount: " << minBuffers;
- std::vector<DequeueBufferResult> dequeueList;
-
// Should now be able to dequeue up to minBuffers times
+ DequeueBufferResult result;
for (int i = 0; i < minBuffers; ++i) {
- DequeueBufferResult result;
EXPECT_EQ(OK, ~IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION &
(dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
TEST_PRODUCER_USAGE_BITS, &result)))
<< "iteration: " << i << ", slot: " << result.slot;
-
- dequeueList.push_back(result);
- }
-
- // Cancel every buffer, so we can set buffer count again
- for (auto& result : dequeueList) {
- mProducer->cancelBuffer(result.slot, result.fence);
}
ASSERT_OK(mProducer->setMaxDequeuedBufferCount(maxBuffers));
+ // queue the first buffer to enable max dequeued buffer count checking
+ IGraphicBufferProducer::QueueBufferInput input = CreateBufferInput();
+ IGraphicBufferProducer::QueueBufferOutput output;
+ sp<GraphicBuffer> buffer;
+ ASSERT_OK(mProducer->requestBuffer(result.slot, &buffer));
+ ASSERT_OK(mProducer->queueBuffer(result.slot, input, &output));
+
+
// Should now be able to dequeue up to maxBuffers times
+ int dequeuedSlot = -1;
+ sp<Fence> dequeuedFence;
for (int i = 0; i < maxBuffers; ++i) {
- int dequeuedSlot = -1;
- sp<Fence> dequeuedFence;
EXPECT_EQ(OK, ~IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION &
(mProducer->dequeueBuffer(&dequeuedSlot, &dequeuedFence,
@@ -535,6 +534,12 @@
TEST_PRODUCER_USAGE_BITS)))
<< "iteration: " << i << ", slot: " << dequeuedSlot;
}
+
+ // Cancel a buffer, so we can decrease the buffer count
+ ASSERT_OK(mProducer->cancelBuffer(dequeuedSlot, dequeuedFence));
+
+ // Should now be able to decrease the max dequeued count by 1
+ ASSERT_OK(mProducer->setMaxDequeuedBufferCount(maxBuffers-1));
}
TEST_F(IGraphicBufferProducerTest, SetMaxDequeuedBufferCount_Fails) {
@@ -553,11 +558,12 @@
EXPECT_EQ(BAD_VALUE, mProducer->setMaxDequeuedBufferCount(maxBuffers + 1))
<< "bufferCount: " << maxBuffers + 1;
- // Prerequisite to fail out a valid setBufferCount call
- {
- int dequeuedSlot = -1;
- sp<Fence> dequeuedFence;
-
+ // Set max dequeue count to 2
+ ASSERT_OK(mProducer->setMaxDequeuedBufferCount(2));
+ // Dequeue 2 buffers
+ int dequeuedSlot = -1;
+ sp<Fence> dequeuedFence;
+ for (int i = 0; i < 2; i++) {
ASSERT_EQ(OK, ~IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION &
(mProducer->dequeueBuffer(&dequeuedSlot, &dequeuedFence,
DEFAULT_WIDTH, DEFAULT_HEIGHT,
@@ -566,8 +572,8 @@
<< "slot: " << dequeuedSlot;
}
- // Client has one or more buffers dequeued
- EXPECT_EQ(BAD_VALUE, mProducer->setMaxDequeuedBufferCount(minBuffers))
+ // Client has too many buffers dequeued
+ EXPECT_EQ(BAD_VALUE, mProducer->setMaxDequeuedBufferCount(1))
<< "bufferCount: " << minBuffers;
// Abandon buffer queue