diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 5f869f8231..805a153c88 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1346,10 +1346,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream return false; } + const rtc::Optional old_rtp_max_bitrate = + rtp_parameters_.encodings[0].max_bitrate_bps; + rtp_parameters_ = parameters; - // parameters.encodings[0].encodings[0].max_bitrate_bps could have changed. - if (config_.send_codec_spec.codec_inst.rate != *send_rate) { + if (rtp_parameters_.encodings[0].max_bitrate_bps != old_rtp_max_bitrate) { // Recreate AudioSendStream with new bit rate. config_.send_codec_spec.codec_inst.rate = *send_rate; RecreateAudioSendStream(); @@ -1381,7 +1383,15 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream RTC_DCHECK(!stream_); if (webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe")) { config_.min_bitrate_bps = kOpusMinBitrateBps; - config_.max_bitrate_bps = kOpusBitrateFbBps; + + // This means that when RtpParameters is reset, we may change the + // encoder's bit rate immediately (through call_->CreateAudioSendStream), + // meanwhile change the cap to the output of BWE. + config_.max_bitrate_bps = + rtp_parameters_.encodings[0].max_bitrate_bps + ? *rtp_parameters_.encodings[0].max_bitrate_bps + : kOpusBitrateFbBps; + // TODO(mflodman): Keep testing this and set proper values. // Note: This is an early experiment currently only supported by Opus. if (send_side_bwe_with_overhead_) { @@ -1390,8 +1400,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream if (!packet_sizes_ms.empty()) { int max_packet_size_ms = *std::max_element(packet_sizes_ms.begin(), packet_sizes_ms.end()); - int min_packet_size_ms = - *std::min_element(packet_sizes_ms.begin(), packet_sizes_ms.end()); // Audio network adaptor will just use 20ms and 60ms frame lengths. // The adaptor will only be active for the Opus encoder. @@ -1402,7 +1410,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream #else max_packet_size_ms = 60; #endif - min_packet_size_ms = 20; } // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) @@ -1411,11 +1418,19 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream int min_overhead_bps = kOverheadPerPacket * 8 * 1000 / max_packet_size_ms; - int max_overhead_bps = - kOverheadPerPacket * 8 * 1000 / min_packet_size_ms; + // We assume that |config_.max_bitrate_bps| before the next line is + // a hard limit on the payload bitrate, so we add min_overhead_bps to + // it to ensure that, when overhead is deducted, the payload rate + // never goes beyond the limit. + // Note: this also means that if a higher overhead is forced, we + // cannot reach the limit. + // TODO(minyue): Reconsider this when the signaling to BWE is done + // through a dedicated API. + config_.max_bitrate_bps += min_overhead_bps; - config_.min_bitrate_bps = kOpusMinBitrateBps + min_overhead_bps; - config_.max_bitrate_bps = kOpusBitrateFbBps + max_overhead_bps; + // In contrast to max_bitrate_bps, we let min_bitrate_bps always be + // reachable. + config_.min_bitrate_bps += min_overhead_bps; } } } diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 44af1fb2c6..b57f4ee568 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -1172,6 +1172,29 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAndGetRtpSendParameters) { EXPECT_EQ(initial_params, channel_->GetRtpSendParameters(kSsrcX)); } +// Test that max_bitrate_bps in send stream config gets updated correctly when +// SetRtpSendParameters is called. +TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterUpdatesMaxBitrate) { + webrtc::test::ScopedFieldTrials override_field_trials( + "WebRTC-Audio-SendSideBwe/Enabled/"); + EXPECT_TRUE(SetupSendStream()); + cricket::AudioSendParameters send_parameters; + send_parameters.codecs.push_back(kOpusCodec); + SetSendParameters(send_parameters); + + webrtc::RtpParameters rtp_parameters = channel_->GetRtpSendParameters(kSsrcX); + // Expect empty on parameters.encodings[0].max_bitrate_bps; + EXPECT_FALSE(rtp_parameters.encodings[0].max_bitrate_bps); + + constexpr int kMaxBitrateBps = 6000; + rtp_parameters.encodings[0].max_bitrate_bps = + rtc::Optional(kMaxBitrateBps); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + + const int max_bitrate = GetSendStreamConfig(kSsrcX).max_bitrate_bps; + EXPECT_EQ(max_bitrate, kMaxBitrateBps); +} + // Test that GetRtpReceiveParameters returns the currently configured codecs. TEST_F(WebRtcVoiceEngineTestFake, GetRtpReceiveParametersCodecs) { EXPECT_TRUE(SetupRecvStream()); @@ -2585,13 +2608,12 @@ TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest, MinAndMaxBitrate) { constexpr int kOpusMaxPtimeMs = WEBRTC_OPUS_SUPPORT_120MS_PTIME ? 120 : 60; constexpr int kMinOverheadBps = kOverheadPerPacket * 8 * 1000 / kOpusMaxPtimeMs; - constexpr int kMaxOverheadBps = kOverheadPerPacket * 8 * 1000 / 10; constexpr int kOpusMinBitrateBps = 6000; EXPECT_EQ(kOpusMinBitrateBps + kMinOverheadBps, GetSendStreamConfig(kSsrcX).min_bitrate_bps); constexpr int kOpusBitrateFbBps = 32000; - EXPECT_EQ(kOpusBitrateFbBps + kMaxOverheadBps, + EXPECT_EQ(kOpusBitrateFbBps + kMinOverheadBps, GetSendStreamConfig(kSsrcX).max_bitrate_bps); parameters.options.audio_network_adaptor = rtc::Optional(true); @@ -2601,15 +2623,42 @@ TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest, MinAndMaxBitrate) { constexpr int kMinOverheadWithAnaBps = kOverheadPerPacket * 8 * 1000 / kOpusMaxPtimeMs; - constexpr int kMaxOverheadWithAnaBps = kOverheadPerPacket * 8 * 1000 / 20; EXPECT_EQ(kOpusMinBitrateBps + kMinOverheadWithAnaBps, GetSendStreamConfig(kSsrcX).min_bitrate_bps); - EXPECT_EQ(kOpusBitrateFbBps + kMaxOverheadWithAnaBps, + EXPECT_EQ(kOpusBitrateFbBps + kMinOverheadWithAnaBps, GetSendStreamConfig(kSsrcX).max_bitrate_bps); } +// This test is similar to +// WebRtcVoiceEngineTestFake.SetRtpSendParameterUpdatesMaxBitrate but with an +// additional field trial. +TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest, + SetRtpSendParameterUpdatesMaxBitrate) { + EXPECT_TRUE(SetupSendStream()); + cricket::AudioSendParameters send_parameters; + send_parameters.codecs.push_back(kOpusCodec); + SetSendParameters(send_parameters); + + webrtc::RtpParameters rtp_parameters = channel_->GetRtpSendParameters(kSsrcX); + // Expect empty on parameters.encodings[0].max_bitrate_bps; + EXPECT_FALSE(rtp_parameters.encodings[0].max_bitrate_bps); + + constexpr int kMaxBitrateBps = 6000; + rtp_parameters.encodings[0].max_bitrate_bps = + rtc::Optional(kMaxBitrateBps); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + + const int max_bitrate = GetSendStreamConfig(kSsrcX).max_bitrate_bps; +#if WEBRTC_OPUS_SUPPORT_120MS_PTIME + constexpr int kMinOverhead = 3333; +#else + constexpr int kMinOverhead = 6666; +#endif + EXPECT_EQ(max_bitrate, kMaxBitrateBps + kMinOverhead); +} + // Test that we can set the outgoing SSRC properly. // SSRC is set in SetupSendStream() by calling AddSendStream. TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrc) {