admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reject offer content with no common codecs

Browse Source 
instead of throwing an error when trying to pick a send codec.

BUG=webrtc:15145,webrtc:4957

Change-Id: I056b145c093348576e1aeaf5def50d5414f2de70
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330122
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41360}
This commit is contained in:
Philipp Hancke 2023-12-08 09:35:00 +01:00 committed by WebRTC LUCI CQ
parent ca98de9714
commit 601ac2eea8
4 changed files with 78 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
media/engine/webrtc_video_engine.cc
View File

@ -1038,15 +1038,21 @@ bool WebRtcVideoSendChannel::GetChangedSenderParameters(
return false;
}
std::vector<VideoCodecSettings> negotiated_codecs =
SelectSendVideoCodecs(MapCodecs(params.codecs));
// We should only fail here if send direction is enabled.
if (params.is_stream_active && negotiated_codecs.empty()) {
RTC_LOG(LS_ERROR) << "No video codecs supported.";
std::vector<VideoCodecSettings> mapped_codecs = MapCodecs(params.codecs);
if (mapped_codecs.empty()) {
// This suggests a failure in MapCodecs, e.g. inconsistent RTX codecs.
return false;
}
std::vector<VideoCodecSettings> negotiated_codecs =
SelectSendVideoCodecs(mapped_codecs);
if (params.is_stream_active && negotiated_codecs.empty()) {
// This is not a failure but will lead to the answer being rejected.
RTC_LOG(LS_ERROR) << "No video codecs in common.";
return true;
}
// Never enable sending FlexFEC, unless we are in the experiment.
if (!IsEnabled(call_->trials(), "WebRTC-FlexFEC-03")) {
for (VideoCodecSettings& codec : negotiated_codecs)

9
media/engine/webrtc_voice_engine.cc
View File

@ -761,9 +761,9 @@ std::vector<AudioCodec> WebRtcVoiceEngine::CollectCodecs(
out.push_back(codec);
if (codec.name == kOpusCodecName) {
std::string redFmtp =
std::string red_fmtp =
rtc::ToString(codec.id) + "/" + rtc::ToString(codec.id);
map_format({kRedCodecName, 48000, 2, {{"", redFmtp}}}, &out);
map_format({kRedCodecName, 48000, 2, {{"", red_fmtp}}}, &out);
}
}
}
@ -1318,7 +1318,7 @@ bool WebRtcVoiceSendChannel::SetSenderParameters(
}
}
if (!SetMaxSendBitrate(params.max_bandwidth_bps)) {
if (send_codec_spec_ && !SetMaxSendBitrate(params.max_bandwidth_bps)) {
return false;
}
return SetOptions(params.options);
@ -1402,7 +1402,8 @@ bool WebRtcVoiceSendChannel::SetSendCodecs(
}
if (!send_codec_spec) {
return false;
// No codecs in common, bail out early.
return true;
}
RTC_DCHECK(voice_codec_info);

36
media/engine/webrtc_voice_engine_unittest.cc
View File

@ -1702,27 +1702,29 @@ TEST_P(WebRtcVoiceEngineTestFake, DontRecreateSendStream) {
// TODO(ossu): Revisit if these tests need to be here, now that these kinds of
// tests should be available in AudioEncoderOpusTest.
// Test that if clockrate is not 48000 for opus, we fail.
// Test that if clockrate is not 48000 for opus, we do not have a send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBadClockrate) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
parameters.codecs.push_back(kOpusCodec);
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].clockrate = 50000;
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that if channels=0 for opus, we fail.
// Test that if channels=0 for opus, we do not have a send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad0ChannelsNoStereo) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
parameters.codecs.push_back(kOpusCodec);
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].channels = 0;
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that if channels=0 for opus, we fail.
// Test that if channels=0 for opus, we do not have a send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad0Channels1Stereo) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
@ -1730,20 +1732,23 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad0Channels1Stereo) {
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].channels = 0;
parameters.codecs[0].params["stereo"] = "1";
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that if channel is 1 for opus and there's no stereo, we fail.
// Test that if channel is 1 for opus and there's no stereo, we do not have a
// send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpus1ChannelNoStereo) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
parameters.codecs.push_back(kOpusCodec);
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].channels = 1;
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that if channel is 1 for opus and stereo=0, we fail.
// Test that if channel is 1 for opus and stereo=0, we do not have a send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad1Channel0Stereo) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
@ -1751,10 +1756,11 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad1Channel0Stereo) {
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].channels = 1;
parameters.codecs[0].params["stereo"] = "0";
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that if channel is 1 for opus and stereo=1, we fail.
// Test that if channel is 1 for opus and stereo=1, we do not have a send codec.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad1Channel1Stereo) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
@ -1762,7 +1768,8 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecOpusBad1Channel1Stereo) {
parameters.codecs[0].bitrate = 0;
parameters.codecs[0].channels = 1;
parameters.codecs[0].params["stereo"] = "1";
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that with bitrate=0 and no stereo, bitrate is 32000.
@ -2035,11 +2042,12 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsBitrate) {
}
}
// Test that we fail if no codecs are specified.
// Test that we do not fail if no codecs are specified.
TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsNoCodecs) {
EXPECT_TRUE(SetupSendStream());
cricket::AudioSenderParameter parameters;
EXPECT_FALSE(send_channel_->SetSenderParameters(parameters));
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
EXPECT_EQ(send_channel_->GetSendCodec(), absl::nullopt);
}
// Test that we can set send codecs even with telephone-event codec as the first

39
pc/sdp_offer_answer_unittest.cc
View File

@ -1096,4 +1096,43 @@ INSTANTIATE_TEST_SUITE_P(SdpOfferAnswerShuffleMediaTypes,
SdpOfferAnswerShuffleMediaTypes,
::testing::Values(true, false));
TEST_F(SdpOfferAnswerTest, OfferWithNoCompatibleCodecsIsRejectedInAnswer) {
auto pc = CreatePeerConnection();
// An offer with no common codecs. This should reject both contents
// in the answer without throwing an error.
std::string sdp =
"v=0\r\n"
"o=- 0 3 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=fingerprint:sha-1 "
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
"a=setup:actpass\r\n"
"a=ice-ufrag:ETEn\r\n"
"a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
"m=audio 9 RTP/SAVPF 97\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=sendrecv\r\n"
"a=rtpmap:97 x-unknown/90000\r\n"
"a=rtcp-mux\r\n"
"m=video 9 RTP/SAVPF 98\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=sendrecv\r\n"
"a=rtpmap:98 H263-1998/90000\r\n"
"a=fmtp:98 CIF=1;QCIF=1\r\n"
"a=rtcp-mux\r\n";
auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
ASSERT_NE(desc, nullptr);
RTCError error;
pc->SetRemoteDescription(std::move(desc), &error);
EXPECT_TRUE(error.ok());
auto answer = pc->CreateAnswer();
auto answer_contents = answer->description()->contents();
ASSERT_EQ(answer_contents.size(), 2u);
EXPECT_EQ(answer_contents[0].rejected, true);
EXPECT_EQ(answer_contents[1].rejected, true);
}
} // namespace webrtc