From 078d7366d28232ea0ef7d614f9526ab33de49b9b Mon Sep 17 00:00:00 2001
From: Patrick Williams <pdwilliams@google.com>
Date: Tue, 27 Aug 2024 10:20:39 -0500
Subject: Block on BufferReleaseChannel when out of buffers

Bug: 294133380
Flag: com.android.graphics.libgui.flags.buffer_release_channel
Test: BLASTBufferQueueTest
Change-Id: I4ffec81ffb9c26546cc50176f3c44ffe6eb90b75
---
 libs/gui/BLASTBufferQueue.cpp | 273 ++++++++++++++++++++++++++++++++----------
 1 file changed, 209 insertions(+), 64 deletions(-)

(limited to 'libs/gui/BLASTBufferQueue.cpp')

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 25e6a52ed1..49f4cba284 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -50,9 +50,28 @@ using namespace com::android::graphics::libgui;
 using namespace std::chrono_literals;
 
 namespace {
+
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+// RAII wrapper to defer arbitrary work until the Deferred instance is deleted.
+template <class F>
+class Deferred {
+public:
+    explicit Deferred(F f) : mF{std::move(f)} {}
+
+    ~Deferred() { mF(); }
+
+    Deferred(const Deferred&) = delete;
+    Deferred& operator=(const Deferred&) = delete;
+
+private:
+    F mF;
+};
+#endif
+
 inline const char* boolToString(bool b) {
     return b ? "true" : "false";
 }
+
 } // namespace
 
 namespace android {
@@ -77,12 +96,6 @@ namespace android {
     std::unique_lock _lock{mutex};        \
     base::ScopedLockAssertion assumeLocked(mutex);
 
-#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK =
-        [](const ReleaseCallbackId&, const sp<Fence>& /*releaseFence*/,
-           std::optional<uint32_t> /*currentMaxAcquiredBufferCount*/) {};
-#endif
-
 void BLASTBufferItemConsumer::onDisconnect() {
     Mutex::Autolock lock(mMutex);
     mPreviouslyConnected = mCurrentlyConnected;
@@ -225,9 +238,8 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati
             this);
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-    std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> bufferReleaseConsumer;
-    gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer);
-    mBufferReleaseReader = std::make_shared<BufferReleaseReader>(std::move(bufferReleaseConsumer));
+    gui::BufferReleaseChannel::open(mName, mBufferReleaseConsumer, mBufferReleaseProducer);
+    mBufferReleaseReader.emplace(*this);
 #endif
 
     BQA_LOGV("BLASTBufferQueue created");
@@ -260,7 +272,7 @@ void BLASTBufferQueue::onFirstRef() {
     // safe default, most producers are expected to override this
     mProducer->setMaxDequeuedBufferCount(2);
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-    mBufferReleaseThread.start(sp<BLASTBufferQueue>::fromExisting(this));
+    mBufferReleaseThread.emplace(sp<BLASTBufferQueue>::fromExisting(this));
 #endif
 }
 
@@ -636,7 +648,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked(
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
     ReleaseBufferCallback releaseBufferCallback =
-            applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk();
+            applyTransaction ? nullptr : makeReleaseBufferCallbackThunk();
 #else
     auto releaseBufferCallback = makeReleaseBufferCallbackThunk();
 #endif
@@ -1137,6 +1149,24 @@ public:
 #endif
 };
 
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+class BBQBufferQueueCore : public BufferQueueCore {
+public:
+    explicit BBQBufferQueueCore(const wp<BLASTBufferQueue>& bbq) : mBLASTBufferQueue{bbq} {}
+
+    void notifyBufferReleased() const override {
+        sp<BLASTBufferQueue> bbq = mBLASTBufferQueue.promote();
+        if (!bbq) {
+            return;
+        }
+        bbq->mBufferReleaseReader->interruptBlockingRead();
+    }
+
+private:
+    wp<BLASTBufferQueue> mBLASTBufferQueue;
+};
+#endif
+
 // Extends the BufferQueueProducer to create a wrapper around the listener so the listener calls
 // can be non-blocking when the producer is in the client process.
 class BBQBufferQueueProducer : public BufferQueueProducer {
@@ -1188,6 +1218,44 @@ public:
         return BufferQueueProducer::query(what, value);
     }
 
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+    status_t waitForBufferRelease(std::unique_lock<std::mutex>& bufferQueueLock,
+                                  nsecs_t timeout) const override {
+        sp<BLASTBufferQueue> bbq = mBLASTBufferQueue.promote();
+        if (!bbq) {
+            return OK;
+        }
+
+        // BufferQueue has already checked if we have a free buffer. If there's an unread interrupt,
+        // we want to ignore it. This must be done before unlocking the BufferQueue lock to ensure
+        // we don't miss an interrupt.
+        bbq->mBufferReleaseReader->clearInterrupts();
+        bbq->mThreadsBlockingOnDequeue++;
+        bufferQueueLock.unlock();
+        Deferred cleanup{[&]() {
+            bufferQueueLock.lock();
+            bbq->mThreadsBlockingOnDequeue--;
+        }};
+
+        ATRACE_FORMAT("waiting for free buffer");
+        ReleaseCallbackId id;
+        sp<Fence> fence;
+        uint32_t maxAcquiredBufferCount;
+        status_t status =
+                bbq->mBufferReleaseReader->readBlocking(id, fence, maxAcquiredBufferCount, timeout);
+        if (status == TIMED_OUT) {
+            return TIMED_OUT;
+        } else if (status != OK) {
+            // Waiting was interrupted or an error occurred. BufferQueueProducer will check if we
+            // have a free buffer and call this method again if not.
+            return OK;
+        }
+
+        bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount);
+        return OK;
+    }
+#endif
+
 private:
     const wp<BLASTBufferQueue> mBLASTBufferQueue;
 };
@@ -1201,14 +1269,18 @@ void BLASTBufferQueue::createBufferQueue(sp<IGraphicBufferProducer>* outProducer
     LOG_ALWAYS_FATAL_IF(outProducer == nullptr, "BLASTBufferQueue: outProducer must not be NULL");
     LOG_ALWAYS_FATAL_IF(outConsumer == nullptr, "BLASTBufferQueue: outConsumer must not be NULL");
 
-    sp<BufferQueueCore> core(new BufferQueueCore());
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+    auto core = sp<BBQBufferQueueCore>::make(this);
+#else
+    auto core = sp<BufferQueueCore>::make();
+#endif
     LOG_ALWAYS_FATAL_IF(core == nullptr, "BLASTBufferQueue: failed to create BufferQueueCore");
 
-    sp<IGraphicBufferProducer> producer(new BBQBufferQueueProducer(core, this));
+    auto producer = sp<BBQBufferQueueProducer>::make(core, this);
     LOG_ALWAYS_FATAL_IF(producer == nullptr,
                         "BLASTBufferQueue: failed to create BBQBufferQueueProducer");
 
-    sp<BufferQueueConsumer> consumer(new BufferQueueConsumer(core));
+    auto consumer = sp<BufferQueueConsumer>::make(core);
     consumer->setAllowExtraAcquire(true);
     LOG_ALWAYS_FATAL_IF(consumer == nullptr,
                         "BLASTBufferQueue: failed to create BufferQueueConsumer");
@@ -1273,10 +1345,8 @@ void BLASTBufferQueue::setApplyToken(sp<IBinder> applyToken) {
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
 
-BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(
-        std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> endpoint)
-      : mEndpoint{std::move(endpoint)} {
-    mEpollFd = android::base::unique_fd{epoll_create1(0)};
+BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(BLASTBufferQueue& bbq) : mBbq{bbq} {
+    mEpollFd = android::base::unique_fd{epoll_create1(EPOLL_CLOEXEC)};
     LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(),
                         "Failed to create buffer release epoll file descriptor. errno=%d "
                         "message='%s'",
@@ -1284,9 +1354,9 @@ BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(
 
     epoll_event registerEndpointFd{};
     registerEndpointFd.events = EPOLLIN;
-    registerEndpointFd.data.fd = mEndpoint->getFd();
-    status_t status =
-            epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), &registerEndpointFd);
+    registerEndpointFd.data.fd = mBbq.mBufferReleaseConsumer->getFd();
+    status_t status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mBbq.mBufferReleaseConsumer->getFd(),
+                                &registerEndpointFd);
     LOG_ALWAYS_FATAL_IF(status == -1,
                         "Failed to register buffer release consumer file descriptor with epoll. "
                         "errno=%d message='%s'",
@@ -1308,78 +1378,153 @@ BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(
                         errno, strerror(errno));
 }
 
-BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=(
-        BufferReleaseReader&& other) {
-    if (this != &other) {
-        ftl::FakeGuard guard{mMutex};
-        ftl::FakeGuard otherGuard{other.mMutex};
-        mEndpoint = std::move(other.mEndpoint);
-        mEpollFd = std::move(other.mEpollFd);
-        mEventFd = std::move(other.mEventFd);
-    }
-    return *this;
-}
-
 status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId,
                                                              sp<Fence>& outFence,
-                                                             uint32_t& outMaxAcquiredBufferCount) {
-    epoll_event event{};
-    while (true) {
-        int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */);
-        if (eventCount == 1) {
-            break;
-        }
-        if (eventCount == -1 && errno != EINTR) {
-            ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno,
-                  strerror(errno));
+                                                             uint32_t& outMaxAcquiredBufferCount,
+                                                             nsecs_t timeout) {
+    // TODO(b/363290953) epoll_wait only has millisecond timeout precision. If timeout is less than
+    // 1ms, then we round timeout up to 1ms. Otherwise, we round timeout to the nearest
+    // millisecond. Once epoll_pwait2 can be used in libgui, we can specify timeout with nanosecond
+    // precision.
+    int timeoutMs = -1;
+    if (timeout == 0) {
+        timeoutMs = 0;
+    } else if (timeout > 0) {
+        const int nsPerMs = 1000000;
+        if (timeout < nsPerMs) {
+            timeoutMs = 1;
+        } else {
+            timeoutMs = static_cast<int>(
+                    std::chrono::round<std::chrono::milliseconds>(std::chrono::nanoseconds{timeout})
+                            .count());
         }
     }
 
+    epoll_event event{};
+    int eventCount;
+    do {
+        eventCount = epoll_wait(mEpollFd.get(), &event, 1 /*maxevents*/, timeoutMs);
+    } while (eventCount == -1 && errno != EINTR);
+
+    if (eventCount == -1) {
+        ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno,
+              strerror(errno));
+        return UNKNOWN_ERROR;
+    }
+
+    if (eventCount == 0) {
+        return TIMED_OUT;
+    }
+
     if (event.data.fd == mEventFd.get()) {
-        uint64_t value;
-        if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) {
-            ALOGE("error while reading from eventfd. errno=%d message='%s'", errno,
-                  strerror(errno));
-        }
+        clearInterrupts();
         return WOULD_BLOCK;
     }
 
-    std::lock_guard lock{mMutex};
-    return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount);
+    return mBbq.mBufferReleaseConsumer->readReleaseFence(outId, outFence,
+                                                         outMaxAcquiredBufferCount);
 }
 
 void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() {
-    uint64_t value = 1;
-    if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) {
+    if (eventfd_write(mEventFd.get(), 1) == -1) {
         ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno));
     }
 }
 
-void BLASTBufferQueue::BufferReleaseThread::start(const sp<BLASTBufferQueue>& bbq) {
-    mRunning = std::make_shared<std::atomic_bool>(true);
-    mReader = bbq->mBufferReleaseReader;
-    std::thread([running = mRunning, reader = mReader, weakBbq = wp<BLASTBufferQueue>(bbq)]() {
+void BLASTBufferQueue::BufferReleaseReader::clearInterrupts() {
+    eventfd_t value;
+    if (eventfd_read(mEventFd.get(), &value) == -1 && errno != EWOULDBLOCK) {
+        ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, strerror(errno));
+    }
+}
+
+BLASTBufferQueue::BufferReleaseThread::BufferReleaseThread(const sp<BLASTBufferQueue>& bbq) {
+    android::base::unique_fd epollFd{epoll_create1(EPOLL_CLOEXEC)};
+    LOG_ALWAYS_FATAL_IF(!epollFd.ok(),
+                        "Failed to create buffer release background thread epoll file descriptor. "
+                        "errno=%d message='%s'",
+                        errno, strerror(errno));
+
+    epoll_event registerEndpointFd{};
+    registerEndpointFd.events = EPOLLIN;
+    registerEndpointFd.data.fd = bbq->mBufferReleaseConsumer->getFd();
+    status_t status = epoll_ctl(epollFd.get(), EPOLL_CTL_ADD, bbq->mBufferReleaseConsumer->getFd(),
+                                &registerEndpointFd);
+    LOG_ALWAYS_FATAL_IF(status == -1,
+                        "Failed to register background thread buffer release consumer file "
+                        "descriptor with epoll. errno=%d message='%s'",
+                        errno, strerror(errno));
+
+    // EventFd is used to break the background thread's loop.
+    android::base::unique_fd eventFd{eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)};
+    LOG_ALWAYS_FATAL_IF(!eventFd.ok(),
+                        "Failed to create background thread buffer release event file descriptor. "
+                        "errno=%d message='%s'",
+                        errno, strerror(errno));
+
+    epoll_event registerEventFd{};
+    registerEventFd.events = EPOLLIN;
+    registerEventFd.data.fd = eventFd.get();
+    status = epoll_ctl(epollFd.get(), EPOLL_CTL_ADD, eventFd.get(), &registerEventFd);
+    LOG_ALWAYS_FATAL_IF(status == -1,
+                        "Failed to register background thread event file descriptor with epoll. "
+                        "errno=%d message='%s'",
+                        errno, strerror(errno));
+
+    mEventFd = eventFd.get();
+
+    std::thread([epollFd = std::move(epollFd), eventFd = std::move(eventFd),
+                 weakBbq = wp<BLASTBufferQueue>(bbq)]() {
         pthread_setname_np(pthread_self(), "BufferReleaseThread");
-        while (*running) {
-            ReleaseCallbackId id;
-            sp<Fence> fence;
-            uint32_t maxAcquiredBufferCount;
-            if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount);
-                status != OK) {
+        while (true) {
+            epoll_event event{};
+            int eventCount;
+            do {
+                eventCount = epoll_wait(epollFd.get(), &event, 1 /*maxevents*/, -1 /*timeout*/);
+            } while (eventCount == -1 && errno != EINTR);
+
+            if (eventCount == -1) {
+                ALOGE("epoll_wait error while waiting for buffer release in background thread. "
+                      "errno=%d message='%s'",
+                      errno, strerror(errno));
                 continue;
             }
+
+            // EventFd is used to join this thread.
+            if (event.data.fd == eventFd.get()) {
+                return;
+            }
+
             sp<BLASTBufferQueue> bbq = weakBbq.promote();
             if (!bbq) {
                 return;
             }
+
+            // If there are threads blocking on dequeue, give those threads priority for handling
+            // the release.
+            if (bbq->mThreadsBlockingOnDequeue > 0) {
+                std::this_thread::sleep_for(0ms);
+                continue;
+            }
+
+            ReleaseCallbackId id;
+            sp<Fence> fence;
+            uint32_t maxAcquiredBufferCount;
+            status_t status = bbq->mBufferReleaseConsumer->readReleaseFence(id, fence,
+                                                                            maxAcquiredBufferCount);
+            if (status != OK) {
+                ALOGE("failed to read from buffer release consumer in background thread. errno=%d "
+                      "message='%s'",
+                      errno, strerror(errno));
+                continue;
+            }
             bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount);
         }
     }).detach();
 }
 
 BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() {
-    *mRunning = false;
-    mReader->interruptBlockingRead();
+    eventfd_write(mEventFd, 1);
 }
 
 #endif
-- 
cgit v1.2.3-59-g8ed1b


From c16a4a5fdef98805af00ee04dcad74622bf3e31b Mon Sep 17 00:00:00 2001
From: Patrick Williams <pdwilliams@google.com>
Date: Sat, 26 Oct 2024 01:48:01 -0500
Subject: Drain buffer release thread in binder callback

Bug: 294133380
Flag: com.android.graphics.libgui.flags.buffer_release_channel
Test: BLASTBufferQueueTest
Change-Id: Ife6abb1ad9253bda836f846907e9bfba225c3dc9
---
 libs/gui/BLASTBufferQueue.cpp           | 154 ++++++++------------------------
 libs/gui/include/gui/BLASTBufferQueue.h |  15 +---
 2 files changed, 37 insertions(+), 132 deletions(-)

(limited to 'libs/gui/BLASTBufferQueue.cpp')

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 49f4cba284..fdc39ed765 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -52,19 +52,18 @@ using namespace std::chrono_literals;
 namespace {
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-// RAII wrapper to defer arbitrary work until the Deferred instance is deleted.
-template <class F>
-class Deferred {
+template <class Mutex>
+class UnlockGuard {
 public:
-    explicit Deferred(F f) : mF{std::move(f)} {}
+    explicit UnlockGuard(Mutex& lock) : mLock{lock} { mLock.unlock(); }
 
-    ~Deferred() { mF(); }
+    ~UnlockGuard() { mLock.lock(); }
 
-    Deferred(const Deferred&) = delete;
-    Deferred& operator=(const Deferred&) = delete;
+    UnlockGuard(const UnlockGuard&) = delete;
+    UnlockGuard& operator=(const UnlockGuard&) = delete;
 
 private:
-    F mF;
+    Mutex& mLock;
 };
 #endif
 
@@ -271,9 +270,6 @@ BLASTBufferQueue::~BLASTBufferQueue() {
 void BLASTBufferQueue::onFirstRef() {
     // safe default, most producers are expected to override this
     mProducer->setMaxDequeuedBufferCount(2);
-#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-    mBufferReleaseThread.emplace(sp<BLASTBufferQueue>::fromExisting(this));
-#endif
 }
 
 void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height,
@@ -297,11 +293,16 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
     mSurfaceControl = surface;
     SurfaceComposerClient::Transaction t;
     if (surfaceControlChanged) {
-        t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
-                   layer_state_t::eEnableBackpressure);
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-        t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer);
+        // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
+        // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. This
+        // transaction is applied separately to ensure we don't lose the other updates.
+        t.setApplyToken(mApplyToken)
+                .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
+                .apply(false /* synchronous */, true /* oneWay */);
 #endif
+        t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
+                   layer_state_t::eEnableBackpressure);
         applyTransaction = true;
     }
     mTransformHint = mSurfaceControl->getTransformHint();
@@ -325,7 +326,7 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
     }
     if (applyTransaction) {
         // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction
-        t.setApplyToken(mApplyToken).apply(false, true);
+        t.setApplyToken(mApplyToken).apply(false /* synchronous */, true /* oneWay */);
     }
 }
 
@@ -419,7 +420,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
                                                     stat.latchTime,
                                                     stat.frameEventStats.dequeueReadyTime);
                 }
-#if !COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
                 auto currFrameNumber = stat.frameEventStats.frameNumber;
                 std::vector<ReleaseCallbackId> staleReleases;
                 for (const auto& [key, value]: mSubmitted) {
@@ -435,7 +435,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
                                                 stat.currentMaxAcquiredBufferCount,
                                                 true /* fakeRelease */);
                 }
-#endif
             } else {
                 BQA_LOGE("Failed to find matching SurfaceControl in transactionCallback");
             }
@@ -469,6 +468,9 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() {
             return;
         }
         bbq->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount);
+#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+        bbq->drainBufferReleaseConsumer();
+#endif
     };
 }
 
@@ -535,8 +537,6 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId,
                                      const sp<Fence>& releaseFence) {
     auto it = mSubmitted.find(callbackId);
     if (it == mSubmitted.end()) {
-        BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s",
-                 callbackId.to_string().c_str());
         return;
     }
     mNumAcquired--;
@@ -646,12 +646,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked(
                            bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform,
                            bufferItem.mScalingMode, crop);
 
-#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-    ReleaseBufferCallback releaseBufferCallback =
-            applyTransaction ? nullptr : makeReleaseBufferCallbackThunk();
-#else
     auto releaseBufferCallback = makeReleaseBufferCallbackThunk();
-#endif
     sp<Fence> fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE;
 
     nsecs_t dequeueTime = -1;
@@ -1230,12 +1225,7 @@ public:
         // we want to ignore it. This must be done before unlocking the BufferQueue lock to ensure
         // we don't miss an interrupt.
         bbq->mBufferReleaseReader->clearInterrupts();
-        bbq->mThreadsBlockingOnDequeue++;
-        bufferQueueLock.unlock();
-        Deferred cleanup{[&]() {
-            bufferQueueLock.lock();
-            bbq->mThreadsBlockingOnDequeue--;
-        }};
+        UnlockGuard unlockGuard{bufferQueueLock};
 
         ATRACE_FORMAT("waiting for free buffer");
         ReleaseCallbackId id;
@@ -1345,6 +1335,21 @@ void BLASTBufferQueue::setApplyToken(sp<IBinder> applyToken) {
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
 
+void BLASTBufferQueue::drainBufferReleaseConsumer() {
+    ATRACE_CALL();
+    while (true) {
+        ReleaseCallbackId id;
+        sp<Fence> fence;
+        uint32_t maxAcquiredBufferCount;
+        status_t status =
+                mBufferReleaseConsumer->readReleaseFence(id, fence, maxAcquiredBufferCount);
+        if (status != OK) {
+            return;
+        }
+        releaseBufferCallback(id, fence, maxAcquiredBufferCount);
+    }
+}
+
 BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(BLASTBufferQueue& bbq) : mBbq{bbq} {
     mEpollFd = android::base::unique_fd{epoll_create1(EPOLL_CLOEXEC)};
     LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(),
@@ -1438,95 +1443,6 @@ void BLASTBufferQueue::BufferReleaseReader::clearInterrupts() {
     }
 }
 
-BLASTBufferQueue::BufferReleaseThread::BufferReleaseThread(const sp<BLASTBufferQueue>& bbq) {
-    android::base::unique_fd epollFd{epoll_create1(EPOLL_CLOEXEC)};
-    LOG_ALWAYS_FATAL_IF(!epollFd.ok(),
-                        "Failed to create buffer release background thread epoll file descriptor. "
-                        "errno=%d message='%s'",
-                        errno, strerror(errno));
-
-    epoll_event registerEndpointFd{};
-    registerEndpointFd.events = EPOLLIN;
-    registerEndpointFd.data.fd = bbq->mBufferReleaseConsumer->getFd();
-    status_t status = epoll_ctl(epollFd.get(), EPOLL_CTL_ADD, bbq->mBufferReleaseConsumer->getFd(),
-                                &registerEndpointFd);
-    LOG_ALWAYS_FATAL_IF(status == -1,
-                        "Failed to register background thread buffer release consumer file "
-                        "descriptor with epoll. errno=%d message='%s'",
-                        errno, strerror(errno));
-
-    // EventFd is used to break the background thread's loop.
-    android::base::unique_fd eventFd{eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)};
-    LOG_ALWAYS_FATAL_IF(!eventFd.ok(),
-                        "Failed to create background thread buffer release event file descriptor. "
-                        "errno=%d message='%s'",
-                        errno, strerror(errno));
-
-    epoll_event registerEventFd{};
-    registerEventFd.events = EPOLLIN;
-    registerEventFd.data.fd = eventFd.get();
-    status = epoll_ctl(epollFd.get(), EPOLL_CTL_ADD, eventFd.get(), &registerEventFd);
-    LOG_ALWAYS_FATAL_IF(status == -1,
-                        "Failed to register background thread event file descriptor with epoll. "
-                        "errno=%d message='%s'",
-                        errno, strerror(errno));
-
-    mEventFd = eventFd.get();
-
-    std::thread([epollFd = std::move(epollFd), eventFd = std::move(eventFd),
-                 weakBbq = wp<BLASTBufferQueue>(bbq)]() {
-        pthread_setname_np(pthread_self(), "BufferReleaseThread");
-        while (true) {
-            epoll_event event{};
-            int eventCount;
-            do {
-                eventCount = epoll_wait(epollFd.get(), &event, 1 /*maxevents*/, -1 /*timeout*/);
-            } while (eventCount == -1 && errno != EINTR);
-
-            if (eventCount == -1) {
-                ALOGE("epoll_wait error while waiting for buffer release in background thread. "
-                      "errno=%d message='%s'",
-                      errno, strerror(errno));
-                continue;
-            }
-
-            // EventFd is used to join this thread.
-            if (event.data.fd == eventFd.get()) {
-                return;
-            }
-
-            sp<BLASTBufferQueue> bbq = weakBbq.promote();
-            if (!bbq) {
-                return;
-            }
-
-            // If there are threads blocking on dequeue, give those threads priority for handling
-            // the release.
-            if (bbq->mThreadsBlockingOnDequeue > 0) {
-                std::this_thread::sleep_for(0ms);
-                continue;
-            }
-
-            ReleaseCallbackId id;
-            sp<Fence> fence;
-            uint32_t maxAcquiredBufferCount;
-            status_t status = bbq->mBufferReleaseConsumer->readReleaseFence(id, fence,
-                                                                            maxAcquiredBufferCount);
-            if (status != OK) {
-                ALOGE("failed to read from buffer release consumer in background thread. errno=%d "
-                      "message='%s'",
-                      errno, strerror(errno));
-                continue;
-            }
-            bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount);
-        }
-    }).detach();
-}
-
-BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() {
-    eventfd_write(mEventFd, 1);
-}
-
 #endif
 
 } // namespace android
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 99c64daa15..4fd44e52f4 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -325,6 +325,8 @@ private:
     std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mBufferReleaseConsumer;
     std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;
 
+    void drainBufferReleaseConsumer();
+
     class BufferReleaseReader {
     public:
         explicit BufferReleaseReader(BLASTBufferQueue&);
@@ -353,19 +355,6 @@ private:
     };
 
     std::optional<BufferReleaseReader> mBufferReleaseReader;
-
-    std::atomic<int> mThreadsBlockingOnDequeue = 0;
-
-    class BufferReleaseThread {
-    public:
-        BufferReleaseThread(const sp<BLASTBufferQueue>&);
-        ~BufferReleaseThread();
-
-    private:
-        int mEventFd;
-    };
-
-    std::optional<BufferReleaseThread> mBufferReleaseThread;
 #endif
 };
 
-- 
cgit v1.2.3-59-g8ed1b


From a419faaaf33e885d614d37aba02b27d2f53a37a9 Mon Sep 17 00:00:00 2001
From: Patrick Williams <pdwilliams@google.com>
Date: Tue, 29 Oct 2024 16:50:27 -0500
Subject: Add warning logs when we fail to set BufferReleaseChannel

Bug: 294133380
Flag: com.android.graphics.libgui.flags.buffer_release_channel
Test: presubmits
Change-Id: I21461ab32ae9c0e75f9cf9851737ed29e4024836
---
 libs/gui/BLASTBufferQueue.cpp           | 21 +++++++++++++++------
 libs/gui/SurfaceComposerClient.cpp      | 15 ++++++++-------
 libs/gui/include/gui/BLASTBufferQueue.h |  6 ++++++
 3 files changed, 29 insertions(+), 13 deletions(-)

(limited to 'libs/gui/BLASTBufferQueue.cpp')

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index fdc39ed765..495418b921 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -294,12 +294,7 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
     SurfaceComposerClient::Transaction t;
     if (surfaceControlChanged) {
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-        // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
-        // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. This
-        // transaction is applied separately to ensure we don't lose the other updates.
-        t.setApplyToken(mApplyToken)
-                .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
-                .apply(false /* synchronous */, true /* oneWay */);
+        updateBufferReleaseProducer();
 #endif
         t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
                    layer_state_t::eEnableBackpressure);
@@ -1335,6 +1330,20 @@ void BLASTBufferQueue::setApplyToken(sp<IBinder> applyToken) {
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
 
+void BLASTBufferQueue::updateBufferReleaseProducer() {
+    // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
+    // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. We send this
+    // transaction independently of any other updates to ensure those updates aren't lost.
+    SurfaceComposerClient::Transaction t;
+    status_t status = t.setApplyToken(mApplyToken)
+                              .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
+                              .apply(false /* synchronous */, true /* oneWay */);
+    if (status != OK) {
+        ALOGW("[%s] %s - failed to set buffer release channel on %s", mName.c_str(),
+              statusToString(status).c_str(), mSurfaceControl->getName().c_str());
+    }
+}
+
 void BLASTBufferQueue::drainBufferReleaseConsumer() {
     ATRACE_CALL();
     while (true) {
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a93fc926c2..13e81bd723 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1347,21 +1347,22 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay
     sp<IBinder> applyToken = mApplyToken ? mApplyToken : getDefaultApplyToken();
 
     sp<ISurfaceComposer> sf(ComposerService::getComposerService());
-    sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,
-                            mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,
-                            mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId,
-                            mMergedTransactionIds);
+    status_t binderStatus =
+            sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags,
+                                    applyToken, mInputWindowCommands, mDesiredPresentTime,
+                                    mIsAutoTimestamp, mUncacheBuffers, hasListenerCallbacks,
+                                    listenerCallbacks, mId, mMergedTransactionIds);
     mId = generateId();
 
     // Clear the current states and flags
     clear();
 
-    if (synchronous) {
+    if (synchronous && binderStatus == OK) {
         syncCallback->wait();
     }
 
     mStatus = NO_ERROR;
-    return NO_ERROR;
+    return binderStatus;
 }
 
 sp<IBinder> SurfaceComposerClient::Transaction::sApplyToken = new BBinder();
@@ -1375,7 +1376,7 @@ sp<IBinder> SurfaceComposerClient::Transaction::getDefaultApplyToken() {
 
 void SurfaceComposerClient::Transaction::setDefaultApplyToken(sp<IBinder> applyToken) {
     std::scoped_lock lock{sApplyTokenMutex};
-    sApplyToken = applyToken;
+    sApplyToken = std::move(applyToken);
 }
 
 status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction(
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 4fd44e52f4..8894b66c6d 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -325,8 +325,14 @@ private:
     std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mBufferReleaseConsumer;
     std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;
 
+    void updateBufferReleaseProducer() REQUIRES(mMutex);
     void drainBufferReleaseConsumer();
 
+    // BufferReleaseReader is used to do blocking but interruptible reads from the buffer
+    // release channel. To implement this, BufferReleaseReader owns an epoll file descriptor that
+    // is configured to wake up when either the BufferReleaseReader::ConsumerEndpoint or an eventfd
+    // becomes readable. Interrupts are necessary because a free buffer may become available for
+    // reasons other than a buffer release from the producer.
     class BufferReleaseReader {
     public:
         explicit BufferReleaseReader(BLASTBufferQueue&);
-- 
cgit v1.2.3-59-g8ed1b


From 628cff4cec8899b1c9cf75d4a3ae80617b97d825 Mon Sep 17 00:00:00 2001
From: Brian Lindahl <blindahl@google.com>
Date: Wed, 30 Oct 2024 11:50:28 -0600
Subject: Allow apps to associate a change in picture profiles alongside a
 buffer

What picture processing a buffer looks best with is often dependent on
the buffer contents itself. It is often necessary for a change in
picture profile to be tightly coupled to a change in buffer.

Bug: 337330263
Test: build
Test: atest BufferQueueTest
Flag: com.android.graphics.libgui.flags.apply_picture_profiles
Change-Id: I8bd3468519fb28a234f6853531638e348b1c5274
---
 libs/gui/BLASTBufferQueue.cpp                   | 18 ++++++-
 libs/gui/BufferItem.cpp                         | 65 +++++++++++++++++--------
 libs/gui/BufferQueueProducer.cpp                |  3 ++
 libs/gui/IGraphicBufferProducerFlattenables.cpp | 37 +++++++++-----
 libs/gui/include/gui/BLASTBufferQueue.h         |  9 +++-
 libs/gui/include/gui/BufferItem.h               |  7 +++
 libs/gui/include/gui/IGraphicBufferProducer.h   | 11 +++++
 libs/gui/tests/BufferQueue_test.cpp             | 58 ++++++++++++++++++++++
 8 files changed, 173 insertions(+), 35 deletions(-)

(limited to 'libs/gui/BLASTBufferQueue.cpp')

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 495418b921..7aee90393b 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -286,18 +286,23 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
     if (surfaceControlChanged && mSurfaceControl != nullptr) {
         BQA_LOGD("Updating SurfaceControl without recreating BBQ");
     }
-    bool applyTransaction = false;
 
     // Always update the native object even though they might have the same layer handle, so we can
     // get the updated transform hint from WM.
     mSurfaceControl = surface;
     SurfaceComposerClient::Transaction t;
+    bool applyTransaction = false;
     if (surfaceControlChanged) {
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
         updateBufferReleaseProducer();
 #endif
         t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
                    layer_state_t::eEnableBackpressure);
+        // Migrate the picture profile handle to the new surface control.
+        if (com_android_graphics_libgui_flags_apply_picture_profiles() &&
+            mPictureProfileHandle.has_value()) {
+            t.setPictureProfileHandle(mSurfaceControl, *mPictureProfileHandle);
+        }
         applyTransaction = true;
     }
     mTransformHint = mSurfaceControl->getTransformHint();
@@ -679,6 +684,17 @@ status_t BLASTBufferQueue::acquireNextBufferLocked(
     if (!bufferItem.mIsAutoTimestamp) {
         t->setDesiredPresentTime(bufferItem.mTimestamp);
     }
+    if (com_android_graphics_libgui_flags_apply_picture_profiles() &&
+        bufferItem.mPictureProfileHandle.has_value()) {
+        t->setPictureProfileHandle(mSurfaceControl, *bufferItem.mPictureProfileHandle);
+        // The current picture profile must be maintained in case the BBQ gets its
+        // SurfaceControl switched out.
+        mPictureProfileHandle = bufferItem.mPictureProfileHandle;
+        // Clear out the picture profile if the requestor has asked for it to be cleared
+        if (mPictureProfileHandle == PictureProfileHandle::NONE) {
+            mPictureProfileHandle = std::nullopt;
+        }
+    }
 
     // Drop stale frame timeline infos
     while (!mPendingFrameTimelines.empty() &&
diff --git a/libs/gui/BufferItem.cpp b/libs/gui/BufferItem.cpp
index 5beba02e63..3b2d337a21 100644
--- a/libs/gui/BufferItem.cpp
+++ b/libs/gui/BufferItem.cpp
@@ -38,26 +38,25 @@ static inline constexpr T to64(const uint32_t lo, const uint32_t hi) {
     return static_cast<T>(static_cast<uint64_t>(hi)<<32 | lo);
 }
 
-BufferItem::BufferItem() :
-    mGraphicBuffer(nullptr),
-    mFence(nullptr),
-    mCrop(Rect::INVALID_RECT),
-    mTransform(0),
-    mScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE),
-    mTimestamp(0),
-    mIsAutoTimestamp(false),
-    mDataSpace(HAL_DATASPACE_UNKNOWN),
-    mFrameNumber(0),
-    mSlot(INVALID_BUFFER_SLOT),
-    mIsDroppable(false),
-    mAcquireCalled(false),
-    mTransformToDisplayInverse(false),
-    mSurfaceDamage(),
-    mAutoRefresh(false),
-    mQueuedBuffer(true),
-    mIsStale(false),
-    mApi(0) {
-}
+BufferItem::BufferItem()
+      : mGraphicBuffer(nullptr),
+        mFence(nullptr),
+        mCrop(Rect::INVALID_RECT),
+        mTransform(0),
+        mScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE),
+        mTimestamp(0),
+        mIsAutoTimestamp(false),
+        mDataSpace(HAL_DATASPACE_UNKNOWN),
+        mFrameNumber(0),
+        mSlot(INVALID_BUFFER_SLOT),
+        mIsDroppable(false),
+        mAcquireCalled(false),
+        mTransformToDisplayInverse(false),
+        mSurfaceDamage(),
+        mAutoRefresh(false),
+        mQueuedBuffer(true),
+        mIsStale(false),
+        mApi(0) {}
 
 BufferItem::~BufferItem() {}
 
@@ -76,6 +75,11 @@ size_t BufferItem::getPodSize() const {
     addAligned(size, high32(mTimestamp));
     addAligned(size, mIsAutoTimestamp);
     addAligned(size, mDataSpace);
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+    addAligned(size, mPictureProfileHandle.has_value());
+    addAligned(size, low32(PictureProfileHandle::NONE.getId()));
+    addAligned(size, high32(PictureProfileHandle::NONE.getId()));
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
     addAligned(size, low32(mFrameNumber));
     addAligned(size, high32(mFrameNumber));
     addAligned(size, mSlot);
@@ -170,6 +174,16 @@ status_t BufferItem::flatten(
     writeAligned(buffer, size, high32(mTimestamp));
     writeAligned(buffer, size, mIsAutoTimestamp);
     writeAligned(buffer, size, mDataSpace);
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+    writeAligned(buffer, size, mPictureProfileHandle.has_value());
+    if (mPictureProfileHandle.has_value()) {
+        writeAligned(buffer, size, low32(mPictureProfileHandle->getId()));
+        writeAligned(buffer, size, high32(mPictureProfileHandle->getId()));
+    } else {
+        writeAligned(buffer, size, low32(PictureProfileHandle::NONE.getId()));
+        writeAligned(buffer, size, high32(PictureProfileHandle::NONE.getId()));
+    }
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
     writeAligned(buffer, size, low32(mFrameNumber));
     writeAligned(buffer, size, high32(mFrameNumber));
     writeAligned(buffer, size, mSlot);
@@ -231,6 +245,7 @@ status_t BufferItem::unflatten(
 
     uint32_t timestampLo = 0, timestampHi = 0;
     uint32_t frameNumberLo = 0, frameNumberHi = 0;
+    int32_t pictureProfileIdLo = 0, pictureProfileIdHi = 0;
 
     readAligned(buffer, size, mCrop);
     readAligned(buffer, size, mTransform);
@@ -240,6 +255,16 @@ status_t BufferItem::unflatten(
     mTimestamp = to64<int64_t>(timestampLo, timestampHi);
     readAligned(buffer, size, mIsAutoTimestamp);
     readAligned(buffer, size, mDataSpace);
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+    bool hasPictureProfileHandle;
+    readAligned(buffer, size, hasPictureProfileHandle);
+    readAligned(buffer, size, pictureProfileIdLo);
+    readAligned(buffer, size, pictureProfileIdHi);
+    mPictureProfileHandle = hasPictureProfileHandle
+            ? std::optional(PictureProfileHandle(
+                      to64<PictureProfileId>(pictureProfileIdLo, pictureProfileIdHi)))
+            : std::nullopt;
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
     readAligned(buffer, size, frameNumberLo);
     readAligned(buffer, size, frameNumberHi);
     mFrameNumber = to64<uint64_t>(frameNumberLo, frameNumberHi);
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 473a374a59..39209f9745 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -938,6 +938,8 @@ status_t BufferQueueProducer::queueBuffer(int slot,
             &getFrameTimestamps);
     const Region& surfaceDamage = input.getSurfaceDamage();
     const HdrMetadata& hdrMetadata = input.getHdrMetadata();
+    const std::optional<PictureProfileHandle>& pictureProfileHandle =
+            input.getPictureProfileHandle();
 
     if (acquireFence == nullptr) {
         BQ_LOGE("queueBuffer: fence is NULL");
@@ -1044,6 +1046,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         item.mIsAutoTimestamp = isAutoTimestamp;
         item.mDataSpace = dataSpace;
         item.mHdrMetadata = hdrMetadata;
+        item.mPictureProfileHandle = pictureProfileHandle;
         item.mFrameNumber = currentFrameNumber;
         item.mSlot = slot;
         item.mFence = acquireFence;
diff --git a/libs/gui/IGraphicBufferProducerFlattenables.cpp b/libs/gui/IGraphicBufferProducerFlattenables.cpp
index c8b9b6751d..4e92a39973 100644
--- a/libs/gui/IGraphicBufferProducerFlattenables.cpp
+++ b/libs/gui/IGraphicBufferProducerFlattenables.cpp
@@ -20,21 +20,19 @@
 namespace android {
 
 constexpr size_t IGraphicBufferProducer::QueueBufferInput::minFlattenedSize() {
-    return sizeof(timestamp) +
-            sizeof(isAutoTimestamp) +
-            sizeof(dataSpace) +
-            sizeof(crop) +
-            sizeof(scalingMode) +
-            sizeof(transform) +
-            sizeof(stickyTransform) +
-            sizeof(getFrameTimestamps) +
-            sizeof(slot);
+    return sizeof(timestamp) + sizeof(isAutoTimestamp) + sizeof(dataSpace) + sizeof(crop) +
+            sizeof(scalingMode) + sizeof(transform) + sizeof(stickyTransform) +
+            sizeof(getFrameTimestamps) + sizeof(slot) +
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+            sizeof(decltype(pictureProfileHandle.has_value())) +
+            sizeof(decltype(pictureProfileHandle.getId()));
+#else
+            0;
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
 }
 
 size_t IGraphicBufferProducer::QueueBufferInput::getFlattenedSize() const {
-    return minFlattenedSize() +
-            fence->getFlattenedSize() +
-            surfaceDamage.getFlattenedSize() +
+    return minFlattenedSize() + fence->getFlattenedSize() + surfaceDamage.getFlattenedSize() +
             hdrMetadata.getFlattenedSize();
 }
 
@@ -57,6 +55,12 @@ status_t IGraphicBufferProducer::QueueBufferInput::flatten(
     FlattenableUtils::write(buffer, size, transform);
     FlattenableUtils::write(buffer, size, stickyTransform);
     FlattenableUtils::write(buffer, size, getFrameTimestamps);
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+    FlattenableUtils::write(buffer, size, pictureProfileHandle.has_value());
+    FlattenableUtils::write(buffer, size,
+                            pictureProfileHandle.has_value() ? pictureProfileHandle->getId()
+                                                             : PictureProfileHandle::NONE.getId());
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
 
     status_t result = fence->flatten(buffer, size, fds, count);
     if (result != NO_ERROR) {
@@ -91,6 +95,15 @@ status_t IGraphicBufferProducer::QueueBufferInput::unflatten(
     FlattenableUtils::read(buffer, size, transform);
     FlattenableUtils::read(buffer, size, stickyTransform);
     FlattenableUtils::read(buffer, size, getFrameTimestamps);
+#if COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
+    bool hasPictureProfileHandle;
+    FlattenableUtils::read(buffer, size, hasPictureProfileHandle);
+    PictureProfileId pictureProfileId;
+    FlattenableUtils::read(buffer, size, pictureProfileId);
+    pictureProfileHandle = hasPictureProfileHandle
+            ? std::optional(PictureProfileHandle(pictureProfileId))
+            : std::nullopt;
+#endif // COM_ANDROID_GRAPHICS_LIBUI_FLAGS_APPLY_PICTURE_PROFILES
 
     fence = new Fence();
     status_t result = fence->unflatten(buffer, size, fds, count);
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 8894b66c6d..07558aa49d 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -17,7 +17,9 @@
 #ifndef ANDROID_GUI_BLAST_BUFFER_QUEUE_H
 #define ANDROID_GUI_BLAST_BUFFER_QUEUE_H
 
-#include <com_android_graphics_libgui_flags.h>
+#include <optional>
+#include <queue>
+
 #include <gui/BufferItem.h>
 #include <gui/BufferItemConsumer.h>
 #include <gui/IGraphicBufferConsumer.h>
@@ -29,7 +31,6 @@
 #include <utils/RefBase.h>
 
 #include <system/window.h>
-#include <queue>
 
 #include <com_android_graphics_libgui_flags.h>
 
@@ -222,6 +223,10 @@ private:
     ui::Size mRequestedSize GUARDED_BY(mMutex);
     int32_t mFormat GUARDED_BY(mMutex);
 
+    // Keep a copy of the current picture profile handle, so it can be moved to a new
+    // SurfaceControl when BBQ migrates via ::update.
+    std::optional<PictureProfileHandle> mPictureProfileHandle;
+
     struct BufferInfo {
         bool hasBuffer = false;
         uint32_t width;
diff --git a/libs/gui/include/gui/BufferItem.h b/libs/gui/include/gui/BufferItem.h
index 218bb424fb..2f85c62a54 100644
--- a/libs/gui/include/gui/BufferItem.h
+++ b/libs/gui/include/gui/BufferItem.h
@@ -17,9 +17,12 @@
 #ifndef ANDROID_GUI_BUFFERITEM_H
 #define ANDROID_GUI_BUFFERITEM_H
 
+#include <optional>
+
 #include <gui/HdrMetadata.h>
 
 #include <ui/FenceTime.h>
+#include <ui/PictureProfileHandle.h>
 #include <ui/Rect.h>
 #include <ui/Region.h>
 
@@ -91,6 +94,10 @@ class BufferItem : public Flattenable<BufferItem> {
     // mHdrMetadata is the HDR metadata associated with this buffer slot.
     HdrMetadata mHdrMetadata;
 
+    // mPictureProfileHandle is a handle that points to a set of parameters that configure picture
+    // processing hardware to enhance the quality of buffer contents.
+    std::optional<PictureProfileHandle> mPictureProfileHandle;
+
     // mFrameNumber is the number of the queued frame for this slot.
     uint64_t mFrameNumber;
 
diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h
index 3aac457a09..001e570982 100644
--- a/libs/gui/include/gui/IGraphicBufferProducer.h
+++ b/libs/gui/include/gui/IGraphicBufferProducer.h
@@ -19,6 +19,7 @@
 
 #include <stdint.h>
 #include <sys/types.h>
+#include <optional>
 
 #include <utils/Errors.h>
 #include <utils/RefBase.h>
@@ -28,6 +29,7 @@
 #include <ui/BufferQueueDefs.h>
 #include <ui/Fence.h>
 #include <ui/GraphicBuffer.h>
+#include <ui/PictureProfileHandle.h>
 #include <ui/Rect.h>
 #include <ui/Region.h>
 
@@ -365,6 +367,14 @@ public:
         const HdrMetadata& getHdrMetadata() const { return hdrMetadata; }
         void setHdrMetadata(const HdrMetadata& metadata) { hdrMetadata = metadata; }
 
+        const std::optional<PictureProfileHandle>& getPictureProfileHandle() const {
+            return pictureProfileHandle;
+        }
+        void setPictureProfileHandle(const PictureProfileHandle& profile) {
+            pictureProfileHandle = profile;
+        }
+        void clearPictureProfileHandle() { pictureProfileHandle = std::nullopt; }
+
         int64_t timestamp{0};
         int isAutoTimestamp{0};
         android_dataspace dataSpace{HAL_DATASPACE_UNKNOWN};
@@ -377,6 +387,7 @@ public:
         bool getFrameTimestamps{false};
         int slot{-1};
         HdrMetadata hdrMetadata;
+        std::optional<PictureProfileHandle> pictureProfileHandle;
     };
 
     struct QueueBufferOutput : public Flattenable<QueueBufferOutput> {
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index 2e6ffcb57f..b026e640aa 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -27,6 +27,7 @@
 #include <gui/Surface.h>
 
 #include <ui/GraphicBuffer.h>
+#include <ui/PictureProfileHandle.h>
 
 #include <android-base/properties.h>
 
@@ -1569,4 +1570,61 @@ TEST_F(BufferQueueTest, TestAdditionalOptions) {
     EXPECT_EQ(ADATASPACE_UNKNOWN, dataSpace);
 }
 
+TEST_F(BufferQueueTest, PassesThroughPictureProfileHandle) {
+    createBufferQueue();
+    sp<MockConsumer> mc(new MockConsumer);
+    mConsumer->consumerConnect(mc, false);
+
+    IGraphicBufferProducer::QueueBufferOutput qbo;
+    mProducer->connect(new StubProducerListener, NATIVE_WINDOW_API_CPU, false, &qbo);
+    mProducer->setMaxDequeuedBufferCount(2);
+    mConsumer->setMaxAcquiredBufferCount(2);
+
+    // First try to pass a valid picture profile handle
+    {
+        int slot;
+        sp<Fence> fence;
+        sp<GraphicBuffer> buf;
+        IGraphicBufferProducer::QueueBufferInput qbi(0, false, HAL_DATASPACE_UNKNOWN,
+                                                     Rect(0, 0, 1, 1),
+                                                     NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
+                                                     Fence::NO_FENCE);
+        qbi.setPictureProfileHandle(PictureProfileHandle(1));
+
+        EXPECT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+                  mProducer->dequeueBuffer(&slot, &fence, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN,
+                                           nullptr, nullptr));
+        EXPECT_EQ(OK, mProducer->requestBuffer(slot, &buf));
+        EXPECT_EQ(OK, mProducer->queueBuffer(slot, qbi, &qbo));
+
+        BufferItem item;
+        EXPECT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+
+        ASSERT_TRUE(item.mPictureProfileHandle.has_value());
+        ASSERT_EQ(item.mPictureProfileHandle, PictureProfileHandle(1));
+    }
+
+    // Then validate that the picture profile handle isn't sticky and is reset for the next buffer
+    {
+        int slot;
+        sp<Fence> fence;
+        sp<GraphicBuffer> buf;
+        IGraphicBufferProducer::QueueBufferInput qbi(0, false, HAL_DATASPACE_UNKNOWN,
+                                                     Rect(0, 0, 1, 1),
+                                                     NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
+                                                     Fence::NO_FENCE);
+
+        EXPECT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+                  mProducer->dequeueBuffer(&slot, &fence, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN,
+                                           nullptr, nullptr));
+        EXPECT_EQ(OK, mProducer->requestBuffer(slot, &buf));
+        EXPECT_EQ(OK, mProducer->queueBuffer(slot, qbi, &qbo));
+
+        BufferItem item;
+        EXPECT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
+
+        ASSERT_FALSE(item.mPictureProfileHandle.has_value());
+    }
+}
+
 } // namespace android
-- 
cgit v1.2.3-59-g8ed1b