diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index f559ffc2c2..027bd235f8 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -383,11 +383,6 @@ void PacedSender::SetPacingFactor(float pacing_factor) { SetEstimatedBitrate(estimated_bitrate_bps_); } -float PacedSender::GetPacingFactor() const { - rtc::CritScope cs(&critsect_); - return pacing_factor_; -} - void PacedSender::SetQueueTimeLimit(int limit_ms) { rtc::CritScope cs(&critsect_); queue_time_limit = limit_ms; diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index c1090ecbc0..56d09ca27e 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -149,7 +149,6 @@ class PacedSender : public Pacer { // Called when the prober is associated with a process thread. void ProcessThreadAttached(ProcessThread* process_thread) override; void SetPacingFactor(float pacing_factor); - float GetPacingFactor() const; void SetQueueTimeLimit(int limit_ms); private: diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index a4d657be34..7ab2a721c4 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -284,6 +284,8 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, void SetTransportOverhead(size_t transport_overhead_per_packet); + rtc::Optional configured_pacing_factor_; + private: class CheckEncoderActivityTask; class EncoderReconfiguredTask; @@ -645,6 +647,10 @@ VideoSendStream::Stats VideoSendStream::GetStats() { return stats_proxy_.GetStats(); } +rtc::Optional VideoSendStream::GetPacingFactorOverride() const { + return send_stream_->configured_pacing_factor_; +} + void VideoSendStream::SignalNetworkState(NetworkState state) { RTC_DCHECK_RUN_ON(&thread_checker_); VideoSendStreamImpl* send_stream = send_stream_.get(); @@ -770,6 +776,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( if (alr_settings) { transport->send_side_cc()->EnablePeriodicAlrProbing(true); transport->pacer()->SetPacingFactor(alr_settings->pacing_factor); + configured_pacing_factor_ = alr_settings->pacing_factor; transport->pacer()->SetQueueTimeLimit(alr_settings->max_paced_queue_time); } } diff --git a/video/video_send_stream.h b/video/video_send_stream.h index 998250cd6e..204a3cee1c 100644 --- a/video/video_send_stream.h +++ b/video/video_send_stream.h @@ -29,6 +29,9 @@ #include "video/video_stream_encoder.h" namespace webrtc { +namespace test { +class VideoSendStreamPeer; +} // namespace test class CallStats; class SendSideCongestionController; @@ -95,9 +98,13 @@ class VideoSendStream : public webrtc::VideoSendStream { void SetTransportOverhead(size_t transport_overhead_per_packet); private: + friend class test::VideoSendStreamPeer; + class ConstructionTask; class DestructAndGetRtpStateTask; + rtc::Optional GetPacingFactorOverride() const; + rtc::ThreadChecker thread_checker_; rtc::TaskQueue* const worker_queue_; rtc::Event thread_sync_event_; diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index f3b92fbc7e..e2f4908286 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -43,11 +43,26 @@ #include "test/rtcp_packet_parser.h" #include "test/testsupport/perf_test.h" +#include "call/video_send_stream.h" #include "video/send_statistics_proxy.h" #include "video/transport_adapter.h" -#include "call/video_send_stream.h" +#include "video/video_send_stream.h" namespace webrtc { +namespace test { +class VideoSendStreamPeer { + public: + explicit VideoSendStreamPeer(webrtc::VideoSendStream* base_class_stream) + : internal_stream_( + static_cast(base_class_stream)) {} + rtc::Optional GetPacingFactorOverride() const { + return internal_stream_->GetPacingFactorOverride(); + } + + private: + internal::VideoSendStream const* const internal_stream_; +}; +} // namespace test enum VideoFormat { kGeneric, kVP8, }; @@ -3539,80 +3554,82 @@ TEST_F(VideoSendStreamTest, SendsKeepAlive) { RunBaseTest(&test); } -TEST_F(VideoSendStreamTest, ConfiguresAlrWhenSendSideOn) { - const std::string kAlrProbingExperiment = - std::string(AlrExperimentSettings::kScreenshareProbingBweExperimentName) + - "/1.0,2875,80,40,-60,3/"; - test::ScopedFieldTrials alr_experiment(kAlrProbingExperiment); - class PacingFactorObserver : public test::SendTest { - public: - PacingFactorObserver(bool configure_send_side, float expected_pacing_factor) - : test::SendTest(kDefaultTimeoutMs), - configure_send_side_(configure_send_side), - expected_pacing_factor_(expected_pacing_factor), - paced_sender_(nullptr) {} +class PacingFactorObserver : public test::SendTest { + public: + PacingFactorObserver(bool configure_send_side, + rtc::Optional expected_pacing_factor) + : test::SendTest(VideoSendStreamTest::kDefaultTimeoutMs), + configure_send_side_(configure_send_side), + expected_pacing_factor_(expected_pacing_factor) {} - void ModifyVideoConfigs( - VideoSendStream::Config* send_config, - std::vector* receive_configs, - VideoEncoderConfig* encoder_config) override { - // Check if send-side bwe extension is already present, and remove it if - // it is not desired. - bool has_send_side = false; - for (auto it = send_config->rtp.extensions.begin(); - it != send_config->rtp.extensions.end(); ++it) { - if (it->uri == RtpExtension::kTransportSequenceNumberUri) { - if (configure_send_side_) { - has_send_side = true; - } else { - send_config->rtp.extensions.erase(it); - } - break; + void ModifyVideoConfigs( + VideoSendStream::Config* send_config, + std::vector* receive_configs, + VideoEncoderConfig* encoder_config) override { + // Check if send-side bwe extension is already present, and remove it if + // it is not desired. + bool has_send_side = false; + for (auto it = send_config->rtp.extensions.begin(); + it != send_config->rtp.extensions.end(); ++it) { + if (it->uri == RtpExtension::kTransportSequenceNumberUri) { + if (configure_send_side_) { + has_send_side = true; + } else { + send_config->rtp.extensions.erase(it); } + break; } - - if (configure_send_side_ && !has_send_side) { - // Want send side, not present by default, so add it. - send_config->rtp.extensions.emplace_back( - RtpExtension::kTransportSequenceNumberUri, - RtpExtension::kTransportSequenceNumberDefaultId); - } - - // ALR only enabled for screenshare. - encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen; } - void OnRtpTransportControllerSendCreated( - RtpTransportControllerSend* controller) override { - // Grab a reference to the pacer. - paced_sender_ = controller->pacer(); + if (configure_send_side_ && !has_send_side) { + // Want send side, not present by default, so add it. + send_config->rtp.extensions.emplace_back( + RtpExtension::kTransportSequenceNumberUri, + RtpExtension::kTransportSequenceNumberDefaultId); } - void OnVideoStreamsCreated( - VideoSendStream* send_stream, - const std::vector& receive_streams) override { - // Video streams created, check that pacer is correctly configured. - EXPECT_EQ(expected_pacing_factor_, paced_sender_->GetPacingFactor()); - observation_complete_.Set(); - } + // ALR only enabled for screenshare. + encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen; + } - void PerformTest() override { - EXPECT_TRUE(Wait()) << "Timed out while waiting for pacer config."; - } + void OnVideoStreamsCreated( + VideoSendStream* send_stream, + const std::vector& receive_streams) override { + auto internal_send_peer = test::VideoSendStreamPeer(send_stream); + // Video streams created, check that pacing factor is correctly configured. + EXPECT_EQ(expected_pacing_factor_, + internal_send_peer.GetPacingFactorOverride()); + observation_complete_.Set(); + } - private: - const bool configure_send_side_; - const float expected_pacing_factor_; - const PacedSender* paced_sender_; - }; + void PerformTest() override { + EXPECT_TRUE(Wait()) << "Timed out while waiting for stream creation."; + } + private: + const bool configure_send_side_; + const rtc::Optional expected_pacing_factor_; +}; + +std::string GetAlrProbingExperimentString() { + return std::string( + AlrExperimentSettings::kScreenshareProbingBweExperimentName) + + "/1.0,2875,80,40,-60,3/"; +} +const float kAlrProbingExperimentPaceMultiplier = 1.0f; + +TEST_F(VideoSendStreamTest, AlrConfiguredWhenSendSideOn) { + test::ScopedFieldTrials alr_experiment(GetAlrProbingExperimentString()); // Send-side bwe on, use pacing factor from |kAlrProbingExperiment| above. - PacingFactorObserver test_with_send_side(true, 1.0f); + PacingFactorObserver test_with_send_side(true, + kAlrProbingExperimentPaceMultiplier); RunBaseTest(&test_with_send_side); +} - // Send-side bwe off, use default pacing factor. - PacingFactorObserver test_without_send_side( - false, PacedSender::kDefaultPaceMultiplier); +TEST_F(VideoSendStreamTest, AlrNotConfiguredWhenSendSideOff) { + test::ScopedFieldTrials alr_experiment(GetAlrProbingExperimentString()); + // Send-side bwe off, use configuration should not be overridden. + PacingFactorObserver test_without_send_side(false, rtc::nullopt); RunBaseTest(&test_without_send_side); }