SF: Improve LayerInfo::calculateAverageFrameTime

This refactors and improves LayerInfo::calculateAverageFrameTime.
The behaviour is changed in two ways:

 * if two consecutive frames are too close to each other we count
   them as one frame and consider the delta between them in
   the total. This gives a better estimation for the average
   refresh rate. See CalculateAverageFrameTimeTest::ignoresSmallPeriods
   which was failing with the previous implementation.

 * if two consecutive frames are too far apart we discard the delta
   between them. This is covered by the test "ignoresLargePeriods".

Fixes: 170476958
Test: atest CalculateAverageFrameTimeTest
Change-Id: If98199bb8198f74c93e93c9996107c021f1bc7ba

5 files changed, 241 insertions, 39 deletions

diff --git a/services/surfaceflinger/Scheduler/LayerInfo.cpp b/services/surfaceflinger/Scheduler/LayerInfo.cpp
index 0fa71f131d..4324855b70 100644
--- a/services/surfaceflinger/Scheduler/LayerInfo.cpp
+++ b/services/surfaceflinger/Scheduler/LayerInfo.cpp
@@ -54,7 +54,7 @@ void LayerInfo::setLastPresentTime(nsecs_t lastPresentTime, nsecs_t now, LayerUp
             break;
         case LayerUpdateType::SetFrameRate:
         case LayerUpdateType::Buffer:
-            FrameTimeData frameTime = {.presetTime = lastPresentTime,
+            FrameTimeData frameTime = {.presentTime = lastPresentTime,
                                        .queueTime = mLastUpdatedTime,
                                        .pendingConfigChange = pendingConfigChange};
             mFrameTimes.push_back(frameTime);
@@ -74,7 +74,7 @@ bool LayerInfo::isFrameTimeValid(const FrameTimeData& frameTime) const {
 bool LayerInfo::isFrequent(nsecs_t now) const {
     // If we know nothing about this layer we consider it as frequent as it might be the start
     // of an animation.
-    if (mFrameTimes.size() < FREQUENT_LAYER_WINDOW_SIZE) {
+    if (mFrameTimes.size() < kFrequentLayerWindowSize) {
         return true;
     }
 
@@ -87,14 +87,14 @@ bool LayerInfo::isFrequent(nsecs_t now) const {
     }
 
     const auto numFrames = std::distance(it, mFrameTimes.end());
-    if (numFrames < FREQUENT_LAYER_WINDOW_SIZE) {
+    if (numFrames < kFrequentLayerWindowSize) {
         return false;
     }
 
     // Layer is considered frequent if the average frame rate is higher than the threshold
     const auto totalTime = mFrameTimes.back().queueTime - it->queueTime;
     return Fps::fromPeriodNsecs(totalTime / (numFrames - 1))
-            .greaterThanOrEqualWithMargin(MIN_FPS_FOR_FREQUENT_LAYER);
+            .greaterThanOrEqualWithMargin(kMinFpsForFrequentLayer);
 }
 
 bool LayerInfo::isAnimating(nsecs_t now) const {
@@ -124,32 +124,21 @@ bool LayerInfo::hasEnoughDataForHeuristic() const {
 }
 
 std::optional<nsecs_t> LayerInfo::calculateAverageFrameTime() const {
-    nsecs_t totalPresentTimeDeltas = 0;
-    nsecs_t totalQueueTimeDeltas = 0;
-    bool missingPresentTime = false;
-    int numFrames = 0;
-    for (auto it = mFrameTimes.begin(); it != mFrameTimes.end() - 1; ++it) {
-        // Ignore frames captured during a config change
-        if (it->pendingConfigChange || (it + 1)->pendingConfigChange) {
-            return std::nullopt;
-        }
-
-        totalQueueTimeDeltas +=
-                std::max(((it + 1)->queueTime - it->queueTime), kMinPeriodBetweenFrames);
-        numFrames++;
-
-        if (!missingPresentTime && (it->presetTime == 0 || (it + 1)->presetTime == 0)) {
-            missingPresentTime = true;
-            // If there are no presentation timestamps and we haven't calculated
-            // one in the past then we can't calculate the refresh rate
-            if (!mLastRefreshRate.reported.isValid()) {
-                return std::nullopt;
-            }
-            continue;
-        }
+    // Ignore frames captured during a config change
+    const bool isDuringConfigChange =
+            std::any_of(mFrameTimes.begin(), mFrameTimes.end(),
+                        [](auto frame) { return frame.pendingConfigChange; });
+    if (isDuringConfigChange) {
+        return std::nullopt;
+    }
 
-        totalPresentTimeDeltas +=
-                std::max(((it + 1)->presetTime - it->presetTime), kMinPeriodBetweenFrames);
+    const bool isMissingPresentTime =
+            std::any_of(mFrameTimes.begin(), mFrameTimes.end(),
+                        [](auto frame) { return frame.presentTime == 0; });
+    if (isMissingPresentTime && !mLastRefreshRate.reported.isValid()) {
+        // If there are no presentation timestamps and we haven't calculated
+        // one in the past then we can't calculate the refresh rate
+        return std::nullopt;
     }
 
     // Calculate the average frame time based on presentation timestamps. If those
@@ -160,9 +149,35 @@ std::optional<nsecs_t> LayerInfo::calculateAverageFrameTime() const {
     // presentation timestamps we look at the queue time to see if the current refresh rate still
     // matches the content.
 
-    const auto averageFrameTime =
-            static_cast<float>(missingPresentTime ? totalQueueTimeDeltas : totalPresentTimeDeltas) /
-            numFrames;
+    auto getFrameTime = isMissingPresentTime ? [](FrameTimeData data) { return data.queueTime; }
+                                             : [](FrameTimeData data) { return data.presentTime; };
+
+    nsecs_t totalDeltas = 0;
+    int numDeltas = 0;
+    auto prevFrame = mFrameTimes.begin();
+    for (auto it = mFrameTimes.begin() + 1; it != mFrameTimes.end(); ++it) {
+        const auto currDelta = getFrameTime(*it) - getFrameTime(*prevFrame);
+        if (currDelta < kMinPeriodBetweenFrames) {
+            // Skip this frame, but count the delta into the next frame
+            continue;
+        }
+
+        prevFrame = it;
+
+        if (currDelta > kMaxPeriodBetweenFrames) {
+            // Skip this frame and the current delta.
+            continue;
+        }
+
+        totalDeltas += currDelta;
+        numDeltas++;
+    }
+
+    if (numDeltas == 0) {
+        return std::nullopt;
+    }
+
+    const auto averageFrameTime = static_cast<double>(totalDeltas) / static_cast<double>(numDeltas);
     return static_cast<nsecs_t>(averageFrameTime);
 }
 
diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h
index 427cc9e2e0..e32ba099f6 100644
--- a/services/surfaceflinger/Scheduler/LayerInfo.h
+++ b/services/surfaceflinger/Scheduler/LayerInfo.h
@@ -49,12 +49,13 @@ class LayerInfo {
     // Layer is considered frequent if the earliest value in the window of most recent present times
     // is within a threshold. If a layer is infrequent, its average refresh rate is disregarded in
     // favor of a low refresh rate.
-    static constexpr size_t FREQUENT_LAYER_WINDOW_SIZE = 3;
-    static constexpr Fps MIN_FPS_FOR_FREQUENT_LAYER{10.0f};
-    static constexpr auto MAX_FREQUENT_LAYER_PERIOD_NS =
-            std::chrono::nanoseconds(MIN_FPS_FOR_FREQUENT_LAYER.getPeriodNsecs()) + 1ms;
+    static constexpr size_t kFrequentLayerWindowSize = 3;
+    static constexpr Fps kMinFpsForFrequentLayer{10.0f};
+    static constexpr auto kMaxPeriodForFrequentLayerNs =
+            std::chrono::nanoseconds(kMinFpsForFrequentLayer.getPeriodNsecs()) + 1ms;
 
     friend class LayerHistoryTest;
+    friend class LayerInfoTest;
 
 public:
     // Holds information about the layer vote
@@ -121,7 +122,7 @@ public:
 private:
     // Used to store the layer timestamps
     struct FrameTimeData {
-        nsecs_t presetTime; // desiredPresentTime, if provided
+        nsecs_t presentTime; // desiredPresentTime, if provided
         nsecs_t queueTime;  // buffer queue time
         bool pendingConfigChange;
     };
@@ -196,6 +197,10 @@ private:
     // Used for sanitizing the heuristic data. If two frames are less than
     // this period apart from each other they'll be considered as duplicates.
     static constexpr nsecs_t kMinPeriodBetweenFrames = Fps(120.f).getPeriodNsecs();
+    // Used for sanitizing the heuristic data. If two frames are more than
+    // this period apart from each other, the interval between them won't be
+    // taken into account when calculating average frame rate.
+    static constexpr nsecs_t kMaxPeriodBetweenFrames = kMinFpsForFrequentLayer.getPeriodNsecs();
     LayerHistory::LayerVoteType mDefaultVote;
 
     LayerVote mLayerVote;
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 13c7c8b976..f2fe3cc5ef 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -49,6 +49,7 @@ cc_test {
         "HWComposerTest.cpp",
         "OneShotTimerTest.cpp",
         "LayerHistoryTest.cpp",
+        "LayerInfoTest.cpp",
         "LayerMetadataTest.cpp",
         "MessageQueueTest.cpp",
         "SurfaceFlinger_CreateDisplayTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
index 2ee9c64478..612ed19ba2 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
@@ -43,8 +43,8 @@ namespace scheduler {
 class LayerHistoryTest : public testing::Test {
 protected:
     static constexpr auto PRESENT_TIME_HISTORY_SIZE = LayerInfo::HISTORY_SIZE;
-    static constexpr auto MAX_FREQUENT_LAYER_PERIOD_NS = LayerInfo::MAX_FREQUENT_LAYER_PERIOD_NS;
-    static constexpr auto FREQUENT_LAYER_WINDOW_SIZE = LayerInfo::FREQUENT_LAYER_WINDOW_SIZE;
+    static constexpr auto MAX_FREQUENT_LAYER_PERIOD_NS = LayerInfo::kMaxPeriodForFrequentLayerNs;
+    static constexpr auto FREQUENT_LAYER_WINDOW_SIZE = LayerInfo::kFrequentLayerWindowSize;
     static constexpr auto PRESENT_TIME_HISTORY_DURATION = LayerInfo::HISTORY_DURATION;
     static constexpr auto REFRESH_RATE_AVERAGE_HISTORY_DURATION =
             LayerInfo::RefreshRateHistory::HISTORY_DURATION;
diff --git a/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp b/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp
new file mode 100644
index 0000000000..d5c9b57c6e
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp
@@ -0,0 +1,181 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "LayerInfoTest"
+
+#include <gtest/gtest.h>
+
+#include "Fps.h"
+#include "Scheduler/LayerHistory.h"
+#include "Scheduler/LayerInfo.h"
+
+namespace android::scheduler {
+
+class LayerInfoTest : public testing::Test {
+protected:
+    using FrameTimeData = LayerInfo::FrameTimeData;
+
+    void setFrameTimes(const std::deque<FrameTimeData>& frameTimes) {
+        layerInfo.mFrameTimes = frameTimes;
+    }
+
+    void setLastRefreshRate(Fps fps) {
+        layerInfo.mLastRefreshRate.reported = fps;
+        layerInfo.mLastRefreshRate.calculated = fps;
+    }
+
+    auto calculateAverageFrameTime() { return layerInfo.calculateAverageFrameTime(); }
+
+    LayerInfo layerInfo{"TestLayerInfo", LayerHistory::LayerVoteType::Heuristic};
+};
+
+namespace {
+
+TEST_F(LayerInfoTest, prefersPresentTime) {
+    std::deque<FrameTimeData> frameTimes;
+    constexpr auto kExpectedFps = Fps(50.0f);
+    constexpr auto kPeriod = kExpectedFps.getPeriodNsecs();
+    constexpr int kNumFrames = 10;
+    for (int i = 1; i <= kNumFrames; i++) {
+        frameTimes.push_back(FrameTimeData{.presentTime = kPeriod * i,
+                                           .queueTime = 0,
+                                           .pendingConfigChange = false});
+    }
+    setFrameTimes(frameTimes);
+    const auto averageFrameTime = calculateAverageFrameTime();
+    ASSERT_TRUE(averageFrameTime.has_value());
+    const auto averageFps = Fps::fromPeriodNsecs(*averageFrameTime);
+    ASSERT_TRUE(kExpectedFps.equalsWithMargin(averageFps))
+            << "Expected " << averageFps << " to be equal to " << kExpectedFps;
+}
+
+TEST_F(LayerInfoTest, fallbacksToQueueTimeIfNoPresentTime) {
+    std::deque<FrameTimeData> frameTimes;
+    constexpr auto kExpectedFps = Fps(50.0f);
+    constexpr auto kPeriod = kExpectedFps.getPeriodNsecs();
+    constexpr int kNumFrames = 10;
+    for (int i = 1; i <= kNumFrames; i++) {
+        frameTimes.push_back(FrameTimeData{.presentTime = 0,
+                                           .queueTime = kPeriod * i,
+                                           .pendingConfigChange = false});
+    }
+    setFrameTimes(frameTimes);
+    setLastRefreshRate(Fps(20.0f)); // Set to some valid value
+    const auto averageFrameTime = calculateAverageFrameTime();
+    ASSERT_TRUE(averageFrameTime.has_value());
+    const auto averageFps = Fps::fromPeriodNsecs(*averageFrameTime);
+    ASSERT_TRUE(kExpectedFps.equalsWithMargin(averageFps))
+            << "Expected " << averageFps << " to be equal to " << kExpectedFps;
+}
+
+TEST_F(LayerInfoTest, returnsNulloptIfThereWasConfigChange) {
+    std::deque<FrameTimeData> frameTimesWithoutConfigChange;
+    const auto period = Fps(50.0f).getPeriodNsecs();
+    constexpr int kNumFrames = 10;
+    for (int i = 1; i <= kNumFrames; i++) {
+        frameTimesWithoutConfigChange.push_back(FrameTimeData{.presentTime = period * i,
+                                                              .queueTime = period * i,
+                                                              .pendingConfigChange = false});
+    }
+
+    setFrameTimes(frameTimesWithoutConfigChange);
+    ASSERT_TRUE(calculateAverageFrameTime().has_value());
+
+    {
+        // Config change in the first record
+        auto frameTimes = frameTimesWithoutConfigChange;
+        frameTimes[0].pendingConfigChange = true;
+        setFrameTimes(frameTimes);
+        ASSERT_FALSE(calculateAverageFrameTime().has_value());
+    }
+
+    {
+        // Config change in the last record
+        auto frameTimes = frameTimesWithoutConfigChange;
+        frameTimes[frameTimes.size() - 1].pendingConfigChange = true;
+        setFrameTimes(frameTimes);
+        ASSERT_FALSE(calculateAverageFrameTime().has_value());
+    }
+
+    {
+        // Config change in the middle
+        auto frameTimes = frameTimesWithoutConfigChange;
+        frameTimes[frameTimes.size() / 2].pendingConfigChange = true;
+        setFrameTimes(frameTimes);
+        ASSERT_FALSE(calculateAverageFrameTime().has_value());
+    }
+}
+
+// A frame can be recorded twice with very close presentation or queue times.
+// Make sure that this doesn't influence the calculated average FPS.
+TEST_F(LayerInfoTest, ignoresSmallPeriods) {
+    std::deque<FrameTimeData> frameTimes;
+    constexpr auto kExpectedFps = Fps(50.0f);
+    constexpr auto kExpectedPeriod = kExpectedFps.getPeriodNsecs();
+    constexpr auto kSmallPeriod = Fps(150.0f).getPeriodNsecs();
+    constexpr int kNumIterations = 10;
+    for (int i = 1; i <= kNumIterations; i++) {
+        frameTimes.push_back(FrameTimeData{.presentTime = kExpectedPeriod * i,
+                                           .queueTime = 0,
+                                           .pendingConfigChange = false});
+
+        // A duplicate frame
+        frameTimes.push_back(FrameTimeData{.presentTime = kExpectedPeriod * i + kSmallPeriod,
+                                           .queueTime = 0,
+                                           .pendingConfigChange = false});
+    }
+    setFrameTimes(frameTimes);
+    const auto averageFrameTime = calculateAverageFrameTime();
+    ASSERT_TRUE(averageFrameTime.has_value());
+    const auto averageFps = Fps::fromPeriodNsecs(*averageFrameTime);
+    ASSERT_TRUE(kExpectedFps.equalsWithMargin(averageFps))
+            << "Expected " << averageFps << " to be equal to " << kExpectedFps;
+}
+
+// There may be a big period of time between two frames. Make sure that
+// this doesn't influence the calculated average FPS.
+TEST_F(LayerInfoTest, ignoresLargePeriods) {
+    std::deque<FrameTimeData> frameTimes;
+    constexpr auto kExpectedFps = Fps(50.0f);
+    constexpr auto kExpectedPeriod = kExpectedFps.getPeriodNsecs();
+    constexpr auto kLargePeriod = Fps(9.0f).getPeriodNsecs();
+
+    auto record = [&](nsecs_t time) {
+        frameTimes.push_back(
+                FrameTimeData{.presentTime = time, .queueTime = 0, .pendingConfigChange = false});
+    };
+
+    auto time = kExpectedPeriod; // Start with non-zero time.
+    record(time);
+    time += kLargePeriod;
+    record(time);
+    constexpr int kNumIterations = 10;
+    for (int i = 1; i <= kNumIterations; i++) {
+        time += kExpectedPeriod;
+        record(time);
+    }
+
+    setFrameTimes(frameTimes);
+    const auto averageFrameTime = calculateAverageFrameTime();
+    ASSERT_TRUE(averageFrameTime.has_value());
+    const auto averageFps = Fps::fromPeriodNsecs(*averageFrameTime);
+    ASSERT_TRUE(kExpectedFps.equalsWithMargin(averageFps))
+            << "Expected " << averageFps << " to be equal to " << kExpectedFps;
+}
+
+} // namespace
+} // namespace android::scheduler