From 28e9d6554aaf9eb70518cee601cde780a392e193 Mon Sep 17 00:00:00 2001 From: Jim Shargo Date: Wed, 10 Jul 2024 23:50:00 +0000 Subject: libgui: Add simpler methods to ConsumerBase and BufferItemConsumer. Now we can detach a buffer directly. BYPASS_IGBP_IGBC_API_REASON=warren buffers Bug: 340933754 Flag: com.android.graphics.libgui.flags.wb_platform_api_improvements Test: new test in libgui_test Change-Id: Ib14e645a04b21b3233522d2d20ee5aaa09fe563c --- libs/gui/BufferItemConsumer.cpp | 32 ++++++++++++++-- libs/gui/ConsumerBase.cpp | 60 ++++++++++++++++++++++++++---- libs/gui/include/gui/BufferItemConsumer.h | 14 ++++++- libs/gui/include/gui/ConsumerBase.h | 16 ++++++-- libs/gui/tests/BufferItemConsumer_test.cpp | 38 ++++++++++++++++++- 5 files changed, 142 insertions(+), 18 deletions(-) (limited to 'libs/gui') diff --git a/libs/gui/BufferItemConsumer.cpp b/libs/gui/BufferItemConsumer.cpp index e6331e7282..eeb8f196e6 100644 --- a/libs/gui/BufferItemConsumer.cpp +++ b/libs/gui/BufferItemConsumer.cpp @@ -21,8 +21,11 @@ #include +#include #include #include +#include +#include #define BI_LOGV(x, ...) ALOGV("[%s] " x, mName.c_str(), ##__VA_ARGS__) // #define BI_LOGD(x, ...) ALOGD("[%s] " x, mName.c_str(), ##__VA_ARGS__) @@ -87,17 +90,38 @@ status_t BufferItemConsumer::acquireBuffer(BufferItem *item, status_t BufferItemConsumer::releaseBuffer(const BufferItem &item, const sp& releaseFence) { - status_t err; + Mutex::Autolock _l(mMutex); + return releaseBufferSlotLocked(item.mSlot, item.mGraphicBuffer, releaseFence); +} +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) +status_t BufferItemConsumer::releaseBuffer(const sp& buffer, + const sp& releaseFence) { Mutex::Autolock _l(mMutex); - err = addReleaseFenceLocked(item.mSlot, item.mGraphicBuffer, releaseFence); + if (buffer == nullptr) { + return BAD_VALUE; + } + + int slotIndex = getSlotForBufferLocked(buffer); + if (slotIndex == INVALID_BUFFER_SLOT) { + return BAD_VALUE; + } + + return releaseBufferSlotLocked(slotIndex, buffer, releaseFence); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + +status_t BufferItemConsumer::releaseBufferSlotLocked(int slotIndex, const sp& buffer, + const sp& releaseFence) { + status_t err; + + err = addReleaseFenceLocked(slotIndex, buffer, releaseFence); if (err != OK) { BI_LOGE("Failed to addReleaseFenceLocked"); } - err = releaseBufferLocked(item.mSlot, item.mGraphicBuffer, EGL_NO_DISPLAY, - EGL_NO_SYNC_KHR); + err = releaseBufferLocked(slotIndex, buffer, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR); if (err != OK && err != IGraphicBufferConsumer::STALE_BUFFER_SLOT) { BI_LOGE("Failed to release buffer: %s (%d)", strerror(-err), err); diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index b625c3f75e..19b9c8b09d 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -30,9 +30,10 @@ #include #include +#include #include #include -#include +#include #include @@ -40,6 +41,8 @@ #include #include +#include + // Macros for including the ConsumerBase name in log messages #define CB_LOGV(x, ...) ALOGV("[%s] " x, mName.c_str(), ##__VA_ARGS__) // #define CB_LOGD(x, ...) ALOGD("[%s] " x, mName.c_str(), ##__VA_ARGS__) @@ -96,6 +99,35 @@ void ConsumerBase::onLastStrongRef(const void* id __attribute__((unused))) { abandon(); } +int ConsumerBase::getSlotForBufferLocked(const sp& buffer) { + if (!buffer) { + return BufferQueue::INVALID_BUFFER_SLOT; + } + + uint64_t id = buffer->getId(); + for (int i = 0; i < BufferQueueDefs::NUM_BUFFER_SLOTS; i++) { + auto& slot = mSlots[i]; + if (slot.mGraphicBuffer && slot.mGraphicBuffer->getId() == id) { + return i; + } + } + + return BufferQueue::INVALID_BUFFER_SLOT; +} + +status_t ConsumerBase::detachBufferLocked(int slotIndex) { + status_t result = mConsumer->detachBuffer(slotIndex); + + if (result != NO_ERROR) { + CB_LOGE("Failed to detach buffer: %d", result); + return result; + } + + freeBufferLocked(slotIndex); + + return result; +} + void ConsumerBase::freeBufferLocked(int slotIndex) { CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); mSlots[slotIndex].mGraphicBuffer = nullptr; @@ -252,16 +284,30 @@ status_t ConsumerBase::detachBuffer(int slot) { return NO_INIT; } - status_t result = mConsumer->detachBuffer(slot); - if (result != NO_ERROR) { - CB_LOGE("Failed to detach buffer: %d", result); - return result; + return detachBufferLocked(slot); +} + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) +status_t ConsumerBase::detachBuffer(const sp& buffer) { + CB_LOGV("detachBuffer"); + Mutex::Autolock lock(mMutex); + + if (mAbandoned) { + CB_LOGE("detachBuffer: ConsumerBase is abandoned!"); + return NO_INIT; + } + if (buffer == nullptr) { + return BAD_VALUE; } - freeBufferLocked(slot); + int slotIndex = getSlotForBufferLocked(buffer); + if (slotIndex == BufferQueue::INVALID_BUFFER_SLOT) { + return BAD_VALUE; + } - return result; + return detachBufferLocked(slotIndex); } +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) status_t ConsumerBase::setDefaultBufferSize(uint32_t width, uint32_t height) { Mutex::Autolock _l(mMutex); diff --git a/libs/gui/include/gui/BufferItemConsumer.h b/libs/gui/include/gui/BufferItemConsumer.h index a905610ee2..e383a407dc 100644 --- a/libs/gui/include/gui/BufferItemConsumer.h +++ b/libs/gui/include/gui/BufferItemConsumer.h @@ -17,13 +17,15 @@ #ifndef ANDROID_GUI_BUFFERITEMCONSUMER_H #define ANDROID_GUI_BUFFERITEMCONSUMER_H -#include +#include #include +#include #define ANDROID_GRAPHICS_BUFFERITEMCONSUMER_JNI_ID "mBufferItemConsumer" namespace android { +class GraphicBuffer; class String8; /** @@ -85,7 +87,15 @@ class BufferItemConsumer: public ConsumerBase status_t releaseBuffer(const BufferItem &item, const sp& releaseFence = Fence::NO_FENCE); - private: +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + status_t releaseBuffer(const sp& buffer, + const sp& releaseFence = Fence::NO_FENCE); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + +private: + status_t releaseBufferSlotLocked(int slotIndex, const sp& buffer, + const sp& releaseFence); + void freeBufferLocked(int slotIndex) override; // mBufferFreedListener is the listener object that will be called when diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h index 8ff0cd0f6e..a031e66f68 100644 --- a/libs/gui/include/gui/ConsumerBase.h +++ b/libs/gui/include/gui/ConsumerBase.h @@ -17,18 +17,16 @@ #ifndef ANDROID_GUI_CONSUMERBASE_H #define ANDROID_GUI_CONSUMERBASE_H +#include #include #include #include #include - #include - #include #include #include - namespace android { // ---------------------------------------------------------------------------- @@ -81,7 +79,13 @@ public: void setFrameAvailableListener(const wp& listener); // See IGraphicBufferConsumer::detachBuffer - status_t detachBuffer(int slot); + status_t detachBuffer(int slot) __attribute(( + deprecated("Please use the GraphicBuffer variant--slots are deprecated."))); + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + // See IGraphicBufferConsumer::detachBuffer + status_t detachBuffer(const sp& buffer); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) // See IGraphicBufferConsumer::setDefaultBufferSize status_t setDefaultBufferSize(uint32_t width, uint32_t height); @@ -150,6 +154,10 @@ protected: virtual void onBuffersReleased() override; virtual void onSidebandStreamChanged() override; + virtual int getSlotForBufferLocked(const sp& buffer); + + virtual status_t detachBufferLocked(int slotIndex); + // freeBufferLocked frees up the given buffer slot. If the slot has been // initialized this will release the reference to the GraphicBuffer in that // slot. Otherwise it has no effect. diff --git a/libs/gui/tests/BufferItemConsumer_test.cpp b/libs/gui/tests/BufferItemConsumer_test.cpp index 6880678050..7d33f283aa 100644 --- a/libs/gui/tests/BufferItemConsumer_test.cpp +++ b/libs/gui/tests/BufferItemConsumer_test.cpp @@ -17,10 +17,12 @@ #define LOG_TAG "BufferItemConsumer_test" //#define LOG_NDEBUG 0 +#include #include #include #include #include +#include namespace android { @@ -42,6 +44,17 @@ class BufferItemConsumerTest : public ::testing::Test { BufferItemConsumerTest* mTest; }; + struct TrackingProducerListener : public BnProducerListener { + TrackingProducerListener(BufferItemConsumerTest* test) : mTest(test) {} + + virtual void onBufferReleased() override {} + virtual bool needsReleaseNotify() override { return true; } + virtual void onBuffersDiscarded(const std::vector&) override {} + virtual void onBufferDetached(int slot) override { mTest->HandleBufferDetached(slot); } + + BufferItemConsumerTest* mTest; + }; + void SetUp() override { BufferQueue::createBufferQueue(&mProducer, &mConsumer); mBIC = @@ -51,7 +64,7 @@ class BufferItemConsumerTest : public ::testing::Test { mBFL = new BufferFreedListener(this); mBIC->setBufferFreedListener(mBFL); - sp producerListener = new StubProducerListener(); + sp producerListener = new TrackingProducerListener(this); IGraphicBufferProducer::QueueBufferOutput bufferOutput; ASSERT_EQ(NO_ERROR, mProducer->connect(producerListener, NATIVE_WINDOW_API_CPU, @@ -71,6 +84,13 @@ class BufferItemConsumerTest : public ::testing::Test { ALOGD("HandleBufferFreed, mFreedBufferCount=%d", mFreedBufferCount); } + void HandleBufferDetached(int slot) { + std::lock_guard lock(mMutex); + mDetachedBufferSlots.push_back(slot); + ALOGD("HandleBufferDetached, slot=%d mDetachedBufferSlots-count=%zu", slot, + mDetachedBufferSlots.size()); + } + void DequeueBuffer(int* outSlot) { ASSERT_NE(outSlot, nullptr); @@ -120,6 +140,7 @@ class BufferItemConsumerTest : public ::testing::Test { std::mutex mMutex; int mFreedBufferCount{0}; + std::vector mDetachedBufferSlots = {}; sp mBIC; sp mBFL; @@ -203,4 +224,19 @@ TEST_F(BufferItemConsumerTest, TriggerBufferFreed_DeleteBufferItemConsumer) { ASSERT_EQ(1, GetFreedBufferCount()); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) +// Test that delete BufferItemConsumer triggers onBufferFreed. +TEST_F(BufferItemConsumerTest, DetachBufferWithBuffer) { + int slot; + // Let buffer go through the cycle at least once. + DequeueBuffer(&slot); + QueueBuffer(slot); + AcquireBuffer(&slot); + + sp buffer = mBuffers[slot]; + EXPECT_EQ(OK, mBIC->detachBuffer(buffer)); + EXPECT_THAT(mDetachedBufferSlots, testing::ElementsAre(slot)); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + } // namespace android -- cgit v1.2.3-59-g8ed1b