Sanitize the codec list before sending it to the media engine

The SDP can assign the same codec to two different payload types
which gets represented as two separate codecs in the SDP structure.
The media engine assumes that the client does not pass down
duplicate codecs. This change adds logic to BaseChannel to filter
out codecs of the same name with different payload types, picking
the one which is listed first in the m= line.

Bug: chromium:987598
Change-Id: I6fa813db1769e572ff7c3f322dc9b1de39817ea2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147602
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28726}
This commit is contained in:
Steve Anton 2019-07-30 18:04:40 -07:00 committed by Commit Bot
parent f6f3ecf4bb
commit add7ef974e
2 changed files with 37 additions and 1 deletions

View file

@ -94,6 +94,20 @@ static void SafeSetError(const std::string& message, std::string* error_desc) {
}
}
template <class Codec>
std::vector<Codec> SanitizeCodecList(const std::vector<Codec>& codecs) {
std::vector<Codec> sanitized;
for (const Codec& codec : codecs) {
if (absl::c_any_of(sanitized, [&](const Codec& other) {
return codec.Matches(other);
})) {
continue;
}
sanitized.push_back(codec);
}
return sanitized;
}
template <class Codec>
void RtpParametersFromMediaDescription(
const MediaContentDescriptionImpl<Codec>* desc,
@ -103,7 +117,7 @@ void RtpParametersFromMediaDescription(
// a description without codecs. Currently the ORTC implementation is relying
// on this.
if (desc->has_codecs()) {
params->codecs = desc->codecs();
params->codecs = SanitizeCodecList(desc->codecs());
}
// TODO(pthatcher): See if we really need
// rtp_header_extensions_set() and remove it if we don't.

View file

@ -45,6 +45,7 @@ using cricket::FakeVoiceMediaChannel;
using cricket::RidDescription;
using cricket::RidDirection;
using cricket::StreamParams;
using testing::ElementsAre;
using webrtc::RtpTransceiverDirection;
using webrtc::SdpType;
@ -2268,6 +2269,27 @@ TEST_F(VideoChannelSingleThreadTest,
EXPECT_EQ(media_channel1_->send_codecs()[0].packetization, absl::nullopt);
}
// Test that if the session description has the same codec assigned to two
// payload types then the MediaChannel will only receive the one that comes
// first in the list.
TEST_F(VideoChannelSingleThreadTest, TestFilterDuplicateDynamicCodecs) {
const char kCodecName[] = "VP8";
cricket::VideoCodec codec(98, kCodecName);
cricket::VideoCodec duplicate(99, kCodecName);
cricket::VideoContentDescription video_content;
video_content.set_codecs({codec, duplicate});
CreateChannels(0, 0);
EXPECT_TRUE(
channel1_->SetRemoteContent(&video_content, SdpType::kOffer, NULL));
EXPECT_TRUE(
channel1_->SetLocalContent(&video_content, SdpType::kAnswer, NULL));
EXPECT_THAT(media_channel1_->recv_codecs(), ElementsAre(codec));
EXPECT_THAT(media_channel1_->send_codecs(), ElementsAre(codec));
}
// VideoChannelDoubleThreadTest
TEST_F(VideoChannelDoubleThreadTest, TestInit) {
Base::TestInit();