diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 95f4d39d8a..0cf17ed984 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -775,9 +775,7 @@ std::vector WebRtcVoiceEngine::CollectCodecs( out.push_back(codec); if (codec.name == kOpusCodecName && audio_red_for_opus_enabled_) { - std::string redFmtp = - rtc::ToString(codec.id) + "/" + rtc::ToString(codec.id); - map_format({kRedCodecName, 48000, 2, {{"", redFmtp}}}, &out); + map_format({kRedCodecName, 48000, 2}, &out); } } } @@ -1656,37 +1654,6 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs( return true; } -// Utility function to check if RED codec and its parameters match a codec spec. -bool CheckRedParameters( - const AudioCodec& red_codec, - const webrtc::AudioSendStream::Config::SendCodecSpec& send_codec_spec) { - if (red_codec.clockrate != send_codec_spec.format.clockrate_hz || - red_codec.channels != send_codec_spec.format.num_channels) { - return false; - } - - // Check the FMTP line for the empty parameter which should match - // /[/...] - auto red_parameters = red_codec.params.find(""); - if (red_parameters == red_codec.params.end()) { - RTC_LOG(LS_WARNING) << "audio/RED missing fmtp parameters."; - return false; - } - std::vector redundant_payloads; - rtc::split(red_parameters->second, '/', &redundant_payloads); - // 32 is chosen as a maximum upper bound for consistency with the - // red payload splitter. - if (redundant_payloads.size() < 2 || redundant_payloads.size() > 32) { - return false; - } - for (auto pt : redundant_payloads) { - if (pt != rtc::ToString(send_codec_spec.payload_type)) { - return false; - } - } - return true; -} - // Utility function called from SetSendParameters() to extract current send // codec settings from the given list of codecs (originally from SDP). Both send // and receive streams may be reconfigured based on the new settings. @@ -1797,7 +1764,8 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( for (const AudioCodec& red_codec : codecs) { if (red_codec_position < send_codec_position && IsCodec(red_codec, kRedCodecName) && - CheckRedParameters(red_codec, *send_codec_spec)) { + red_codec.clockrate == send_codec_spec->format.clockrate_hz && + red_codec.channels == send_codec_spec->format.num_channels) { send_codec_spec->red_payload_type = red_codec.id; break; } diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 9f031ed4ef..4b127af234 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -1028,12 +1028,10 @@ TEST_P(WebRtcVoiceEngineTestFake, RecvRedDefault) { cricket::AudioRecvParameters parameters; parameters.codecs.push_back(kOpusCodec); parameters.codecs.push_back(kRed48000Codec); - parameters.codecs[1].params[""] = "111/111"; EXPECT_TRUE(channel_->SetRecvParameters(parameters)); EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map, (ContainerEq>( - {{111, {"opus", 48000, 2}}, - {112, {"red", 48000, 2, {{"", "111/111"}}}}}))); + {{111, {"opus", 48000, 2}}, {112, {"red", 48000, 2}}}))); } // Test that we disable Opus/Red with the kill switch. @@ -1497,13 +1495,15 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecs) { EXPECT_FALSE(channel_->CanInsertDtmf()); } -// Test that we use Opus/Red by default when it is -// listed as the first codec and there is an fmtp line. +// Test that we use Opus/Red under the field trial when it is +// listed as the first codec. TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRed) { + webrtc::test::ScopedFieldTrials override_field_trials( + "WebRTC-Audio-Red-For-Opus/Enabled/"); + EXPECT_TRUE(SetupSendStream()); cricket::AudioSendParameters parameters; parameters.codecs.push_back(kRed48000Codec); - parameters.codecs[0].params[""] = "111/111"; parameters.codecs.push_back(kOpusCodec); SetSendParameters(parameters); const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; @@ -1512,41 +1512,15 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRed) { EXPECT_EQ(112, send_codec_spec.red_payload_type); } -// Test that we do not use Opus/Red by default when it is -// listed as the first codec but there is no fmtp line. -TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedNoFmtp) { - EXPECT_TRUE(SetupSendStream()); - cricket::AudioSendParameters parameters; - parameters.codecs.push_back(kRed48000Codec); - parameters.codecs.push_back(kOpusCodec); - SetSendParameters(parameters); - const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; - EXPECT_EQ(111, send_codec_spec.payload_type); - EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str()); - EXPECT_EQ(absl::nullopt, send_codec_spec.red_payload_type); -} - -// Test that we do not use Opus/Red by default. +// Test that we do not use Opus/Red under the field trial by default. TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedDefault) { - EXPECT_TRUE(SetupSendStream()); - cricket::AudioSendParameters parameters; - parameters.codecs.push_back(kOpusCodec); - parameters.codecs.push_back(kRed48000Codec); - parameters.codecs[1].params[""] = "111/111"; - SetSendParameters(parameters); - const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; - EXPECT_EQ(111, send_codec_spec.payload_type); - EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str()); - EXPECT_EQ(absl::nullopt, send_codec_spec.red_payload_type); -} + webrtc::test::ScopedFieldTrials override_field_trials( + "WebRTC-Audio-Red-For-Opus/Enabled/"); -// Test that the RED fmtp line must match the payload type. -TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedFmtpMismatch) { EXPECT_TRUE(SetupSendStream()); cricket::AudioSendParameters parameters; - parameters.codecs.push_back(kRed48000Codec); - parameters.codecs[0].params[""] = "8/8"; parameters.codecs.push_back(kOpusCodec); + parameters.codecs.push_back(kRed48000Codec); SetSendParameters(parameters); const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; EXPECT_EQ(111, send_codec_spec.payload_type); @@ -1554,34 +1528,6 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedFmtpMismatch) { EXPECT_EQ(absl::nullopt, send_codec_spec.red_payload_type); } -// Test that the RED fmtp line must show 2..32 payloads. -TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedFmtpAmountOfRedundancy) { - EXPECT_TRUE(SetupSendStream()); - cricket::AudioSendParameters parameters; - parameters.codecs.push_back(kRed48000Codec); - parameters.codecs[0].params[""] = "111"; - parameters.codecs.push_back(kOpusCodec); - SetSendParameters(parameters); - const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; - EXPECT_EQ(111, send_codec_spec.payload_type); - EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str()); - EXPECT_EQ(absl::nullopt, send_codec_spec.red_payload_type); - for (int i = 1; i < 32; i++) { - parameters.codecs[0].params[""] += "/111"; - SetSendParameters(parameters); - const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; - EXPECT_EQ(111, send_codec_spec.payload_type); - EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str()); - EXPECT_EQ(112, send_codec_spec.red_payload_type); - } - parameters.codecs[0].params[""] += "/111"; - SetSendParameters(parameters); - const auto& send_codec_spec2 = *GetSendStreamConfig(kSsrcX).send_codec_spec; - EXPECT_EQ(111, send_codec_spec2.payload_type); - EXPECT_STRCASEEQ("opus", send_codec_spec2.format.name.c_str()); - EXPECT_EQ(absl::nullopt, send_codec_spec2.red_payload_type); -} - // Test that WebRtcVoiceEngine reconfigures, rather than recreates its // AudioSendStream. TEST_P(WebRtcVoiceEngineTestFake, DontRecreateSendStream) {