Bug 1300863 - Part 2: Don't reject m-sections that should just be inactive. r?drno draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Wed, 14 Sep 2016 16:57:27 -0500
changeset 415653 2bbba65fbe20ef46358f8fa94a528dbb2fd65ee8
parent 415652 b3daf8a3a104276ec7268a86c8af6f5d0f11de31
child 531660 4e72574cbe14e52ce5516f37da98609bc5ff7a05
push id29924
push userbcampen@mozilla.com
push dateTue, 20 Sep 2016 19:34:30 +0000
reviewersdrno
bugs1300863
milestone52.0a1
Bug 1300863 - Part 2: Don't reject m-sections that should just be inactive. r?drno MozReview-Commit-ID: BJUIKUNpo1u
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepTrack.cpp
media/webrtc/signaling/src/jsep/JsepTrack.h
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -924,21 +924,16 @@ JsepSessionImpl::CreateAnswerMSection(co
     rv = BindMatchingLocalTrackToAnswer(&msection);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   if (remoteMsection.IsSending()) {
     BindMatchingRemoteTrackToAnswer(&msection);
   }
 
-  if (!msection.IsReceiving() && !msection.IsSending()) {
-    mSdpHelper.DisableMsection(sdp, &msection);
-    return NS_OK;
-  }
-
   // Add extmap attributes.
   AddCommonExtmaps(remoteMsection, &msection);
 
   if (msection.GetFormats().empty()) {
     // Could not negotiate anything. Disable m-section.
     mSdpHelper.DisableMsection(sdp, &msection);
   }
 
@@ -989,16 +984,24 @@ JsepSessionImpl::BindMatchingLocalTrackT
     MOZ_ASSERT(track != mLocalTracks.end());
   }
 
   if (track != mLocalTracks.end()) {
     track->mAssignedMLine = Some(msection->GetLevel());
     track->mTrack->AddToAnswer(
         mPendingRemoteDescription->GetMediaSection(msection->GetLevel()),
         msection);
+  } else {
+    // Create a dummy track, and have it add codecs
+    RefPtr<JsepTrack> dummy =
+      new JsepTrack(msection->GetMediaType(), "", "", sdp::kSend);
+    dummy->PopulateCodecs(mSupportedCodecs.values);
+    dummy->AddCodecsToAnswer(
+        mPendingRemoteDescription->GetMediaSection(msection->GetLevel()),
+        msection);
   }
 
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::BindMatchingRemoteTrackToAnswer(SdpMediaSection* msection)
 {
@@ -1349,17 +1352,19 @@ JsepSessionImpl::HandleNegotiatedSession
     rv = MakeNegotiatedTrackPair(remote->GetMediaSection(i),
                                  local->GetMediaSection(i),
                                  transport,
                                  usingBundle,
                                  transportLevel,
                                  &trackPair);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    trackPairs.push_back(trackPair);
+    if (trackPair.mReceiving || trackPair.mSending) {
+      trackPairs.push_back(trackPair);
+    }
   }
 
   JsepTrack::SetUniquePayloadTypes(GetTracks(mRemoteTracks));
 
   // Ouch, this probably needs some dirty bit instead of just clearing
   // stuff for renegotiation.
   mNegotiatedTrackPairs = trackPairs;
 
@@ -1407,43 +1412,31 @@ JsepSessionImpl::MakeNegotiatedTrackPair
     trackPairOut->mBundleLevel = Some(transportLevel);
   }
 
   auto sendTrack = FindTrackByLevel(mLocalTracks, local.GetLevel());
   if (sendTrack != mLocalTracks.end()) {
     sendTrack->mTrack->Negotiate(answer, remote);
     sendTrack->mTrack->SetActive(sending);
     trackPairOut->mSending = sendTrack->mTrack;
-  } else if (sending) {
-    JSEP_SET_ERROR("Failed to find local track for level " <<
-                   local.GetLevel()
-                   << " in local SDP. This should never happen.");
-    NS_ASSERTION(false, "Failed to find local track for level");
-    return NS_ERROR_FAILURE;
   }
 
   auto recvTrack = FindTrackByLevel(mRemoteTracks, local.GetLevel());
   if (recvTrack != mRemoteTracks.end()) {
     recvTrack->mTrack->Negotiate(answer, remote);
     recvTrack->mTrack->SetActive(receiving);
     trackPairOut->mReceiving = recvTrack->mTrack;
 
     if (receiving &&
         trackPairOut->mBundleLevel.isSome() &&
         recvTrack->mTrack->GetSsrcs().empty() &&
         recvTrack->mTrack->GetMediaType() != SdpMediaSection::kApplication) {
       MOZ_MTLOG(ML_ERROR, "Bundled m-section has no ssrc attributes. "
                           "This may cause media packets to be dropped.");
     }
-  } else if (receiving) {
-    JSEP_SET_ERROR("Failed to find remote track for level "
-                   << local.GetLevel()
-                   << " in remote SDP. This should never happen.");
-    NS_ASSERTION(false, "Failed to find remote track for level");
-    return NS_ERROR_FAILURE;
   }
 
   trackPairOut->mRtpTransport = transport;
 
   if (transport->mComponents == 2) {
     // RTCP MUX or not.
     // TODO(bug 1095743): verify that the PTs are consistent with mux.
     MOZ_MTLOG(ML_DEBUG, "RTCP-MUX is off");
--- a/media/webrtc/signaling/src/jsep/JsepTrack.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.cpp
@@ -97,56 +97,71 @@ JsepTrack::PopulateCodecs(const std::vec
   }
 
   EnsureNoDuplicatePayloadTypes(&mPrototypeCodecs.values);
 }
 
 void
 JsepTrack::AddToOffer(SdpMediaSection* offer) const
 {
-  AddToMsection(mPrototypeCodecs.values, offer);
+  AddCodecsToMsection(mPrototypeCodecs.values, offer);
+  AddToMsection(offer);
   if (mDirection == sdp::kSend) {
     AddToMsection(mJsEncodeConstraints, sdp::kSend, offer);
   }
 }
 
 void
-JsepTrack::AddToAnswer(const SdpMediaSection& offer,
-                       SdpMediaSection* answer) const
+JsepTrack::AddCodecsToAnswer(const SdpMediaSection& offer,
+                             SdpMediaSection* answer) const
 {
   // We do not modify mPrototypeCodecs here, since we're only creating an
   // answer. Once offer/answer concludes, we will update mPrototypeCodecs.
   PtrVector<JsepCodecDescription> codecs;
   codecs.values = GetCodecClones();
   NegotiateCodecs(offer, &codecs.values);
   if (codecs.values.empty()) {
     return;
   }
 
-  AddToMsection(codecs.values, answer);
+  AddCodecsToMsection(codecs.values, answer);
+}
+
+void
+JsepTrack::AddToAnswer(const SdpMediaSection& offer,
+                       SdpMediaSection* answer) const
+{
+  AddCodecsToAnswer(offer, answer);
+  AddToMsection(answer);
 
   if (mDirection == sdp::kSend) {
     std::vector<JsConstraints> constraints;
     std::vector<SdpRidAttributeList::Rid> rids;
     GetRids(offer, sdp::kRecv, &rids);
     NegotiateRids(rids, &constraints);
     AddToMsection(constraints, sdp::kSend, answer);
   }
 }
 
 void
-JsepTrack::AddToMsection(const std::vector<JsepCodecDescription*>& codecs,
-                         SdpMediaSection* msection) const
+JsepTrack::AddCodecsToMsection(const std::vector<JsepCodecDescription*>& codecs,
+                               SdpMediaSection* msection) const
 {
   MOZ_ASSERT(msection->GetMediaType() == mType);
   MOZ_ASSERT(!codecs.empty());
 
   for (const JsepCodecDescription* codec : codecs) {
     codec->AddToMediaSection(*msection);
   }
+}
+
+void
+JsepTrack::AddToMsection(SdpMediaSection* msection) const
+{
+  MOZ_ASSERT(msection->GetMediaType() == mType);
 
   if (mDirection == sdp::kSend) {
     if (msection->GetMediaType() != SdpMediaSection::kApplication) {
       msection->SetSsrcs(mSsrcs, mCNAME);
       msection->AddMsid(mStreamId, mTrackId);
     }
     msection->SetSending(true);
   } else {
--- a/media/webrtc/signaling/src/jsep/JsepTrack.h
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.h
@@ -163,16 +163,18 @@ public:
   template <class BinaryPredicate>
   void SortCodecs(BinaryPredicate sorter)
   {
     std::stable_sort(mPrototypeCodecs.values.begin(),
                      mPrototypeCodecs.values.end(), sorter);
   }
 
   virtual void AddToOffer(SdpMediaSection* offer) const;
+  virtual void AddCodecsToAnswer(const SdpMediaSection& offer,
+                                 SdpMediaSection* answer) const;
   virtual void AddToAnswer(const SdpMediaSection& offer,
                            SdpMediaSection* answer) const;
   virtual void Negotiate(const SdpMediaSection& answer,
                          const SdpMediaSection& remote);
   static void SetUniquePayloadTypes(
       const std::vector<RefPtr<JsepTrack>>& tracks);
   virtual void GetNegotiatedPayloadTypes(std::vector<uint16_t>* payloadTypes);
 
@@ -231,18 +233,19 @@ private:
   std::vector<JsepCodecDescription*> GetCodecClones() const;
   static void EnsureNoDuplicatePayloadTypes(
       std::vector<JsepCodecDescription*>* codecs);
   static void GetPayloadTypes(
       const std::vector<JsepCodecDescription*>& codecs,
       std::vector<uint16_t>* pts);
   static void EnsurePayloadTypeIsUnique(std::set<uint16_t>* uniquePayloadTypes,
                                         JsepCodecDescription* codec);
-  void AddToMsection(const std::vector<JsepCodecDescription*>& codecs,
-                     SdpMediaSection* msection) const;
+  void AddToMsection(SdpMediaSection* msection) const;
+  void AddCodecsToMsection(const std::vector<JsepCodecDescription*>& codecs,
+                           SdpMediaSection* msection) const;
   void GetRids(const SdpMediaSection& msection,
                sdp::Direction direction,
                std::vector<SdpRidAttributeList::Rid>* rids) const;
   void CreateEncodings(
       const SdpMediaSection& remote,
       const std::vector<JsepCodecDescription*>& negotiatedCodecs,
       JsepTrackNegotiatedDetails* details);