From b2540bb99fa974bc8cb66eeff9c6b6d7fd0c36eb Mon Sep 17 00:00:00 2001 From: Irfan Sheriff Date: Mon, 12 Sep 2016 12:28:54 -0700 Subject: [PATCH] Probing: Add support for exponential startup probing Adds support for exponentially probing the bandwidth at start-up to allow ramp-up to real capacity of the network. BUG=webrtc:6332 R=philipel@webrtc.org, stefan@webrtc.org Review URL: https://codereview.webrtc.org/2235373004 . Cr-Commit-Position: refs/heads/master@{#14189} --- webrtc/modules/BUILD.gn | 1 + .../bitrate_controller_impl.cc | 12 +- .../bitrate_controller_impl.h | 9 +- .../bitrate_controller_unittest.cc | 2 +- .../include/bitrate_controller.h | 7 +- webrtc/modules/congestion_controller/BUILD.gn | 2 + .../congestion_controller.cc | 37 ++---- .../congestion_controller.gypi | 2 + .../congestion_controller_unittest.cc | 1 + .../congestion_controller/delay_based_bwe.cc | 81 +++++++------ .../congestion_controller/delay_based_bwe.h | 7 +- .../delay_based_bwe_unittest_helper.cc | 5 +- .../include/congestion_controller.h | 3 +- .../congestion_controller/probe_controller.cc | 113 ++++++++++++++++++ .../congestion_controller/probe_controller.h | 62 ++++++++++ .../probe_controller_unittest.cc | 82 +++++++++++++ webrtc/modules/modules.gyp | 1 + webrtc/modules/pacing/bitrate_prober.cc | 1 + .../modules/pacing/mock/mock_paced_sender.h | 1 + webrtc/modules/pacing/paced_sender.h | 2 +- .../include/remote_bitrate_estimator.h | 1 - .../transport_feedback_adapter.cc | 12 -- .../transport_feedback_adapter.h | 14 +-- .../transport_feedback_adapter_unittest.cc | 5 +- webrtc/tools/event_log_visualizer/analyzer.cc | 2 +- 25 files changed, 353 insertions(+), 112 deletions(-) create mode 100644 webrtc/modules/congestion_controller/probe_controller.cc create mode 100644 webrtc/modules/congestion_controller/probe_controller.h create mode 100644 webrtc/modules/congestion_controller/probe_controller_unittest.cc diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn index f997e1eda9..0c8befcabc 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -351,6 +351,7 @@ if (rtc_include_tests) { "congestion_controller/delay_based_bwe_unittest_helper.cc", "congestion_controller/delay_based_bwe_unittest_helper.h", "congestion_controller/probe_bitrate_estimator_unittest.cc", + "congestion_controller/probe_controller_unittest.cc", "media_file/media_file_unittest.cc", "module_common_types_unittest.cc", "pacing/bitrate_prober_unittest.cc", diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index 1231c782a9..e94baa7b67 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -31,7 +31,7 @@ class BitrateControllerImpl::RtcpBandwidthObserverImpl } // Received RTCP REMB or TMMBR. void OnReceivedEstimatedBitrate(uint32_t bitrate) override { - owner_->OnReceivedEstimatedBitrate(bitrate); + owner_->OnReceiverEstimatedBitrate(bitrate); } // Received RTCP receiver block. void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks, @@ -173,7 +173,8 @@ void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { +// This is called upon reception of REMB or TMMBR. +void BitrateControllerImpl::OnReceiverEstimatedBitrate(uint32_t bitrate) { { rtc::CritScope cs(&critsect_); bandwidth_estimation_.UpdateReceiverEstimate(clock_->TimeInMilliseconds(), @@ -182,7 +183,7 @@ void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::UpdateProbeBitrate(uint32_t bitrate_bps) { +void BitrateControllerImpl::OnProbeBitrate(uint32_t bitrate_bps) { { rtc::CritScope cs(&critsect_); bandwidth_estimation_.SetSendBitrate(bitrate_bps); @@ -190,7 +191,10 @@ void BitrateControllerImpl::UpdateProbeBitrate(uint32_t bitrate_bps) { MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::UpdateDelayBasedEstimate(uint32_t bitrate_bps) { +// TODO(isheriff): Perhaps need new interface for invocation from DelayBasedBwe. +void BitrateControllerImpl::OnReceiveBitrateChanged( + const std::vector& ssrcs, + uint32_t bitrate_bps) { { rtc::CritScope cs(&critsect_); bandwidth_estimation_.UpdateDelayBasedEstimate(clock_->TimeInMilliseconds(), diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index e5f595d09e..c8bb10282b 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -19,6 +19,7 @@ #include #include +#include #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" @@ -52,7 +53,6 @@ class BitrateControllerImpl : public BitrateController { int min_bitrate_bps, int max_bitrate_bps) override; - void UpdateDelayBasedEstimate(uint32_t bitrate_bps) override; void SetReservedBitrate(uint32_t reserved_bitrate_bps) override; @@ -61,7 +61,10 @@ class BitrateControllerImpl : public BitrateController { uint8_t* fraction_loss, int64_t* rtt) override; - void UpdateProbeBitrate(uint32_t bitrate_bps) override; + // RemoteBitrateObserver overrides. + void OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) override; + void OnProbeBitrate(uint32_t bitrate_bps) override; int64_t TimeUntilNextProcess() override; void Process() override; @@ -70,7 +73,7 @@ class BitrateControllerImpl : public BitrateController { class RtcpBandwidthObserverImpl; // Called by BitrateObserver's direct from the RTCP module. - void OnReceivedEstimatedBitrate(uint32_t bitrate); + void OnReceiverEstimatedBitrate(uint32_t bitrate); void OnReceivedRtcpReceiverReport(uint8_t fraction_loss, int64_t rtt, diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index a70eecf241..bfc44ea709 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -170,7 +170,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); // Test that a low delay-based estimate limits the combined estimate. - controller_->UpdateDelayBasedEstimate(280000); + controller_->OnReceiveBitrateChanged({0}, 280000); EXPECT_EQ(280000, bitrate_observer_.last_bitrate_); // Test that a low REMB limits the combined estimate. diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index 4c4578adf0..90b6471390 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -19,6 +19,7 @@ #include "webrtc/modules/include/module.h" #include "webrtc/modules/pacing/paced_sender.h" +#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" namespace webrtc { @@ -43,7 +44,7 @@ class BitrateObserver { virtual ~BitrateObserver() {} }; -class BitrateController : public Module { +class BitrateController : public Module, public RemoteBitrateObserver { // This class collects feedback from all streams sent to a peer (via // RTCPBandwidthObservers). It does one aggregated send side bandwidth // estimation and divide the available bitrate between all its registered @@ -77,10 +78,6 @@ class BitrateController : public Module { int min_bitrate_bps, int max_bitrate_bps) = 0; - virtual void UpdateDelayBasedEstimate(uint32_t bitrate_bps) = 0; - - virtual void UpdateProbeBitrate(uint32_t bitrate_bps) = 0; - // Gets the available payload bandwidth in bits per second. Note that // this bandwidth excludes packet headers. virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index a05f25b165..a07acc5171 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -16,6 +16,8 @@ rtc_source_set("congestion_controller") { "include/congestion_controller.h", "probe_bitrate_estimator.cc", "probe_bitrate_estimator.h", + "probe_controller.cc", + "probe_controller.h", ] # TODO(jschuh): Bug 1348: fix this warning. diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index ae11d93fbb..c0ec3da0f9 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -22,6 +22,7 @@ #include "webrtc/base/thread_annotations.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" +#include "webrtc/modules/congestion_controller/probe_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" @@ -166,13 +167,13 @@ CongestionController::CongestionController( new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), + probe_controller_(new ProbeController(pacer_.get(), clock_)), retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(bitrate_controller_.get(), clock_), + transport_feedback_adapter_(clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), max_bitrate_bps_(0), - initial_probing_triggered_(false), last_reported_bitrate_bps_(0), last_reported_fraction_loss_(0), last_reported_rtt_(0), @@ -197,13 +198,13 @@ CongestionController::CongestionController( // construction. bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), + probe_controller_(new ProbeController(pacer_.get(), clock_)), retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(bitrate_controller_.get(), clock_), + transport_feedback_adapter_(clock_), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), max_bitrate_bps_(0), - initial_probing_triggered_(false), last_reported_bitrate_bps_(0), last_reported_fraction_loss_(0), last_reported_rtt_(0), @@ -215,7 +216,7 @@ CongestionController::~CongestionController() {} void CongestionController::Init() { transport_feedback_adapter_.SetBitrateEstimator( - new DelayBasedBwe(&transport_feedback_adapter_, clock_)); + new DelayBasedBwe(bitrate_controller_.get(), clock_)); transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( min_bitrate_bps_); } @@ -228,25 +229,8 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, min_bitrate_bps, max_bitrate_bps); - { - rtc::CritScope cs(&critsect_); - if (!initial_probing_triggered_) { - pacer_->CreateProbeCluster(start_bitrate_bps * 3, 6); - pacer_->CreateProbeCluster(start_bitrate_bps * 6, 5); - initial_probing_triggered_ = true; - } - - // Only do probing if: - // - we are mid-call, which we consider to be if - // |last_reported_bitrate_bps_| != 0, and - // - the current bitrate is lower than the new |max_bitrate_bps|, and - // - we actually want to increase the |max_bitrate_bps_|. - if (last_reported_bitrate_bps_ != 0 && - last_reported_bitrate_bps_ < static_cast(max_bitrate_bps) && - max_bitrate_bps > max_bitrate_bps_) { - pacer_->CreateProbeCluster(max_bitrate_bps, 5); - } - } + probe_controller_->SetBitrates(min_bitrate_bps, start_bitrate_bps, + max_bitrate_bps); max_bitrate_bps_ = max_bitrate_bps; if (remote_bitrate_estimator_) @@ -272,8 +256,8 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps, if (remote_bitrate_estimator_) remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps); - RemoteBitrateEstimator* rbe = new DelayBasedBwe( - &transport_feedback_adapter_, clock_); + RemoteBitrateEstimator* rbe = + new DelayBasedBwe(bitrate_controller_.get(), clock_); transport_feedback_adapter_.SetBitrateEstimator(rbe); rbe->SetMinBitrate(min_bitrate_bps); // TODO(holmer): Trigger a new probe once mid-call probing is implemented. @@ -361,6 +345,7 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { &bitrate_bps, &fraction_loss, &rtt); if (estimate_changed) { pacer_->SetEstimatedBitrate(bitrate_bps); + probe_controller_->SetEstimatedBitrate(bitrate_bps); retransmission_rate_limiter_->SetMaxRate(bitrate_bps); } diff --git a/webrtc/modules/congestion_controller/congestion_controller.gypi b/webrtc/modules/congestion_controller/congestion_controller.gypi index c7fdd04455..5c7ddf2784 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.gypi +++ b/webrtc/modules/congestion_controller/congestion_controller.gypi @@ -23,6 +23,8 @@ 'include/congestion_controller.h', 'probe_bitrate_estimator.cc', 'probe_bitrate_estimator.h', + 'probe_controller.cc', + 'probe_controller.h', ], # TODO(jschuh): Bug 1348: fix size_t to int truncations. 'msvs_disabled_warnings': [ 4267, ], diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 53ccf3dda1..79cb2ed0af 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -12,6 +12,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/call/mock/mock_rtc_event_log.h" #include "webrtc/modules/pacing/mock/mock_paced_sender.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h" diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index bd9f3eef31..b7a21be73a 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -18,6 +18,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/logging.h" #include "webrtc/base/thread_annotations.h" +#include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" @@ -46,7 +47,6 @@ DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock) estimator_(), detector_(OverUseDetectorOptions()), incoming_bitrate_(kBitrateWindowMs, 8000), - first_packet_time_ms_(-1), last_update_ms_(-1), last_seen_packet_ms_(-1), uma_recorded_(false) { @@ -71,11 +71,8 @@ void DelayBasedBwe::IncomingPacketFeedbackVector( void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { int64_t now_ms = clock_->TimeInMilliseconds(); - if (first_packet_time_ms_ == -1) - first_packet_time_ms_ = now_ms; - incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms); - bool update_estimate = false; + bool delay_based_bwe_changed = false; uint32_t target_bitrate_bps = 0; { rtc::CritScope lock(&crit_); @@ -90,15 +87,6 @@ void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { } last_seen_packet_ms_ = now_ms; - if (info.probe_cluster_id != PacketInfo::kNotAProbe) { - int bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); - if (bps > 0) { - remote_rate_.SetEstimate(bps, info.arrival_time_ms); - observer_->OnProbeBitrate(bps); - update_estimate = true; - } - } - uint32_t send_time_24bits = static_cast(((static_cast(info.send_time_ms) << kAbsSendTimeFraction) + @@ -121,41 +109,56 @@ void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { estimator_->num_of_deltas(), info.arrival_time_ms); } - if (!update_estimate) { - // Check if it's time for a periodic update or if we should update because - // of an over-use. - if (last_update_ms_ == -1 || - now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval()) { - update_estimate = true; - } else if (detector_.State() == kBwOverusing) { - rtc::Optional incoming_rate = - incoming_bitrate_.Rate(info.arrival_time_ms); - if (incoming_rate && - remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) { - update_estimate = true; - } - } + int probing_bps = 0; + if (info.probe_cluster_id != PacketInfo::kNotAProbe) { + probing_bps = + probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); } - if (update_estimate) { - // The first overuse should immediately trigger a new estimate. - // We also have to update the estimate immediately if we are overusing - // and the target bitrate is too high compared to what we are receiving. - const RateControlInput input(detector_.State(), - incoming_bitrate_.Rate(info.arrival_time_ms), - estimator_->var_noise()); - remote_rate_.Update(&input, now_ms); - target_bitrate_bps = remote_rate_.UpdateBandwidthEstimate(now_ms); - update_estimate = remote_rate_.ValidEstimate(); + // Currently overusing the bandwidth. + if (detector_.State() == kBwOverusing) { + rtc::Optional incoming_rate = + incoming_bitrate_.Rate(info.arrival_time_ms); + if (incoming_rate && + remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) { + delay_based_bwe_changed = + UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); + } + } else if (probing_bps > 0) { + // No overuse, but probing measured a bitrate. + remote_rate_.SetEstimate(probing_bps, info.arrival_time_ms); + observer_->OnProbeBitrate(probing_bps); + delay_based_bwe_changed = + UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); + } + if (!delay_based_bwe_changed && + (last_update_ms_ == -1 || + now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval())) { + delay_based_bwe_changed = + UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); } } - if (update_estimate) { + if (delay_based_bwe_changed) { last_update_ms_ = now_ms; observer_->OnReceiveBitrateChanged({kFixedSsrc}, target_bitrate_bps); } } +bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, + int64_t now_ms, + uint32_t* target_bitrate_bps) { + // The first overuse should immediately trigger a new estimate. + // We also have to update the estimate immediately if we are overusing + // and the target bitrate is too high compared to what we are receiving. + const RateControlInput input(detector_.State(), + incoming_bitrate_.Rate(arrival_time_ms), + estimator_->var_noise()); + remote_rate_.Update(&input, now_ms); + *target_bitrate_bps = remote_rate_.UpdateBandwidthEstimate(now_ms); + return remote_rate_.ValidEstimate(); +} + void DelayBasedBwe::Process() {} int64_t DelayBasedBwe::TimeUntilNextProcess() { diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 19410c658a..3e0a0149a4 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -57,6 +57,12 @@ class DelayBasedBwe : public RemoteBitrateEstimator { private: void IncomingPacketInfo(const PacketInfo& info); + // Updates the current remote rate estimate and returns true if a valid + // estimate exists. + bool UpdateEstimate(int64_t packet_arrival_time_ms, + int64_t now_ms, + uint32_t* target_bitrate_bps) + EXCLUSIVE_LOCKS_REQUIRED(crit_); rtc::ThreadChecker network_thread_; Clock* const clock_; @@ -65,7 +71,6 @@ class DelayBasedBwe : public RemoteBitrateEstimator { std::unique_ptr estimator_; OveruseDetector detector_; RateStatistics incoming_bitrate_; - int64_t first_packet_time_ms_; int64_t last_update_ms_; int64_t last_seen_packet_ms_; bool uma_recorded_; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index 0ed7816e50..34692511b9 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -152,9 +152,8 @@ DelayBasedBweTest::DelayBasedBweTest() : clock_(100000000), bitrate_observer_(new test::TestBitrateObserver), bitrate_estimator_(new DelayBasedBwe(bitrate_observer_.get(), &clock_)), - stream_generator_( - new test::StreamGenerator(1e6, // Capacity. - clock_.TimeInMicroseconds())), + stream_generator_(new test::StreamGenerator(1e6, // Capacity. + clock_.TimeInMicroseconds())), arrival_time_offset_ms_(0) {} DelayBasedBweTest::~DelayBasedBweTest() {} diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 57c2c64ff4..862f2cd6be 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -30,6 +30,7 @@ namespace webrtc { class BitrateController; class Clock; +class ProbeController; class ProcessThread; class RateLimiter; class RemoteBitrateEstimator; @@ -120,12 +121,12 @@ class CongestionController : public CallStatsObserver, public Module { const std::unique_ptr pacer_; const std::unique_ptr remote_bitrate_estimator_; const std::unique_ptr bitrate_controller_; + const std::unique_ptr probe_controller_; const std::unique_ptr retransmission_rate_limiter_; RemoteEstimatorProxy remote_estimator_proxy_; TransportFeedbackAdapter transport_feedback_adapter_; int min_bitrate_bps_; int max_bitrate_bps_; - bool initial_probing_triggered_; rtc::CriticalSection critsect_; uint32_t last_reported_bitrate_bps_ GUARDED_BY(critsect_); uint8_t last_reported_fraction_loss_ GUARDED_BY(critsect_); diff --git a/webrtc/modules/congestion_controller/probe_controller.cc b/webrtc/modules/congestion_controller/probe_controller.cc new file mode 100644 index 0000000000..cf7f3e1b61 --- /dev/null +++ b/webrtc/modules/congestion_controller/probe_controller.cc @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2016 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 "webrtc/modules/congestion_controller/probe_controller.h" + +#include + +#include "webrtc/base/logging.h" + +namespace webrtc { + +namespace { + +// Number of deltas between probes per cluster. On the very first cluster, +// we will need kProbeDeltasPerCluster + 1 probes, but on a cluster following +// another, we need kProbeDeltasPerCluster probes. +constexpr int kProbeDeltasPerCluster = 5; + +// Maximum waiting time from the time of initiating probing to getting +// the measured results back. +constexpr int64_t kMaxWaitingTimeForProbingResultMs = 1000; + +// Value of |min_bitrate_to_probe_further_bps_| that indicates +// further probing is disabled. +constexpr int kExponentialProbingDisabled = 0; + +} // namespace + +ProbeController::ProbeController(PacedSender* pacer, Clock* clock) + : pacer_(pacer), + clock_(clock), + state_(State::kInit), + min_bitrate_to_probe_further_bps_(kExponentialProbingDisabled), + time_last_probing_initiated_ms_(0), + estimated_bitrate_bps_(0), + max_bitrate_bps_(0) {} + +void ProbeController::SetBitrates(int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) { + rtc::CritScope cs(&critsect_); + if (state_ == State::kInit) { + // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of + // 1.2 Mbps to continue probing. + InitiateProbing({3 * start_bitrate_bps, 6 * start_bitrate_bps}, + 4 * start_bitrate_bps); + } + + // Only do probing if: + // - we are mid-call, which we consider to be if + // |estimated_bitrate_bps_| != 0, and + // - the current bitrate is lower than the new |max_bitrate_bps|, and + // - we actually want to increase the |max_bitrate_bps_|. + if (estimated_bitrate_bps_ != 0 && estimated_bitrate_bps_ < max_bitrate_bps && + max_bitrate_bps > max_bitrate_bps_) { + InitiateProbing({max_bitrate_bps}, kExponentialProbingDisabled); + } + max_bitrate_bps_ = max_bitrate_bps; +} + +void ProbeController::SetEstimatedBitrate(int bitrate_bps) { + rtc::CritScope cs(&critsect_); + if (state_ == State::kWaitingForProbingResult) { + if ((clock_->TimeInMilliseconds() - time_last_probing_initiated_ms_) > + kMaxWaitingTimeForProbingResultMs) { + LOG(LS_INFO) << "kWaitingForProbingResult: timeout"; + state_ = State::kProbingComplete; + min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; + } else { + // Continue probing if probing results indicate channel has greater + // capacity. + LOG(LS_INFO) << "Measured bitrate: " << bitrate_bps + << " Minimum to probe further: " + << min_bitrate_to_probe_further_bps_; + if (min_bitrate_to_probe_further_bps_ != kExponentialProbingDisabled && + bitrate_bps > min_bitrate_to_probe_further_bps_) { + // Double the probing bitrate and expect a minimum of 25% gain to + // continue probing. + InitiateProbing({2 * bitrate_bps}, 1.25 * bitrate_bps); + } + } + } + estimated_bitrate_bps_ = bitrate_bps; +} + +void ProbeController::InitiateProbing( + std::initializer_list bitrates_to_probe, + int min_bitrate_to_probe_further_bps) { + bool first_cluster = true; + for (int bitrate : bitrates_to_probe) { + if (first_cluster) { + pacer_->CreateProbeCluster(bitrate, kProbeDeltasPerCluster + 1); + first_cluster = false; + } else { + pacer_->CreateProbeCluster(bitrate, kProbeDeltasPerCluster); + } + } + min_bitrate_to_probe_further_bps_ = min_bitrate_to_probe_further_bps; + time_last_probing_initiated_ms_ = clock_->TimeInMilliseconds(); + if (min_bitrate_to_probe_further_bps == kExponentialProbingDisabled) + state_ = State::kProbingComplete; + else + state_ = State::kWaitingForProbingResult; +} + +} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probe_controller.h b/webrtc/modules/congestion_controller/probe_controller.h new file mode 100644 index 0000000000..4cf34c3c87 --- /dev/null +++ b/webrtc/modules/congestion_controller/probe_controller.h @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2016 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. + */ + +#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ + +#include + +#include "webrtc/base/criticalsection.h" +#include "webrtc/common_types.h" +#include "webrtc/modules/pacing/paced_sender.h" + +namespace webrtc { + +class Clock; + +// This class controls initiation of probing to estimate initial channel +// capacity. There is also support for probing during a session when max +// bitrate is adjusted by an application. +class ProbeController { + public: + ProbeController(PacedSender* pacer, Clock* clock); + + void SetBitrates(int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps); + void SetEstimatedBitrate(int bitrate_bps); + + private: + void InitiateProbing(std::initializer_list bitrates_to_probe, + int min_bitrate_to_probe_further_bps) + EXCLUSIVE_LOCKS_REQUIRED(critsect_); + enum class State { + // Initial state where no probing has been triggered yet. + kInit, + // Waiting for probing results to continue further probing. + kWaitingForProbingResult, + // Probing is complete. + kProbingComplete, + }; + rtc::CriticalSection critsect_; + PacedSender* const pacer_; + Clock* const clock_; + State state_ GUARDED_BY(critsect_); + int min_bitrate_to_probe_further_bps_ GUARDED_BY(critsect_); + int64_t time_last_probing_initiated_ms_ GUARDED_BY(critsect_); + int estimated_bitrate_bps_ GUARDED_BY(critsect_); + int max_bitrate_bps_ GUARDED_BY(critsect_); + + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(ProbeController); +}; + +} // namespace webrtc + +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ diff --git a/webrtc/modules/congestion_controller/probe_controller_unittest.cc b/webrtc/modules/congestion_controller/probe_controller_unittest.cc new file mode 100644 index 0000000000..693277552c --- /dev/null +++ b/webrtc/modules/congestion_controller/probe_controller_unittest.cc @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2016 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 "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/logging.h" +#include "webrtc/modules/congestion_controller/probe_controller.h" +#include "webrtc/modules/pacing/mock/mock_paced_sender.h" +#include "webrtc/system_wrappers/include/clock.h" + +using testing::_; +using testing::AtLeast; +using testing::NiceMock; + +namespace webrtc { +namespace test { + +namespace { + +constexpr int kMinBitrateBps = 100; +constexpr int kStartBitrateBps = 300; +constexpr int kMaxBitrateBps = 1000; + +} // namespace + +class ProbeControllerTest : public ::testing::Test { + protected: + ProbeControllerTest() : clock_(0) { + probe_controller_.reset(new ProbeController(&pacer_, &clock_)); + } + ~ProbeControllerTest() override {} + + SimulatedClock clock_; + NiceMock pacer_; + std::unique_ptr probe_controller_; +}; + +TEST_F(ProbeControllerTest, InitiatesProbingAtStart) { + EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(AtLeast(2)); + probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps); +} + +TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { + EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(AtLeast(2)); + probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps); + clock_.AdvanceTimeMilliseconds(25); + + probe_controller_->SetEstimatedBitrate(kStartBitrateBps); + EXPECT_CALL(pacer_, CreateProbeCluster(kMaxBitrateBps + 100, _)); + probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps + 100); +} + +TEST_F(ProbeControllerTest, TestExponentialProbing) { + probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps); + EXPECT_CALL(pacer_, CreateProbeCluster(2 * 1800, _)); + probe_controller_->SetEstimatedBitrate(1800); +} + +TEST_F(ProbeControllerTest, TestExponentialProbingTimeout) { + probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps); + + // Advance far enough to cause a time out in waiting for probing result. + clock_.AdvanceTimeMilliseconds(5000); + EXPECT_CALL(pacer_, CreateProbeCluster(2 * 1800, _)).Times(0); + probe_controller_->SetEstimatedBitrate(1800); +} + +} // namespace test +} // namespace webrtc diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 2b552c0314..68149b6814 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -275,6 +275,7 @@ 'congestion_controller/delay_based_bwe_unittest_helper.cc', 'congestion_controller/delay_based_bwe_unittest_helper.h', 'congestion_controller/probe_bitrate_estimator_unittest.cc', + 'congestion_controller/probe_controller_unittest.cc', 'media_file/media_file_unittest.cc', 'module_common_types_unittest.cc', 'pacing/bitrate_prober_unittest.cc', diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index e2cea73305..936be3929e 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -66,6 +66,7 @@ void BitrateProber::OnIncomingPacket(size_t packet_size) { } void BitrateProber::CreateProbeCluster(int bitrate_bps, int num_packets) { + RTC_DCHECK(probing_state_ != ProbingState::kDisabled); ProbeCluster cluster; cluster.max_probe_packets = num_packets; cluster.probe_bitrate_bps = bitrate_bps; diff --git a/webrtc/modules/pacing/mock/mock_paced_sender.h b/webrtc/modules/pacing/mock/mock_paced_sender.h index bd7d7aaa49..e429962aed 100644 --- a/webrtc/modules/pacing/mock/mock_paced_sender.h +++ b/webrtc/modules/pacing/mock/mock_paced_sender.h @@ -29,6 +29,7 @@ class MockPacedSender : public PacedSender { int64_t capture_time_ms, size_t bytes, bool retransmission)); + MOCK_METHOD2(CreateProbeCluster, void(int, int)); MOCK_METHOD1(SetEstimatedBitrate, void(uint32_t)); MOCK_CONST_METHOD0(QueueInMs, int64_t()); MOCK_CONST_METHOD0(QueueInPackets, int()); diff --git a/webrtc/modules/pacing/paced_sender.h b/webrtc/modules/pacing/paced_sender.h index cb9b84930b..f74a97ab4d 100644 --- a/webrtc/modules/pacing/paced_sender.h +++ b/webrtc/modules/pacing/paced_sender.h @@ -71,7 +71,7 @@ class PacedSender : public Module, public RtpPacketSender { virtual ~PacedSender(); - void CreateProbeCluster(int bitrate_bps, int num_packets); + virtual void CreateProbeCluster(int bitrate_bps, int num_packets); // Temporarily pause all sending. void Pause(); diff --git a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h index 738570c718..dfeedfce85 100644 --- a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h +++ b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h @@ -34,7 +34,6 @@ class RemoteBitrateObserver { // incoming streams. virtual void OnReceiveBitrateChanged(const std::vector& ssrcs, uint32_t bitrate) = 0; - virtual void OnProbeBitrate(uint32_t bitrate) {} virtual ~RemoteBitrateObserver() {} diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc index d7067b3ef8..66ef7f0495 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc @@ -39,10 +39,8 @@ class PacketInfoComparator { }; TransportFeedbackAdapter::TransportFeedbackAdapter( - BitrateController* bitrate_controller, Clock* clock) : send_time_history_(clock, kSendTimeHistoryWindowMs), - bitrate_controller_(bitrate_controller), clock_(clock), current_offset_ms_(kNoTimestamp), last_timestamp_us_(kNoTimestamp) {} @@ -141,16 +139,6 @@ std::vector TransportFeedbackAdapter::GetTransportFeedbackVector() return last_packet_feedback_vector_; } -void TransportFeedbackAdapter::OnReceiveBitrateChanged( - const std::vector& ssrcs, - uint32_t bitrate) { - bitrate_controller_->UpdateDelayBasedEstimate(bitrate); -} - -void TransportFeedbackAdapter::OnProbeBitrate(uint32_t bitrate) { - bitrate_controller_->UpdateProbeBitrate(bitrate); -} - void TransportFeedbackAdapter::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { RTC_DCHECK(bitrate_estimator_.get() != nullptr); diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h index 525dc714e9..2db6603984 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h @@ -16,7 +16,6 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/thread_annotations.h" -#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" @@ -24,13 +23,11 @@ namespace webrtc { class ProcessThread; -class RemoteBitrateEstimator; class TransportFeedbackAdapter : public TransportFeedbackObserver, - public CallStatsObserver, - public RemoteBitrateObserver { + public CallStatsObserver { public: - TransportFeedbackAdapter(BitrateController* bitrate_controller, Clock* clock); + explicit TransportFeedbackAdapter(Clock* clock); virtual ~TransportFeedbackAdapter(); void SetBitrateEstimator(RemoteBitrateEstimator* rbe); @@ -51,18 +48,11 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; private: - // Implements RemoteBitrateObserver. - void OnReceiveBitrateChanged(const std::vector& ssrcs, - uint32_t bitrate) override; - - void OnProbeBitrate(uint32_t bitrate) override; - std::vector GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback); rtc::CriticalSection lock_; SendTimeHistory send_time_history_ GUARDED_BY(&lock_); - BitrateController* bitrate_controller_; std::unique_ptr bitrate_estimator_; Clock* const clock_; int64_t current_offset_ms_; diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc index f9ac8c9fa1..2a60da7669 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc @@ -40,7 +40,7 @@ class TransportFeedbackAdapterTest : public ::testing::Test { virtual ~TransportFeedbackAdapterTest() {} virtual void SetUp() { - adapter_.reset(new TransportFeedbackAdapter(&bitrate_controller_, &clock_)); + adapter_.reset(new TransportFeedbackAdapter(&clock_)); bitrate_estimator_ = new MockRemoteBitrateEstimator(); adapter_->SetBitrateEstimator(bitrate_estimator_); @@ -60,7 +60,8 @@ class TransportFeedbackAdapterTest : public ::testing::Test { ~MockBitrateControllerAdapter() override {} - void UpdateDelayBasedEstimate(uint32_t bitrate_bps) override { + void OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) override { owner_->receiver_estimated_bitrate_ = bitrate_bps; } diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index 5a8733c074..eab4dcf798 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -1008,7 +1008,7 @@ void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { } SimulatedClock clock(0); - TransportFeedbackAdapter feedback_adapter(nullptr, &clock); + TransportFeedbackAdapter feedback_adapter(&clock); TimeSeries time_series; time_series.label = "Network Delay Change";