Merge "Account for release callbacks when determing if sync is finished" into tm-qpr-dev am: 4b383323f7 am: 23a7c8a1a5

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/18740006

Change-Id: I3f59ae397ae474db2b9a5b316c9e020bea6f50df
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

1 files changed, 24 insertions, 18 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index f34061492a..453bdae9cc 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -288,18 +288,17 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,
 
                 // We need to check if we were waiting for a transaction callback in order to
                 // process any pending buffers and unblock. It's possible to get transaction
-                // callbacks for previous requests so we need to ensure the frame from this
-                // transaction callback matches the last acquired buffer. Since acquireNextBuffer
-                // will stop processing buffers when mWaitForTransactionCallback is set, we know
-                // that mLastAcquiredFrameNumber is the frame we're waiting on.
-                // We also want to check if mNextTransaction is null because it's possible another
+                // callbacks for previous requests so we need to ensure that there are no pending
+                // frame numbers that were in a sync. We remove the frame from mSyncedFrameNumbers
+                // set and then check if it's empty. If there are no more pending syncs, we can
+                // proceed with flushing the shadow queue.
+                // We also want to check if mSyncTransaction is null because it's possible another
                 // sync request came in while waiting, but it hasn't started processing yet. In that
                 // case, we don't actually want to flush the frames in between since they will get
                 // processed and merged with the sync transaction and released earlier than if they
                 // were sent to SF
-                if (mWaitForTransactionCallback && mSyncTransaction == nullptr &&
-                    currFrameNumber >= mLastAcquiredFrameNumber) {
-                    mWaitForTransactionCallback = false;
+                mSyncedFrameNumbers.erase(currFrameNumber);
+                if (mSyncedFrameNumbers.empty() && mSyncTransaction == nullptr) {
                     flushShadowQueue();
                 }
             } else {
@@ -417,9 +416,11 @@ void BLASTBufferQueue::releaseBufferCallback(
         const auto releasedBuffer = mPendingRelease.front();
         mPendingRelease.pop_front();
         releaseBuffer(releasedBuffer.callbackId, releasedBuffer.releaseFence);
-        // Don't process the transactions here if mWaitForTransactionCallback is set. Instead, let
-        // onFrameAvailable handle processing them since it will merge with the syncTransaction.
-        if (!mWaitForTransactionCallback) {
+        // Don't process the transactions here if mSyncedFrameNumbers is not empty. That means
+        // are still transactions that have sync buffers in them that have not been applied or
+        // dropped. Instead, let onFrameAvailable handle processing them since it will merge with
+        // the syncTransaction.
+        if (mSyncedFrameNumbers.empty()) {
             acquireNextBufferLocked(std::nullopt);
         }
     }
@@ -443,6 +444,9 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId,
     BQA_LOGV("released %s", callbackId.to_string().c_str());
     mBufferItemConsumer->releaseBuffer(it->second, releaseFence);
     mSubmitted.erase(it);
+    // Remove the frame number from mSyncedFrameNumbers since we can get a release callback
+    // without getting a transaction committed if the buffer was dropped.
+    mSyncedFrameNumbers.erase(callbackId.framenumber);
 }
 
 void BLASTBufferQueue::acquireNextBufferLocked(
@@ -609,7 +613,7 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() {
 }
 
 void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock) {
-    if (mWaitForTransactionCallback && mNumFrameAvailable > 0) {
+    if (!mSyncedFrameNumbers.empty() && mNumFrameAvailable > 0) {
         // We are waiting on a previous sync's transaction callback so allow another sync
         // transaction to proceed.
         //
@@ -636,6 +640,8 @@ void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& l
 void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
     std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;
     SurfaceComposerClient::Transaction* prevTransaction = nullptr;
+    bool waitForTransactionCallback = !mSyncedFrameNumbers.empty();
+
     {
         BBQ_TRACE();
         std::unique_lock _lock{mMutex};
@@ -667,7 +673,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
 
         // add to shadow queue
         mNumFrameAvailable++;
-        if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) {
+        if (waitForTransactionCallback && mNumFrameAvailable >= 2) {
             acquireAndReleaseBuffer();
         }
         ATRACE_INT(mQueuedBufferTrace.c_str(),
@@ -684,14 +690,14 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
             incStrong((void*)transactionCommittedCallbackThunk);
             mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk,
                                                               static_cast<void*>(this));
-            mWaitForTransactionCallback = true;
+            mSyncedFrameNumbers.emplace(item.mFrameNumber);
             if (mAcquireSingleBuffer) {
                 prevCallback = mTransactionReadyCallback;
                 prevTransaction = mSyncTransaction;
                 mTransactionReadyCallback = nullptr;
                 mSyncTransaction = nullptr;
             }
-        } else if (!mWaitForTransactionCallback) {
+        } else if (!waitForTransactionCallback) {
             acquireNextBufferLocked(std::nullopt);
         }
     }
@@ -1098,9 +1104,9 @@ void BLASTBufferQueue::abandon() {
     }
 
     // Clear sync states
-    if (mWaitForTransactionCallback) {
-        BQA_LOGD("mWaitForTransactionCallback cleared");
-        mWaitForTransactionCallback = false;
+    if (!mSyncedFrameNumbers.empty()) {
+        BQA_LOGD("mSyncedFrameNumbers cleared");
+        mSyncedFrameNumbers.clear();
     }
 
     if (mSyncTransaction != nullptr) {