commit 22f842ba04c32cef2faf855dc304eb0ab131b9ec
author Shuzhen Wang <shuzhenwang@google.com> Wed Jan 18 23:02:36 2017 -0800
committer Shuzhen Wang <shuzhenwang@google.com> Fri Jan 20 11:26:13 2017 -0800
tree dd6cc0fb014748838c5c39b1b91480999514b96c
parent 90b25ed559f3d04777dbbc195ad5c95287990bf7

BufferQueue: Add bufferReplaced flag in QueueBufferOutput

For async buffer queue, when queueBuffer overwrites a previously queued
buffer, because consumer won't be able to acquire the overwritten
buffer, onBufferReleased isn't triggered either.

This makes it difficult to track which buffer in the buffer queue
becomes free other than calling dequeueBuffer(). Adding a flag in
QueueBufferOutput addresses this issue.

Test: TestBufferReplacedInQueueBuffer in BufferQueue_tests
Bug: 34481539
Change-Id: Ic7f4be2d3f9691dd6b007a450240614c0c420f0d

libs/gui/BufferQueueProducer.cpp

diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 39b9a0b..3f69b1f 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -879,6 +879,7 @@
             mCore->mSharedBufferCache.dataspace = dataSpace;
         }
 
+        output->bufferReplaced = false;
         if (mCore->mQueue.empty()) {
             // When the queue is empty, we can ignore mDequeueBufferCannotBlock
             // and simply queue this buffer
@@ -905,6 +906,7 @@
                     if (!mSlots[last.mSlot].mBufferState.isShared()) {
                         mCore->mActiveBuffers.erase(last.mSlot);
                         mCore->mFreeBuffers.push_back(last.mSlot);
+                        output->bufferReplaced = true;
                     }
                 }
 
@@ -1158,6 +1160,7 @@
             output->numPendingBuffers =
                     static_cast<uint32_t>(mCore->mQueue.size());
             output->nextFrameNumber = mCore->mFrameCounter + 1;
+            output->bufferReplaced = false;
 
             if (listener != NULL) {
                 // Set up a death notification so that we can disconnect

libs/gui/IGraphicBufferProducer.cpp

diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp
index e37b65b..1799e0b 100644
--- a/libs/gui/IGraphicBufferProducer.cpp
+++ b/libs/gui/IGraphicBufferProducer.cpp
@@ -830,7 +830,8 @@
             sizeof(height) +
             sizeof(transformHint) +
             sizeof(numPendingBuffers) +
-            sizeof(nextFrameNumber);
+            sizeof(nextFrameNumber) +
+            sizeof(bufferReplaced);
 }
 
 size_t IGraphicBufferProducer::QueueBufferOutput::getFlattenedSize() const {
@@ -853,6 +854,7 @@
     FlattenableUtils::write(buffer, size, transformHint);
     FlattenableUtils::write(buffer, size, numPendingBuffers);
     FlattenableUtils::write(buffer, size, nextFrameNumber);
+    FlattenableUtils::write(buffer, size, bufferReplaced);
 
     return frameTimestamps.flatten(buffer, size, fds, count);
 }
@@ -869,6 +871,7 @@
     FlattenableUtils::read(buffer, size, transformHint);
     FlattenableUtils::read(buffer, size, numPendingBuffers);
     FlattenableUtils::read(buffer, size, nextFrameNumber);
+    FlattenableUtils::read(buffer, size, bufferReplaced);
 
     return frameTimestamps.unflatten(buffer, size, fds, count);
 }

libs/gui/tests/BufferQueue_test.cpp

diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index 98c0449..91ce531 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -1076,4 +1076,45 @@
     }
 }
 
+TEST_F(BufferQueueTest, TestBufferReplacedInQueueBuffer) {
+    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));
+    ASSERT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(1));
+
+    int slot = BufferQueue::INVALID_BUFFER_SLOT;
+    sp<Fence> fence = Fence::NO_FENCE;
+    sp<GraphicBuffer> buffer = nullptr;
+    IGraphicBufferProducer::QueueBufferInput input(0ull, true,
+        HAL_DATASPACE_UNKNOWN, Rect::INVALID_RECT,
+        NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE);
+    BufferItem item{};
+
+    // Preallocate, dequeue, request, and cancel 2 buffers so we don't get
+    // BUFFER_NEEDS_REALLOCATION below
+    int slots[2] = {};
+    ASSERT_EQ(OK, mProducer->setMaxDequeuedBufferCount(2));
+    for (size_t i = 0; i < 2; ++i) {
+        status_t result = mProducer->dequeueBuffer(&slots[i], &fence,
+                0, 0, 0, 0, nullptr);
+        ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result);
+        ASSERT_EQ(OK, mProducer->requestBuffer(slots[i], &buffer));
+    }
+    for (size_t i = 0; i < 2; ++i) {
+        ASSERT_EQ(OK, mProducer->cancelBuffer(slots[i], Fence::NO_FENCE));
+    }
+
+    // Fill 2 buffers without consumer consuming them. Verify that all
+    // queued buffer returns proper bufferReplaced flag
+    ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0, nullptr));
+    ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+    ASSERT_EQ(false, output.bufferReplaced);
+    ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0, nullptr));
+    ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+    ASSERT_EQ(true, output.bufferReplaced);
+}
+
 } // namespace android