From cb9ba301f0651a58dfa20c9fea0333d69f5b2fbf Mon Sep 17 00:00:00 2001 From: philipel Date: Tue, 7 Mar 2017 06:30:59 -0800 Subject: [PATCH] Perform probing on network route change. BUG=webrtc:7208 Review-Url: https://codereview.webrtc.org/2714503002 Cr-Commit-Position: refs/heads/master@{#17096} --- .../congestion_controller.cc | 6 +++- .../congestion_controller_unittest.cc | 13 +++++++++ .../congestion_controller/probe_controller.cc | 28 +++++++++++-------- .../congestion_controller/probe_controller.h | 5 ++++ .../transport_feedback_adapter.cc | 5 ++++ .../transport_feedback_adapter.h | 2 ++ .../send_time_history_unittest.cc | 22 +++++++++++++++ 7 files changed, 68 insertions(+), 13 deletions(-) diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 34b174ebc6..bc2f29cf0c 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -230,10 +230,14 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps, // no longer exposed outside CongestionController. remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps); + transport_feedback_adapter_.ClearSendTimeHistory(); transport_feedback_adapter_.InitBwe(); transport_feedback_adapter_.SetStartBitrate(bitrate_bps); transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps); - // TODO(holmer): Trigger a new probe once mid-call probing is implemented. + + probe_controller_->Reset(); + probe_controller_->SetBitrates(min_bitrate_bps, bitrate_bps, max_bitrate_bps); + MaybeTriggerOnNetworkChanged(); } diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index ca0d28fcd5..925631b9bb 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -59,6 +59,8 @@ class CongestionControllerTest : public ::testing::Test { // to be updated. EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps)); + EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 3)); + EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 5)); controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps); } @@ -143,6 +145,7 @@ TEST_F(CongestionControllerTest, SignalNetworkState) { TEST_F(CongestionControllerTest, ResetBweAndBitrates) { int new_bitrate = 200000; + testing::Mock::VerifyAndClearExpectations(pacer_); EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(new_bitrate)); controller_->ResetBweAndBitrates(new_bitrate, -1, -1); @@ -240,5 +243,15 @@ TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) { ASSERT_EQ(1u, ssrcs.size()); EXPECT_EQ(header.ssrc, ssrcs[0]); } + +TEST_F(CongestionControllerTest, ProbeOnBweReset) { + testing::Mock::VerifyAndClearExpectations(pacer_); + EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 6)); + EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 12)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _)); + controller_->ResetBweAndBitrates(2 * kInitialBitrateBps, 0, + 20 * kInitialBitrateBps); +} + } // namespace test } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probe_controller.cc b/webrtc/modules/congestion_controller/probe_controller.cc index e8b3a2a1cb..654f8b98f4 100644 --- a/webrtc/modules/congestion_controller/probe_controller.cc +++ b/webrtc/modules/congestion_controller/probe_controller.cc @@ -48,18 +48,9 @@ constexpr int kRepeatedProbeMinPercentage = 70; } // namespace ProbeController::ProbeController(PacedSender* pacer, Clock* clock) - : pacer_(pacer), - clock_(clock), - network_state_(kNetworkUp), - state_(State::kInit), - min_bitrate_to_probe_further_bps_(kExponentialProbingDisabled), - time_last_probing_initiated_ms_(0), - estimated_bitrate_bps_(0), - start_bitrate_bps_(0), - max_bitrate_bps_(0), - last_alr_probing_time_(clock_->TimeInMilliseconds()), - enable_periodic_alr_probing_(false), - mid_call_probing_waiting_for_result_(false) {} + : pacer_(pacer), clock_(clock), enable_periodic_alr_probing_(false) { + Reset(); +} void ProbeController::SetBitrates(int64_t min_bitrate_bps, int64_t start_bitrate_bps, @@ -186,6 +177,19 @@ void ProbeController::EnablePeriodicAlrProbing(bool enable) { enable_periodic_alr_probing_ = enable; } +void ProbeController::Reset() { + rtc::CritScope cs(&critsect_); + network_state_ = kNetworkUp; + state_ = State::kInit; + min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; + time_last_probing_initiated_ms_ = 0; + estimated_bitrate_bps_ = 0; + start_bitrate_bps_ = 0; + max_bitrate_bps_ = 0; + last_alr_probing_time_ = clock_->TimeInMilliseconds(); + mid_call_probing_waiting_for_result_ = false; +} + void ProbeController::Process() { rtc::CritScope cs(&critsect_); diff --git a/webrtc/modules/congestion_controller/probe_controller.h b/webrtc/modules/congestion_controller/probe_controller.h index a4201e63b5..ccce7d3ec6 100644 --- a/webrtc/modules/congestion_controller/probe_controller.h +++ b/webrtc/modules/congestion_controller/probe_controller.h @@ -37,6 +37,11 @@ class ProbeController { void SetEstimatedBitrate(int64_t bitrate_bps); void EnablePeriodicAlrProbing(bool enable); + + // Resets the ProbeController to a state equivalent to as if it was just + // created EXCEPT for |enable_periodic_alr_probing_|. + void Reset(); + void Process(); private: diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index 38f6173e22..2d173391f2 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -99,6 +99,11 @@ int64_t TransportFeedbackAdapter::GetProbingIntervalMs() const { return delay_based_bwe_->GetProbingIntervalMs(); } +void TransportFeedbackAdapter::ClearSendTimeHistory() { + rtc::CritScope cs(&lock_); + send_time_history_.Clear(); +} + std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback) { int64_t timestamp_us = feedback.GetBaseTimeUs(); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h index bbc245fe1e..0819ad969c 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -57,6 +57,8 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, int64_t GetProbingIntervalMs() const; + void ClearSendTimeHistory(); + private: std::vector GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback); diff --git a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc index 96e54203da..93c7b8f06a 100644 --- a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc @@ -213,5 +213,27 @@ TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) { EXPECT_EQ(packets[2], packet3); } +TEST_F(SendTimeHistoryTest, Clear) { + const uint16_t kSeqNo = 1; + const int64_t kTimestamp = 2; + const PacedPacketInfo kPacingInfo(0, 5, 1200); + + PacketFeedback packets[] = {{0, kTimestamp, kSeqNo, 0, kPacingInfo}, + {0, kTimestamp + 1, kSeqNo + 1, 0, kPacingInfo}}; + + AddPacketWithSendTime(packets[0].sequence_number, packets[0].payload_size, + packets[0].send_time_ms, kPacingInfo); + AddPacketWithSendTime(packets[1].sequence_number, packets[1].payload_size, + packets[1].send_time_ms, kPacingInfo); + PacketFeedback info(0, 0, packets[0].sequence_number, 0, kPacingInfo); + EXPECT_TRUE(history_.GetFeedback(&info, true)); + EXPECT_EQ(packets[0], info); + + history_.Clear(); + + PacketFeedback info2(0, 0, packets[1].sequence_number, 0, kPacingInfo); + EXPECT_FALSE(history_.GetFeedback(&info2, true)); +} + } // namespace test } // namespace webrtc