From add7ef974ee2642a3b55a36ec80be50a615bc60a Mon Sep 17 00:00:00 2001
From: Steve Anton <steveanton@webrtc.org>
Date: Tue, 30 Jul 2019 18:04:40 -0700
Subject: [PATCH] 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}
---
 pc/channel.cc          | 16 +++++++++++++++-
 pc/channel_unittest.cc | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/pc/channel.cc b/pc/channel.cc
index caf8c93956..916c403a00 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -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.
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index db0e8a8d02..952570e1c2 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -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();