BufferQueues: Always respect setMaxDequeuedBufferCount
For some reason, this wouldn't be respected if no buffers had been
queued. This could lead to surprising behavior of a (with unlimited
buffers, essentially unbounded) buffers being dequeued for new BQs.
This is flag gated to capture potential issues, since I'm worried about
chesterton's fence.
Bug: 399328309
Flag: com.android.graphics.libgui.flags.bq_always_use_max_dequeued_buffer_count
Test: new tests
Change-Id: I756eb4f621f6eec1a5bb43a490305018ec69c477
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 5961b41..bcf61e4 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -364,8 +364,10 @@
// 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) {
+ using namespace com::android::graphics::libgui::flags;
+ bool flagGatedBufferHasBeenQueued =
+ bq_always_use_max_dequeued_buffer_count() || mCore->mBufferHasBeenQueued;
+ if (flagGatedBufferHasBeenQueued && dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
// Supress error logs when timeout is non-negative.
if (mDequeueTimeout < 0) {
BQ_LOGE("%s: attempting to exceed the max dequeued buffer "
diff --git a/libs/gui/libgui_flags.aconfig b/libs/gui/libgui_flags.aconfig
index 2c3222d..a893b84 100644
--- a/libs/gui/libgui_flags.aconfig
+++ b/libs/gui/libgui_flags.aconfig
@@ -153,3 +153,14 @@
}
is_fixed_read_only: true
} # allocate_buffer_priority
+
+flag {
+ name: "bq_always_use_max_dequeued_buffer_count"
+ namespace: "core_graphics"
+ description: "BufferQueueProducer::dequeue's respects setMaxDequeuedBufferCount even before a buffer is dequeued."
+ bug: "399328309"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+ is_fixed_read_only: true
+} # bq_always_use_max_dequeued_buffer_count
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index e7690e2..c479662 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -2246,6 +2246,52 @@
ASSERT_EQ(NO_ERROR, surface->disconnect(NATIVE_WINDOW_API_CPU));
}
+TEST_F(SurfaceTest, setMaxDequeuedBufferCount_setMaxAcquiredBufferCount_allocations) {
+ //
+ // Set up the consumer and producer--nothing fancy.
+ //
+ auto [consumer, surface] =
+ BufferItemConsumer::create(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_HW_RENDER);
+ sp<SurfaceListener> surfaceListener = sp<StubSurfaceListener>::make();
+ surface->connect(NATIVE_WINDOW_API_CPU, surfaceListener);
+ sp<GraphicBuffer> buffer;
+ sp<Fence> fence;
+
+ //
+ // These values are independent. The consumer can dequeue 3 and the consumer can acquire 3 at
+ // the same time.
+ //
+ ASSERT_EQ(OK, consumer->setMaxAcquiredBufferCount(3));
+ ASSERT_EQ(OK, surface->setMaxDequeuedBufferCount(3));
+
+ //
+ // Take all three buffers out of the queue--a fourth can't be retrieved. Then queue them.
+ //
+ std::vector<Surface::BatchBuffer> dequeuedBuffers(3);
+ EXPECT_EQ(OK, surface->dequeueBuffers(&dequeuedBuffers));
+ if (::com::android::graphics::libgui::flags::bq_always_use_max_dequeued_buffer_count()) {
+ EXPECT_EQ(INVALID_OPERATION, surface->dequeueBuffer(&buffer, &fence));
+ }
+
+ for (auto& batchBuffer : dequeuedBuffers) {
+ EXPECT_EQ(OK,
+ surface->queueBuffer(GraphicBuffer::from(batchBuffer.buffer),
+ sp<Fence>::make(batchBuffer.fenceFd)));
+ }
+ dequeuedBuffers.assign(3, {});
+
+ //
+ // Acquire all three, then we should be able to dequeue 3 more.
+ //
+ std::vector<BufferItem> acquiredBuffers(3);
+ for (auto& bufferItem : acquiredBuffers) {
+ EXPECT_EQ(OK, consumer->acquireBuffer(&bufferItem, 0));
+ }
+
+ EXPECT_EQ(OK, surface->dequeueBuffers(&dequeuedBuffers));
+ EXPECT_EQ(INVALID_OPERATION, surface->dequeueBuffer(&buffer, &fence));
+}
+
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
TEST_F(SurfaceTest, PlatformBufferMethods) {