diff --git a/webrtc/base/OWNERS b/webrtc/base/OWNERS index 2f400904c6..b5d02af6b8 100644 --- a/webrtc/base/OWNERS +++ b/webrtc/base/OWNERS @@ -15,4 +15,6 @@ per-file *.gyp=* per-file *.gypi=* per-file BUILD.gn=kjellander@webrtc.org +per-file rate_statistics*=sprang@webrtc.org +per-file rate_statistics*=stefan@webrtc.org diff --git a/webrtc/base/rate_statistics.cc b/webrtc/base/rate_statistics.cc index 8db2851e68..6529aa1f7a 100644 --- a/webrtc/base/rate_statistics.cc +++ b/webrtc/base/rate_statistics.cc @@ -10,7 +10,9 @@ #include "webrtc/base/rate_statistics.h" -#include +#include + +#include "webrtc/base/checks.h" namespace webrtc { @@ -20,7 +22,7 @@ RateStatistics::RateStatistics(uint32_t window_size_ms, float scale) accumulated_count_(0), oldest_time_(0), oldest_index_(0), - scale_(scale / (num_buckets_ - 1)) {} + scale_(scale) {} RateStatistics::~RateStatistics() {} @@ -42,7 +44,7 @@ void RateStatistics::Update(size_t count, int64_t now_ms) { EraseOld(now_ms); int now_offset = static_cast(now_ms - oldest_time_); - assert(now_offset < num_buckets_); + RTC_DCHECK_LT(now_offset, num_buckets_); int index = oldest_index_ + now_offset; if (index >= num_buckets_) { index -= num_buckets_; @@ -53,18 +55,20 @@ void RateStatistics::Update(size_t count, int64_t now_ms) { uint32_t RateStatistics::Rate(int64_t now_ms) { EraseOld(now_ms); - return static_cast(accumulated_count_ * scale_ + 0.5f); + float scale = scale_ / (now_ms - oldest_time_ + 1); + return static_cast(accumulated_count_ * scale + 0.5f); } void RateStatistics::EraseOld(int64_t now_ms) { int64_t new_oldest_time = now_ms - num_buckets_ + 1; if (new_oldest_time <= oldest_time_) { + if (accumulated_count_ == 0) + oldest_time_ = now_ms; return; } - while (oldest_time_ < new_oldest_time) { size_t count_in_oldest_bucket = buckets_[oldest_index_]; - assert(accumulated_count_ >= count_in_oldest_bucket); + RTC_DCHECK_GE(accumulated_count_, count_in_oldest_bucket); accumulated_count_ -= count_in_oldest_bucket; buckets_[oldest_index_] = 0; if (++oldest_index_ >= num_buckets_) { @@ -74,6 +78,7 @@ void RateStatistics::EraseOld(int64_t now_ms) { if (accumulated_count_ == 0) { // This guarantees we go through all the buckets at most once, even if // |new_oldest_time| is far greater than |oldest_time_|. + new_oldest_time = now_ms; break; } } diff --git a/webrtc/base/rate_statistics_unittest.cc b/webrtc/base/rate_statistics_unittest.cc index 0270253d5e..9702da0699 100644 --- a/webrtc/base/rate_statistics_unittest.cc +++ b/webrtc/base/rate_statistics_unittest.cc @@ -8,6 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include + #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/rate_statistics.h" @@ -15,9 +17,11 @@ namespace { using webrtc::RateStatistics; +const int64_t kWindowMs = 500; + class RateStatisticsTest : public ::testing::Test { protected: - RateStatisticsTest() : stats_(500, 8000) {} + RateStatisticsTest() : stats_(kWindowMs, 8000) {} RateStatistics stats_; }; @@ -26,8 +30,9 @@ TEST_F(RateStatisticsTest, TestStrictMode) { // Should be initialized to 0. EXPECT_EQ(0u, stats_.Rate(now_ms)); stats_.Update(1500, now_ms); - // Expecting 24 kbps given a 500 ms window with one 1500 bytes packet. - EXPECT_EQ(24000u, stats_.Rate(now_ms)); + // Expecting 1200 kbps since the window is initially kept small and grows as + // we have more data. + EXPECT_EQ(12000000u, stats_.Rate(now_ms)); stats_.Reset(); // Expecting 0 after init. EXPECT_EQ(0u, stats_.Rate(now_ms)); @@ -37,12 +42,12 @@ TEST_F(RateStatisticsTest, TestStrictMode) { } // Approximately 1200 kbps expected. Not exact since when packets // are removed we will jump 10 ms to the next packet. - if (now_ms > 0 && now_ms % 500 == 0) { - EXPECT_NEAR(1200000u, stats_.Rate(now_ms), 24000u); + if (now_ms > 0 && now_ms % kWindowMs == 0) { + EXPECT_NEAR(1200000u, stats_.Rate(now_ms), 22000u); } now_ms += 1; } - now_ms += 500; + now_ms += kWindowMs; // The window is 2 seconds. If nothing has been received for that time // the estimate should be 0. EXPECT_EQ(0u, stats_.Rate(now_ms)); @@ -54,25 +59,26 @@ TEST_F(RateStatisticsTest, IncreasingThenDecreasingBitrate) { // Expecting 0 after init. uint32_t bitrate = stats_.Rate(now_ms); EXPECT_EQ(0u, bitrate); + const uint32_t kExpectedBitrate = 8000000; // 1000 bytes per millisecond until plateau is reached. + int prev_error = kExpectedBitrate; while (++now_ms < 10000) { stats_.Update(1000, now_ms); - uint32_t new_bitrate = stats_.Rate(now_ms); - if (new_bitrate != bitrate) { - // New bitrate must be higher than previous one. - EXPECT_GT(new_bitrate, bitrate); - } else { - // Plateau reached, 8000 kbps expected. - EXPECT_NEAR(8000000u, bitrate, 80000u); - break; - } - bitrate = new_bitrate; + bitrate = stats_.Rate(now_ms); + int error = kExpectedBitrate - bitrate; + error = std::abs(error); + // Expect the estimation error to decrease as the window is extended. + EXPECT_LE(error, prev_error + 1); + prev_error = error; } + // Window filled, expect to be close to 8000000. + EXPECT_EQ(kExpectedBitrate, bitrate); + // 1000 bytes per millisecond until 10-second mark, 8000 kbps expected. while (++now_ms < 10000) { stats_.Update(1000, now_ms); bitrate = stats_.Rate(now_ms); - EXPECT_NEAR(8000000u, bitrate, 80000u); + EXPECT_EQ(kExpectedBitrate, bitrate); } // Zero bytes per millisecond until 0 is reached. while (++now_ms < 20000) { @@ -94,4 +100,33 @@ TEST_F(RateStatisticsTest, IncreasingThenDecreasingBitrate) { EXPECT_EQ(0u, stats_.Rate(now_ms)); } } + +TEST_F(RateStatisticsTest, ResetAfterSilence) { + int64_t now_ms = 0; + stats_.Reset(); + // Expecting 0 after init. + uint32_t bitrate = stats_.Rate(now_ms); + EXPECT_EQ(0u, bitrate); + const uint32_t kExpectedBitrate = 8000000; + // 1000 bytes per millisecond until the window has been filled. + int prev_error = kExpectedBitrate; + while (++now_ms < 10000) { + stats_.Update(1000, now_ms); + bitrate = stats_.Rate(now_ms); + int error = kExpectedBitrate - bitrate; + error = std::abs(error); + // Expect the estimation error to decrease as the window is extended. + EXPECT_LE(error, prev_error + 1); + prev_error = error; + } + // Window filled, expect to be close to 8000000. + EXPECT_EQ(kExpectedBitrate, bitrate); + + now_ms += kWindowMs + 1; + EXPECT_EQ(0u, stats_.Rate(now_ms)); + stats_.Update(1000, now_ms); + // We expect one sample of 1000 bytes, and that the bitrate is measured over + // 1 ms, i.e., 8 * 1000 / 0.001 = 8000000. + EXPECT_EQ(kExpectedBitrate, stats_.Rate(now_ms)); +} } // namespace diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index 5bd0627afe..8ad60aea65 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -176,8 +176,7 @@ RemoteBitrateEstimatorAbsSendTime::ProcessClusters(int64_t now_ms) { std::min(best_it->GetSendBitrateBps(), best_it->GetRecvBitrateBps()); // Make sure that a probe sent on a lower bitrate than our estimate can't // reduce the estimate. - if (IsBitrateImproving(probe_bitrate_bps) && - probe_bitrate_bps > static_cast(incoming_bitrate_.Rate(now_ms))) { + if (IsBitrateImproving(probe_bitrate_bps)) { LOG(LS_INFO) << "Probe successful, sent at " << best_it->GetSendBitrateBps() << " bps, received at " << best_it->GetRecvBitrateBps() diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc index 8f57bbb58b..a57fcb5114 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc @@ -35,15 +35,15 @@ TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseReordering) { } TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(1232); + RateIncreaseRtpTimestampsTestHelper(1229); } TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStream) { - CapacityDropTestHelper(1, false, 633); + CapacityDropTestHelper(1, false, 667); } TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStreamWrap) { - CapacityDropTestHelper(1, true, 633); + CapacityDropTestHelper(1, true, 667); } TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropTwoStreamsWrap) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc index 6fd0ad11b5..97e3abaa32 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc @@ -47,7 +47,7 @@ TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStreamWrap) { } TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropTwoStreamsWrap) { - CapacityDropTestHelper(2, true, 767); + CapacityDropTestHelper(2, true, 600); } TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThreeStreamsWrap) { diff --git a/webrtc/modules/video_coding/bitrate_adjuster_unittest.cc b/webrtc/modules/video_coding/bitrate_adjuster_unittest.cc index 1d14ee3160..1421082aeb 100644 --- a/webrtc/modules/video_coding/bitrate_adjuster_unittest.cc +++ b/webrtc/modules/video_coding/bitrate_adjuster_unittest.cc @@ -86,7 +86,7 @@ TEST_F(BitrateAdjusterTest, VaryingBitrates) { SimulateBitrateBps(actual_bitrate_bps); VerifyAdjustment(); adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps(); - EXPECT_LT(adjusted_bitrate_bps, last_adjusted_bitrate_bps); + EXPECT_LE(adjusted_bitrate_bps, last_adjusted_bitrate_bps); last_adjusted_bitrate_bps = adjusted_bitrate_bps; // After two cycles we should've stabilized and hit the lower bound. EXPECT_EQ(GetTargetBitrateBpsPct(kMinAdjustedBitratePct),