red: only enable RED if its preferred as send codec
only enables RFC 2198 redundancy if it has a higher preference
than Opus. This means it not used by default but can be
chosen with setCodecPreferences.
BUG=webrtc:11640
Change-Id: I84ff2ca518da70440297a667dedba5cf4484eed7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178742
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31830}
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index 016c00f..590c31e 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -1719,6 +1719,7 @@
send_codec_spec;
webrtc::BitrateConstraints bitrate_config;
absl::optional<webrtc::AudioCodecInfo> voice_codec_info;
+ size_t send_codec_position = 0;
for (const AudioCodec& voice_codec : codecs) {
if (!(IsCodec(voice_codec, kCnCodecName) ||
IsCodec(voice_codec, kDtmfCodecName) ||
@@ -1742,6 +1743,7 @@
bitrate_config = GetBitrateConfigForCodec(voice_codec);
break;
}
+ send_codec_position++;
}
if (!send_codec_spec) {
@@ -1783,13 +1785,16 @@
if (IsAudioRedForOpusFieldTrialEnabled()) {
// 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 (IsCodec(red_codec, kRedCodecName) &&
+ if (red_codec_position < send_codec_position &&
+ IsCodec(red_codec, kRedCodecName) &&
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;
}
+ red_codec_position++;
}
}
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index d70019e..1bd631f 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -1507,33 +1507,35 @@
EXPECT_FALSE(channel_->CanInsertDtmf());
}
-// Test that we set Opus/Red under the field trial.
+// 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.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(112, send_codec_spec.red_payload_type);
+}
+
+// Test that we do not use Opus/Red under the field trial by default.
+TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedDefault) {
+ webrtc::test::ScopedFieldTrials override_field_trials(
+ "WebRTC-Audio-Red-For-Opus/Enabled/");
+
+ EXPECT_TRUE(SetupSendStream());
+ cricket::AudioSendParameters parameters;
parameters.codecs.push_back(kOpusCodec);
parameters.codecs.push_back(kRed48000Codec);
- parameters.codecs[0].id = 96;
SetSendParameters(parameters);
const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec;
- EXPECT_EQ(96, send_codec_spec.payload_type);
- EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str());
- EXPECT_EQ(112, send_codec_spec.red_payload_type);
-}
-
-// Test that we set do not interpret Opus/Red by default.
-TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedDefault) {
- EXPECT_TRUE(SetupSendStream());
- cricket::AudioSendParameters parameters;
- parameters.codecs.push_back(kOpusCodec);
- parameters.codecs.push_back(kRed48000Codec);
- parameters.codecs[0].id = 96;
- SetSendParameters(parameters);
- const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec;
- EXPECT_EQ(96, send_codec_spec.payload_type);
+ 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);
}
