Merge changes I13283720,Iddd8f902 into main

* changes:
  BufferItemConsumer: Add/expose methods
  BufferQueue: Fix deadlock in setMaxAcquiredBufferCount

Author: Jim Shargo

libs/gui/BufferItemConsumer.cpp

@@ -17,6 +17,7 @@
 //#define LOG_NDEBUG 0
 #define LOG_TAG "BufferItemConsumer"
 //#define ATRACE_TAG ATRACE_TAG_GRAPHICS
+#include <utils/Errors.h>
 #include <utils/Log.h>
 
 #include <inttypes.h>
@@ -132,13 +133,36 @@ status_t BufferItemConsumer::acquireBuffer(BufferItem *item,
     return OK;
 }
 
+status_t BufferItemConsumer::attachBuffer(const sp<GraphicBuffer>& buffer) {
+    if (!buffer) {
+        BI_LOGE("BufferItemConsumer::attachBuffer no input buffer specified.");
+        return BAD_VALUE;
+    }
+
+    Mutex::Autolock _l(mMutex);
+
+    int slot = INVALID_BUFFER_SLOT;
+    status_t status = mConsumer->attachBuffer(&slot, buffer);
+    if (status != OK) {
+        BI_LOGE("BufferItemConsumer::attachBuffer unable to attach buffer %d", status);
+        return status;
+    }
+
+    mSlots[slot] = {
+            .mGraphicBuffer = buffer,
+            .mFence = nullptr,
+            .mFrameNumber = 0,
+    };
+
+    return OK;
+}
+
 status_t BufferItemConsumer::releaseBuffer(const BufferItem &item,
         const sp<Fence>& releaseFence) {
     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<GraphicBuffer>& buffer,
                                            const sp<Fence>& releaseFence) {
     Mutex::Autolock _l(mMutex);
@@ -154,7 +178,6 @@ status_t BufferItemConsumer::releaseBuffer(const sp<GraphicBuffer>& buffer,
 
     return releaseBufferSlotLocked(slotIndex, buffer, releaseFence);
 }
-#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
 
 status_t BufferItemConsumer::releaseBufferSlotLocked(int slotIndex, const sp<GraphicBuffer>& buffer,
                                                      const sp<Fence>& releaseFence) {

libs/gui/BufferQueueConsumer.cpp

@@ -14,10 +14,6 @@
  * limitations under the License.
  */
 
-#include <inttypes.h>
-#include <pwd.h>
-#include <sys/types.h>
-
 #define LOG_TAG "BufferQueueConsumer"
 #define ATRACE_TAG ATRACE_TAG_GRAPHICS
 //#define LOG_NDEBUG 0
@@ -48,6 +44,11 @@
 
 #include <com_android_graphics_libgui_flags.h>
 
+#include <inttypes.h>
+#include <pwd.h>
+#include <sys/types.h>
+#include <optional>
+
 namespace android {
 
 // Macros for include BufferQueueCore information in log messages
@@ -767,11 +768,15 @@ status_t BufferQueueConsumer::setMaxBufferCount(int bufferCount) {
     return NO_ERROR;
 }
 
+status_t BufferQueueConsumer::setMaxAcquiredBufferCount(int maxAcquiredBuffers) {
+    return setMaxAcquiredBufferCount(maxAcquiredBuffers, std::nullopt);
+}
+
 status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
-        int maxAcquiredBuffers) {
+        int maxAcquiredBuffers, std::optional<OnBufferReleasedCallback> onBuffersReleasedCallback) {
     ATRACE_FORMAT("%s(%d)", __func__, maxAcquiredBuffers);
 
-    sp<IConsumerListener> listener;
+    std::optional<OnBufferReleasedCallback> callback;
     { // Autolock scope
         std::unique_lock<std::mutex> lock(mCore->mMutex);
 
@@ -833,13 +838,20 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
         BQ_LOGV("setMaxAcquiredBufferCount: %d", maxAcquiredBuffers);
         mCore->mMaxAcquiredBufferCount = maxAcquiredBuffers;
         VALIDATE_CONSISTENCY();
-        if (delta < 0 && mCore->mBufferReleasedCbEnabled) {
-            listener = mCore->mConsumerListener;
+        if (delta < 0) {
+            if (onBuffersReleasedCallback) {
+                callback = std::move(onBuffersReleasedCallback);
+            } else if (mCore->mBufferReleasedCbEnabled) {
+                callback = [listener = mCore->mConsumerListener]() {
+                    listener->onBuffersReleased();
+                };
+            }
         }
     }
+
     // Call back without lock held
-    if (listener != nullptr) {
-        listener->onBuffersReleased();
+    if (callback) {
+        (*callback)();
     }
 
     return NO_ERROR;

libs/gui/ConsumerBase.cpp

@@ -264,7 +264,10 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) {
 
 void ConsumerBase::onBuffersReleased() {
     Mutex::Autolock lock(mMutex);
+    onBuffersReleasedLocked();
+}
 
+void ConsumerBase::onBuffersReleasedLocked() {
     CB_LOGV("onBuffersReleased");
 
     if (mAbandoned) {
@@ -481,7 +484,8 @@ status_t ConsumerBase::setMaxAcquiredBufferCount(int maxAcquiredBuffers) {
         CB_LOGE("setMaxAcquiredBufferCount: ConsumerBase is abandoned!");
         return NO_INIT;
     }
-    return mConsumer->setMaxAcquiredBufferCount(maxAcquiredBuffers);
+    return mConsumer->setMaxAcquiredBufferCount(maxAcquiredBuffers,
+                                                {[this]() { onBuffersReleasedLocked(); }});
 }
 
 status_t ConsumerBase::setConsumerIsProtected(bool isProtected) {

libs/gui/include/gui/BufferItemConsumer.h

@@ -96,6 +96,14 @@ class BufferItemConsumer: public ConsumerBase
     status_t acquireBuffer(BufferItem* item, nsecs_t presentWhen,
             bool waitForFence = true);
 
+    // Transfer ownership of a buffer to the BufferQueue. On NO_ERROR, the buffer
+    // is considered as if it were acquired. Buffer must not be null.
+    //
+    // Returns
+    //  - BAD_VALUE if buffer is null
+    //  - INVALID_OPERATION if too many buffers have already been acquired
+    status_t attachBuffer(const sp<GraphicBuffer>& buffer);
+
     // Returns an acquired buffer to the queue, allowing it to be reused. Since
     // only a fixed number of buffers may be acquired at a time, old buffers
     // must be released by calling releaseBuffer to ensure new buffers can be
@@ -105,10 +113,8 @@ class BufferItemConsumer: public ConsumerBase
     status_t releaseBuffer(const BufferItem &item,
             const sp<Fence>& releaseFence = Fence::NO_FENCE);
 
-#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
     status_t releaseBuffer(const sp<GraphicBuffer>& buffer,
                            const sp<Fence>& releaseFence = Fence::NO_FENCE);
-#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
 
 protected:
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)

libs/gui/include/gui/BufferQueueConsumer.h

@@ -122,7 +122,10 @@ class BufferQueueConsumer : public IGraphicBufferConsumer {
     // setMaxAcquiredBufferCount sets the maximum number of buffers that can
     // be acquired by the consumer at one time (default 1).  This call will
     // fail if a producer is connected to the BufferQueue.
-    virtual status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers);
+    virtual status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers) override;
+    virtual status_t setMaxAcquiredBufferCount(
+            int maxAcquiredBuffers,
+            std::optional<OnBufferReleasedCallback> onBuffersReleasedCallback) override;
 
     // setConsumerName sets the name used in logging
     status_t setConsumerName(const String8& name) override;

libs/gui/include/gui/ConsumerBase.h

@@ -191,6 +191,8 @@ class ConsumerBase : public virtual RefBase,
 #endif
     virtual int getSlotForBufferLocked(const sp<GraphicBuffer>& buffer);
 
+    virtual void onBuffersReleasedLocked();
+
     virtual status_t detachBufferLocked(int slotIndex);
 
     // freeBufferLocked frees up the given buffer slot.  If the slot has been

libs/gui/include/gui/IGraphicBufferConsumer.h

@@ -243,6 +243,9 @@ class IGraphicBufferConsumer : public RefBase {
     // maxAcquiredBuffers must be (inclusive) between 1 and MAX_MAX_ACQUIRED_BUFFERS. It also cannot
     // cause the maxBufferCount value to be exceeded.
     //
+    // If called with onBuffersReleasedCallback, that call back will be called in lieu of
+    // IConsumerListener::onBuffersReleased.
+    //
     // Return of a value other than NO_ERROR means an error has occurred:
     // * NO_INIT - the BufferQueue has been abandoned
     // * BAD_VALUE - one of the below conditions occurred:
@@ -253,6 +256,11 @@ class IGraphicBufferConsumer : public RefBase {
     // * INVALID_OPERATION - attempting to call this after a producer connected.
     virtual status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers) = 0;
 
+    using OnBufferReleasedCallback = std::function<void(void)>;
+    virtual status_t setMaxAcquiredBufferCount(
+            int maxAcquiredBuffers,
+            std::optional<OnBufferReleasedCallback> onBuffersReleasedCallback) = 0;
+
     // setConsumerName sets the name used in logging
     virtual status_t setConsumerName(const String8& name) = 0;
 

libs/gui/include/gui/mock/GraphicBufferConsumer.h

@@ -47,6 +47,7 @@ class GraphicBufferConsumer : public IGraphicBufferConsumer {
     MOCK_METHOD2(setDefaultBufferSize, status_t(uint32_t, uint32_t));
     MOCK_METHOD1(setMaxBufferCount, status_t(int));
     MOCK_METHOD1(setMaxAcquiredBufferCount, status_t(int));
+    MOCK_METHOD2(setMaxAcquiredBufferCount, status_t(int, std::optional<OnBufferReleasedCallback>));
     MOCK_METHOD1(setConsumerName, status_t(const String8&));
     MOCK_METHOD1(setDefaultBufferFormat, status_t(PixelFormat));
     MOCK_METHOD1(setDefaultBufferDataSpace, status_t(android_dataspace));

libs/gui/tests/BufferItemConsumer_test.cpp

@@ -24,6 +24,7 @@
 #include <gui/Surface.h>
 #include <ui/BufferQueueDefs.h>
 #include <ui/GraphicBuffer.h>
+#include <utils/Errors.h>
 
 #include <unordered_set>
 
@@ -235,6 +236,38 @@ TEST_F(BufferItemConsumerTest, TriggerBufferFreed_DeleteBufferItemConsumer) {
     ASSERT_EQ(1, GetFreedBufferCount());
 }
 
+TEST_F(BufferItemConsumerTest, ResizeAcquireCount) {
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers + 1));
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers + 2));
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers - 1));
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers - 2));
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers + 1));
+    EXPECT_EQ(OK, mBIC->setMaxAcquiredBufferCount(kMaxLockedBuffers - 1));
+}
+
+TEST_F(BufferItemConsumerTest, AttachBuffer) {
+    ASSERT_EQ(OK, mBIC->setMaxAcquiredBufferCount(1));
+
+    int slot;
+    DequeueBuffer(&slot);
+    QueueBuffer(slot);
+    AcquireBuffer(&slot);
+
+    sp<GraphicBuffer> newBuffer1 = sp<GraphicBuffer>::make(kWidth, kHeight, kFormat, kUsage);
+    sp<GraphicBuffer> newBuffer2 = sp<GraphicBuffer>::make(kWidth, kHeight, kFormat, kUsage);
+
+    // For some reason, you can attach an extra buffer?
+    // b/400973991 to investigate
+    EXPECT_EQ(OK, mBIC->attachBuffer(newBuffer1));
+    EXPECT_EQ(INVALID_OPERATION, mBIC->attachBuffer(newBuffer2));
+
+    ReleaseBuffer(slot);
+
+    EXPECT_EQ(OK, mBIC->attachBuffer(newBuffer2));
+    EXPECT_EQ(OK, mBIC->releaseBuffer(newBuffer1, Fence::NO_FENCE));
+    EXPECT_EQ(OK, mBIC->releaseBuffer(newBuffer2, Fence::NO_FENCE));
+}
+
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)
 // Test that delete BufferItemConsumer triggers onBufferFreed.
 TEST_F(BufferItemConsumerTest, DetachBufferWithBuffer) {