diff options
author | Chavi Weingarten <chaviw@google.com> | 2022-12-27 22:00:24 +0000 |
---|---|---|
committer | Chavi Weingarten <chaviw@google.com> | 2023-01-09 22:08:55 +0000 |
commit | 92bf8b0893b7341075728ca5c3d9c58f05215fd1 (patch) | |
tree | c1f6c35cf73d9a479341654bad11e463919f4638 | |
parent | ab97d6e335f56e6c8f8d06b89bf2d44200b5fb6a (diff) | |
download | native-92bf8b0893b7341075728ca5c3d9c58f05215fd1.tar.gz |
[BBQ] Only wait when acquiring a buffer if in a sync.
There's no need to do the explicit maxBuffersAcquired check since the
acquireBuffer call will return a status of NO_BUFFER_AVAILABLE if it
can't acquire any more buffers. Therefore, the code can be simplified so
it will return early in most cases if NO_BUFFER_AVAILABLE is returned.
If there was a request to sync the buffer, then we need to wait on a
free buffer and not return early.
Test: BBQ syncs
Bug: 263340543
Bug: 263570429
Change-Id: I11a81a440bc0d22ba85bcf4d4a068b853ca5cf9c
Merged-In: I11a81a440bc0d22ba85bcf4d4a068b853ca5cf9c
-rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 80 | ||||
-rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 4 |
2 files changed, 28 insertions, 56 deletions
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 24a5295112..8ee9b14945 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -480,20 +480,18 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, mSyncedFrameNumbers.erase(callbackId.framenumber); } -void BLASTBufferQueue::acquireNextBufferLocked( +status_t BLASTBufferQueue::acquireNextBufferLocked( const std::optional<SurfaceComposerClient::Transaction*> transaction) { // If the next transaction is set, we want to guarantee the our acquire will not fail, so don't // include the extra buffer when checking if we can acquire the next buffer. - const bool includeExtraAcquire = !transaction; - const bool maxAcquired = maxBuffersAcquired(includeExtraAcquire); - if (mNumFrameAvailable == 0 || maxAcquired) { - BQA_LOGV("Can't process next buffer maxBuffersAcquired=%s", boolToString(maxAcquired)); - return; + if (mNumFrameAvailable == 0) { + BQA_LOGV("Can't process next buffer. No available frames"); + return NOT_ENOUGH_DATA; } if (mSurfaceControl == nullptr) { BQA_LOGE("ERROR : surface control is null"); - return; + return NAME_NOT_FOUND; } SurfaceComposerClient::Transaction localTransaction; @@ -510,10 +508,10 @@ void BLASTBufferQueue::acquireNextBufferLocked( mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); if (status == BufferQueue::NO_BUFFER_AVAILABLE) { BQA_LOGV("Failed to acquire a buffer, err=NO_BUFFER_AVAILABLE"); - return; + return status; } else if (status != OK) { BQA_LOGE("Failed to acquire a buffer, err=%s", statusToString(status).c_str()); - return; + return status; } auto buffer = bufferItem.mGraphicBuffer; @@ -523,7 +521,7 @@ void BLASTBufferQueue::acquireNextBufferLocked( if (buffer == nullptr) { mBufferItemConsumer->releaseBuffer(bufferItem, Fence::NO_FENCE); BQA_LOGE("Buffer was empty"); - return; + return BAD_VALUE; } if (rejectBuffer(bufferItem)) { @@ -532,8 +530,7 @@ void BLASTBufferQueue::acquireNextBufferLocked( mSize.width, mSize.height, mRequestedSize.width, mRequestedSize.height, buffer->getWidth(), buffer->getHeight(), bufferItem.mTransform); mBufferItemConsumer->releaseBuffer(bufferItem, Fence::NO_FENCE); - acquireNextBufferLocked(transaction); - return; + return acquireNextBufferLocked(transaction); } mNumAcquired++; @@ -621,6 +618,7 @@ void BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "", static_cast<uint32_t>(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), bufferItem.mAutoRefresh ? " mAutoRefresh" : "", bufferItem.mTransform); + return OK; } Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { @@ -643,31 +641,6 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } -void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock) { - if (!mSyncedFrameNumbers.empty() && mNumFrameAvailable > 0) { - // We are waiting on a previous sync's transaction callback so allow another sync - // transaction to proceed. - // - // We need to first flush out the transactions that were in between the two syncs. - // We do this by merging them into mSyncTransaction so any buffer merging will get - // a release callback invoked. The release callback will be async so we need to wait - // on max acquired to make sure we have the capacity to acquire another buffer. - if (maxBuffersAcquired(false /* includeExtraAcquire */)) { - BQA_LOGD("waiting to flush shadow queue..."); - mCallbackCV.wait(lock); - } - while (mNumFrameAvailable > 0) { - // flush out the shadow queue - acquireAndReleaseBuffer(); - } - } - - while (maxBuffersAcquired(false /* includeExtraAcquire */)) { - BQA_LOGD("waiting for free buffer."); - mCallbackCV.wait(lock); - } -} - void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr; SurfaceComposerClient::Transaction* prevTransaction = nullptr; @@ -680,7 +653,6 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { BQA_LOGV("onFrameAvailable-start syncTransactionSet=%s", boolToString(syncTransactionSet)); if (syncTransactionSet) { - bool mayNeedToWaitForBuffer = true; // If we are going to re-use the same mSyncTransaction, release the buffer that may // already be set in the Transaction. This is to allow us a free slot early to continue // processing a new buffer. @@ -691,14 +663,20 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { bufferData->frameNumber); releaseBuffer(bufferData->generateReleaseCallbackId(), bufferData->acquireFence); - // Because we just released a buffer, we know there's no need to wait for a free - // buffer. - mayNeedToWaitForBuffer = false; } } - if (mayNeedToWaitForBuffer) { - flushAndWaitForFreeBuffer(_lock); + if (waitForTransactionCallback) { + // We are waiting on a previous sync's transaction callback so allow another sync + // transaction to proceed. + // + // We need to first flush out the transactions that were in between the two syncs. + // We do this by merging them into mSyncTransaction so any buffer merging will get + // a release callback invoked. + while (mNumFrameAvailable > 0) { + // flush out the shadow queue + acquireAndReleaseBuffer(); + } } } @@ -714,7 +692,12 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { item.mFrameNumber, boolToString(syncTransactionSet)); if (syncTransactionSet) { - acquireNextBufferLocked(mSyncTransaction); + // If there's no available buffer and we're in a sync transaction, we need to wait + // instead of returning since we guarantee a buffer will be acquired for the sync. + while (acquireNextBufferLocked(mSyncTransaction) == BufferQueue::NO_BUFFER_AVAILABLE) { + BQA_LOGD("waiting for available buffer"); + mCallbackCV.wait(_lock); + } // Only need a commit callback when syncing to ensure the buffer that's synced has been // sent to SF @@ -824,15 +807,6 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { return mSize != bufferSize; } -// Check if we have acquired the maximum number of buffers. -// Consumer can acquire an additional buffer if that buffer is not droppable. Set -// includeExtraAcquire is true to include this buffer to the count. Since this depends on the state -// of the buffer, the next acquire may return with NO_BUFFER_AVAILABLE. -bool BLASTBufferQueue::maxBuffersAcquired(bool includeExtraAcquire) const { - int maxAcquiredBuffers = mMaxAcquiredBuffers + (includeExtraAcquire ? 2 : 1); - return mNumAcquired >= maxAcquiredBuffers; -} - class BBQSurface : public Surface { private: std::mutex mMutex; diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 48226b9641..f074bd37d1 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -136,12 +136,11 @@ private: void createBufferQueue(sp<IGraphicBufferProducer>* outProducer, sp<IGraphicBufferConsumer>* outConsumer); - void acquireNextBufferLocked( + status_t acquireNextBufferLocked( const std::optional<SurfaceComposerClient::Transaction*> transaction) REQUIRES(mMutex); Rect computeCrop(const BufferItem& item) REQUIRES(mMutex); // Return true if we need to reject the buffer based on the scaling mode and the buffer size. bool rejectBuffer(const BufferItem& item) REQUIRES(mMutex); - bool maxBuffersAcquired(bool includeExtraAcquire) const REQUIRES(mMutex); static PixelFormat convertBufferFormat(PixelFormat& format); void mergePendingTransactions(SurfaceComposerClient::Transaction* t, uint64_t frameNumber) REQUIRES(mMutex); @@ -150,7 +149,6 @@ private: void acquireAndReleaseBuffer() REQUIRES(mMutex); void releaseBuffer(const ReleaseCallbackId& callbackId, const sp<Fence>& releaseFence) REQUIRES(mMutex); - void flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock); std::string mName; // Represents the queued buffer count from buffer queue, |