Fix unexpected FrameEvents on BufferQueue reconnect
Helps reduce the number of ALOGE's being hit
when switching between apps.
* Notify Layer when the Producer disconnects.
* Avoid sending event deltas from a previous connection.
* Avoid releasing a frame more than once.
Test: adb shell /data/nativetest/libgui_test/libgui_test
--gtest_filter=*GetFrameTimestamps*
Change-Id: I64f314be72ddb154b584d726ac382cd468e345bf
diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index a523cd8..c95c535 100644
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -62,11 +62,12 @@
public:
explicit ProxyConsumerListener(const wp<ConsumerListener>& consumerListener);
virtual ~ProxyConsumerListener();
- virtual void onFrameAvailable(const BufferItem& item) override;
- virtual void onFrameReplaced(const BufferItem& item) override;
- virtual void onBuffersReleased() override;
- virtual void onSidebandStreamChanged() override;
- virtual void addAndGetFrameTimestamps(
+ void onDisconnect() override;
+ void onFrameAvailable(const BufferItem& item) override;
+ void onFrameReplaced(const BufferItem& item) override;
+ void onBuffersReleased() override;
+ void onSidebandStreamChanged() override;
+ void addAndGetFrameTimestamps(
const NewFrameEventsEntry* newTimestamps,
FrameEventHistoryDelta* outDelta) override;
private:
diff --git a/include/gui/FrameTimestamps.h b/include/gui/FrameTimestamps.h
index 0c4af46..bda3c5c 100644
--- a/include/gui/FrameTimestamps.h
+++ b/include/gui/FrameTimestamps.h
@@ -78,6 +78,7 @@
void dump(String8& outString) const;
bool valid{false};
+ int connectId{0};
uint64_t frameNumber{0};
// Whether or not certain points in the frame's life cycle have been
@@ -212,6 +213,8 @@
public:
~ConsumerFrameEventHistory() override;
+ void onDisconnect();
+
void initializeCompositorTiming(const CompositorTiming& compositorTiming);
void addQueue(const NewFrameEventsEntry& newEntry);
@@ -233,11 +236,13 @@
const std::array<FrameEvents, MAX_FRAME_HISTORY>::iterator& frame);
std::array<FrameEventDirtyFields, MAX_FRAME_HISTORY> mFramesDirty;
+
size_t mQueueOffset{0};
size_t mCompositionOffset{0};
size_t mRetireOffset{0};
size_t mReleaseOffset{0};
+ int mCurrentConnectId{0};
bool mProducerWantsEvents{false};
};
diff --git a/include/gui/IConsumerListener.h b/include/gui/IConsumerListener.h
index 93dd4ac..a3c7d64 100644
--- a/include/gui/IConsumerListener.h
+++ b/include/gui/IConsumerListener.h
@@ -43,6 +43,9 @@
ConsumerListener() { }
virtual ~ConsumerListener();
+ // onDisconnect is called when a producer disconnects from the BufferQueue.
+ virtual void onDisconnect() {} /* Asynchronous */
+
// onFrameAvailable is called from queueBuffer each time an additional
// frame becomes available for consumption. This means that frames that
// are queued while in asynchronous mode only trigger the callback if no
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index f76a282..13692eb 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -31,6 +31,13 @@
BufferQueue::ProxyConsumerListener::~ProxyConsumerListener() {}
+void BufferQueue::ProxyConsumerListener::onDisconnect() {
+ sp<ConsumerListener> listener(mConsumerListener.promote());
+ if (listener != NULL) {
+ listener->onDisconnect();
+ }
+}
+
void BufferQueue::ProxyConsumerListener::onFrameAvailable(
const BufferItem& item) {
sp<ConsumerListener> listener(mConsumerListener.promote());
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 3f69b1f..be0dc20 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -1272,6 +1272,7 @@
// Call back without lock held
if (listener != NULL) {
listener->onBuffersReleased();
+ listener->onDisconnect();
}
return status;
diff --git a/libs/gui/FrameTimestamps.cpp b/libs/gui/FrameTimestamps.cpp
index f427e68..019a11e 100644
--- a/libs/gui/FrameTimestamps.cpp
+++ b/libs/gui/FrameTimestamps.cpp
@@ -329,7 +329,7 @@
void ProducerFrameEventHistory::applyFenceDelta(FenceTimeline* timeline,
std::shared_ptr<FenceTime>* dst, const FenceTime::Snapshot& src) const {
- if (CC_UNLIKELY(dst == nullptr)) {
+ if (CC_UNLIKELY(dst == nullptr || dst->get() == nullptr)) {
ALOGE("applyFenceDelta: dst is null.");
return;
}
@@ -364,6 +364,11 @@
ConsumerFrameEventHistory::~ConsumerFrameEventHistory() = default;
+void ConsumerFrameEventHistory::onDisconnect() {
+ mCurrentConnectId++;
+ mProducerWantsEvents = false;
+}
+
void ConsumerFrameEventHistory::initializeCompositorTiming(
const CompositorTiming& compositorTiming) {
mCompositorTiming = compositorTiming;
@@ -372,6 +377,7 @@
void ConsumerFrameEventHistory::addQueue(const NewFrameEventsEntry& newEntry) {
// Overwrite all fields of the frame with default values unless set here.
FrameEvents newTimestamps;
+ newTimestamps.connectId = mCurrentConnectId;
newTimestamps.frameNumber = newEntry.frameNumber;
newTimestamps.postedTime = newEntry.postedTime;
newTimestamps.requestedPresentTime = newEntry.requestedPresentTime;
@@ -453,8 +459,8 @@
void ConsumerFrameEventHistory::addRelease(uint64_t frameNumber,
nsecs_t dequeueReadyTime, std::shared_ptr<FenceTime>&& release) {
FrameEvents* frame = getFrame(frameNumber, &mReleaseOffset);
- if (CC_UNLIKELY(frame == nullptr)) {
- ALOGE("addRelease: Did not find frame (%" PRIu64 ").", frameNumber);
+ if (frame == nullptr) {
+ ALOGE_IF(mProducerWantsEvents, "addRelease: Did not find frame.");
return;
}
frame->addReleaseCalled = true;
@@ -469,13 +475,19 @@
mProducerWantsEvents = true;
size_t i = static_cast<size_t>(std::distance(mFrames.begin(), frame));
if (mFramesDirty[i].anyDirty()) {
- delta->mDeltas.emplace_back(i, *frame, mFramesDirty[i]);
+ // Make sure only to send back deltas for the current connection
+ // since the producer won't have the correct state to apply a delta
+ // from a previous connection.
+ if (mFrames[i].connectId == mCurrentConnectId) {
+ delta->mDeltas.emplace_back(i, *frame, mFramesDirty[i]);
+ }
mFramesDirty[i].reset();
}
}
void ConsumerFrameEventHistory::getAndResetDelta(
FrameEventHistoryDelta* delta) {
+ mProducerWantsEvents = true;
delta->mCompositorTiming = mCompositorTiming;
// Write these in order of frame number so that it is easy to
diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp
index 3d893b1..8cadc4d 100644
--- a/libs/gui/IConsumerListener.cpp
+++ b/libs/gui/IConsumerListener.cpp
@@ -28,7 +28,8 @@
// ---------------------------------------------------------------------------
enum {
- ON_FRAME_AVAILABLE = IBinder::FIRST_CALL_TRANSACTION,
+ ON_DISCONNECT = IBinder::FIRST_CALL_TRANSACTION,
+ ON_FRAME_AVAILABLE,
ON_BUFFER_RELEASED,
ON_SIDEBAND_STREAM_CHANGED,
GET_FRAME_TIMESTAMPS
@@ -43,6 +44,12 @@
virtual ~BpConsumerListener();
+ virtual void onDisconnect() {
+ Parcel data, reply;
+ data.writeInterfaceToken(IConsumerListener::getInterfaceDescriptor());
+ remote()->transact(ON_DISCONNECT, data, &reply, IBinder::FLAG_ONEWAY);
+ }
+
virtual void onFrameAvailable(const BufferItem& item) {
Parcel data, reply;
data.writeInterfaceToken(IConsumerListener::getInterfaceDescriptor());
@@ -75,6 +82,10 @@
uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
{
switch(code) {
+ case ON_DISCONNECT: {
+ CHECK_INTERFACE(IConsumerListener, data, reply);
+ onDisconnect();
+ return NO_ERROR; }
case ON_FRAME_AVAILABLE: {
CHECK_INTERFACE(IConsumerListener, data, reply);
BufferItem item;
diff --git a/libs/gui/tests/DummyConsumer.h b/libs/gui/tests/DummyConsumer.h
index 0511e16..502bdf9 100644
--- a/libs/gui/tests/DummyConsumer.h
+++ b/libs/gui/tests/DummyConsumer.h
@@ -19,9 +19,9 @@
namespace android {
struct DummyConsumer : public BnConsumerListener {
- virtual void onFrameAvailable(const BufferItem& /* item */) {}
- virtual void onBuffersReleased() {}
- virtual void onSidebandStreamChanged() {}
+ void onFrameAvailable(const BufferItem& /* item */) override {}
+ void onBuffersReleased() override {}
+ void onSidebandStreamChanged() override {}
};
} // namespace android
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index 0329a6d..aa071f6 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -17,6 +17,8 @@
#define LOG_TAG "IGraphicBufferProducer_test"
//#define LOG_NDEBUG 0
+#include "DummyConsumer.h"
+
#include <gtest/gtest.h>
#include <utils/String8.h>
@@ -64,12 +66,6 @@
const sp<Fence> QUEUE_BUFFER_INPUT_FENCE = Fence::NO_FENCE;
}; // namespace anonymous
-struct DummyConsumer : public BnConsumerListener {
- virtual void onFrameAvailable(const BufferItem& /* item */) {}
- virtual void onBuffersReleased() {}
- virtual void onSidebandStreamChanged() {}
-};
-
class IGraphicBufferProducerTest : public ::testing::Test {
protected:
diff --git a/opengl/tests/EGLTest/EGL_test.cpp b/opengl/tests/EGLTest/EGL_test.cpp
index 1cd40b3..1b3086b 100644
--- a/opengl/tests/EGLTest/EGL_test.cpp
+++ b/opengl/tests/EGLTest/EGL_test.cpp
@@ -106,9 +106,9 @@
EXPECT_TRUE(eglChooseConfig(mEglDisplay, attrs, &config, 1, &numConfigs));
struct DummyConsumer : public BnConsumerListener {
- virtual void onFrameAvailable(const BufferItem& /* item */) {}
- virtual void onBuffersReleased() {}
- virtual void onSidebandStreamChanged() {}
+ void onFrameAvailable(const BufferItem& /* item */) override {}
+ void onBuffersReleased() override {}
+ void onSidebandStreamChanged() override {}
};
// Create a EGLSurface
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 8a6ec3c..49f7480 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1944,7 +1944,10 @@
#ifdef USE_HWC2
void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) {
- mSurfaceFlingerConsumer->releasePendingBuffer();
+ if (!mSurfaceFlingerConsumer->releasePendingBuffer()) {
+ return;
+ }
+
auto releaseFenceTime = std::make_shared<FenceTime>(
mSurfaceFlingerConsumer->getPrevFinalReleaseFence());
mReleaseTimeline.push(releaseFenceTime);
@@ -2377,6 +2380,11 @@
mFrameEventHistory.dump(result);
}
+void Layer::onDisconnect() {
+ Mutex::Autolock lock(mFrameEventHistoryMutex);
+ mFrameEventHistory.onDisconnect();
+}
+
void Layer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
FrameEventHistoryDelta *outDelta) {
Mutex::Autolock lock(mFrameEventHistoryMutex);
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 8227dae..d979a41 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -446,6 +446,7 @@
std::vector<OccupancyTracker::Segment> getOccupancyHistory(bool forceFlush);
+ void onDisconnect();
void addAndGetFrameTimestamps(const NewFrameEventsEntry* newEntry,
FrameEventHistoryDelta* outDelta);
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.cpp b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
index 942af13..bb1ff06 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.cpp
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
@@ -211,11 +211,11 @@
}
}
-void SurfaceFlingerConsumer::releasePendingBuffer()
+bool SurfaceFlingerConsumer::releasePendingBuffer()
{
if (!mPendingRelease.isPending) {
ALOGV("Pending buffer already released");
- return;
+ return false;
}
ALOGV("Releasing pending buffer");
Mutex::Autolock lock(mMutex);
@@ -225,6 +225,7 @@
ALOGE_IF(result != NO_ERROR, "releasePendingBuffer failed: %s (%d)",
strerror(-result), result);
mPendingRelease = PendingRelease();
+ return true;
}
#endif
@@ -261,6 +262,13 @@
}
}
+void SurfaceFlingerConsumer::onDisconnect() {
+ sp<Layer> l = mLayer.promote();
+ if (l.get()) {
+ l->onDisconnect();
+ }
+}
+
void SurfaceFlingerConsumer::addAndGetFrameTimestamps(
const NewFrameEventsEntry* newTimestamps,
FrameEventHistoryDelta *outDelta) {
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.h b/services/surfaceflinger/SurfaceFlingerConsumer.h
index 7713ed2..1531e2c 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.h
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.h
@@ -82,10 +82,11 @@
sp<Fence> getPrevFinalReleaseFence() const;
#ifdef USE_HWC2
virtual void setReleaseFence(const sp<Fence>& fence) override;
- void releasePendingBuffer();
+ bool releasePendingBuffer();
#endif
- virtual void addAndGetFrameTimestamps(
+ void onDisconnect() override;
+ void addAndGetFrameTimestamps(
const NewFrameEventsEntry* newTimestamps,
FrameEventHistoryDelta* outDelta) override;