Adding triple buffering without wait
Bug: 141939236
Test: build, boot, libgui_test
Change-Id: Ibffe3f10f07493700c0ead97a96548bd13cae953
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 3c31d74..06a5f06 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#undef LOG_TAG
+#define LOG_TAG "BLASTBufferQueue"
+
#include <gui/BLASTBufferQueue.h>
#include <gui/BufferItemConsumer.h>
@@ -24,8 +27,14 @@
namespace android {
BLASTBufferQueue::BLASTBufferQueue(const sp<SurfaceControl>& surface, int width, int height)
- : mSurfaceControl(surface), mWidth(width), mHeight(height) {
+ : mSurfaceControl(surface),
+ mPendingCallbacks(0),
+ mWidth(width),
+ mHeight(height),
+ mNextTransaction(nullptr) {
BufferQueue::createBufferQueue(&mProducer, &mConsumer);
+ mConsumer->setMaxBufferCount(MAX_BUFFERS);
+ mProducer->setMaxDequeuedBufferCount(MAX_BUFFERS - 1);
mBufferItemConsumer =
new BufferItemConsumer(mConsumer, AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER, 1, true);
mBufferItemConsumer->setName(String8("BLAST Consumer"));
@@ -34,6 +43,8 @@
mBufferItemConsumer->setDefaultBufferSize(mWidth, mHeight);
mBufferItemConsumer->setDefaultBufferFormat(PIXEL_FORMAT_RGBA_8888);
mBufferItemConsumer->setTransformHint(mSurfaceControl->getTransformHint());
+
+ mAcquired = false;
}
void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, int width, int height) {
@@ -59,20 +70,32 @@
const std::vector<SurfaceControlStats>& stats) {
std::unique_lock _lock{mMutex};
- if (stats.size() > 0 && mNextCallbackBufferItem.mGraphicBuffer != nullptr) {
+ if (stats.size() > 0 && !mShadowQueue.empty()) {
mBufferItemConsumer->releaseBuffer(mNextCallbackBufferItem,
stats[0].previousReleaseFence
? stats[0].previousReleaseFence
: Fence::NO_FENCE);
+ mAcquired = false;
mNextCallbackBufferItem = BufferItem();
mBufferItemConsumer->setTransformHint(stats[0].transformHint);
}
- mDequeueWaitCV.notify_all();
+ mPendingCallbacks--;
+ processNextBufferLocked();
+ mCallbackCV.notify_all();
decStrong((void*)transactionCallbackThunk);
}
-void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
- std::unique_lock _lock{mMutex};
+void BLASTBufferQueue::processNextBufferLocked() {
+ if (mShadowQueue.empty()) {
+ return;
+ }
+
+ if (mAcquired) {
+ return;
+ }
+
+ BufferItem item = std::move(mShadowQueue.front());
+ mShadowQueue.pop();
SurfaceComposerClient::Transaction localTransaction;
bool applyTransaction = true;
@@ -83,11 +106,11 @@
applyTransaction = false;
}
- int status = OK;
mNextCallbackBufferItem = mLastSubmittedBufferItem;
-
mLastSubmittedBufferItem = BufferItem();
- status = mBufferItemConsumer->acquireBuffer(&mLastSubmittedBufferItem, -1, false);
+
+ status_t status = mBufferItemConsumer->acquireBuffer(&mLastSubmittedBufferItem, -1, false);
+ mAcquired = true;
if (status != OK) {
ALOGE("Failed to acquire?");
}
@@ -99,7 +122,6 @@
return;
}
-
// Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback.
incStrong((void*)transactionCallbackThunk);
@@ -112,17 +134,21 @@
t->setCrop(mSurfaceControl, {0, 0, (int32_t)buffer->getWidth(), (int32_t)buffer->getHeight()});
if (applyTransaction) {
- ALOGE("Apply transaction");
t->apply();
-
- if (mNextCallbackBufferItem.mGraphicBuffer != nullptr) {
- mDequeueWaitCV.wait_for(_lock, 5000ms);
- }
}
}
+void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {
+ std::lock_guard _lock{mMutex};
+
+ // add to shadow queue
+ mShadowQueue.push(item);
+ processNextBufferLocked();
+ mPendingCallbacks++;
+}
+
void BLASTBufferQueue::setNextTransaction(SurfaceComposerClient::Transaction* t) {
- std::unique_lock _lock{mMutex};
+ std::lock_guard _lock{mMutex};
mNextTransaction = t;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a538e14..2c8d4ca 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -189,48 +189,49 @@
}
void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
- std::lock_guard<std::mutex> lock(mMutex);
+ std::unordered_map<CallbackId, CallbackTranslation> callbacksMap;
+ {
+ std::lock_guard<std::mutex> lock(mMutex);
- /* This listener knows all the sp<IBinder> to sp<SurfaceControl> for all its registered
- * callbackIds, except for when Transactions are merged together. This probably cannot be
- * solved before this point because the Transactions could be merged together and applied in a
- * different process.
- *
- * Fortunately, we get all the callbacks for this listener for the same frame together at the
- * same time. This means if any Transactions were merged together, we will get their callbacks
- * at the same time. We can combine all the sp<IBinder> to sp<SurfaceControl> maps for all the
- * callbackIds to generate one super map that contains all the sp<IBinder> to sp<SurfaceControl>
- * that could possibly exist for the callbacks.
- */
- std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>
- surfaceControls;
- for (const auto& transactionStats : listenerStats.transactionStats) {
- for (auto callbackId : transactionStats.callbackIds) {
- auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
- surfaceControls.insert(callbackSurfaceControls.begin(), callbackSurfaceControls.end());
+ /* This listener knows all the sp<IBinder> to sp<SurfaceControl> for all its registered
+ * callbackIds, except for when Transactions are merged together. This probably cannot be
+ * solved before this point because the Transactions could be merged together and applied in
+ * a different process.
+ *
+ * Fortunately, we get all the callbacks for this listener for the same frame together at
+ * the same time. This means if any Transactions were merged together, we will get their
+ * callbacks at the same time. We can combine all the sp<IBinder> to sp<SurfaceControl> maps
+ * for all the callbackIds to generate one super map that contains all the sp<IBinder> to
+ * sp<SurfaceControl> that could possibly exist for the callbacks.
+ */
+ callbacksMap = mCallbacks;
+ for (const auto& transactionStats : listenerStats.transactionStats) {
+ for (auto& callbackId : transactionStats.callbackIds) {
+ mCallbacks.erase(callbackId);
+ }
}
}
-
for (const auto& transactionStats : listenerStats.transactionStats) {
for (auto callbackId : transactionStats.callbackIds) {
- auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
+ auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId];
if (!callbackFunction) {
ALOGE("cannot call null callback function, skipping");
continue;
}
std::vector<SurfaceControlStats> surfaceControlStats;
for (const auto& surfaceStats : transactionStats.surfaceStats) {
- surfaceControlStats.emplace_back(surfaceControls[surfaceStats.surfaceControl],
- surfaceStats.acquireTime,
- surfaceStats.previousReleaseFence,
- surfaceStats.transformHint);
- surfaceControls[surfaceStats.surfaceControl]->setTransformHint(
- surfaceStats.transformHint);
+ surfaceControlStats
+ .emplace_back(callbacksMap[callbackId]
+ .surfaceControls[surfaceStats.surfaceControl],
+ surfaceStats.acquireTime, surfaceStats.previousReleaseFence,
+ surfaceStats.transformHint);
+ callbacksMap[callbackId]
+ .surfaceControls[surfaceStats.surfaceControl]
+ ->setTransformHint(surfaceStats.transformHint);
}
callbackFunction(transactionStats.latchTime, transactionStats.presentFence,
surfaceControlStats);
- mCallbacks.erase(callbackId);
}
}
}
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 6320556..f1758a2 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -27,6 +27,7 @@
#include <utils/RefBase.h>
#include <system/window.h>
+#include <thread>
namespace android {
@@ -50,7 +51,6 @@
void setNextTransaction(SurfaceComposerClient::Transaction *t);
void update(const sp<SurfaceControl>& surface, int width, int height);
-
virtual ~BLASTBufferQueue() = default;
@@ -61,32 +61,35 @@
BLASTBufferQueue& operator = (const BLASTBufferQueue& rhs);
BLASTBufferQueue(const BLASTBufferQueue& rhs);
- sp<SurfaceControl> mSurfaceControl;
-
- mutable std::mutex mMutex;
+ void processNextBufferLocked() REQUIRES(mMutex);
- static const int MAX_BUFFERS = 2;
+ sp<SurfaceControl> mSurfaceControl;
+
+ std::mutex mMutex;
+ std::condition_variable mCallbackCV;
+ uint64_t mPendingCallbacks GUARDED_BY(mMutex);
+
+ static const int MAX_BUFFERS = 3;
struct BufferInfo {
sp<GraphicBuffer> buffer;
int fence;
};
-
- int mDequeuedBuffers = 0;
- int mWidth;
- int mHeight;
+ std::queue<const BufferItem> mShadowQueue GUARDED_BY(mMutex);
+ bool mAcquired GUARDED_BY(mMutex);
- BufferItem mLastSubmittedBufferItem;
- BufferItem mNextCallbackBufferItem;
- sp<Fence> mLastFence;
+ int mWidth GUARDED_BY(mMutex);
+ int mHeight GUARDED_BY(mMutex);
- std::condition_variable mDequeueWaitCV;
+ BufferItem mLastSubmittedBufferItem GUARDED_BY(mMutex);
+ BufferItem mNextCallbackBufferItem GUARDED_BY(mMutex);
+ sp<Fence> mLastFence GUARDED_BY(mMutex);
sp<IGraphicBufferConsumer> mConsumer;
sp<IGraphicBufferProducer> mProducer;
sp<BufferItemConsumer> mBufferItemConsumer;
- SurfaceComposerClient::Transaction* mNextTransaction = nullptr;
+ SurfaceComposerClient::Transaction* mNextTransaction GUARDED_BY(mMutex);
};
} // namespace android
diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp
index ff22913..6ecdae5 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -19,6 +19,8 @@
#include <gui/BLASTBufferQueue.h>
#include <android/hardware/graphics/common/1.2/types.h>
+#include <gui/BufferQueueCore.h>
+#include <gui/BufferQueueProducer.h>
#include <gui/IGraphicBufferProducer.h>
#include <gui/IProducerListener.h>
#include <gui/SurfaceComposerClient.h>
@@ -65,9 +67,11 @@
return mBlastBufferQueueAdapter->mSurfaceControl;
}
- void waitForCallback() {
+ void waitForCallbacks() {
std::unique_lock lock{mBlastBufferQueueAdapter->mMutex};
- mBlastBufferQueueAdapter->mDequeueWaitCV.wait_for(lock, 1s);
+ while (mBlastBufferQueueAdapter->mPendingCallbacks > 0) {
+ mBlastBufferQueueAdapter->mCallbackCV.wait(lock);
+ }
}
private:
@@ -116,6 +120,17 @@
.apply();
}
+ void setUpProducer(BLASTBufferQueueHelper adapter, sp<IGraphicBufferProducer>& producer) {
+ auto igbProducer = adapter.getIGraphicBufferProducer();
+ ASSERT_NE(nullptr, igbProducer.get());
+ IGraphicBufferProducer::QueueBufferOutput qbOutput;
+ ASSERT_EQ(NO_ERROR,
+ igbProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
+ &qbOutput));
+ ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
+ producer = igbProducer;
+ }
+
void fillBuffer(uint32_t* bufData, uint32_t width, uint32_t height, uint32_t stride, uint8_t r,
uint8_t g, uint8_t b) {
for (uint32_t row = 0; row < height; row++) {
@@ -195,14 +210,8 @@
uint8_t b = 0;
BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);
- auto igbProducer = adapter.getIGraphicBufferProducer();
- ASSERT_NE(nullptr, igbProducer.get());
- IGraphicBufferProducer::QueueBufferOutput qbOutput;
- ASSERT_EQ(NO_ERROR,
- igbProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false,
- &qbOutput));
- ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(3));
- ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
+ sp<IGraphicBufferProducer> igbProducer;
+ setUpProducer(adapter, igbProducer);
int slot;
sp<Fence> fence;
@@ -219,6 +228,7 @@
fillBuffer(bufData, buf->getWidth(), buf->getHeight(), buf->getStride(), r, g, b);
buf->unlock();
+ IGraphicBufferProducer::QueueBufferOutput qbOutput;
IGraphicBufferProducer::QueueBufferInput input(systemTime(), false, HAL_DATASPACE_UNKNOWN,
Rect(mDisplayWidth, mDisplayHeight),
NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
@@ -226,7 +236,7 @@
igbProducer->queueBuffer(slot, input, &qbOutput);
ASSERT_NE(ui::Transform::orientation_flags::ROT_INVALID, qbOutput.transformHint);
- adapter.waitForCallback();
+ sleep(1);
// capture screen and verify that it is red
bool capturedSecureLayers;
@@ -237,4 +247,43 @@
/*useIdentityTransform*/ false));
ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b));
}
+
+TEST_F(BLASTBufferQueueTest, TripleBuffering) {
+ BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);
+ sp<IGraphicBufferProducer> igbProducer;
+ setUpProducer(adapter, igbProducer);
+
+ std::vector<std::pair<int, sp<Fence>>> allocated;
+ for (int i = 0; i < 3; i++) {
+ int slot;
+ sp<Fence> fence;
+ sp<GraphicBuffer> buf;
+ auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight,
+ PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN,
+ nullptr, nullptr);
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret);
+ ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf));
+ allocated.push_back({slot, fence});
+ }
+ for (int i = 0; i < allocated.size(); i++) {
+ igbProducer->cancelBuffer(allocated[i].first, allocated[i].second);
+ }
+
+ for (int i = 0; i < 10; i++) {
+ int slot;
+ sp<Fence> fence;
+ sp<GraphicBuffer> buf;
+ auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight,
+ PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN,
+ nullptr, nullptr);
+ ASSERT_EQ(NO_ERROR, ret);
+ IGraphicBufferProducer::QueueBufferOutput qbOutput;
+ IGraphicBufferProducer::QueueBufferInput input(systemTime(), false, HAL_DATASPACE_UNKNOWN,
+ Rect(mDisplayWidth, mDisplayHeight),
+ NATIVE_WINDOW_SCALING_MODE_FREEZE, 0,
+ Fence::NO_FENCE);
+ igbProducer->queueBuffer(slot, input, &qbOutput);
+ }
+ adapter.waitForCallbacks();
+}
} // namespace android
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index aef7aed..103f775 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -543,7 +543,6 @@
// Should now be able to dequeue up to minBuffers times
DequeueBufferResult result;
for (int i = 0; i < minBuffers; ++i) {
-
EXPECT_EQ(OK, ~IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION &
(dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
TEST_PRODUCER_USAGE_BITS, &result)))