Bug 1398820 - Do not add duplicate rtp extensions; r?bwc draft
authorDan Minor <dminor@mozilla.com>
Thu, 14 Sep 2017 15:55:33 -0400
changeset 665492 3427f9259afa294e214a97c3a4fdbae2a516b74d
parent 665390 893fe1549e1e7342a66514b65960f08d40810a34
child 731807 16f12bd2d485a47b1595ad03b6673c78460bdeba
push id80089
push userbmo:dminor@mozilla.com
push dateFri, 15 Sep 2017 16:16:37 +0000
reviewersbwc
bugs1398820
milestone57.0a1
Bug 1398820 - Do not add duplicate rtp extensions; r?bwc MozReview-Commit-ID: G6wLXW7z05d
media/webrtc/signaling/gtest/jsep_session_unittest.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -4126,16 +4126,46 @@ TEST_F(JsepSessionTest, TestExtmap)
   ASSERT_TRUE(answerMediaAttrs.HasAttribute(SdpAttribute::kExtmapAttribute));
   auto& answerExtmap = answerMediaAttrs.GetExtmap().mExtmaps;
   ASSERT_EQ(1U, answerExtmap.size());
   // We ensure that the entry for "bar" matches what was in the offer
   ASSERT_EQ("bar", answerExtmap[0].extensionname);
   ASSERT_EQ(3U, answerExtmap[0].entry);
 }
 
+TEST_F(JsepSessionTest, TestExtmapWithDuplicates)
+{
+  AddTracks(*mSessionOff, "audio");
+  AddTracks(*mSessionAns, "audio");
+  // ssrc-audio-level will be extmap 1 for both
+  mSessionOff->AddAudioRtpExtension("foo"); // Default mapping of 2
+  mSessionOff->AddAudioRtpExtension("bar"); // Default mapping of 3
+  mSessionOff->AddAudioRtpExtension("bar"); // Should be ignored
+  mSessionOff->AddAudioRtpExtension("bar"); // Should be ignored
+  mSessionOff->AddAudioRtpExtension("baz"); // Default mapping of 4
+  mSessionOff->AddAudioRtpExtension("bar"); // Should be ignored
+
+  std::string offer = CreateOffer();
+  UniquePtr<Sdp> parsedOffer(Parse(offer));
+  ASSERT_EQ(1U, parsedOffer->GetMediaSectionCount());
+
+  auto& offerMediaAttrs = parsedOffer->GetMediaSection(0).GetAttributeList();
+  ASSERT_TRUE(offerMediaAttrs.HasAttribute(SdpAttribute::kExtmapAttribute));
+  auto& offerExtmap = offerMediaAttrs.GetExtmap().mExtmaps;
+  ASSERT_EQ(4U, offerExtmap.size());
+  ASSERT_EQ("urn:ietf:params:rtp-hdrext:ssrc-audio-level",
+      offerExtmap[0].extensionname);
+  ASSERT_EQ(1U, offerExtmap[0].entry);
+  ASSERT_EQ("foo", offerExtmap[1].extensionname);
+  ASSERT_EQ(2U, offerExtmap[1].entry);
+  ASSERT_EQ("bar", offerExtmap[2].extensionname);
+  ASSERT_EQ(3U, offerExtmap[2].entry);
+}
+
+
 TEST_F(JsepSessionTest, TestRtcpFbStar)
 {
   AddTracks(*mSessionOff, "video");
   AddTracks(*mSessionAns, "video");
 
   std::string offer = CreateOffer();
 
   UniquePtr<Sdp> parsedOffer(Parse(offer));
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -226,16 +226,23 @@ JsepSessionImpl::AddRtpExtension(std::ve
 {
   mLastError.clear();
 
   if (extensions.size() + 1 > UINT16_MAX) {
     JSEP_SET_ERROR("Too many rtp extensions have been added");
     return NS_ERROR_FAILURE;
   }
 
+  // Avoid adding duplicate entries
+  for (auto ext = extensions.begin(); ext != extensions.end(); ++ext) {
+    if (ext->direction == direction && ext->extensionname == extensionName) {
+      return NS_OK;
+    }
+  }
+
   SdpExtmapAttributeList::Extmap extmap =
       { static_cast<uint16_t>(extensions.size() + 1),
         direction,
         direction != SdpDirectionAttribute::kSendrecv, // do we want to specify direction?
         extensionName,
         "" };
 
   extensions.push_back(extmap);