Add the notion of a last callback time into DispSync.
If we do not track real callback times we run into two issues:
* We can potentially fall into double-rate vsyncs due to error in thread
wakeup latency. For instance, we only correct up to 1.5 ms when wakeup
latency can be up to 5 ms in duration. To correct for this error, we
need a sanity check that callback timings won't fall into double-rate
vsyncs.
* Registering and unregistering event listeners can occur rapidly within
the same vsyncs, so that without any correction we can initially
double-fire events. If we only rely on inferring the last event time,
then we may end up skipping a frame, so we'll require the calling
thread registering and unregistering listeners to best-effort cache the
lastest callback time.
Bug: 124383894
Test: systrace
Change-Id: I7bc8f00502a3b798073a77c5ba4794c12cd14341
diff --git a/services/surfaceflinger/Scheduler/DispSync.h b/services/surfaceflinger/Scheduler/DispSync.h
index 6f3bd00..de2b874 100644
--- a/services/surfaceflinger/Scheduler/DispSync.h
+++ b/services/surfaceflinger/Scheduler/DispSync.h
@@ -54,8 +54,9 @@
virtual void setPeriod(nsecs_t period) = 0;
virtual nsecs_t getPeriod() = 0;
virtual void setRefreshSkipCount(int count) = 0;
- virtual status_t addEventListener(const char* name, nsecs_t phase, Callback* callback) = 0;
- virtual status_t removeEventListener(Callback* callback) = 0;
+ virtual status_t addEventListener(const char* name, nsecs_t phase, Callback* callback,
+ nsecs_t lastCallbackTime) = 0;
+ virtual status_t removeEventListener(Callback* callback, nsecs_t* outLastCallback) = 0;
virtual status_t changePhaseOffset(Callback* callback, nsecs_t phase) = 0;
virtual nsecs_t computeNextRefresh(int periodOffset) const = 0;
virtual void setIgnorePresentFences(bool ignore) = 0;
@@ -139,12 +140,21 @@
// given phase offset from the hardware vsync events. The callback is
// called from a separate thread and it should return reasonably quickly
// (i.e. within a few hundred microseconds).
- status_t addEventListener(const char* name, nsecs_t phase, Callback* callback) override;
+ // If the callback was previously registered, and the last clock time the
+ // callback was invoked was known to the caller (e.g. via removeEventListener),
+ // then the caller may pass that through to lastCallbackTime, so that
+ // callbacks do not accidentally double-fire if they are unregistered and
+ // reregistered in rapid succession.
+ status_t addEventListener(const char* name, nsecs_t phase, Callback* callback,
+ nsecs_t lastCallbackTime) override;
// removeEventListener removes an already-registered event callback. Once
// this method returns that callback will no longer be called by the
// DispSync object.
- status_t removeEventListener(Callback* callback) override;
+ // outLastCallbackTime will contain the last time that the callback was invoked.
+ // If the caller wishes to reregister the same callback, they should pass the
+ // callback time back into lastCallbackTime (see addEventListener).
+ status_t removeEventListener(Callback* callback, nsecs_t* outLastCallbackTime) override;
// changePhaseOffset changes the phase offset of an already-registered event callback. The
// method will make sure that there is no skipping or double-firing on the listener per frame,