diff --git a/webrtc/media/engine/webrtcmediaengine.cc b/webrtc/media/engine/webrtcmediaengine.cc index 51a110b88b..1531be36b5 100644 --- a/webrtc/media/engine/webrtcmediaengine.cc +++ b/webrtc/media/engine/webrtcmediaengine.cc @@ -168,4 +168,30 @@ std::vector FilterRtpExtensions( return result; } + +webrtc::Call::Config::BitrateConfig GetBitrateConfigForCodec( + const Codec& codec) { + webrtc::Call::Config::BitrateConfig config; + int bitrate_kbps = 0; + if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) && + bitrate_kbps > 0) { + config.min_bitrate_bps = bitrate_kbps * 1000; + } else { + config.min_bitrate_bps = 0; + } + if (codec.GetParam(kCodecParamStartBitrate, &bitrate_kbps) && + bitrate_kbps > 0) { + config.start_bitrate_bps = bitrate_kbps * 1000; + } else { + // Do not reconfigure start bitrate unless it's specified and positive. + config.start_bitrate_bps = -1; + } + if (codec.GetParam(kCodecParamMaxBitrate, &bitrate_kbps) && + bitrate_kbps > 0) { + config.max_bitrate_bps = bitrate_kbps * 1000; + } else { + config.max_bitrate_bps = -1; + } + return config; +} } // namespace cricket diff --git a/webrtc/media/engine/webrtcmediaengine.h b/webrtc/media/engine/webrtcmediaengine.h index dae63477d6..af85b2f99f 100644 --- a/webrtc/media/engine/webrtcmediaengine.h +++ b/webrtc/media/engine/webrtcmediaengine.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/call.h" #include "webrtc/config.h" #include "webrtc/media/base/mediaengine.h" @@ -58,6 +59,9 @@ std::vector FilterRtpExtensions( bool (*supported)(const std::string&), bool filter_redundant_extensions); +webrtc::Call::Config::BitrateConfig GetBitrateConfigForCodec( + const Codec& codec); + } // namespace cricket #endif // WEBRTC_MEDIA_ENGINE_WEBRTCMEDIAENGINE_H_ diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 3f9d5f6596..fc91ca6f0f 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -73,32 +73,6 @@ class EncoderFactoryAdapter : public webrtc::VideoEncoderFactory { cricket::WebRtcVideoEncoderFactory* const factory_; }; -webrtc::Call::Config::BitrateConfig GetBitrateConfigForCodec( - const VideoCodec& codec) { - webrtc::Call::Config::BitrateConfig config; - int bitrate_kbps; - if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) && - bitrate_kbps > 0) { - config.min_bitrate_bps = bitrate_kbps * 1000; - } else { - config.min_bitrate_bps = 0; - } - if (codec.GetParam(kCodecParamStartBitrate, &bitrate_kbps) && - bitrate_kbps > 0) { - config.start_bitrate_bps = bitrate_kbps * 1000; - } else { - // Do not reconfigure start bitrate unless it's specified and positive. - config.start_bitrate_bps = -1; - } - if (codec.GetParam(kCodecParamMaxBitrate, &bitrate_kbps) && - bitrate_kbps > 0) { - config.max_bitrate_bps = bitrate_kbps * 1000; - } else { - config.max_bitrate_bps = -1; - } - return config; -} - // An encoder factory that wraps Create requests for simulcastable codec types // with a webrtc::SimulcastEncoderAdapter. Non simulcastable codec type // requests are just passed through to the contained encoder factory. diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 96f9ed70d4..eec9340640 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1616,6 +1616,14 @@ bool WebRtcVoiceMediaChannel::SetSendParameters( return false; } + if (params.max_bandwidth_bps >= 0) { + // Note that max_bandwidth_bps intentionally takes priority over the + // bitrate config for the codec. + bitrate_config_.max_bitrate_bps = + params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps; + } + call_->SetBitrateConfig(bitrate_config_); + if (!ValidateRtpExtensions(params.extensions)) { return false; } @@ -1921,6 +1929,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( send_codec_spec.transport_cc_enabled = HasTransportCc(*codec); send_codec_spec.nack_enabled = HasNack(*codec); + bitrate_config_ = GetBitrateConfigForCodec(*codec); // For Opus as the send codec, we are to determine inband FEC, maximum // playback rate, and opus internal dtx. @@ -1987,12 +1996,16 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } } - // Apply new settings to all streams. if (send_codec_spec_ != send_codec_spec) { send_codec_spec_ = std::move(send_codec_spec); + // Apply new settings to all streams. for (const auto& kv : send_streams_) { kv.second->RecreateAudioSendStream(send_codec_spec_); } + } else { + // If the codec isn't changing, set the start bitrate to -1 which means + // "unchanged" so that BWE isn't affected. + bitrate_config_.start_bitrate_bps = -1; } // Check if the transport cc feedback or NACK status has changed on the diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index 03b493744b..3bed1a0317 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -259,6 +259,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool playout_ = false; bool send_ = false; webrtc::Call* const call_ = nullptr; + webrtc::Call::Config::BitrateConfig bitrate_config_; // SSRC of unsignalled receive stream, or -1 if there isn't one. int64_t default_recv_ssrc_ = -1; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 674924eda8..43142ea367 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -356,6 +356,29 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_EQ(expected_codec_bitrate, GetCodecBitrate(kSsrc1)); } + void SetSendCodecsShouldWorkForBitrates(const char* min_bitrate_kbps, + int expected_min_bitrate_bps, + const char* start_bitrate_kbps, + int expected_start_bitrate_bps, + const char* max_bitrate_kbps, + int expected_max_bitrate_bps) { + EXPECT_TRUE(SetupSendStream()); + auto& codecs = send_parameters_.codecs; + codecs.clear(); + codecs.push_back(kOpusCodec); + 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_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); + } + void TestSetSendRtpHeaderExtensions(const std::string& ext) { EXPECT_TRUE(SetupSendStream()); @@ -1402,6 +1425,37 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusMaxAverageBitrate) { EXPECT_EQ(200000, GetCodecBitrate(kSsrc1)); } +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsWithBitrates) { + SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", + 200000); +} + +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsWithHighMaxBitrate) { + SetSendCodecsShouldWorkForBitrates("", 0, "", -1, "10000", 10000000); +} + +TEST_F(WebRtcVoiceEngineTestFake, + SetSendCodecsWithoutBitratesUsesCorrectDefaults) { + SetSendCodecsShouldWorkForBitrates("", 0, "", -1, "", -1); +} + +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCapsMinAndStartBitrate) { + SetSendCodecsShouldWorkForBitrates("-1", 0, "-100", -1, "", -1); +} + +TEST_F(WebRtcVoiceEngineTestFake, + SetMaxSendBandwidthShouldPreserveOtherBitrates) { + SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", + 200000); + send_parameters_.max_bandwidth_bps = 300000; + 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(300000, call_.GetConfig().bitrate_config.max_bitrate_bps); +} + // Test that we can enable NACK with opus as caller. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackAsCaller) { EXPECT_TRUE(SetupSendStream());