zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "red: generate and parse the red fmtp format"

Browse Source 
This reverts commit 9d0730942677a520ce7e184d081b4c5a2469fc48.

Reason for revert: Speculative revert due to failing downstream test. If the test recovers, I'll assign the issue to the tests owners.

Original change's description:
> red: generate and parse the red fmtp format
>
> generates a fmtp line like
>   a=fmtp:<red payloadtype> <opus payloadtype>/<opus payloadtype>
> and matches the incoming redundant payload types against the
> send codec one. Offers without an FMTP line will not use RED.
> Redundancy levels of 1 (plus main packet ) to 32 are accepted but
> this is not wired up to the encoder since the O/A semantic of
> RFC 2198 is not clear.
>
> This decreases the chance of a collision with the SATIN codec
> which also runs on 48khz (but so far does not specify a channelCount of 2)
>
> BUG=webrtc:11640
>
> Change-Id: I8755e5b1e944d105212f1bbe4f330cf4e0753e67
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229583
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#34848}

TBR=henrik.lundin@webrtc.org,hta@webrtc.org,philipp.hancke@googlemail.com,webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I5a0816a22a2a213679ab047c61e3b1dda40c4f59
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11640
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230140
Reviewed-by: Björn Terelius <terelius@google.com>
Commit-Queue: Björn Terelius <terelius@google.com>
Cr-Commit-Position: refs/heads/main@{#34850}
This commit is contained in:
Björn Terelius
2021-08-25 11:46:07 +00:00
committed by WebRTC LUCI CQ
parent b7aac6f5f4
commit 75bbd1fbe6
2 changed files with 13 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
media/engine/webrtc_voice_engine.cc
View File

@ -775,9 +775,7 @@ std::vector<AudioCodec> 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
// <primary codec>/<primary codec>[/...]
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<std::string> 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;
}

74
media/engine/webrtc_voice_engine_unittest.cc
View File

@ -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<std::map<int, webrtc::SdpAudioFormat>>(
{{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) {