From 7f775bc94c0b5b4329b4dd5ec2d9ea66161ce789 Mon Sep 17 00:00:00 2001
From: Johannes Kron <kron@webrtc.org>
Date: Tue, 12 Nov 2024 13:31:06 +0100
Subject: [PATCH] Ensure accurate FPS calculation for low frame rates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When receiving streams with frame rates around 1 fps, the decode and
render fps were incorrectly reported as 0, even though frames were being
decoded successfully.

This commit addresses the issue by adjusting the calculation in
RateStatistics to better handle streams with frame intervals that are
close to the window size.

1 fps streams are an important special case that occur frequently in
in screen share scenarios.

Fixed: webrtc:354625675
Change-Id: I1362768229a3abab5929220ba4bbd5ccb06a33d2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368080
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43417}
---
 rtc_base/rate_statistics.cc          | 11 ++++++-
 rtc_base/rate_statistics_unittest.cc | 43 ++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/rtc_base/rate_statistics.cc b/rtc_base/rate_statistics.cc
index 401ca24b28..036cfad460 100644
--- a/rtc_base/rate_statistics.cc
+++ b/rtc_base/rate_statistics.cc
@@ -57,8 +57,17 @@ void RateStatistics::Reset() {
 void RateStatistics::Update(int64_t count, int64_t now_ms) {
   RTC_DCHECK_GE(count, 0);
 
+  // Don't reset `first_timestamp_` if the last sample removed by EraseOld() was
+  // recent. This ensures that the window maintains its intended duration even
+  // when samples are received near the boundary. Use a margin of 50% of the
+  // current window size.
+  const int64_t recent_sample_time_margin = 1.5 * current_window_size_ms_;
+  bool last_sample_is_recent =
+      !buckets_.empty() &&
+      buckets_.back().timestamp > now_ms - recent_sample_time_margin;
+
   EraseOld(now_ms);
-  if (first_timestamp_ == -1 || num_samples_ == 0) {
+  if (first_timestamp_ == -1 || (num_samples_ == 0 && !last_sample_is_recent)) {
     first_timestamp_ = now_ms;
   }
 
diff --git a/rtc_base/rate_statistics_unittest.cc b/rtc_base/rate_statistics_unittest.cc
index 5f9707a8ee..ce982b7cdf 100644
--- a/rtc_base/rate_statistics_unittest.cc
+++ b/rtc_base/rate_statistics_unittest.cc
@@ -315,4 +315,47 @@ TEST_F(RateStatisticsTest, HandlesSomewhatLargeNumbers) {
   EXPECT_FALSE(stats_.Rate(now_ms));
 }
 
+TEST_F(RateStatisticsTest, HandlesLowFps) {
+  RateStatistics fps_stats(/*window_size_ms=*/1000, /*scale=*/1000);
+
+  const int64_t kExpectedFps = 1;
+  constexpr int64_t kTimeDelta = 1000 / kExpectedFps;
+
+  int64_t now_ms = 0;
+  EXPECT_FALSE(stats_.Rate(now_ms));
+  // Fill 1 s window.
+  while (now_ms < 1000) {
+    fps_stats.Update(1, now_ms);
+    now_ms += kTimeDelta;
+  }
+
+  // Simulate 1 fps stream for 10 seconds.
+  while (now_ms < 10000) {
+    fps_stats.Update(1, now_ms);
+    EXPECT_EQ(kExpectedFps, fps_stats.Rate(now_ms));
+    now_ms += kTimeDelta;
+  }
+}
+
+TEST_F(RateStatisticsTest, Handles25Fps) {
+  RateStatistics fps_stats(/*window_size_ms=*/1000, /*scale=*/1000);
+
+  constexpr int64_t kExpectedFps = 25;
+  constexpr int64_t kTimeDelta = 1000 / kExpectedFps;
+
+  int64_t now_ms = 0;
+  EXPECT_FALSE(stats_.Rate(now_ms));
+  // Fill 1 s window.
+  while (now_ms < 1000) {
+    fps_stats.Update(1, now_ms);
+    now_ms += kTimeDelta;
+  }
+  // Simulate 25 fps stream for 10 seconds.
+  while (now_ms < 10000) {
+    fps_stats.Update(1, now_ms);
+    EXPECT_EQ(kExpectedFps, fps_stats.Rate(now_ms));
+    now_ms += kTimeDelta;
+  }
+}
+
 }  // namespace