diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index ac9866b314..9915a77258 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -303,7 +303,7 @@ Call::Call(const Call::Config& config) RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); RTC_DCHECK(config.event_log != nullptr); RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0); - RTC_DCHECK_GE(config.bitrate_config.start_bitrate_bps, + RTC_DCHECK_GT(config.bitrate_config.start_bitrate_bps, config.bitrate_config.min_bitrate_bps); if (config.bitrate_config.max_bitrate_bps != -1) { RTC_DCHECK_GE(config.bitrate_config.max_bitrate_bps, @@ -809,6 +809,7 @@ void Call::SetBitrateConfig( if (bitrate_config.start_bitrate_bps > 0) config_.bitrate_config.start_bitrate_bps = bitrate_config.start_bitrate_bps; config_.bitrate_config.max_bitrate_bps = bitrate_config.max_bitrate_bps; + RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0); congestion_controller_->SetBweBitrates(bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps, bitrate_config.max_bitrate_bps); @@ -905,6 +906,7 @@ void Call::OnNetworkRouteChanged(const std::string& transport_name, << " bps, start: " << config_.bitrate_config.start_bitrate_bps << " bps, max: " << config_.bitrate_config.start_bitrate_bps << " bps."; + RTC_DCHECK_GT(config_.bitrate_config.start_bitrate_bps, 0); congestion_controller_->ResetBweAndBitrates( config_.bitrate_config.start_bitrate_bps, config_.bitrate_config.min_bitrate_bps, diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index d8f0592ec4..46e97cd2a5 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -36,9 +36,11 @@ std::vector GenerateSsrcs(size_t num_streams, uint32_t ssrc_offset) { RampUpTester::RampUpTester(size_t num_video_streams, size_t num_audio_streams, unsigned int start_bitrate_bps, + int64_t min_run_time_ms, const std::string& extension_type, bool rtx, - bool red) + bool red, + bool report_perf_stats) : EndToEndTest(test::CallTest::kLongTimeoutMs), event_(false, false), clock_(Clock::GetRealTimeClock()), @@ -49,7 +51,8 @@ RampUpTester::RampUpTester(size_t num_video_streams, sender_call_(nullptr), send_stream_(nullptr), start_bitrate_bps_(start_bitrate_bps), - start_bitrate_verified_(false), + min_run_time_ms_(min_run_time_ms), + report_perf_stats_(report_perf_stats), expected_bitrate_bps_(0), test_start_ms_(-1), ramp_up_finished_ms_(-1), @@ -253,17 +256,11 @@ bool RampUpTester::PollStats() { if (sender_call_) { Call::Stats stats = sender_call_->GetStats(); - RTC_DCHECK_GT(expected_bitrate_bps_, 0); - if (!start_bitrate_verified_ && start_bitrate_bps_ != 0) { - // For tests with an explicitly set start bitrate, verify the first - // bitrate estimate is close to the start bitrate and lower than the - // test target bitrate. This is to verify a call respects the configured - // start bitrate, but due to the BWE implementation we can't guarantee the - // first estimate really is as high as the start bitrate. - EXPECT_GT(stats.send_bandwidth_bps, 0.9 * start_bitrate_bps_); - start_bitrate_verified_ = true; - } - if (stats.send_bandwidth_bps >= expected_bitrate_bps_) { + EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_); + EXPECT_GE(expected_bitrate_bps_, 0); + if (stats.send_bandwidth_bps >= expected_bitrate_bps_ && + (min_run_time_ms_ == -1 || + clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) { ramp_up_finished_ms_ = clock_->TimeInMilliseconds(); observation_complete_.Set(); } @@ -325,21 +322,23 @@ void RampUpTester::TriggerTestDone() { &rtx_total_sent, &rtx_padding_sent, &rtx_media_sent); } - ReportResult("ramp-up-total-packets-sent", total_packets_sent, "packets"); - ReportResult("ramp-up-total-sent", total_sent, "bytes"); - ReportResult("ramp-up-media-sent", media_sent, "bytes"); - ReportResult("ramp-up-padding-sent", padding_sent, "bytes"); - ReportResult("ramp-up-rtx-total-packets-sent", rtx_total_packets_sent, - "packets"); - ReportResult("ramp-up-rtx-total-sent", rtx_total_sent, "bytes"); - ReportResult("ramp-up-rtx-media-sent", rtx_media_sent, "bytes"); - ReportResult("ramp-up-rtx-padding-sent", rtx_padding_sent, "bytes"); - if (ramp_up_finished_ms_ >= 0) { - ReportResult("ramp-up-time", ramp_up_finished_ms_ - test_start_ms_, - "milliseconds"); + if (report_perf_stats_) { + ReportResult("ramp-up-total-packets-sent", total_packets_sent, "packets"); + ReportResult("ramp-up-total-sent", total_sent, "bytes"); + ReportResult("ramp-up-media-sent", media_sent, "bytes"); + ReportResult("ramp-up-padding-sent", padding_sent, "bytes"); + ReportResult("ramp-up-rtx-total-packets-sent", rtx_total_packets_sent, + "packets"); + ReportResult("ramp-up-rtx-total-sent", rtx_total_sent, "bytes"); + ReportResult("ramp-up-rtx-media-sent", rtx_media_sent, "bytes"); + ReportResult("ramp-up-rtx-padding-sent", rtx_padding_sent, "bytes"); + if (ramp_up_finished_ms_ >= 0) { + ReportResult("ramp-up-time", ramp_up_finished_ms_ - test_start_ms_, + "milliseconds"); + } + ReportResult("ramp-up-average-network-latency", + send_transport_->GetAverageDelayMs(), "milliseconds"); } - ReportResult("ramp-up-average-network-latency", - send_transport_->GetAverageDelayMs(), "milliseconds"); } void RampUpTester::PerformTest() { @@ -359,9 +358,11 @@ RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams, : RampUpTester(num_video_streams, num_audio_streams, start_bitrate_bps, + 0, extension_type, rtx, - red), + red, + true), test_state_(kFirstRampup), state_start_ms_(clock_->TimeInMilliseconds()), interval_start_ms_(clock_->TimeInMilliseconds()), @@ -525,35 +526,45 @@ TEST_F(RampUpTest, UpDownUpAudioTransportSequenceNumberRtx) { } TEST_F(RampUpTest, TOffsetSimulcastRedRtx) { - RampUpTester test(3, 0, 0, RtpExtension::kTimestampOffsetUri, true, true); + RampUpTester test(3, 0, 0, 0, RtpExtension::kTimestampOffsetUri, true, true, + true); RunBaseTest(&test); } TEST_F(RampUpTest, AbsSendTime) { - RampUpTester test(1, 0, 0, RtpExtension::kAbsSendTimeUri, false, false); + RampUpTester test(1, 0, 0, 0, RtpExtension::kAbsSendTimeUri, false, false, + true); RunBaseTest(&test); } TEST_F(RampUpTest, AbsSendTimeSimulcastRedRtx) { - RampUpTester test(3, 0, 0, RtpExtension::kAbsSendTimeUri, true, true); + RampUpTester test(3, 0, 0, 0, RtpExtension::kAbsSendTimeUri, true, true, + true); RunBaseTest(&test); } TEST_F(RampUpTest, TransportSequenceNumber) { - RampUpTester test(1, 0, 0, RtpExtension::kTransportSequenceNumberUri, false, - false); + RampUpTester test(1, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri, + false, false, true); RunBaseTest(&test); } TEST_F(RampUpTest, TransportSequenceNumberSimulcast) { - RampUpTester test(3, 0, 0, RtpExtension::kTransportSequenceNumberUri, false, - false); + RampUpTester test(3, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri, + false, false, true); RunBaseTest(&test); } TEST_F(RampUpTest, TransportSequenceNumberSimulcastRedRtx) { - RampUpTester test(3, 0, 0, RtpExtension::kTransportSequenceNumberUri, true, - true); + RampUpTester test(3, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri, + true, true, true); + RunBaseTest(&test); +} + +TEST_F(RampUpTest, AudioTransportSequenceNumber) { + RampUpTester test(0, 1, 300000, 10000, + RtpExtension::kTransportSequenceNumberUri, false, false, + false); RunBaseTest(&test); } } // namespace webrtc diff --git a/webrtc/call/rampup_tests.h b/webrtc/call/rampup_tests.h index 606a8cd5d2..5cf4c4fab5 100644 --- a/webrtc/call/rampup_tests.h +++ b/webrtc/call/rampup_tests.h @@ -34,9 +34,11 @@ class RampUpTester : public test::EndToEndTest { RampUpTester(size_t num_video_streams, size_t num_audio_streams, unsigned int start_bitrate_bps, + int64_t min_run_time_ms, const std::string& extension_type, bool rtx, - bool red); + bool red, + bool report_perf_stats); ~RampUpTester() override; size_t GetNumVideoStreams() const override; @@ -91,7 +93,8 @@ class RampUpTester : public test::EndToEndTest { static bool BitrateStatsPollingThread(void* obj); const int start_bitrate_bps_; - bool start_bitrate_verified_; + const int64_t min_run_time_ms_; + const bool report_perf_stats_; int expected_bitrate_bps_; int64_t test_start_ms_; int64_t ramp_up_finished_ms_; diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 51fc775266..fd0d070ba0 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -210,6 +210,7 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps); min_bitrate_bps_ = min_bitrate_bps; + transport_feedback_adapter_.SetStartBitrate(start_bitrate_bps); transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_); MaybeTriggerOnNetworkChanged(); } @@ -229,6 +230,7 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps, remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps); 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. MaybeTriggerOnNetworkChanged(); diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 3850797b9e..ca0d28fcd5 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -240,6 +240,5 @@ TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) { ASSERT_EQ(1u, ssrcs.size()); EXPECT_EQ(header.ssrc, ssrcs[0]); } - } // namespace test } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 7a20b3eedb..0febf6f435 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -383,6 +383,11 @@ bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, return true; } +void DelayBasedBwe::SetStartBitrate(int start_bitrate_bps) { + LOG(LS_WARNING) << "BWE Setting start bitrate to: " << start_bitrate_bps; + rate_control_.SetStartBitrate(start_bitrate_bps); +} + void DelayBasedBwe::SetMinBitrate(int min_bitrate_bps) { // Called from both the configuration thread and the network thread. Shouldn't // be called from the network thread in the future. diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 6e7a6f8d63..6342d05927 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -52,6 +52,7 @@ class DelayBasedBwe { void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms); bool LatestEstimate(std::vector* ssrcs, uint32_t* bitrate_bps) const; + void SetStartBitrate(int start_bitrate_bps); void SetMinBitrate(int min_bitrate_bps); int64_t GetProbingIntervalMs() const; diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index f8da97ae75..b9440ac959 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -74,6 +74,11 @@ void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number, send_time_history_.OnSentPacket(sequence_number, send_time_ms); } +void TransportFeedbackAdapter::SetStartBitrate(int start_bitrate_bps) { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_->SetStartBitrate(start_bitrate_bps); +} + void TransportFeedbackAdapter::SetMinBitrate(int min_bitrate_bps) { rtc::CritScope cs(&bwe_lock_); delay_based_bwe_->SetMinBitrate(min_bitrate_bps); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h index b7085be91d..5ff48aaf97 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -48,6 +48,7 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, // Implements CallStatsObserver. void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; + void SetStartBitrate(int start_bitrate_bps); void SetMinBitrate(int min_bitrate_bps); void SetTransportOverhead(int transport_overhead_bytes_per_packet); diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc index e0c9ccd3ba..fdee6df867 100644 --- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -43,6 +43,13 @@ AimdRateControl::AimdRateControl() rtt_(kDefaultRttMs), in_experiment_(!AdaptiveThresholdExperimentIsDisabled()) {} +AimdRateControl::~AimdRateControl() {} + +void AimdRateControl::SetStartBitrate(int start_bitrate_bps) { + current_bitrate_bps_ = start_bitrate_bps; + bitrate_is_initialized_ = true; +} + void AimdRateControl::SetMinBitrate(int min_bitrate_bps) { min_configured_bitrate_bps_ = min_bitrate_bps; current_bitrate_bps_ = std::max(min_bitrate_bps, current_bitrate_bps_); diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h index ddfa667a87..b3c5928e05 100644 --- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -24,11 +24,12 @@ namespace webrtc { class AimdRateControl { public: AimdRateControl(); - virtual ~AimdRateControl() {} + virtual ~AimdRateControl(); // Returns true if there is a valid estimate of the incoming bitrate, false // otherwise. bool ValidEstimate() const; + void SetStartBitrate(int start_bitrate_bps); void SetMinBitrate(int min_bitrate_bps); int64_t GetFeedbackInterval() const; // Returns true if the bitrate estimate hasn't been changed for more than