Fix rate statistic when time window running out of samples
Current rate statistic tracker has assumption, the tracking window will
always be full after first filled up. This assumption looks not always
true. One example is the input_framerate_ tracker inside
video_stream_sender.cc which is used for setup frame droper and encoder.
Whenever there is a gap in video stream, like mute/unmute,
pacer pause/unpause etc. The fps detected from the rate_statistics
becomes samples_filled_partial_window / full_window_size, which could
be extremely low for a while. This creates a misalignment between the
fps we told encoder/frame dropper, and the real fps we fed into them,
which causes short-term serious overshot and very bad experience on
delay, avsync, congestion etc. This may also depends on how fast
encoder could react to the gap between set fps and real fps, but
libvpx and openh264 at least cannot handle this well.
So propose a fix to update first timestamp after tracker window
drained. This will give more accurate fps estimate similar based on
active window after sample gets drained
Bug: webrtc:13403
Change-Id: I96792c11091fe8bfa63e669f4360a3b3e95593e1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237720
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35447}
diff --git a/rtc_base/rate_statistics.cc b/rtc_base/rate_statistics.cc
index 85621fa..5c83796 100644
--- a/rtc_base/rate_statistics.cc
+++ b/rtc_base/rate_statistics.cc
@@ -58,7 +58,7 @@
RTC_DCHECK_GE(count, 0);
EraseOld(now_ms);
- if (first_timestamp_ == -1) {
+ if (first_timestamp_ == -1 || num_samples_ == 0) {
first_timestamp_ = now_ms;
}
diff --git a/rtc_base/rate_statistics_unittest.cc b/rtc_base/rate_statistics_unittest.cc
index 7356770..8f1a838 100644
--- a/rtc_base/rate_statistics_unittest.cc
+++ b/rtc_base/rate_statistics_unittest.cc
@@ -148,14 +148,15 @@
now_ms += kWindowMs + 1;
EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+ // Silence over window size should trigger auto reset for coming sample.
stats_.Update(1000, now_ms);
++now_ms;
stats_.Update(1000, now_ms);
// We expect two samples of 1000 bytes, and that the bitrate is measured over
- // 500 ms, i.e. 2 * 8 * 1000 / 0.500 = 32000.
- EXPECT_EQ(32000u, *stats_.Rate(now_ms));
+ // active window instead of full window, which is now_ms - first_timestamp + 1
+ EXPECT_EQ(kExpectedBitrate, *stats_.Rate(now_ms));
- // Reset, add the same samples again.
+ // Manual reset, add the same samples again.
stats_.Reset();
EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
stats_.Update(1000, now_ms);
@@ -272,9 +273,16 @@
EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
// Move window a long way out.
+ // This will cause an automatic reset of the window
+ // First data point won't give a valid result
now_ms += 2 * kWindowMs;
stats_.Update(0, now_ms);
bitrate = stats_.Rate(now_ms);
+ EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+ // Second data point gives valid result
+ ++now_ms;
+ stats_.Update(0, now_ms);
+ bitrate = stats_.Rate(now_ms);
EXPECT_TRUE(static_cast<bool>(bitrate));
EXPECT_EQ(0u, *bitrate);
}
