From 83d4804a50a9b8ee5e029ef1cb62659611a03d7b Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Mon, 10 Nov 2014 13:55:16 +0000 Subject: [PATCH] Put send-side bwe probing under finch experiment. BUG=crbug/425925 R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/26079004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7668 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../bitrate_controller_unittest.cc | 21 ----- .../send_side_bandwidth_estimation.cc | 29 ++++--- .../send_side_bandwidth_estimation.h | 3 + ...send_side_bandwidth_estimation_unittest.cc | 80 +++++++++++++++++++ webrtc/modules/modules.gyp | 1 + webrtc/webrtc_tests.gypi | 2 +- 6 files changed, 102 insertions(+), 34 deletions(-) create mode 100644 webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index fddab9da7e..040cfec384 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -83,27 +83,6 @@ TEST_F(BitrateControllerTest, Basic) { controller_->RemoveBitrateObserver(&bitrate_observer); } -TEST_F(BitrateControllerTest, InitialRemb) { - TestBitrateObserver bitrate_observer; - controller_->SetBitrateObserver(&bitrate_observer, 200000, 100000, 1500000); - const uint32_t kRemb = 1000000u; - const uint32_t kSecondRemb = kRemb + 500000u; - - // Initial REMB applies immediately. - bandwidth_observer_->OnReceivedEstimatedBitrate(kRemb); - webrtc::ReportBlockList report_blocks; - report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); - bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1); - report_blocks.clear(); - EXPECT_EQ(kRemb, bitrate_observer.last_bitrate_); - - // Second REMB doesn't apply immediately. - bandwidth_observer_->OnReceivedEstimatedBitrate(kRemb + 500000); - report_blocks.push_back(CreateReportBlock(1, 2, 0, 21)); - bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001); - EXPECT_LT(bitrate_observer.last_bitrate_, kSecondRemb); -} - TEST_F(BitrateControllerTest, UpdatingBitrateObserver) { TestBitrateObserver bitrate_observer; controller_->SetBitrateObserver(&bitrate_observer, 200000, 100000, 1500000); diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc index 9b55dad70c..d238362603 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc @@ -12,6 +12,7 @@ #include +#include "webrtc/system_wrappers/interface/field_trial.h" #include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/system_wrappers/interface/metrics.h" @@ -103,6 +104,9 @@ void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss, uint32_t rtt, int number_of_packets, uint32_t now_ms) { + if (first_report_time_ms_ == -1) + first_report_time_ms_ = now_ms; + // Update RTT. last_round_trip_time_ms_ = rtt; @@ -129,12 +133,7 @@ void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss, } time_last_receiver_block_ms_ = now_ms; UpdateEstimate(now_ms); - - if (first_report_time_ms_ == -1) { - first_report_time_ms_ = now_ms; - } else { - UpdateUmaStats(now_ms, rtt, (fraction_loss * number_of_packets) >> 8); - } + UpdateUmaStats(now_ms, rtt, (fraction_loss * number_of_packets) >> 8); } void SendSideBandwidthEstimation::UpdateUmaStats(int64_t now_ms, @@ -167,12 +166,14 @@ void SendSideBandwidthEstimation::UpdateUmaStats(int64_t now_ms, void SendSideBandwidthEstimation::UpdateEstimate(uint32_t now_ms) { // We trust the REMB during the first 2 seconds if we haven't had any // packet loss reported, to allow startup bitrate probing. - if (last_fraction_loss_ == 0 && IsInStartPhase(now_ms) && - bwe_incoming_ > bitrate_) { - bitrate_ = CapBitrateToThresholds(bwe_incoming_); - min_bitrate_history_.clear(); - min_bitrate_history_.push_back(std::make_pair(now_ms, bitrate_)); - return; + if (ProbingExperimentIsEnabled()) { + if (last_fraction_loss_ == 0 && IsInStartPhase(now_ms) && + bwe_incoming_ > bitrate_) { + bitrate_ = CapBitrateToThresholds(bwe_incoming_); + min_bitrate_history_.clear(); + min_bitrate_history_.push_back(std::make_pair(now_ms, bitrate_)); + return; + } } UpdateMinHistory(now_ms); // Only start updating bitrate when receiving receiver blocks. @@ -265,4 +266,8 @@ uint32_t SendSideBandwidthEstimation::CapBitrateToThresholds(uint32_t bitrate) { return bitrate; } +bool SendSideBandwidthEstimation::ProbingExperimentIsEnabled() const { + return webrtc::field_trial::FindFullName("WebRTC-BitrateProbing") == + "Enabled"; +} } // namespace webrtc diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h index 3361904bf7..67fb0ddf43 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h @@ -42,6 +42,9 @@ class SendSideBandwidthEstimation { void SetMinMaxBitrate(uint32_t min_bitrate, uint32_t max_bitrate); void SetMinBitrate(uint32_t min_bitrate); + protected: + virtual bool ProbingExperimentIsEnabled() const; + private: enum UmaState { kNoUpdate, kFirstDone, kDone }; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc new file mode 100644 index 0000000000..4fe9ea970f --- /dev/null +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include + +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h" + +namespace webrtc { + +class TestBandwidthEstimation : public SendSideBandwidthEstimation { + public: + explicit TestBandwidthEstimation(bool in_experiment) + : in_experiment_(in_experiment) {} + + virtual bool ProbingExperimentIsEnabled() const OVERRIDE { + return in_experiment_; + } + + private: + bool in_experiment_; +}; + +TEST(SendSideBweTest, InitialRembWithProbing) { + TestBandwidthEstimation bwe(true); + bwe.SetMinMaxBitrate(100000, 1500000); + bwe.SetSendBitrate(200000); + + const uint32_t kRemb = 1000000u; + const uint32_t kSecondRemb = kRemb + 500000u; + int64_t now_ms = 0; + + bwe.UpdateReceiverBlock(0, 50, 1, now_ms); + + // Initial REMB applies immediately. + bwe.UpdateReceiverEstimate(kRemb); + bwe.UpdateEstimate(now_ms); + uint32_t bitrate; + uint8_t fraction_loss; + uint32_t rtt; + bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt); + EXPECT_EQ(kRemb, bitrate); + + // Second REMB doesn't apply immediately. + now_ms += 2001; + bwe.UpdateReceiverEstimate(kSecondRemb); + bwe.UpdateEstimate(now_ms); + bitrate = 0; + bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt); + EXPECT_EQ(kRemb, bitrate); +} + +TEST(SendSideBweTest, InitialRembWithoutProbing) { + TestBandwidthEstimation bwe(false); + bwe.SetMinMaxBitrate(100000, 1500000); + const uint32_t kStartBitrate = 200000; + bwe.SetSendBitrate(kStartBitrate); + + int64_t now_ms = 0; + bwe.UpdateReceiverBlock(0, 50, 1, now_ms); + + // Initial REMB doesn't apply immediately. + const uint32_t kRemb = 1000000u; + bwe.UpdateReceiverEstimate(kRemb); + bwe.UpdateEstimate(now_ms); + uint32_t bitrate; + uint8_t fraction_loss; + uint32_t rtt; + bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt); + EXPECT_EQ(kStartBitrate, bitrate); +} +} // namespace webrtc diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 3caf41f928..58d5cdd548 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -166,6 +166,7 @@ 'audio_processing/utility/delay_estimator_unittest.cc', 'audio_processing/utility/ring_buffer_unittest.cc', 'bitrate_controller/bitrate_controller_unittest.cc', + 'bitrate_controller/send_side_bandwidth_estimation_unittest.cc', 'desktop_capture/desktop_and_cursor_composer_unittest.cc', 'desktop_capture/desktop_region_unittest.cc', 'desktop_capture/differ_block_unittest.cc', diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi index dc17e707e2..86a0d3e576 100644 --- a/webrtc/webrtc_tests.gypi +++ b/webrtc/webrtc_tests.gypi @@ -58,7 +58,7 @@ 'test/webrtc_test_common.gyp:webrtc_test_renderer', '<(webrtc_root)/modules/modules.gyp:video_capture_module_internal_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_impl', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', 'test/test.gyp:test_main', 'webrtc', ],