BBQ: Recreate BBQ when SurfaceControl changes 1/2
Alternative approach to fab15e55446080bdcfc05ba315e8ef914b0a6f65 which
was racy because the disconnect callback is called without the
BQ lock and the client can continue to modify the BQ state after
disconnecting from the queue.
This approach resets the BQ and BBQ states when the SurfaceControl is
updated. This solves one concrete problem of not relying on the old
SurfaceControl to be destroyed in order to release the currently presented
buffer back to BQ.
In addition this change resets the sync state in BBQ with the rationale
the system does not want to sync on buffers presented on an older
SurfaceControl.
Bug: 197269223
Test: atest BLASTBufferQueueTest
Test: labtest ag/16407859 cf-foldable * 3 (b/197269223#comment40)
Change-Id: Id7049c3fcb7f68ed1bcaed8b82bd13b4397af000
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 85a4b67..34faf87 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -164,8 +164,7 @@
mCurrentMaxAcquiredBufferCount = mMaxAcquiredBuffers;
mNumAcquired = 0;
mNumFrameAvailable = 0;
- BQA_LOGV("BLASTBufferQueue created width=%d height=%d format=%d mTransformHint=%d", mSize.width,
- mSize.height, mFormat, mTransformHint);
+ BQA_LOGV("BLASTBufferQueue created");
}
BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp<SurfaceControl>& surface,
@@ -182,14 +181,14 @@
BQA_LOGE("Applying pending transactions on dtor %d",
static_cast<uint32_t>(mPendingTransactions.size()));
SurfaceComposerClient::Transaction t;
- for (auto& [targetFrameNumber, transaction] : mPendingTransactions) {
- t.merge(std::move(transaction));
- }
- t.apply();
+ mergePendingTransactions(&t, std::numeric_limits<uint64_t>::max() /* frameNumber */);
+ t.setApplyToken(mApplyToken).apply();
}
void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height,
int32_t format, SurfaceComposerClient::Transaction* outTransaction) {
+ LOG_ALWAYS_FATAL_IF(surface == nullptr, "BLASTBufferQueue: mSurfaceControl must not be NULL");
+
std::unique_lock _lock{mMutex};
if (mFormat != format) {
mFormat = format;
@@ -197,21 +196,20 @@
}
SurfaceComposerClient::Transaction t;
- const bool setBackpressureFlag = !SurfaceControl::isSameSurface(mSurfaceControl, surface);
+ const bool surfaceControlChanged = !SurfaceControl::isSameSurface(mSurfaceControl, surface);
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;
- if (mSurfaceControl != nullptr) {
- if (setBackpressureFlag) {
- t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
- layer_state_t::eEnableBackpressure);
- applyTransaction = true;
- }
- mTransformHint = mSurfaceControl->getTransformHint();
- mBufferItemConsumer->setTransformHint(mTransformHint);
+ if (surfaceControlChanged) {
+ BQA_LOGD("Updating SurfaceControl without recreating BBQ");
+ t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
+ layer_state_t::eEnableBackpressure);
+ applyTransaction = true;
}
+ mTransformHint = mSurfaceControl->getTransformHint();
+ mBufferItemConsumer->setTransformHint(mTransformHint);
BQA_LOGV("update width=%d height=%d format=%d mTransformHint=%d", width, height, format,
mTransformHint);
@@ -225,11 +223,9 @@
mSize = mRequestedSize;
SurfaceComposerClient::Transaction* destFrameTransaction =
(outTransaction) ? outTransaction : &t;
- if (mSurfaceControl != nullptr) {
- destFrameTransaction->setDestinationFrame(mSurfaceControl,
- Rect(0, 0, newSize.getWidth(),
- newSize.getHeight()));
- }
+ destFrameTransaction->setDestinationFrame(mSurfaceControl,
+ Rect(0, 0, newSize.getWidth(),
+ newSize.getHeight()));
applyTransaction = true;
}
}
@@ -640,7 +636,7 @@
// add to shadow queue
mNumFrameAvailable++;
- if (mWaitForTransactionCallback && mNumFrameAvailable == 2) {
+ if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) {
acquireAndReleaseBuffer();
}
ATRACE_INT(mQueuedBufferTrace.c_str(),
@@ -717,7 +713,7 @@
// 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;
+ return mNumAcquired >= maxAcquiredBuffers;
}
class BBQSurface : public Surface {
@@ -991,4 +987,54 @@
return mLastAcquiredFrameNumber;
}
+void BLASTBufferQueue::abandon() {
+ std::unique_lock _lock{mMutex};
+ // flush out the shadow queue
+ while (mNumFrameAvailable > 0) {
+ acquireAndReleaseBuffer();
+ }
+
+ // Clear submitted buffer states
+ mNumAcquired = 0;
+ mSubmitted.clear();
+ mPendingRelease.clear();
+
+ if (!mPendingTransactions.empty()) {
+ BQA_LOGD("Applying pending transactions on abandon %d",
+ static_cast<uint32_t>(mPendingTransactions.size()));
+ SurfaceComposerClient::Transaction t;
+ mergePendingTransactions(&t, std::numeric_limits<uint64_t>::max() /* frameNumber */);
+ t.setApplyToken(mApplyToken).apply();
+ }
+
+ // Clear sync states
+ if (mWaitForTransactionCallback) {
+ BQA_LOGD("mWaitForTransactionCallback cleared");
+ mWaitForTransactionCallback = false;
+ }
+
+ if (mSyncTransaction != nullptr) {
+ BQA_LOGD("mSyncTransaction cleared mAcquireSingleBuffer=%s",
+ mAcquireSingleBuffer ? "true" : "false");
+ mSyncTransaction = nullptr;
+ mAcquireSingleBuffer = false;
+ }
+
+ // abandon buffer queue
+ if (mBufferItemConsumer != nullptr) {
+ mBufferItemConsumer->abandon();
+ mBufferItemConsumer->setFrameAvailableListener(nullptr);
+ mBufferItemConsumer->setBufferFreedListener(nullptr);
+ mBufferItemConsumer->setBlastBufferQueue(nullptr);
+ }
+ mBufferItemConsumer = nullptr;
+ mConsumer = nullptr;
+ mProducer = nullptr;
+}
+
+bool BLASTBufferQueue::isSameSurfaceControl(const sp<SurfaceControl>& surfaceControl) const {
+ std::unique_lock _lock{mMutex};
+ return SurfaceControl::isSameSurface(mSurfaceControl, surfaceControl);
+}
+
} // namespace android