DO NOT MERGE ANYWHERE: BufferQueue consumers: Add discardFreeBuffer method
This method releases all free buffers owned by the buffer queue,
in order to save memory (at the cost of potential future
reallocation of buffers).
Bug: 28695173
Change-Id: I458d10373e639e3144faf673af2ba01aca36e65a
(cherry picked from commit 8211047138ea7892c73f4e6f6291a85a11759e0c)
diff --git a/include/gui/BufferQueueConsumer.h b/include/gui/BufferQueueConsumer.h
index b2daae4..3b681ee 100644
--- a/include/gui/BufferQueueConsumer.h
+++ b/include/gui/BufferQueueConsumer.h
@@ -136,6 +136,9 @@
// Retrieve the sideband buffer stream, if any.
virtual sp<NativeHandle> getSidebandStream() const;
+ // See IGraphicBufferConsumer::discardFreeBuffers
+ virtual status_t discardFreeBuffers() override;
+
// dump our state in a String
virtual void dump(String8& result, const char* prefix) const;
diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h
index 0371db7..35c9170 100644
--- a/include/gui/BufferQueueCore.h
+++ b/include/gui/BufferQueueCore.h
@@ -124,6 +124,11 @@
// all slots, even if they're currently dequeued, queued, or acquired.
void freeAllBuffersLocked();
+ // discardFreeBuffersLocked releases all currently-free buffers held by the
+ // queue, in order to reduce the memory consumption of the queue to the
+ // minimum possible without discarding data.
+ void discardFreeBuffersLocked();
+
// If delta is positive, makes more slots available. If negative, takes
// away slots. Returns false if the request can't be met.
bool adjustAvailableSlotsLocked(int delta);
diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 9307a26..ba86478 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -85,6 +85,9 @@
// See IGraphicBufferConsumer::setDefaultBufferDataSpace
status_t setDefaultBufferDataSpace(android_dataspace defaultDataSpace);
+ // See IGraphicBufferConsumer::discardFreeBuffers
+ status_t discardFreeBuffers();
+
private:
ConsumerBase(const ConsumerBase&);
void operator=(const ConsumerBase&);
diff --git a/include/gui/IGraphicBufferConsumer.h b/include/gui/IGraphicBufferConsumer.h
index e983c16..f03d1ce 100644
--- a/include/gui/IGraphicBufferConsumer.h
+++ b/include/gui/IGraphicBufferConsumer.h
@@ -265,6 +265,11 @@
// Retrieve the sideband buffer stream, if any.
virtual sp<NativeHandle> getSidebandStream() const = 0;
+ // discardFreeBuffers releases all currently-free buffers held by the queue,
+ // in order to reduce the memory consumption of the queue to the minimum
+ // possible without discarding data.
+ virtual status_t discardFreeBuffers() = 0;
+
// dump state into a string
virtual void dump(String8& result, const char* prefix) const = 0;
diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index cbc8893..525938e 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -717,6 +717,12 @@
return mCore->mSidebandStream;
}
+status_t BufferQueueConsumer::discardFreeBuffers() {
+ Mutex::Autolock lock(mCore->mMutex);
+ mCore->discardFreeBuffersLocked();
+ return NO_ERROR;
+}
+
void BufferQueueConsumer::dump(String8& result, const char* prefix) const {
const IPCThreadState* ipc = IPCThreadState::self();
const pid_t pid = ipc->getCallingPid();
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index 569b8f9..9cb9c62 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -244,6 +244,16 @@
VALIDATE_CONSISTENCY();
}
+void BufferQueueCore::discardFreeBuffersLocked() {
+ for (int s : mFreeBuffers) {
+ mFreeSlots.insert(s);
+ clearBufferSlotLocked(s);
+ }
+ mFreeBuffers.clear();
+
+ VALIDATE_CONSISTENCY();
+}
+
bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) {
if (delta >= 0) {
// If we're going to fail, do so before modifying anything
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index a6a9712..c00526d 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -235,6 +235,15 @@
return mConsumer->setDefaultBufferDataSpace(defaultDataSpace);
}
+status_t ConsumerBase::discardFreeBuffers() {
+ Mutex::Autolock _l(mMutex);
+ if (mAbandoned) {
+ CB_LOGE("discardFreeBuffers: ConsumerBase is abandoned!");
+ return NO_INIT;
+ }
+ return mConsumer->discardFreeBuffers();
+}
+
void ConsumerBase::dump(String8& result) const {
dump(result, "");
}
diff --git a/libs/gui/IGraphicBufferConsumer.cpp b/libs/gui/IGraphicBufferConsumer.cpp
index cb1ad35..af6800b 100644
--- a/libs/gui/IGraphicBufferConsumer.cpp
+++ b/libs/gui/IGraphicBufferConsumer.cpp
@@ -51,6 +51,7 @@
SET_CONSUMER_USAGE_BITS,
SET_TRANSFORM_HINT,
GET_SIDEBAND_STREAM,
+ DISCARD_FREE_BUFFERS,
DUMP,
};
@@ -260,6 +261,21 @@
return stream;
}
+ virtual status_t discardFreeBuffers() {
+ Parcel data, reply;
+ data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor());
+ status_t error = remote()->transact(DISCARD_FREE_BUFFERS, data, &reply);
+ if (error != NO_ERROR) {
+ return error;
+ }
+ int32_t result = NO_ERROR;
+ error = reply.readInt32(&result);
+ if (error != NO_ERROR) {
+ return error;
+ }
+ return result;
+ }
+
virtual void dump(String8& result, const char* prefix) const {
Parcel data, reply;
data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor());
@@ -409,6 +425,12 @@
}
return NO_ERROR;
}
+ case DISCARD_FREE_BUFFERS: {
+ CHECK_INTERFACE(IGraphicBufferConsumer, data, reply);
+ status_t result = discardFreeBuffers();
+ status_t error = reply->writeInt32(result);
+ return error;
+ }
case DUMP: {
CHECK_INTERFACE(IGraphicBufferConsumer, data, reply);
String8 result = data.readString8();
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index 85d63b4..721667c 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -850,4 +850,81 @@
returnedBuffer->getNativeBuffer()->handle);
}
+TEST_F(BufferQueueTest, TestDiscardFreeBuffers) {
+ createBufferQueue();
+ sp<DummyConsumer> dc(new DummyConsumer);
+ ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false));
+ IGraphicBufferProducer::QueueBufferOutput output;
+ ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener,
+ NATIVE_WINDOW_API_CPU, false, &output));
+
+ 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 4 buffers so we don't get
+ // BUFFER_NEEDS_REALLOCATION below
+ int slots[4] = {};
+ mProducer->setMaxDequeuedBufferCount(4);
+ for (size_t i = 0; i < 4; ++i) {
+ status_t result = mProducer->dequeueBuffer(&slots[i], &fence,
+ 0, 0, 0, 0);
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result);
+ ASSERT_EQ(OK, mProducer->requestBuffer(slots[i], &buffer));
+ }
+ for (size_t i = 0; i < 4; ++i) {
+ ASSERT_EQ(OK, mProducer->cancelBuffer(slots[i], Fence::NO_FENCE));
+ }
+
+ // Get buffers in all states: dequeued, filled, acquired, free
+
+ // Fill 3 buffers
+ ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+ ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+ ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
+ ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
+ // Dequeue 1 buffer
+ ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
+
+ // Acquire and free 1 buffer
+ ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+ ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
+ EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
+ // Acquire 1 buffer, leaving 1 filled buffer in queue
+ ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+
+ // Now discard the free buffers
+ ASSERT_EQ(OK, mConsumer->discardFreeBuffers());
+
+ // Check no free buffers in dump
+ String8 dumpString;
+ mConsumer->dump(dumpString, nullptr);
+
+ // Parse the dump to ensure that all buffer slots that are FREE also
+ // have a null GraphicBuffer
+ // Fragile - assumes the following format for the dump for a buffer entry:
+ // ":%p\][^:]*state=FREE" where %p is the buffer pointer in hex.
+ ssize_t idx = dumpString.find("state=FREE");
+ while (idx != -1) {
+ ssize_t bufferPtrIdx = idx - 1;
+ while (bufferPtrIdx > 0) {
+ if (dumpString[bufferPtrIdx] == ':') {
+ bufferPtrIdx++;
+ break;
+ }
+ bufferPtrIdx--;
+ }
+ ASSERT_GT(bufferPtrIdx, 0) << "Can't parse queue dump to validate";
+ ssize_t nullPtrIdx = dumpString.find("0x0]", bufferPtrIdx);
+ ASSERT_EQ(bufferPtrIdx, nullPtrIdx) << "Free buffer not discarded";
+ idx = dumpString.find("FREE", idx + 1);
+ }
+}
+
} // namespace android
