Merge "Changed setSyncTransaction to syncNextTransaction with callback logic." into tm-dev
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index b7a7aa0..8e23eb87 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -137,6 +137,7 @@
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::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));
- 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 (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();
+ }
+ 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);
}
}
-
- // 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;
- }
- mWaitForTransactionCallback = true;
- } else if (!mWaitForTransactionCallback) {
- acquireNextBufferLocked(std::nullopt);
+ if (prevCallback) {
+ prevCallback(prevTransaction);
}
}
@@ -680,12 +690,37 @@
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 74addea..265ae24 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -95,7 +95,9 @@
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 @@
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 179bdd7..0c3236c 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -110,15 +110,27 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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;