From 9fd6b98f4473ffc108265630bfaa95123243f263 Mon Sep 17 00:00:00 2001 From: philipel Date: Thu, 19 Apr 2018 16:58:07 +0200 Subject: [PATCH] Don't interrupt exponential probing when VideoStream bitrates are reconfigured. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: chromium:834255, webrtc:8955 Change-Id: Iba316e940ddc2d0d5891e57c24b808c68e0bbcfe Reviewed-on: https://webrtc-review.googlesource.com/70760 Reviewed-by: Erik Språng Reviewed-by: Sebastian Jansson Commit-Queue: Philip Eliasson Cr-Commit-Position: refs/heads/master@{#22943} --- .../goog_cc/probe_controller.cc | 9 ++++++++- modules/congestion_controller/probe_controller.cc | 10 +++++++++- video/end_to_end_tests/probing_tests.cc | 12 ++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 66838d3557..c05c094c2c 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -137,7 +137,8 @@ void ProbeController::OnMaxTotalAllocatedBitrate( int64_t at_time_ms) { // TODO(philipel): Should |max_total_allocated_bitrate| be used as a limit for // ALR probing? - if (max_total_allocated_bitrate != max_total_allocated_bitrate_ && + if (state_ == State::kProbingComplete && + max_total_allocated_bitrate != max_total_allocated_bitrate_ && estimated_bitrate_bps_ != 0 && (max_bitrate_bps_ <= 0 || estimated_bitrate_bps_ < max_bitrate_bps_) && estimated_bitrate_bps_ < max_total_allocated_bitrate) { @@ -147,6 +148,12 @@ void ProbeController::OnMaxTotalAllocatedBitrate( void ProbeController::OnNetworkAvailability(NetworkAvailability msg) { network_available_ = msg.network_available; + + if (!network_available_ && state_ == State::kWaitingForProbingResult) { + state_ = State::kProbingComplete; + min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; + } + if (network_available_ && state_ == State::kInit && start_bitrate_bps_ > 0) InitiateExponentialProbing(msg.at_time.ms()); } diff --git a/modules/congestion_controller/probe_controller.cc b/modules/congestion_controller/probe_controller.cc index 6fbe01e92e..ea1f45ca5b 100644 --- a/modules/congestion_controller/probe_controller.cc +++ b/modules/congestion_controller/probe_controller.cc @@ -130,7 +130,8 @@ void ProbeController::OnMaxTotalAllocatedBitrate( rtc::CritScope cs(&critsect_); // TODO(philipel): Should |max_total_allocated_bitrate| be used as a limit for // ALR probing? - if (max_total_allocated_bitrate != max_total_allocated_bitrate_ && + if (state_ == State::kProbingComplete && + max_total_allocated_bitrate != max_total_allocated_bitrate_ && estimated_bitrate_bps_ != 0 && (max_bitrate_bps_ <= 0 || estimated_bitrate_bps_ < max_bitrate_bps_) && estimated_bitrate_bps_ < max_total_allocated_bitrate) { @@ -143,6 +144,13 @@ void ProbeController::OnMaxTotalAllocatedBitrate( void ProbeController::OnNetworkStateChanged(NetworkState network_state) { rtc::CritScope cs(&critsect_); network_state_ = network_state; + + if (network_state_ == kNetworkDown && + state_ == State::kWaitingForProbingResult) { + state_ = State::kProbingComplete; + min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; + } + if (network_state_ == kNetworkUp && state_ == State::kInit) InitiateExponentialProbing(); } diff --git a/video/end_to_end_tests/probing_tests.cc b/video/end_to_end_tests/probing_tests.cc index 777a346754..e709505cf5 100644 --- a/video/end_to_end_tests/probing_tests.cc +++ b/video/end_to_end_tests/probing_tests.cc @@ -216,6 +216,11 @@ TEST_P(ProbingEndToEndTest, ProbeOnVideoEncoderReconfiguration) { send_stream_ = send_stream; } + void OnRtpTransportControllerSendCreated( + RtpTransportControllerSend* transport_controller) override { + transport_controller_ = transport_controller; + } + test::PacketTransport* CreateSendTransport( test::SingleThreadedTaskQueueForTesting* task_queue, Call* sender_call) override { @@ -244,6 +249,12 @@ TEST_P(ProbingEndToEndTest, ProbeOnVideoEncoderReconfiguration) { config.link_capacity_kbps = 200; send_transport_->SetConfig(config); + // In order to speed up the test we can interrupt exponential + // probing by toggling the network availability. The alternative + // is to wait for it to time out (1000 ms). + transport_controller_->OnNetworkAvailability(false); + transport_controller_->OnNetworkAvailability(true); + ++state_; } break; @@ -279,6 +290,7 @@ TEST_P(ProbingEndToEndTest, ProbeOnVideoEncoderReconfiguration) { test::PacketTransport* send_transport_; VideoSendStream* send_stream_; VideoEncoderConfig* encoder_config_; + RtpTransportControllerSend* transport_controller_; }; bool success = false;