Bug 1398820 - Do not add duplicate rtp extensions; r=bwc
authorDan Minor <dminor@mozilla.com>
Thu, 14 Sep 2017 15:55:33 -0400
changeset 433122 f73a6867afeead095c1804adb63815fb885af2f3
parent 433121 880d772e282e47a015605622ca0046569c80bd71
child 433147 8998495d56dcb03aac5376b72ccd73ab19dbfc8c
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1398820
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);