Add warning logs when we fail to set BufferReleaseChannel
Bug: 294133380
Flag: com.android.graphics.libgui.flags.buffer_release_channel
Test: presubmits
Change-Id: I21461ab32ae9c0e75f9cf9851737ed29e4024836
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index fdc39ed..495418b 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -294,12 +294,7 @@
SurfaceComposerClient::Transaction t;
if (surfaceControlChanged) {
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
- // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
- // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. This
- // transaction is applied separately to ensure we don't lose the other updates.
- t.setApplyToken(mApplyToken)
- .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
- .apply(false /* synchronous */, true /* oneWay */);
+ updateBufferReleaseProducer();
#endif
t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
layer_state_t::eEnableBackpressure);
@@ -1335,6 +1330,20 @@
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
+void BLASTBufferQueue::updateBufferReleaseProducer() {
+ // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
+ // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. We send this
+ // transaction independently of any other updates to ensure those updates aren't lost.
+ SurfaceComposerClient::Transaction t;
+ status_t status = t.setApplyToken(mApplyToken)
+ .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
+ .apply(false /* synchronous */, true /* oneWay */);
+ if (status != OK) {
+ ALOGW("[%s] %s - failed to set buffer release channel on %s", mName.c_str(),
+ statusToString(status).c_str(), mSurfaceControl->getName().c_str());
+ }
+}
+
void BLASTBufferQueue::drainBufferReleaseConsumer() {
ATRACE_CALL();
while (true) {
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a93fc92..13e81bd 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1347,21 +1347,22 @@
sp<IBinder> applyToken = mApplyToken ? mApplyToken : getDefaultApplyToken();
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
- sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,
- mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,
- mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId,
- mMergedTransactionIds);
+ status_t binderStatus =
+ sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags,
+ applyToken, mInputWindowCommands, mDesiredPresentTime,
+ mIsAutoTimestamp, mUncacheBuffers, hasListenerCallbacks,
+ listenerCallbacks, mId, mMergedTransactionIds);
mId = generateId();
// Clear the current states and flags
clear();
- if (synchronous) {
+ if (synchronous && binderStatus == OK) {
syncCallback->wait();
}
mStatus = NO_ERROR;
- return NO_ERROR;
+ return binderStatus;
}
sp<IBinder> SurfaceComposerClient::Transaction::sApplyToken = new BBinder();
@@ -1375,7 +1376,7 @@
void SurfaceComposerClient::Transaction::setDefaultApplyToken(sp<IBinder> applyToken) {
std::scoped_lock lock{sApplyTokenMutex};
- sApplyToken = applyToken;
+ sApplyToken = std::move(applyToken);
}
status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction(
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 4fd44e5..8894b66 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -325,8 +325,14 @@
std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mBufferReleaseConsumer;
std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;
+ void updateBufferReleaseProducer() REQUIRES(mMutex);
void drainBufferReleaseConsumer();
+ // BufferReleaseReader is used to do blocking but interruptible reads from the buffer
+ // release channel. To implement this, BufferReleaseReader owns an epoll file descriptor that
+ // is configured to wake up when either the BufferReleaseReader::ConsumerEndpoint or an eventfd
+ // becomes readable. Interrupts are necessary because a free buffer may become available for
+ // reasons other than a buffer release from the producer.
class BufferReleaseReader {
public:
explicit BufferReleaseReader(BLASTBufferQueue&);