BQ: Get rid of setBufferCount
- Remove setBufferCount from BufferQueueProducer and
IGraphicsBufferQueueProducer.
- Get rid of the unit tests for it.
- In Surface, convert setBufferCount calls into calls to
setMaxDequeuedBufferCount.
- Change mOverrideMaxBufferCount to a boolean since it can now be
derived from mMaxAcquiredBufferCount, mMaxDequeuedBufferCount, and
mAsyncMode.
Bug 13174928
Change-Id: Ia0adc737fae9e13f186df832b8428a0829041bf9
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index 70d80a7..d297112 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -55,7 +55,6 @@
mQueue(),
mFreeSlots(),
mFreeBuffers(),
- mOverrideMaxBufferCount(0),
mDequeueCondition(),
mUseAsyncBuffer(true),
mDequeueBufferCannotBlock(false),
@@ -66,6 +65,7 @@
mDefaultMaxBufferCount(2),
mMaxAcquiredBufferCount(1),
mMaxDequeuedBufferCount(1),
+ mOverrideMaxBufferCount(false),
mBufferHasBeenQueued(false),
mFrameCounter(0),
mTransformHint(0),
@@ -164,9 +164,11 @@
int minMaxBufferCount = getMinMaxBufferCountLocked(async);
int maxBufferCount = max(mDefaultMaxBufferCount, minMaxBufferCount);
- if (mOverrideMaxBufferCount != 0) {
- assert(mOverrideMaxBufferCount >= minMaxBufferCount);
- maxBufferCount = mOverrideMaxBufferCount;
+ if (mOverrideMaxBufferCount) {
+ int bufferCount = mMaxAcquiredBufferCount + mMaxDequeuedBufferCount +
+ (async ? 1 : 0);
+ assert(bufferCount >= minMaxBufferCount);
+ maxBufferCount = bufferCount;
}
// Any buffers that are dequeued by the producer or sitting in the queue
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 5ea4a10..61878f6 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -72,66 +72,6 @@
return NO_ERROR;
}
-status_t BufferQueueProducer::setBufferCount(int bufferCount) {
- ATRACE_CALL();
- BQ_LOGV("setBufferCount: count = %d", bufferCount);
-
- sp<IConsumerListener> listener;
- { // Autolock scope
- Mutex::Autolock lock(mCore->mMutex);
- mCore->waitWhileAllocatingLocked();
-
- if (mCore->mIsAbandoned) {
- BQ_LOGE("setBufferCount: BufferQueue has been abandoned");
- return NO_INIT;
- }
-
- if (bufferCount > BufferQueueDefs::NUM_BUFFER_SLOTS) {
- BQ_LOGE("setBufferCount: bufferCount %d too large (max %d)",
- bufferCount, BufferQueueDefs::NUM_BUFFER_SLOTS);
- return BAD_VALUE;
- }
-
- // There must be no dequeued buffers when changing the buffer count.
- for (int s = 0; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
- if (mSlots[s].mBufferState == BufferSlot::DEQUEUED) {
- BQ_LOGE("setBufferCount: buffer owned by producer");
- return BAD_VALUE;
- }
- }
-
- if (bufferCount == 0) {
- mCore->mOverrideMaxBufferCount = 0;
- mCore->mDequeueCondition.broadcast();
- return NO_ERROR;
- }
-
- const int minBufferSlots = mCore->getMinMaxBufferCountLocked(false);
- if (bufferCount < minBufferSlots) {
- BQ_LOGE("setBufferCount: requested buffer count %d is less than "
- "minimum %d", bufferCount, minBufferSlots);
- 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.
- mCore->freeAllBuffersLocked();
- mCore->mMaxDequeuedBufferCount = bufferCount - minBufferSlots + 1;
- mCore->mOverrideMaxBufferCount = bufferCount;
- mCore->mDequeueCondition.broadcast();
- listener = mCore->mConsumerListener;
- } // Autolock scope
-
- // Call back without lock held
- if (listener != NULL) {
- listener->onBuffersReleased();
- }
-
- return NO_ERROR;
-}
-
status_t BufferQueueProducer::setMaxDequeuedBufferCount(
int maxDequeuedBuffers) {
ATRACE_CALL();
@@ -181,7 +121,7 @@
// get displayed.
mCore->freeAllBuffersLocked();
mCore->mMaxDequeuedBufferCount = maxDequeuedBuffers;
- mCore->mOverrideMaxBufferCount = bufferCount;
+ mCore->mOverrideMaxBufferCount = true;
mCore->mDequeueCondition.broadcast();
listener = mCore->mConsumerListener;
} // Autolock scope
@@ -217,6 +157,7 @@
}
mCore->mAsyncMode = async;
+ mCore->mOverrideMaxBufferCount = true;
mCore->mDequeueCondition.broadcast();
listener = mCore->mConsumerListener;
} // Autolock scope
@@ -238,16 +179,6 @@
}
const int maxBufferCount = mCore->getMaxBufferCountLocked(async);
- if (async && mCore->mOverrideMaxBufferCount) {
- // FIXME: Some drivers are manually setting the buffer count
- // (which they shouldn't), so we do this extra test here to
- // handle that case. This is TEMPORARY until we get this fixed.
- if (mCore->mOverrideMaxBufferCount < maxBufferCount) {
- BQ_LOGE("%s: async mode is invalid with buffer count override",
- caller);
- return BAD_VALUE;
- }
- }
// Free up any buffers that are in slots beyond the max buffer count
for (int s = maxBufferCount; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
@@ -273,33 +204,16 @@
}
}
- // Producers are not allowed to dequeue more than one buffer if they
- // did not set a buffer count
- if (!mCore->mOverrideMaxBufferCount && dequeuedCount) {
- BQ_LOGE("%s: can't dequeue multiple buffers without setting the "
- "buffer count", caller);
+ // Producers are not allowed to dequeue more than
+ // mMaxDequeuedBufferCount buffers.
+ // This check is only done if a buffer has already been queued
+ if (mCore->mBufferHasBeenQueued &&
+ dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
+ BQ_LOGE("%s: attempting to exceed the max dequeued buffer count "
+ "(%d)", caller, mCore->mMaxDequeuedBufferCount);
return INVALID_OPERATION;
}
- // See whether a buffer has been queued since the last
- // setBufferCount so we know whether to perform the min undequeued
- // buffers check below
- if (mCore->mBufferHasBeenQueued) {
- // Make sure the producer is not trying to dequeue more buffers
- // than allowed
- const int newUndequeuedCount =
- maxBufferCount - (dequeuedCount + 1);
- const int minUndequeuedCount =
- mCore->getMinUndequeuedBufferCountLocked(async);
- if (newUndequeuedCount < minUndequeuedCount) {
- BQ_LOGE("%s: min undequeued buffer count (%d) exceeded "
- "(dequeued=%d undequeued=%d)",
- caller, minUndequeuedCount,
- dequeuedCount, newUndequeuedCount);
- return INVALID_OPERATION;
- }
- }
-
*found = BufferQueueCore::INVALID_BUFFER_SLOT;
// If we disconnect and reconnect quickly, we can be in a state where
@@ -683,16 +597,6 @@
}
const int maxBufferCount = mCore->getMaxBufferCountLocked(async);
- if (async && mCore->mOverrideMaxBufferCount) {
- // FIXME: Some drivers are manually setting the buffer count
- // (which they shouldn't), so we do this extra test here to
- // handle that case. This is TEMPORARY until we get this fixed.
- if (mCore->mOverrideMaxBufferCount < maxBufferCount) {
- BQ_LOGE("queueBuffer: async mode is invalid with "
- "buffer count override");
- return BAD_VALUE;
- }
- }
if (slot < 0 || slot >= maxBufferCount) {
BQ_LOGE("queueBuffer: slot index %d out of range [0, %d)",
diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp
index 4a6bada..6495aa9 100644
--- a/libs/gui/IGraphicBufferProducer.cpp
+++ b/libs/gui/IGraphicBufferProducer.cpp
@@ -34,7 +34,6 @@
enum {
REQUEST_BUFFER = IBinder::FIRST_CALL_TRANSACTION,
- SET_BUFFER_COUNT,
DEQUEUE_BUFFER,
DETACH_BUFFER,
DETACH_NEXT_BUFFER,
@@ -84,19 +83,6 @@
return result;
}
- virtual status_t setBufferCount(int bufferCount)
- {
- Parcel data, reply;
- data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());
- data.writeInt32(bufferCount);
- status_t result =remote()->transact(SET_BUFFER_COUNT, data, &reply);
- if (result != NO_ERROR) {
- return result;
- }
- result = reply.readInt32();
- return result;
- }
-
virtual status_t setMaxDequeuedBufferCount(int maxDequeuedBuffers) {
Parcel data, reply;
data.writeInterfaceToken(
@@ -364,13 +350,6 @@
reply->writeInt32(result);
return NO_ERROR;
}
- case SET_BUFFER_COUNT: {
- CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
- int bufferCount = data.readInt32();
- int result = setBufferCount(bufferCount);
- reply->writeInt32(result);
- return NO_ERROR;
- }
case SET_MAX_DEQUEUED_BUFFER_COUNT: {
CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
int maxDequeuedBuffers = data.readInt32();
diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index 2a9273d..27f7ed6 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -800,14 +800,26 @@
ALOGV("Surface::setBufferCount");
Mutex::Autolock lock(mMutex);
- status_t err = mGraphicBufferProducer->setBufferCount(bufferCount);
- ALOGE_IF(err, "IGraphicBufferProducer::setBufferCount(%d) returned %s",
- bufferCount, strerror(-err));
+ status_t err = NO_ERROR;
+ if (bufferCount == 0) {
+ err = mGraphicBufferProducer->setMaxDequeuedBufferCount(1);
+ } else {
+ int minUndequeuedBuffers = 0;
+ err = mGraphicBufferProducer->query(
+ NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, &minUndequeuedBuffers);
+ if (err == NO_ERROR) {
+ err = mGraphicBufferProducer->setMaxDequeuedBufferCount(
+ bufferCount - minUndequeuedBuffers);
+ }
+ }
if (err == NO_ERROR) {
freeAllBuffers();
}
+ ALOGE_IF(err, "IGraphicBufferProducer::setBufferCount(%d) returned %s",
+ bufferCount, strerror(-err));
+
return err;
}
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index ced1555..d6bd8a8 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -481,94 +481,6 @@
mProducer->cancelBuffer(dequeuedSlot, dequeuedFence);
}
-TEST_F(IGraphicBufferProducerTest, SetBufferCount_Succeeds) {
-
- // The producer does not wish to set a buffer count
- EXPECT_OK(mProducer->setBufferCount(0)) << "bufferCount: " << 0;
- // TODO: how to test "0" buffer count?
-
- int minBuffers;
- ASSERT_OK(mProducer->query(NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, &minBuffers));
-
- // The MIN_UNDEQUEUED_BUFFERS limit is exclusive, so need to increment by at least 1
- minBuffers++;
-
- ASSERT_OK(mProducer->setBufferCount(minBuffers)) << "bufferCount: " << minBuffers;
-
- std::vector<DequeueBufferResult> dequeueList;
-
- // Should now be able to dequeue up to minBuffers times
- for (int i = 0; i < minBuffers; ++i) {
- DequeueBufferResult result;
-
- EXPECT_LE(OK,
- dequeueBuffer(QUEUE_BUFFER_INPUT_ASYNC,
- 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 (int i = 0; i < minBuffers; ++i) {
- DequeueBufferResult& result = dequeueList[i];
- mProducer->cancelBuffer(result.slot, result.fence);
- }
-
- ASSERT_OK(mProducer->setBufferCount(BufferQueue::NUM_BUFFER_SLOTS));
-
- // Should now be able to dequeue up to NUM_BUFFER_SLOTS times
- for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; ++i) {
- int dequeuedSlot = -1;
- sp<Fence> dequeuedFence;
-
- EXPECT_LE(OK,
- mProducer->dequeueBuffer(&dequeuedSlot, &dequeuedFence,
- QUEUE_BUFFER_INPUT_ASYNC,
- DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
- TEST_PRODUCER_USAGE_BITS))
- << "iteration: " << i << ", slot: " << dequeuedSlot;
- }
-}
-
-TEST_F(IGraphicBufferProducerTest, SetBufferCount_Fails) {
- int minBuffers;
- ASSERT_OK(mProducer->query(NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, &minBuffers));
-
- // The MIN_UNDEQUEUED_BUFFERS limit is exclusive, so need to increment by at least 1
- minBuffers++;
-
- // Buffer count was out of range
- EXPECT_EQ(BAD_VALUE, mProducer->setBufferCount(-1)) << "bufferCount: " << -1;
- EXPECT_EQ(BAD_VALUE, mProducer->setBufferCount(minBuffers - 1)) << "bufferCount: " << minBuffers - 1;
- EXPECT_EQ(BAD_VALUE, mProducer->setBufferCount(BufferQueue::NUM_BUFFER_SLOTS + 1))
- << "bufferCount: " << BufferQueue::NUM_BUFFER_SLOTS + 1;
-
- // Pre-requisite to fail out a valid setBufferCount call
- {
- int dequeuedSlot = -1;
- sp<Fence> dequeuedFence;
-
- ASSERT_LE(OK,
- mProducer->dequeueBuffer(&dequeuedSlot, &dequeuedFence,
- QUEUE_BUFFER_INPUT_ASYNC,
- DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
- TEST_PRODUCER_USAGE_BITS))
- << "slot: " << dequeuedSlot;
- }
-
- // Client has one or more buffers dequeued
- EXPECT_EQ(BAD_VALUE, mProducer->setBufferCount(minBuffers)) << "bufferCount: " << minBuffers;
-
- // Abandon buffer queue
- ASSERT_OK(mConsumer->consumerDisconnect());
-
- // Fail because the buffer queue was abandoned
- EXPECT_EQ(NO_INIT, mProducer->setBufferCount(minBuffers)) << "bufferCount: " << minBuffers;
-
-}
-
TEST_F(IGraphicBufferProducerTest, SetMaxDequeuedBufferCount_Succeeds) {
int minUndequeuedBuffers;
ASSERT_OK(mProducer->query(NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS,