From 6563934be3ef43e38f1ff8237c9c3c7689b2e6d1 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Fri, 2 Aug 2019 08:27:51 +0000 Subject: [PATCH] Revert "Sanitize the codec list before sending it to the media engine" This reverts commit add7ef974ee2642a3b55a36ec80be50a615bc60a. Reason for revert: Cause regression in pc_full_stack_tests.cc Original change's description: > 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 > Commit-Queue: Steve Anton > Cr-Commit-Position: refs/heads/master@{#28726} TBR=steveanton@webrtc.org,amithi@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:987598 Change-Id: I4ffbfcd90c81c6c6c8ee8f872f7e217d8291c857 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147864 Commit-Queue: Artem Titov Reviewed-by: Artem Titov Cr-Commit-Position: refs/heads/master@{#28744} --- pc/channel.cc | 16 +--------------- pc/channel_unittest.cc | 22 ---------------------- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/pc/channel.cc b/pc/channel.cc index 916c403a00..caf8c93956 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -94,20 +94,6 @@ static void SafeSetError(const std::string& message, std::string* error_desc) { } } -template -std::vector SanitizeCodecList(const std::vector& codecs) { - std::vector 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 void RtpParametersFromMediaDescription( const MediaContentDescriptionImpl* desc, @@ -117,7 +103,7 @@ void RtpParametersFromMediaDescription( // a description without codecs. Currently the ORTC implementation is relying // on this. if (desc->has_codecs()) { - params->codecs = SanitizeCodecList(desc->codecs()); + params->codecs = desc->codecs(); } // TODO(pthatcher): See if we really need // rtp_header_extensions_set() and remove it if we don't. diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 952570e1c2..db0e8a8d02 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -45,7 +45,6 @@ using cricket::FakeVoiceMediaChannel; using cricket::RidDescription; using cricket::RidDirection; using cricket::StreamParams; -using testing::ElementsAre; using webrtc::RtpTransceiverDirection; using webrtc::SdpType; @@ -2269,27 +2268,6 @@ 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();