diff options
| -rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 139 | ||||
| -rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 6 | ||||
| -rw-r--r-- | libs/gui/tests/BLASTBufferQueue_test.cpp | 57 | 
3 files changed, 127 insertions, 75 deletions
| diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index b7a7aa0ccb..8e23eb8766 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -137,6 +137,7 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati          mSize(1, 1),          mRequestedSize(mSize),          mFormat(PIXEL_FORMAT_RGBA_8888), +        mTransactionReadyCallback(nullptr),          mSyncTransaction(nullptr),          mUpdateDestinationFrame(updateDestinationFrame) {      createBufferQueue(&mProducer, &mConsumer); @@ -608,60 +609,69 @@ void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& l  }  void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { -    BBQ_TRACE(); -    std::unique_lock _lock{mMutex}; +    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr; +    SurfaceComposerClient::Transaction* prevTransaction = nullptr; +    { +        BBQ_TRACE(); +        std::unique_lock _lock{mMutex}; +        const bool syncTransactionSet = mTransactionReadyCallback != nullptr; +        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. +            if (!mAcquireSingleBuffer) { +                auto bufferData = mSyncTransaction->getAndClearBuffer(mSurfaceControl); +                if (bufferData) { +                    BQA_LOGD("Releasing previous buffer when syncing: framenumber=%" PRIu64, +                             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; +                } +            } -    const bool syncTransactionSet = mSyncTransaction != nullptr; -    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. -        if (!mAcquireSingleBuffer) { -            auto bufferData = mSyncTransaction->getAndClearBuffer(mSurfaceControl); -            if (bufferData) { -                BQA_LOGD("Releasing previous buffer when syncing: framenumber=%" PRIu64, -                         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 (mayNeedToWaitForBuffer) { -            flushAndWaitForFreeBuffer(_lock); +        // add to shadow queue +        mNumFrameAvailable++; +        if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) { +            acquireAndReleaseBuffer();          } -    } - -    // add to shadow queue -    mNumFrameAvailable++; -    if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) { -        acquireAndReleaseBuffer(); -    } -    ATRACE_INT(mQueuedBufferTrace.c_str(), -               mNumFrameAvailable + mNumAcquired - mPendingRelease.size()); - -    BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " syncTransactionSet=%s", item.mFrameNumber, -             boolToString(syncTransactionSet)); - -    if (syncTransactionSet) { -        acquireNextBufferLocked(mSyncTransaction); - -        // Only need a commit callback when syncing to ensure the buffer that's synced has been sent -        // to SF -        incStrong((void*)transactionCommittedCallbackThunk); -        mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk, -                                                          static_cast<void*>(this)); - -        if (mAcquireSingleBuffer) { -            mSyncTransaction = nullptr; +        ATRACE_INT(mQueuedBufferTrace.c_str(), +                   mNumFrameAvailable + mNumAcquired - mPendingRelease.size()); + +        BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " syncTransactionSet=%s", +                 item.mFrameNumber, boolToString(syncTransactionSet)); + +        if (syncTransactionSet) { +            acquireNextBufferLocked(mSyncTransaction); + +            // Only need a commit callback when syncing to ensure the buffer that's synced has been +            // sent to SF +            incStrong((void*)transactionCommittedCallbackThunk); +            mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk, +                                                              static_cast<void*>(this)); +            mWaitForTransactionCallback = true; +            if (mAcquireSingleBuffer) { +                prevCallback = mTransactionReadyCallback; +                prevTransaction = mSyncTransaction; +                mTransactionReadyCallback = nullptr; +                mSyncTransaction = nullptr; +            } +        } else if (!mWaitForTransactionCallback) { +            acquireNextBufferLocked(std::nullopt);          } -        mWaitForTransactionCallback = true; -    } else if (!mWaitForTransactionCallback) { -        acquireNextBufferLocked(std::nullopt); +    } +    if (prevCallback) { +        prevCallback(prevTransaction);      }  } @@ -680,12 +690,37 @@ void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) {      mDequeueTimestamps.erase(bufferId);  }; -void BLASTBufferQueue::setSyncTransaction(SurfaceComposerClient::Transaction* t, -                                          bool acquireSingleBuffer) { +void BLASTBufferQueue::syncNextTransaction( +        std::function<void(SurfaceComposerClient::Transaction*)> callback, +        bool acquireSingleBuffer) {      BBQ_TRACE();      std::lock_guard _lock{mMutex}; -    mSyncTransaction = t; -    mAcquireSingleBuffer = mSyncTransaction ? acquireSingleBuffer : true; +    mTransactionReadyCallback = callback; +    if (callback) { +        mSyncTransaction = new SurfaceComposerClient::Transaction(); +    } else { +        mSyncTransaction = nullptr; +    } +    mAcquireSingleBuffer = mTransactionReadyCallback ? acquireSingleBuffer : true; +} + +void BLASTBufferQueue::stopContinuousSyncTransaction() { +    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr; +    SurfaceComposerClient::Transaction* prevTransaction = nullptr; +    { +        std::lock_guard _lock{mMutex}; +        bool invokeCallback = mTransactionReadyCallback && !mAcquireSingleBuffer; +        if (invokeCallback) { +            prevCallback = mTransactionReadyCallback; +            prevTransaction = mSyncTransaction; +        } +        mTransactionReadyCallback = nullptr; +        mSyncTransaction = nullptr; +        mAcquireSingleBuffer = true; +    } +    if (prevCallback) { +        prevCallback(prevTransaction); +    }  }  bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 74addeac4a..265ae24255 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -95,7 +95,9 @@ public:                                       const std::vector<SurfaceControlStats>& stats);      void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,                                 std::optional<uint32_t> currentMaxAcquiredBufferCount); -    void setSyncTransaction(SurfaceComposerClient::Transaction* t, bool acquireSingleBuffer = true); +    void syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback, +                             bool acquireSingleBuffer = true); +    void stopContinuousSyncTransaction();      void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);      void applyPendingTransactions(uint64_t frameNumber);      SurfaceComposerClient::Transaction* gatherPendingTransactions(uint64_t frameNumber); @@ -213,6 +215,8 @@ private:      sp<IGraphicBufferProducer> mProducer;      sp<BLASTBufferItemConsumer> mBufferItemConsumer; +    std::function<void(SurfaceComposerClient::Transaction*)> mTransactionReadyCallback +            GUARDED_BY(mMutex);      SurfaceComposerClient::Transaction* mSyncTransaction GUARDED_BY(mMutex);      std::vector<std::tuple<uint64_t /* framenumber */, SurfaceComposerClient::Transaction>>              mPendingTransactions GUARDED_BY(mMutex); diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 179bdd76aa..0c3236c9ce 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -110,15 +110,27 @@ public:          mBlastBufferQueueAdapter->update(sc, width, height, PIXEL_FORMAT_RGBA_8888);      } -    void setSyncTransaction(Transaction* next, bool acquireSingleBuffer = true) { -        mBlastBufferQueueAdapter->setSyncTransaction(next, acquireSingleBuffer); +    void setSyncTransaction(Transaction& next, bool acquireSingleBuffer = true) { +        auto callback = [&next](Transaction* t) { next.merge(std::move(*t)); }; +        mBlastBufferQueueAdapter->syncNextTransaction(callback, acquireSingleBuffer); +    } + +    void syncNextTransaction(std::function<void(Transaction*)> callback, +                             bool acquireSingleBuffer = true) { +        mBlastBufferQueueAdapter->syncNextTransaction(callback, acquireSingleBuffer); +    } + +    void stopContinuousSyncTransaction() { +        mBlastBufferQueueAdapter->stopContinuousSyncTransaction();      }      int getWidth() { return mBlastBufferQueueAdapter->mSize.width; }      int getHeight() { return mBlastBufferQueueAdapter->mSize.height; } -    Transaction* getSyncTransaction() { return mBlastBufferQueueAdapter->mSyncTransaction; } +    std::function<void(Transaction*)> getTransactionReadyCallback() { +        return mBlastBufferQueueAdapter->mTransactionReadyCallback; +    }      sp<IGraphicBufferProducer> getIGraphicBufferProducer() {          return mBlastBufferQueueAdapter->getIGraphicBufferProducer(); @@ -343,7 +355,7 @@ TEST_F(BLASTBufferQueueTest, CreateBLASTBufferQueue) {      ASSERT_EQ(mSurfaceControl, adapter.getSurfaceControl());      ASSERT_EQ(mDisplayWidth, adapter.getWidth());      ASSERT_EQ(mDisplayHeight, adapter.getHeight()); -    ASSERT_EQ(nullptr, adapter.getSyncTransaction()); +    ASSERT_EQ(nullptr, adapter.getTransactionReadyCallback());  }  TEST_F(BLASTBufferQueueTest, Update) { @@ -364,11 +376,12 @@ TEST_F(BLASTBufferQueueTest, Update) {      ASSERT_EQ(mDisplayHeight / 2, height);  } -TEST_F(BLASTBufferQueueTest, SetSyncTransaction) { +TEST_F(BLASTBufferQueueTest, SyncNextTransaction) {      BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); -    Transaction sync; -    adapter.setSyncTransaction(&sync); -    ASSERT_EQ(&sync, adapter.getSyncTransaction()); +    ASSERT_EQ(nullptr, adapter.getTransactionReadyCallback()); +    auto callback = [](Transaction*) {}; +    adapter.syncNextTransaction(callback); +    ASSERT_NE(nullptr, adapter.getTransactionReadyCallback());  }  TEST_F(BLASTBufferQueueTest, DISABLED_onFrameAvailable_ApplyDesiredPresentTime) { @@ -808,7 +821,7 @@ TEST_F(BLASTBufferQueueTest, SyncThenNoSync) {      setUpProducer(adapter, igbProducer);      Transaction sync; -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, 0, 255, 0, 0);      // queue non sync buffer, so this one should get blocked @@ -848,12 +861,12 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactions) {      Transaction mainTransaction;      Transaction sync; -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, 0, 255, 0, 0);      mainTransaction.merge(std::move(sync)); -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, r, g, b, 0);      mainTransaction.merge(std::move(sync)); @@ -889,7 +902,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactionWithNonSync) {      Transaction sync;      // queue a sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, 0, 255, 0, 0);      mainTransaction.merge(std::move(sync)); @@ -898,7 +911,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactionWithNonSync) {      queueBuffer(igbProducer, 0, 0, 255, 0);      // queue another sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, r, g, b, 0);      // Expect 1 buffer to be released because the non sync transaction should merge      // with the sync @@ -937,7 +950,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncRunOutOfBuffers) {      Transaction sync;      // queue a sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, 0, 255, 0, 0);      mainTransaction.merge(std::move(sync)); @@ -948,7 +961,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncRunOutOfBuffers) {      queueBuffer(igbProducer, 0, 0, 255, 0);      // queue another sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, r, g, b, 0);      // Expect 3 buffers to be released because the non sync transactions should merge      // with the sync @@ -994,7 +1007,7 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) {      Transaction sync;      // queue a sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, 0, 255, 0, 0);      mainTransaction.merge(std::move(sync)); @@ -1008,7 +1021,7 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) {      mainTransaction.apply();      // queue another sync transaction -    adapter.setSyncTransaction(&sync); +    adapter.setSyncTransaction(sync);      queueBuffer(igbProducer, r, g, b, 0);      // Expect 2 buffers to be released because the non sync transactions should merge      // with the sync @@ -1031,19 +1044,19 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) {              checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight}));  } -TEST_F(BLASTBufferQueueTest, SetSyncTransactionAcquireMultipleBuffers) { +TEST_F(BLASTBufferQueueTest, SyncNextTransactionAcquireMultipleBuffers) {      BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);      sp<IGraphicBufferProducer> igbProducer;      setUpProducer(adapter, igbProducer);      Transaction next; -    adapter.setSyncTransaction(&next, false); +    adapter.setSyncTransaction(next, false);      queueBuffer(igbProducer, 0, 255, 0, 0);      queueBuffer(igbProducer, 0, 0, 255, 0);      // There should only be one frame submitted since the first frame will be released.      adapter.validateNumFramesSubmitted(1); -    adapter.setSyncTransaction(nullptr); +    adapter.stopContinuousSyncTransaction();      // queue non sync buffer, so this one should get blocked      // Add a present delay to allow the first screenshot to get taken. @@ -1097,7 +1110,7 @@ TEST_F(BLASTBufferQueueTest, DISABLED_DisconnectProducerTest) {          Transaction next;          queueBuffer(igbProducer, 0, 255, 0, 0);          queueBuffer(igbProducer, 0, 0, 255, 0); -        adapter.setSyncTransaction(&next, false); +        adapter.setSyncTransaction(next, true);          queueBuffer(igbProducer, 255, 0, 0, 0);          CallbackHelper transactionCallback; @@ -1140,7 +1153,7 @@ TEST_F(BLASTBufferQueueTest, DISABLED_UpdateSurfaceControlTest) {          Transaction next;          queueBuffer(igbProducer, 0, 255, 0, 0);          queueBuffer(igbProducer, 0, 0, 255, 0); -        adapter.setSyncTransaction(&next, false); +        adapter.setSyncTransaction(next, true);          queueBuffer(igbProducer, 255, 0, 0, 0);          CallbackHelper transactionCallback; |