Skip to content

Commit a62f051

Browse files
author
Jim Shargo
committed
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
1 parent 6075c6a commit a62f051

3 files changed

Lines changed: 61 additions & 2 deletions

File tree

libs/gui/BufferQueueProducer.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,10 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller,
364364
// Producers are not allowed to dequeue more than
365365
// mMaxDequeuedBufferCount buffers.
366366
// This check is only done if a buffer has already been queued
367-
if (mCore->mBufferHasBeenQueued &&
368-
dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
367+
using namespace com::android::graphics::libgui::flags;
368+
bool flagGatedBufferHasBeenQueued =
369+
bq_always_use_max_dequeued_buffer_count() || mCore->mBufferHasBeenQueued;
370+
if (flagGatedBufferHasBeenQueued && dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
369371
// Supress error logs when timeout is non-negative.
370372
if (mDequeueTimeout < 0) {
371373
BQ_LOGE("%s: attempting to exceed the max dequeued buffer "

libs/gui/libgui_flags.aconfig

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,3 +153,14 @@ flag {
153153
}
154154
is_fixed_read_only: true
155155
} # allocate_buffer_priority
156+
157+
flag {
158+
name: "bq_always_use_max_dequeued_buffer_count"
159+
namespace: "core_graphics"
160+
description: "BufferQueueProducer::dequeue's respects setMaxDequeuedBufferCount even before a buffer is dequeued."
161+
bug: "399328309"
162+
metadata {
163+
purpose: PURPOSE_BUGFIX
164+
}
165+
is_fixed_read_only: true
166+
} # bq_always_use_max_dequeued_buffer_count

libs/gui/tests/Surface_test.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2246,6 +2246,52 @@ TEST_F(SurfaceTest, BatchIllegalOperations) {
22462246
ASSERT_EQ(NO_ERROR, surface->disconnect(NATIVE_WINDOW_API_CPU));
22472247
}
22482248

2249+
TEST_F(SurfaceTest, setMaxDequeuedBufferCount_setMaxAcquiredBufferCount_allocations) {
2250+
//
2251+
// Set up the consumer and producer--nothing fancy.
2252+
//
2253+
auto [consumer, surface] =
2254+
BufferItemConsumer::create(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_HW_RENDER);
2255+
sp<SurfaceListener> surfaceListener = sp<StubSurfaceListener>::make();
2256+
surface->connect(NATIVE_WINDOW_API_CPU, surfaceListener);
2257+
sp<GraphicBuffer> buffer;
2258+
sp<Fence> fence;
2259+
2260+
//
2261+
// These values are independent. The consumer can dequeue 3 and the consumer can acquire 3 at
2262+
// the same time.
2263+
//
2264+
ASSERT_EQ(OK, consumer->setMaxAcquiredBufferCount(3));
2265+
ASSERT_EQ(OK, surface->setMaxDequeuedBufferCount(3));
2266+
2267+
//
2268+
// Take all three buffers out of the queue--a fourth can't be retrieved. Then queue them.
2269+
//
2270+
std::vector<Surface::BatchBuffer> dequeuedBuffers(3);
2271+
EXPECT_EQ(OK, surface->dequeueBuffers(&dequeuedBuffers));
2272+
if (::com::android::graphics::libgui::flags::bq_always_use_max_dequeued_buffer_count()) {
2273+
EXPECT_EQ(INVALID_OPERATION, surface->dequeueBuffer(&buffer, &fence));
2274+
}
2275+
2276+
for (auto& batchBuffer : dequeuedBuffers) {
2277+
EXPECT_EQ(OK,
2278+
surface->queueBuffer(GraphicBuffer::from(batchBuffer.buffer),
2279+
sp<Fence>::make(batchBuffer.fenceFd)));
2280+
}
2281+
dequeuedBuffers.assign(3, {});
2282+
2283+
//
2284+
// Acquire all three, then we should be able to dequeue 3 more.
2285+
//
2286+
std::vector<BufferItem> acquiredBuffers(3);
2287+
for (auto& bufferItem : acquiredBuffers) {
2288+
EXPECT_EQ(OK, consumer->acquireBuffer(&bufferItem, 0));
2289+
}
2290+
2291+
EXPECT_EQ(OK, surface->dequeueBuffers(&dequeuedBuffers));
2292+
EXPECT_EQ(INVALID_OPERATION, surface->dequeueBuffer(&buffer, &fence));
2293+
}
2294+
22492295
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
22502296

22512297
TEST_F(SurfaceTest, PlatformBufferMethods) {

0 commit comments

Comments
 (0)