Bug 1165129: Allow JS to reorder codecs in a local answer. Also means that we'll take the ordering more seriously when we see multiple codecs in a remote answer. r=jesup
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 14 May 2015 16:05:36 -0700
changeset 246121 001de2286250cc4c63def8524bfec52ed3ad621d
parent 246120 d29db7f0d3f059c86a4a82a0e01fb35621187028
child 246122 1ce0647eb223246f1417c367d614453ca6d63387
push id60367
push userbcampen@mozilla.com
push dateThu, 28 May 2015 23:20:19 +0000
treeherdermozilla-inbound@001de2286250 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1165129
milestone41.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 1165129: Allow JS to reorder codecs in a local answer. Also means that we'll take the ordering more seriously when we see multiple codecs in a remote answer. r=jesup
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.h
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -898,21 +898,22 @@ JsepSessionImpl::GetRtpExtensions(SdpMed
 
 static bool
 CompareCodec(const JsepCodecDescription* lhs, const JsepCodecDescription* rhs)
 {
   return lhs->mStronglyPreferred && !rhs->mStronglyPreferred;
 }
 
 PtrVector<JsepCodecDescription>
-JsepSessionImpl::GetCommonCodecs(const SdpMediaSection& remoteMsection)
+JsepSessionImpl::GetCommonCodecs(const SdpMediaSection& offerMsection)
 {
+  MOZ_ASSERT(!mIsOfferer);
   PtrVector<JsepCodecDescription> matchingCodecs;
-  for (const std::string& fmt : remoteMsection.GetFormats()) {
-    JsepCodecDescription* codec = FindMatchingCodec(fmt, remoteMsection);
+  for (const std::string& fmt : offerMsection.GetFormats()) {
+    JsepCodecDescription* codec = FindMatchingCodec(fmt, offerMsection);
     if (codec) {
       codec->mDefaultPt = fmt; // Remember the other side's PT
       matchingCodecs.values.push_back(codec->Clone());
     }
   }
 
   std::stable_sort(matchingCodecs.values.begin(),
                    matchingCodecs.values.end(),
@@ -1710,21 +1711,35 @@ JsepSessionImpl::NegotiateTrack(const Sd
                                 const SdpMediaSection& localMsection,
                                 JsepTrack::Direction direction,
                                 RefPtr<JsepTrack>* track)
 {
   UniquePtr<JsepTrackNegotiatedDetailsImpl> negotiatedDetails =
       MakeUnique<JsepTrackNegotiatedDetailsImpl>();
   negotiatedDetails->mProtocol = remoteMsection.GetProtocol();
 
-  // Insert all the codecs we jointly support.
-  PtrVector<JsepCodecDescription> commonCodecs(
-      GetCommonCodecs(remoteMsection));
-
-  for (JsepCodecDescription* codec : commonCodecs.values) {
+  auto& answerMsection = mIsOfferer ? remoteMsection : localMsection;
+
+  for (auto& format : answerMsection.GetFormats()) {
+    JsepCodecDescription* origCodec = FindMatchingCodec(format, answerMsection);
+    if (!origCodec) {
+      continue;
+    }
+
+    // Make sure codec->mDefaultPt is consistent with what is in the remote
+    // msection, since the following logic needs to look stuff up there.
+    for (auto& remoteFormat : remoteMsection.GetFormats()) {
+      if (origCodec->Matches(remoteFormat, remoteMsection)) {
+        origCodec->mDefaultPt = remoteFormat;
+        break;
+      }
+    }
+
+    UniquePtr<JsepCodecDescription> codec(origCodec->Clone());
+
     bool sending = (direction == JsepTrack::kJsepTrackSending);
 
     // Everywhere else in JsepSessionImpl, a JsepCodecDescription describes
     // what one side puts in its SDP. However, we don't want that here; we want
     // a JsepCodecDescription that instead encapsulates all the parameters
     // that deal with sending (or receiving). For sending, some of these
     // parameters will come from the remote SDP (eg; max-fps), and others can
     // only be determined by inspecting both local config and remote SDP (eg;
@@ -1746,27 +1761,25 @@ JsepSessionImpl::NegotiateTrack(const Sd
       if (!codec->GetPtAsInt(&payloadType) ||
           payloadType > UINT8_MAX) {
         JSEP_SET_ERROR("audio/video payload type is not an 8 bit unsigned int: "
                        << codec->mDefaultPt);
         return NS_ERROR_INVALID_ARG;
       }
     }
 
-    negotiatedDetails->mCodecs.values.push_back(codec->Clone());
+    negotiatedDetails->mCodecs.values.push_back(codec.release());
     break;
   }
 
   if (negotiatedDetails->mCodecs.values.empty()) {
     JSEP_SET_ERROR("Failed to negotiate codec details for all codecs");
     return NS_ERROR_INVALID_ARG;
   }
 
-  auto& answerMsection = mIsOfferer ? remoteMsection : localMsection;
-
   if (answerMsection.GetAttributeList().HasAttribute(
         SdpAttribute::kExtmapAttribute)) {
     auto& extmap = answerMsection.GetAttributeList().GetExtmap().mExtmaps;
     for (auto i = extmap.begin(); i != extmap.end(); ++i) {
       negotiatedDetails->mExtmap[i->extensionname] = *i;
     }
   }
 
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -182,17 +182,17 @@ private:
   void AddLocalIds(const JsepTrack& track, SdpMediaSection* msection) const;
   JsepCodecDescription* FindMatchingCodec(
       const std::string& pt,
       const SdpMediaSection& msection) const;
   const std::vector<SdpExtmapAttributeList::Extmap>* GetRtpExtensions(
       SdpMediaSection::MediaType type) const;
 
   PtrVector<JsepCodecDescription> GetCommonCodecs(
-      const SdpMediaSection& remoteMsection);
+      const SdpMediaSection& offerMsection);
   void AddCommonExtmaps(const SdpMediaSection& remoteMsection,
                         SdpMediaSection* msection);
   nsresult SetupIds();
   nsresult CreateSsrc(uint32_t* ssrc);
   void SetupDefaultCodecs();
   void SetupDefaultRtpExtensions();
   void SetState(JsepSignalingState state);
   // Non-const so it can set mLastError