From ff05c5c805e2a7652507c3f610c66e4676268933 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Thu, 3 Mar 2022 12:09:16 +0100 Subject: [PATCH] audio/red: cleanup killswitch this has been enable by default since M96 BUG=webrtc:11640 Change-Id: I5d310d3929882007211eae12bc3ac1366107ca87 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253400 Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#36123} --- media/engine/webrtc_voice_engine.cc | 37 +++++++------------- media/engine/webrtc_voice_engine.h | 5 --- media/engine/webrtc_voice_engine_unittest.cc | 12 ------- 3 files changed, 13 insertions(+), 41 deletions(-) diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 829cb82af1..7f01b95642 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -210,11 +210,6 @@ bool IsEnabled(const webrtc::WebRtcKeyValueConfig& config, return absl::StartsWith(config.Lookup(trial), "Enabled"); } -bool IsDisabled(const webrtc::WebRtcKeyValueConfig& config, - absl::string_view trial) { - return absl::StartsWith(config.Lookup(trial), "Disabled"); -} - struct AdaptivePtimeConfig { bool enabled = false; webrtc::DataRate min_payload_bitrate = webrtc::DataRate::KilobitsPerSec(16); @@ -311,8 +306,6 @@ WebRtcVoiceEngine::WebRtcVoiceEngine( audio_mixer_(audio_mixer), apm_(audio_processing), audio_frame_processor_(audio_frame_processor), - audio_red_for_opus_enabled_( - !IsDisabled(trials, "WebRTC-Audio-Red-For-Opus")), minimized_remsampling_on_mobile_trial_enabled_( IsEnabled(trials, "WebRTC-Audio-MinimizeResamplingOnMobile")) { // This may be called from any thread, so detach thread checkers. @@ -738,7 +731,7 @@ std::vector WebRtcVoiceEngine::CollectCodecs( out.push_back(codec); - if (codec.name == kOpusCodecName && audio_red_for_opus_enabled_) { + if (codec.name == kOpusCodecName) { std::string redFmtp = rtc::ToString(codec.id) + "/" + rtc::ToString(codec.id); map_format({kRedCodecName, 48000, 2, {{"", redFmtp}}}, &out); @@ -1311,9 +1304,7 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel( engine_(engine), call_(call), audio_config_(config.audio), - crypto_options_(crypto_options), - audio_red_for_opus_enabled_( - !IsDisabled(call->trials(), "WebRTC-Audio-Red-For-Opus")) { + crypto_options_(crypto_options) { network_thread_checker_.Detach(); RTC_LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel"; RTC_DCHECK(call); @@ -1574,7 +1565,7 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs( } auto format = AudioCodecToSdpAudioFormat(codec); if (!IsCodec(codec, kCnCodecName) && !IsCodec(codec, kDtmfCodecName) && - (!audio_red_for_opus_enabled_ || !IsCodec(codec, kRedCodecName)) && + !IsCodec(codec, kRedCodecName) && !engine()->decoder_factory_->IsSupportedDecoder(format)) { RTC_LOG(LS_ERROR) << "Unsupported codec: " << rtc::ToString(format); return false; @@ -1760,19 +1751,17 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } } - if (audio_red_for_opus_enabled_) { - // Loop through the codecs to find the RED codec that matches opus - // with respect to clockrate and number of channels. - size_t red_codec_position = 0; - for (const AudioCodec& red_codec : codecs) { - if (red_codec_position < send_codec_position && - IsCodec(red_codec, kRedCodecName) && - CheckRedParameters(red_codec, *send_codec_spec)) { - send_codec_spec->red_payload_type = red_codec.id; - break; - } - red_codec_position++; + // Loop through the codecs to find the RED codec that matches opus + // with respect to clockrate and number of channels. + size_t red_codec_position = 0; + for (const AudioCodec& red_codec : codecs) { + if (red_codec_position < send_codec_position && + IsCodec(red_codec, kRedCodecName) && + CheckRedParameters(red_codec, *send_codec_spec)) { + send_codec_spec->red_payload_type = red_codec.id; + break; } + red_codec_position++; } if (send_codec_spec_ != send_codec_spec) { diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 1061d7a129..1a0b54472e 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -130,9 +130,6 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { int audio_jitter_buffer_min_delay_ms_ = 0; bool audio_jitter_buffer_enable_rtx_handling_ = false; - // If this field is enabled, we will negotiate and use RFC 2198 - // redundancy for opus audio. - const bool audio_red_for_opus_enabled_; const bool minimized_remsampling_on_mobile_trial_enabled_; }; @@ -325,8 +322,6 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, // Unsignaled streams have an option to have a frame decryptor set on them. rtc::scoped_refptr unsignaled_frame_decryptor_; - - const bool audio_red_for_opus_enabled_; }; } // namespace cricket diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index ad15a638bb..a9f4f22cdc 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -1035,18 +1035,6 @@ TEST_P(WebRtcVoiceEngineTestFake, RecvRedDefault) { {112, {"red", 48000, 2, {{"", "111/111"}}}}}))); } -// Test that we disable Opus/Red with the kill switch. -TEST_P(WebRtcVoiceEngineTestFake, RecvRed) { - webrtc::test::ScopedFieldTrials override_field_trials( - "WebRTC-Audio-Red-For-Opus/Disabled/"); - - EXPECT_TRUE(SetupRecvStream()); - cricket::AudioRecvParameters parameters; - parameters.codecs.push_back(kOpusCodec); - parameters.codecs.push_back(kRed48000Codec); - EXPECT_FALSE(channel_->SetRecvParameters(parameters)); -} - TEST_P(WebRtcVoiceEngineTestFake, SetSendBandwidthAuto) { EXPECT_TRUE(SetupSendStream());