From 8f83b42946dd7d5c2cf420c83887cfd4f62b472c Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 21 Feb 2018 13:07:13 +0100 Subject: [PATCH] Moved bitrate config interface from Call class. Moving usage of bitrate configuration related interface from Call interface to the corresponding methods in the RtpSendTransportController interface. SetBitrateConfig was replaced with SetSdpBitrateParameters SetBitrateConfigMask was replaced with SetClientBitratePreferences OnNetworkRouteChanged was replaced with OnNetworkRouteChanged This makes it more clear that RtpSendTransportController owns bitrate configuration and fits a longer term ambition to reduce the scope of the Call class. Bug: webrtc:8415 Change-Id: I6d04eaad22a54ecd5ed60096e01689b0c67e9c65 Reviewed-on: https://webrtc-review.googlesource.com/54365 Commit-Queue: Sebastian Jansson Reviewed-by: Stefan Holmer Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#22131} --- call/BUILD.gn | 1 + call/call.cc | 34 ++------ call/call.h | 26 ++---- call/call_perf_tests.cc | 3 +- call/rtp_bitrate_configurator.h | 18 ++--- call/rtp_transport_controller_send.cc | 4 +- .../test/mock_rtp_transport_controller_send.h | 54 +++++++++++++ media/BUILD.gn | 1 + media/engine/fakewebrtccall.cc | 19 +---- media/engine/fakewebrtccall.h | 28 ++++--- media/engine/webrtcvideoengine.cc | 7 +- media/engine/webrtcvideoengine_unittest.cc | 80 +++++++++---------- media/engine/webrtcvoiceengine.cc | 6 +- media/engine/webrtcvoiceengine_unittest.cc | 33 ++++---- pc/channelmanager_unittest.cc | 4 +- pc/peerconnection.cc | 2 +- pc/rtpsenderreceiver_unittest.cc | 2 +- video/end_to_end_tests/probing_tests.cc | 6 +- video/video_send_stream_tests.cc | 23 ++++-- 19 files changed, 189 insertions(+), 162 deletions(-) create mode 100644 call/test/mock_rtp_transport_controller_send.h diff --git a/call/BUILD.gn b/call/BUILD.gn index 683f5282c1..1abe131a5e 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -309,6 +309,7 @@ if (rtc_include_tests) { sources = [ "test/mock_rtp_packet_sink_interface.h", + "test/mock_rtp_transport_controller_send.h", ] deps = [ ":rtp_interfaces", diff --git a/call/call.cc b/call/call.cc index 9f526cc672..0322a8624d 100644 --- a/call/call.cc +++ b/call/call.cc @@ -202,6 +202,8 @@ class Call : public webrtc::Call, void DestroyFlexfecReceiveStream( FlexfecReceiveStream* receive_stream) override; + RtpTransportControllerSendInterface* GetTransportControllerSend() override; + Stats GetStats() const override; // Implements PacketReceiver. @@ -212,12 +214,6 @@ class Call : public webrtc::Call, // Implements RecoveredPacketReceiver. void OnRecoveredPacket(const uint8_t* packet, size_t length) override; - void SetBitrateConfig( - const webrtc::BitrateConstraints& bitrate_config) override; - - void SetBitrateConfigMask( - const webrtc::BitrateConstraintsMask& bitrate_config) override; - void SetBitrateAllocationStrategy( std::unique_ptr bitrate_allocation_strategy) override; @@ -227,9 +223,6 @@ class Call : public webrtc::Call, void OnTransportOverheadChanged(MediaType media, int transport_overhead_per_packet) override; - void OnNetworkRouteChanged(const std::string& transport_name, - const rtc::NetworkRoute& network_route) override; - void OnSentPacket(const rtc::SentPacket& sent_packet) override; // Implements BitrateObserver. @@ -907,6 +900,10 @@ void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { delete receive_stream; } +RtpTransportControllerSendInterface* Call::GetTransportControllerSend() { + return transport_send_.get(); +} + Call::Stats Call::GetStats() const { // TODO(solenberg): Some test cases in EndToEndTest use this from a different // thread. Re-enable once that is fixed. @@ -930,18 +927,6 @@ Call::Stats Call::GetStats() const { return stats; } -void Call::SetBitrateConfig(const BitrateConstraints& bitrate_config) { - TRACE_EVENT0("webrtc", "Call::SetBitrateConfig"); - RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); - transport_send_->SetSdpBitrateParameters(bitrate_config); -} - -void Call::SetBitrateConfigMask(const webrtc::BitrateConstraintsMask& mask) { - TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask"); - RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); - transport_send_->SetClientBitratePreferences(mask); -} - void Call::SetBitrateAllocationStrategy( std::unique_ptr bitrate_allocation_strategy) { @@ -1020,13 +1005,6 @@ void Call::OnTransportOverheadChanged(MediaType media, } } -// TODO(honghaiz): Add tests for this method. -void Call::OnNetworkRouteChanged(const std::string& transport_name, - const rtc::NetworkRoute& network_route) { - RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); - transport_send_->OnNetworkRouteChanged(transport_name, network_route); -} - void Call::UpdateAggregateNetworkState() { RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); diff --git a/call/call.h b/call/call.h index 117c40c785..8630815a95 100644 --- a/call/call.h +++ b/call/call.h @@ -147,25 +147,17 @@ class Call { // Call instance exists. virtual PacketReceiver* Receiver() = 0; + // This is used to access the transport controller send instance owned by + // Call. The send transport controller is currently owned by Call for legacy + // reasons. (for instance variants of call tests are built on this assumtion) + // TODO(srte): Move ownership of transport controller send out of Call and + // remove this method interface. + virtual RtpTransportControllerSendInterface* GetTransportControllerSend() = 0; + // Returns the call statistics, such as estimated send and receive bandwidth, // pacing delay, etc. virtual Stats GetStats() const = 0; - // The greater min and smaller max set by this and SetBitrateConfigMask will - // be used. The latest non-negative start value from either call will be used. - // Specifying a start bitrate (>0) will reset the current bitrate estimate. - // This is due to how the 'x-google-start-bitrate' flag is currently - // implemented. Passing -1 leaves the start bitrate unchanged. Behavior is not - // guaranteed for other negative values or 0. - virtual void SetBitrateConfig(const BitrateConstraints& bitrate_config) = 0; - - // The greater min and smaller max set by this and SetBitrateConfig will be - // used. The latest non-negative start value form either call will be used. - // Specifying a start bitrate will reset the current bitrate estimate. - // Assumes 0 <= min <= start <= max holds for set parameters. - virtual void SetBitrateConfigMask( - const BitrateConstraintsMask& bitrate_mask) = 0; - virtual void SetBitrateAllocationStrategy( std::unique_ptr bitrate_allocation_strategy) = 0; @@ -180,10 +172,6 @@ class Call { MediaType media, int transport_overhead_per_packet) = 0; - virtual void OnNetworkRouteChanged( - const std::string& transport_name, - const rtc::NetworkRoute& network_route) = 0; - virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; virtual ~Call() {} diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc index e8c3601a66..d96160e50b 100644 --- a/call/call_perf_tests.cc +++ b/call/call_perf_tests.cc @@ -892,7 +892,8 @@ void CallPerfTest::TestMinAudioVideoBitrate( bitrate_config.min_bitrate_bps = min_bwe_; bitrate_config.start_bitrate_bps = start_bwe_; bitrate_config.max_bitrate_bps = max_bwe_; - sender_call->SetBitrateConfig(bitrate_config); + sender_call->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); if (use_bitrate_allocation_strategy_) { sender_call->SetBitrateAllocationStrategy( std::move(allocation_strategy_)); diff --git a/call/rtp_bitrate_configurator.h b/call/rtp_bitrate_configurator.h index df84b5fad9..9115243cf7 100644 --- a/call/rtp_bitrate_configurator.h +++ b/call/rtp_bitrate_configurator.h @@ -24,19 +24,19 @@ class RtpBitrateConfigurator { ~RtpBitrateConfigurator(); BitrateConstraints GetConfig() const; - // The greater min and smaller max set by this and SetBitrateConfigMask will - // be used. The latest non-negative start value from either call will be used. - // Specifying a start bitrate (>0) will reset the current bitrate estimate. - // This is due to how the 'x-google-start-bitrate' flag is currently + // The greater min and smaller max set by this and SetClientBitratePreferences + // will be used. The latest non-negative start value from either call will be + // used. Specifying a start bitrate (>0) will reset the current bitrate + // estimate. This is due to how the 'x-google-start-bitrate' flag is currently // implemented. Passing -1 leaves the start bitrate unchanged. Behavior is not // guaranteed for other negative values or 0. // The optional return value is set with new configuration if it was updated. rtc::Optional UpdateWithSdpParameters( const BitrateConstraints& bitrate_config_); - // The greater min and smaller max set by this and SetBitrateConfig will be - // used. The latest non-negative start value form either call will be used. - // Specifying a start bitrate will reset the current bitrate estimate. + // The greater min and smaller max set by this and SetSdpBitrateParameters + // will be used. The latest non-negative start value form either call will be + // used. Specifying a start bitrate will reset the current bitrate estimate. // Assumes 0 <= min <= start <= max holds for set parameters. // Update the bitrate configuration // The optional return value is set with new configuration if it was updated. @@ -53,11 +53,11 @@ class RtpBitrateConfigurator { // used to cap total bitrate used. This comes from the remote connection. BitrateConstraints bitrate_config_; - // The config mask set by SetBitrateConfigMask. + // The config mask set by SetClientBitratePreferences. // 0 <= min <= start <= max BitrateConstraintsMask bitrate_config_mask_; - // The config set by SetBitrateConfig. + // The config set by SetSdpBitrateParameters. // min >= 0, start != 0, max == -1 || max > 0 BitrateConstraints base_bitrate_config_; diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 71be5cbef8..5e054a85b0 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -166,7 +166,7 @@ void RtpTransportControllerSend::SetSdpBitrateParameters( updated->max_bitrate_bps); } else { RTC_LOG(LS_VERBOSE) - << "WebRTC.RtpTransportControllerSend.SetBitrateConfig: " + << "WebRTC.RtpTransportControllerSend.SetSdpBitrateParameters: " << "nothing to update"; } } @@ -181,7 +181,7 @@ void RtpTransportControllerSend::SetClientBitratePreferences( updated->max_bitrate_bps); } else { RTC_LOG(LS_VERBOSE) - << "WebRTC.RtpTransportControllerSend.SetBitrateConfigMask: " + << "WebRTC.RtpTransportControllerSend.SetClientBitratePreferences: " << "nothing to update"; } } diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h new file mode 100644 index 0000000000..6c9b47cba4 --- /dev/null +++ b/call/test/mock_rtp_transport_controller_send.h @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2018 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 CALL_TEST_MOCK_RTP_TRANSPORT_CONTROLLER_SEND_H_ +#define CALL_TEST_MOCK_RTP_TRANSPORT_CONTROLLER_SEND_H_ + +#include + +#include "call/rtp_transport_controller_send_interface.h" +#include "test/gmock.h" + +namespace webrtc { + +class MockRtpTransportControllerSend + : public RtpTransportControllerSendInterface { + public: + MOCK_METHOD0(packet_router, PacketRouter*()); + MOCK_METHOD0(transport_feedback_observer, TransportFeedbackObserver*()); + MOCK_METHOD0(packet_sender, RtpPacketSender*()); + MOCK_CONST_METHOD0(keepalive_config, RtpKeepAliveConfig&()); + MOCK_METHOD2(SetAllocatedSendBitrateLimits, void(int, int)); + MOCK_METHOD0(GetPacerModule, Module*()); + MOCK_METHOD1(SetPacingFactor, void(float)); + MOCK_METHOD1(SetQueueTimeLimit, void(int)); + MOCK_METHOD0(GetModule, Module*()); + MOCK_METHOD0(GetCallStatsObserver, CallStatsObserver*()); + MOCK_METHOD1(RegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); + MOCK_METHOD1(DeRegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); + MOCK_METHOD1(RegisterNetworkObserver, void(NetworkChangedObserver*)); + MOCK_METHOD1(DeRegisterNetworkObserver, void(NetworkChangedObserver*)); + MOCK_METHOD2(OnNetworkRouteChanged, + void(const std::string&, const rtc::NetworkRoute&)); + MOCK_METHOD1(OnNetworkAvailability, void(bool)); + MOCK_METHOD1(SetTransportOverhead, void(size_t)); + MOCK_METHOD0(GetBandwidthObserver, RtcpBandwidthObserver*()); + MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t*)); + MOCK_CONST_METHOD0(GetPacerQueuingDelayMs, int64_t()); + MOCK_CONST_METHOD0(GetFirstPacketTimeMs, int64_t()); + MOCK_METHOD0(GetRetransmissionRateLimiter, RateLimiter*()); + MOCK_METHOD1(EnablePeriodicAlrProbing, void(bool)); + MOCK_METHOD1(OnSentPacket, void(const rtc::SentPacket&)); + MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&)); + MOCK_METHOD1(SetClientBitratePreferences, + void(const BitrateConstraintsMask&)); +}; +} // namespace webrtc +#endif // CALL_TEST_MOCK_RTP_TRANSPORT_CONTROLLER_SEND_H_ diff --git a/media/BUILD.gn b/media/BUILD.gn index 1b2e9c8c58..67dd00836b 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -488,6 +488,7 @@ if (rtc_include_tests) { "../api:video_frame_api", "../api/video_codecs:video_codecs_api", "../call:call_interfaces", + "../call:mock_rtp_interfaces", "../rtc_base:rtc_base", "../rtc_base:rtc_base_approved", "../rtc_base:rtc_base_tests_utils", diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc index c742a677d4..9aae516de1 100644 --- a/media/engine/fakewebrtccall.cc +++ b/media/engine/fakewebrtccall.cc @@ -384,9 +384,8 @@ void FakeFlexfecReceiveStream::OnRtpPacket(const webrtc::RtpPacketReceived&) { RTC_NOTREACHED() << "Not implemented."; } -FakeCall::FakeCall(const webrtc::Call::Config& config) - : config_(config), - audio_network_state_(webrtc::kNetworkUp), +FakeCall::FakeCall() + : audio_network_state_(webrtc::kNetworkUp), video_network_state_(webrtc::kNetworkUp), num_created_send_streams_(0), num_created_receive_streams_(0), @@ -400,10 +399,6 @@ FakeCall::~FakeCall() { EXPECT_EQ(0u, audio_receive_streams_.size()); } -webrtc::Call::Config FakeCall::GetConfig() const { - return config_; -} - const std::vector& FakeCall::GetVideoSendStreams() { return video_send_streams_; } @@ -617,16 +612,6 @@ webrtc::Call::Stats FakeCall::GetStats() const { return stats_; } -void FakeCall::SetBitrateConfig( - const webrtc::BitrateConstraints& bitrate_config) { - config_.bitrate_config = bitrate_config; -} - -void FakeCall::SetBitrateConfigMask( - const webrtc::BitrateConstraintsMask& mask) { - // TODO(zstein): not implemented -} - void FakeCall::SetBitrateAllocationStrategy( std::unique_ptr bitrate_allocation_strategy) { diff --git a/media/engine/fakewebrtccall.h b/media/engine/fakewebrtccall.h index dca061938e..9b8ea23646 100644 --- a/media/engine/fakewebrtccall.h +++ b/media/engine/fakewebrtccall.h @@ -29,10 +29,11 @@ #include "call/audio_send_stream.h" #include "call/call.h" #include "call/flexfec_receive_stream.h" -#include "modules/rtp_rtcp/source/rtp_packet_received.h" -#include "rtc_base/buffer.h" +#include "call/test/mock_rtp_transport_controller_send.h" #include "call/video_receive_stream.h" #include "call/video_send_stream.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" +#include "rtc_base/buffer.h" namespace cricket { class FakeAudioSendStream final : public webrtc::AudioSendStream { @@ -241,10 +242,13 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { public: - explicit FakeCall(const webrtc::Call::Config& config); + FakeCall(); ~FakeCall() override; - webrtc::Call::Config GetConfig() const; + webrtc::MockRtpTransportControllerSend* GetMockTransportControllerSend() { + return &transport_controller_send_; + } + const std::vector& GetVideoSendStreams(); const std::vector& GetVideoReceiveStreams(); @@ -299,24 +303,26 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { rtc::CopyOnWriteBuffer packet, const webrtc::PacketTime& packet_time) override; + webrtc::RtpTransportControllerSendInterface* GetTransportControllerSend() + override { + return &transport_controller_send_; + } + webrtc::Call::Stats GetStats() const override; - void SetBitrateConfig( - const webrtc::BitrateConstraints& bitrate_config) override; - void SetBitrateConfigMask( - const webrtc::BitrateConstraintsMask& mask) override; void SetBitrateAllocationStrategy( std::unique_ptr bitrate_allocation_strategy) override; - void OnNetworkRouteChanged(const std::string& transport_name, - const rtc::NetworkRoute& network_route) override {} + void SignalChannelNetworkState(webrtc::MediaType media, webrtc::NetworkState state) override; void OnTransportOverheadChanged(webrtc::MediaType media, int transport_overhead_per_packet) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; - webrtc::Call::Config config_; + testing::NiceMock + transport_controller_send_; + webrtc::NetworkState audio_network_state_; webrtc::NetworkState video_network_state_; rtc::SentPacket last_sent_packet_; diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 9c555c60b4..f0b511df95 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -776,7 +776,8 @@ bool WebRtcVideoChannel::SetSendParameters(const VideoSendParameters& params) { bitrate_config_.max_bitrate_bps = params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps; } - call_->SetBitrateConfig(bitrate_config_); + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config_); } { @@ -1481,8 +1482,8 @@ void WebRtcVideoChannel::OnReadyToSend(bool ready) { void WebRtcVideoChannel::OnNetworkRouteChanged( const std::string& transport_name, const rtc::NetworkRoute& network_route) { - // TODO(zhihaung): Merge these two callbacks. - call_->OnNetworkRouteChanged(transport_name, network_route); + call_->GetTransportControllerSend()->OnNetworkRouteChanged(transport_name, + network_route); call_->OnTransportOverheadChanged(webrtc::MediaType::VIDEO, network_route.packet_overhead); } diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 97fbf39293..29653541e5 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -42,7 +42,9 @@ #include "test/gmock.h" using cricket::FakeVideoCapturerWithTaskQueue; +using webrtc::BitrateConstraints; using webrtc::RtpExtension; +using testing::Field; namespace { static const int kDefaultQpMax = 56; @@ -475,7 +477,7 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForExternalCodec) { void WebRtcVideoEngineTest::TestExtendedEncoderOveruse( bool use_external_encoder) { std::unique_ptr channel; - FakeCall* fake_call = new FakeCall(webrtc::Call::Config(&event_log_)); + FakeCall* fake_call = new FakeCall(); call_.reset(fake_call); if (use_external_encoder) { encoder_factory_->AddSupportedVideoCodecType("VP8"); @@ -519,7 +521,7 @@ TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) { TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - FakeCall* fake_call = new FakeCall(webrtc::Call::Config(&event_log_)); + FakeCall* fake_call = new FakeCall(); call_.reset(fake_call); std::unique_ptr channel(SetUpForExternalEncoderFactory()); @@ -1083,8 +1085,7 @@ TEST_F(WebRtcVideoEngineTest, StreamParamsIdPassedToDecoderFactory) { TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - std::unique_ptr fake_call( - new FakeCall(webrtc::Call::Config(&event_log_))); + std::unique_ptr fake_call(new FakeCall()); std::unique_ptr channel(SetUpForExternalEncoderFactory()); ASSERT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -1231,7 +1232,7 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { explicit WebRtcVideoChannelTest(const char* field_trials) : WebRtcVideoEngineTest(field_trials), last_ssrc_(0) {} void SetUp() override { - fake_call_.reset(new FakeCall(webrtc::Call::Config(&event_log_))); + fake_call_.reset(new FakeCall()); channel_.reset(engine_.CreateChannel(fake_call_.get(), GetMediaConfig(), VideoOptions())); channel_->OnReadyToSend(true); @@ -1278,6 +1279,9 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { int expected_start_bitrate_bps, const char* max_bitrate_kbps, int expected_max_bitrate_bps) { + ExpectSetBitrateParameters(expected_min_bitrate_bps, + expected_start_bitrate_bps, + expected_max_bitrate_bps); auto& codecs = send_parameters_.codecs; codecs.clear(); codecs.push_back(GetEngineCodec("VP8")); @@ -1285,13 +1289,23 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { codecs[0].params[kCodecParamStartBitrate] = start_bitrate_kbps; codecs[0].params[kCodecParamMaxBitrate] = max_bitrate_kbps; EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); + } - EXPECT_EQ(expected_min_bitrate_bps, - fake_call_->GetConfig().bitrate_config.min_bitrate_bps); - EXPECT_EQ(expected_start_bitrate_bps, - fake_call_->GetConfig().bitrate_config.start_bitrate_bps); - EXPECT_EQ(expected_max_bitrate_bps, - fake_call_->GetConfig().bitrate_config.max_bitrate_bps); + void ExpectSetBitrateParameters(int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) { + EXPECT_CALL( + *fake_call_->GetMockTransportControllerSend(), + SetSdpBitrateParameters(AllOf( + Field(&BitrateConstraints::min_bitrate_bps, min_bitrate_bps), + Field(&BitrateConstraints::start_bitrate_bps, start_bitrate_bps), + Field(&BitrateConstraints::max_bitrate_bps, max_bitrate_bps)))); + } + + void ExpectSetMaxBitrate(int max_bitrate_bps) { + EXPECT_CALL(*fake_call_->GetMockTransportControllerSend(), + SetSdpBitrateParameters(Field( + &BitrateConstraints::max_bitrate_bps, max_bitrate_bps))); } void TestSetSendRtpHeaderExtensions(const std::string& ext_uri) { @@ -3117,8 +3131,7 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithHighMaxBitrate) { TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithoutBitratesUsesCorrectDefaults) { - SetSendCodecsShouldWorkForBitrates( - "", 0, "", -1, "", -1); + SetSendCodecsShouldWorkForBitrates("", 0, "", -1, "", -1); } TEST_F(WebRtcVideoChannelTest, SetSendCodecsCapsMinAndStartBitrate) { @@ -3138,34 +3151,26 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithBitratesAndMaxSendBandwidth) { send_parameters_.codecs[0].params[kCodecParamStartBitrate] = "200"; send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "300"; send_parameters_.max_bandwidth_bps = 400000; - EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps); - EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps); // We expect max_bandwidth_bps to take priority, if set. - EXPECT_EQ(400000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); + ExpectSetBitrateParameters(100000, 200000, 400000); + EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); + // Since the codec isn't changing, start_bitrate_bps should be -1. + ExpectSetBitrateParameters(100000, -1, 350000); // Decrease max_bandwidth_bps. send_parameters_.max_bandwidth_bps = 350000; EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps); - // Since the codec isn't changing, start_bitrate_bps should be -1. - EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.start_bitrate_bps); - EXPECT_EQ(350000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); // Now try again with the values flipped around. send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "400"; send_parameters_.max_bandwidth_bps = 300000; + ExpectSetBitrateParameters(100000, 200000, 300000); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps); - EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps); - EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); // If we change the codec max, max_bandwidth_bps should still apply. send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "350"; + ExpectSetBitrateParameters(100000, 200000, 300000); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps); - EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps); - EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); } TEST_F(WebRtcVideoChannelTest, @@ -3173,39 +3178,34 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", 200000); send_parameters_.max_bandwidth_bps = 300000; + // Setting max bitrate should keep previous min bitrate. + // Setting max bitrate should not reset start bitrate. + ExpectSetBitrateParameters(100000, -1, 300000); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps) - << "Setting max bitrate should keep previous min bitrate."; - EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.start_bitrate_bps) - << "Setting max bitrate should not reset start bitrate."; - EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); } TEST_F(WebRtcVideoChannelTest, SetMaxSendBandwidthShouldBeRemovable) { send_parameters_.max_bandwidth_bps = 300000; + ExpectSetMaxBitrate(300000); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); // -1 means to disable max bitrate (set infinite). send_parameters_.max_bandwidth_bps = -1; + ExpectSetMaxBitrate(-1); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.max_bitrate_bps) - << "Setting zero max bitrate did not reset start bitrate."; } TEST_F(WebRtcVideoChannelTest, SetMaxSendBandwidthAndAddSendStream) { send_parameters_.max_bandwidth_bps = 99999; FakeVideoSendStream* stream = AddSendStream(); + ExpectSetMaxBitrate(send_parameters_.max_bandwidth_bps); ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(send_parameters_.max_bandwidth_bps, - fake_call_->GetConfig().bitrate_config.max_bitrate_bps); ASSERT_EQ(1u, stream->GetVideoStreams().size()); EXPECT_EQ(send_parameters_.max_bandwidth_bps, stream->GetVideoStreams()[0].max_bitrate_bps); send_parameters_.max_bandwidth_bps = 77777; + ExpectSetMaxBitrate(send_parameters_.max_bandwidth_bps); ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); - EXPECT_EQ(send_parameters_.max_bandwidth_bps, - fake_call_->GetConfig().bitrate_config.max_bitrate_bps); EXPECT_EQ(send_parameters_.max_bandwidth_bps, stream->GetVideoStreams()[0].max_bitrate_bps); } @@ -4902,7 +4902,7 @@ TEST_F(WebRtcVideoChannelTest, ConfiguresLocalSsrcOnExistingReceivers) { class WebRtcVideoChannelSimulcastTest : public testing::Test { public: WebRtcVideoChannelSimulcastTest() - : fake_call_(webrtc::Call::Config(&event_log_)), + : fake_call_(), encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory), decoder_factory_(new cricket::FakeWebRtcVideoDecoderFactory), engine_(std::unique_ptr( diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index 2b18ed7a6d..b1fb14d64a 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -1665,7 +1665,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( // "unchanged" so that BWE isn't affected. bitrate_config.start_bitrate_bps = -1; } - call_->SetBitrateConfig(bitrate_config); + call_->GetTransportControllerSend()->SetSdpBitrateParameters(bitrate_config); // Check if the transport cc feedback or NACK status has changed on the // preferred send codec, and in that case reconfigure all receive streams. @@ -2040,8 +2040,8 @@ void WebRtcVoiceMediaChannel::OnNetworkRouteChanged( const std::string& transport_name, const rtc::NetworkRoute& network_route) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - // TODO(zhihaung): Merge these two callbacks. - call_->OnNetworkRouteChanged(transport_name, network_route); + call_->GetTransportControllerSend()->OnNetworkRouteChanged(transport_name, + network_route); call_->OnTransportOverheadChanged(webrtc::MediaType::AUDIO, network_route.packet_overhead); } diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 31a092caf0..d71d77d8a8 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -36,12 +36,14 @@ using testing::_; using testing::ContainerEq; +using testing::Field; using testing::Return; using testing::ReturnPointee; using testing::SaveArg; using testing::StrictMock; namespace { +using webrtc::BitrateConstraints; constexpr uint32_t kMaxUnsignaledRecvStreams = 4; @@ -160,7 +162,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { apm_ec_(*apm_->echo_cancellation()), apm_ns_(*apm_->noise_suppression()), apm_vd_(*apm_->voice_detection()), - call_(webrtc::Call::Config(&event_log_)), + call_(), override_field_trials_(field_trials) { // AudioDeviceModule. AdmSetupExpectations(&adm_); @@ -425,14 +427,16 @@ class WebRtcVoiceEngineTestFake : public testing::Test { codecs[0].params[cricket::kCodecParamMinBitrate] = min_bitrate_kbps; codecs[0].params[cricket::kCodecParamStartBitrate] = start_bitrate_kbps; codecs[0].params[cricket::kCodecParamMaxBitrate] = max_bitrate_kbps; - SetSendParameters(send_parameters_); + EXPECT_CALL(*call_.GetMockTransportControllerSend(), + SetSdpBitrateParameters( + AllOf(Field(&BitrateConstraints::min_bitrate_bps, + expected_min_bitrate_bps), + Field(&BitrateConstraints::start_bitrate_bps, + expected_start_bitrate_bps), + Field(&BitrateConstraints::max_bitrate_bps, + expected_max_bitrate_bps)))); - EXPECT_EQ(expected_min_bitrate_bps, - call_.GetConfig().bitrate_config.min_bitrate_bps); - EXPECT_EQ(expected_start_bitrate_bps, - call_.GetConfig().bitrate_config.start_bitrate_bps); - EXPECT_EQ(expected_max_bitrate_bps, - call_.GetConfig().bitrate_config.max_bitrate_bps); + SetSendParameters(send_parameters_); } void TestSetSendRtpHeaderExtensions(const std::string& ext) { @@ -698,7 +702,6 @@ class WebRtcVoiceEngineTestFake : public testing::Test { webrtc::test::MockEchoCancellation& apm_ec_; webrtc::test::MockNoiseSuppression& apm_ns_; webrtc::test::MockVoiceDetection& apm_vd_; - webrtc::RtcEventLogNullImpl event_log_; cricket::FakeCall call_; std::unique_ptr engine_; cricket::VoiceMediaChannel* channel_ = nullptr; @@ -1531,12 +1534,14 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", 200000); send_parameters_.max_bandwidth_bps = 100000; + // Setting max bitrate should keep previous min bitrate + // Setting max bitrate should not reset start bitrate. + EXPECT_CALL(*call_.GetMockTransportControllerSend(), + SetSdpBitrateParameters( + AllOf(Field(&BitrateConstraints::min_bitrate_bps, 100000), + Field(&BitrateConstraints::start_bitrate_bps, -1), + Field(&BitrateConstraints::max_bitrate_bps, 200000)))); SetSendParameters(send_parameters_); - EXPECT_EQ(100000, call_.GetConfig().bitrate_config.min_bitrate_bps) - << "Setting max bitrate should keep previous min bitrate."; - EXPECT_EQ(-1, call_.GetConfig().bitrate_config.start_bitrate_bps) - << "Setting max bitrate should not reset start bitrate."; - EXPECT_EQ(200000, call_.GetConfig().bitrate_config.max_bitrate_bps); } // Test that we can enable NACK with opus as caller. diff --git a/pc/channelmanager_unittest.cc b/pc/channelmanager_unittest.cc index 18f84752f7..d318ac5b20 100644 --- a/pc/channelmanager_unittest.cc +++ b/pc/channelmanager_unittest.cc @@ -11,7 +11,6 @@ #include #include -#include "logging/rtc_event_log/rtc_event_log.h" #include "media/base/fakemediaengine.h" #include "media/base/fakevideocapturer.h" #include "media/base/testutils.h" @@ -48,14 +47,13 @@ class ChannelManagerTest : public testing::Test { std::unique_ptr(fdme_), rtc::Thread::Current(), rtc::Thread::Current())), - fake_call_(webrtc::Call::Config(&event_log_)), + fake_call_(), transport_controller_( new cricket::FakeTransportController(ICEROLE_CONTROLLING)) { fme_->SetAudioCodecs(MAKE_VECTOR(kAudioCodecs)); fme_->SetVideoCodecs(MAKE_VECTOR(kVideoCodecs)); } - webrtc::RtcEventLogNullImpl event_log_; std::unique_ptr network_; std::unique_ptr worker_; // |fme_| and |fdme_| are actually owned by |cm_|. diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 14cff94183..3b2f71c64a 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2884,7 +2884,7 @@ RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) { mask.max_bitrate_bps = bitrate.max_bitrate_bps; RTC_DCHECK(call_.get()); - call_->SetBitrateConfigMask(mask); + call_->GetTransportControllerSend()->SetClientBitratePreferences(mask); return RTCError::OK(); } diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 4223264599..a09791c292 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -64,7 +64,7 @@ class RtpSenderReceiverTest : public testing::Test, rtc::MakeUnique(), worker_thread_, network_thread_), - fake_call_(Call::Config(&event_log_)), + fake_call_(), local_stream_(MediaStream::Create(kStreamLabel1)) { // Create channels to be used by the RtpSenders and RtpReceivers. channel_manager_.Init(); diff --git a/video/end_to_end_tests/probing_tests.cc b/video/end_to_end_tests/probing_tests.cc index 209ffc94bc..4069a68223 100644 --- a/video/end_to_end_tests/probing_tests.cc +++ b/video/end_to_end_tests/probing_tests.cc @@ -140,7 +140,8 @@ TEST_P(ProbingEndToEndTest, TriggerMidCallProbing) { BitrateConstraints bitrate_config; bitrate_config.max_bitrate_bps = 100000; task_queue_->SendTask([this, &bitrate_config]() { - sender_call_->SetBitrateConfig(bitrate_config); + sender_call_->GetTransportControllerSend() + ->SetSdpBitrateParameters(bitrate_config); }); ++state_; } @@ -150,7 +151,8 @@ TEST_P(ProbingEndToEndTest, TriggerMidCallProbing) { BitrateConstraints bitrate_config; bitrate_config.max_bitrate_bps = 2500000; task_queue_->SendTask([this, &bitrate_config]() { - sender_call_->SetBitrateConfig(bitrate_config); + sender_call_->GetTransportControllerSend() + ->SetSdpBitrateParameters(bitrate_config); }); ++state_; } diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 193505f676..043264488c 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -1593,9 +1593,11 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) { BitrateConstraints bitrate_config; task_queue_->SendTask([this, &new_route, &bitrate_config]() { - call_->OnNetworkRouteChanged("transport", new_route); + call_->GetTransportControllerSend()->OnNetworkRouteChanged("transport", + new_route); bitrate_config.start_bitrate_bps = kStartBitrateBps; - call_->SetBitrateConfig(bitrate_config); + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); }); EXPECT_TRUE(Wait()) @@ -1604,11 +1606,13 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) { task_queue_->SendTask([this, &new_route, &bitrate_config]() { bitrate_config.start_bitrate_bps = -1; bitrate_config.max_bitrate_bps = kNewMaxBitrateBps; - call_->SetBitrateConfig(bitrate_config); + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); // TODO(holmer): We should set the last sent packet id here and verify // that we correctly ignore any packet loss reported prior to that id. ++new_route.local_network_id; - call_->OnNetworkRouteChanged("transport", new_route); + call_->GetTransportControllerSend()->OnNetworkRouteChanged("transport", + new_route); EXPECT_GE(call_->GetStats().send_bandwidth_bps, kStartBitrateBps); }); } @@ -1957,7 +1961,8 @@ TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) { BitrateConstraints bitrate_config; bitrate_config.start_bitrate_bps = 2 * video_encoder_config_.max_bitrate_bps; - sender_call_->SetBitrateConfig(bitrate_config); + sender_call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); StartBitrateObserver encoder; video_send_config_.encoder_settings.encoder = &encoder; @@ -2868,7 +2873,8 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { bitrate_config.start_bitrate_bps = kIncreasedStartBitrateKbps * 1000; bitrate_config.max_bitrate_bps = kIncreasedMaxBitrateKbps * 1000; task_queue_->SendTask([this, &bitrate_config]() { - call_->SetBitrateConfig(bitrate_config); + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); }); // Encoder rate is capped by EncoderConfig max_bitrate_bps. WaitForSetRates(kMaxBitrateKbps); @@ -2887,7 +2893,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_EQ(3, num_initializations_) << "Encoder should have been reconfigured with the new value."; // Expected target bitrate is the start bitrate set in the call to - // call_->SetBitrateConfig. + // call_->GetTransportControllerSend()->SetSdpBitrateParameters. WaitForSetRates(kIncreasedStartBitrateKbps); } @@ -3568,7 +3574,8 @@ TEST_F(VideoSendStreamTest, RemoveOverheadFromBandwidth) { bitrate_config.max_bitrate_bps = kMaxBitrateBps; bitrate_config.min_bitrate_bps = kMinBitrateBps; task_queue_->SendTask([this, &bitrate_config]() { - call_->SetBitrateConfig(bitrate_config); + call_->GetTransportControllerSend()->SetSdpBitrateParameters( + bitrate_config); call_->OnTransportOverheadChanged(webrtc::MediaType::VIDEO, 40); });