Bug 1531803 - Part 3: Only set track id on JsepTrack if we're configured to emit track ids in SDP, and simplify some code. r=mjf
authorByron Campen [:bwc] <docfaraday@gmail.com>
Mon, 29 Apr 2019 15:51:30 +0000
changeset 530750 f8a6a019f873aaebc47740ea1423c0985aaedf69
parent 530749 14e1bfbd006034df2786ed5c16ec1c4d60315ff8
child 530751 4864459fde540d61387aa5bb4ec8b05b607d3b4d
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1531803
milestone68.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 1531803 - Part 3: Only set track id on JsepTrack if we're configured to emit track ids in SDP, and simplify some code. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D25796
media/webrtc/signaling/gtest/jsep_track_unittest.cpp
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/gtest/jsep_track_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_track_unittest.cpp
@@ -122,27 +122,27 @@ class JsepTrackTest : public ::testing::
     helper.SetupMsidSemantic(msids, mAnswer.get());
   }
 
   SdpMediaSection& GetOffer() { return mOffer->GetMediaSection(0); }
 
   SdpMediaSection& GetAnswer() { return mAnswer->GetMediaSection(0); }
 
   void CreateOffer() {
-    mSendOff.AddToOffer(mSsrcGenerator, true, &GetOffer());
-    mRecvOff.AddToOffer(mSsrcGenerator, true, &GetOffer());
+    mSendOff.AddToOffer(mSsrcGenerator, &GetOffer());
+    mRecvOff.AddToOffer(mSsrcGenerator, &GetOffer());
   }
 
   void CreateAnswer() {
     if (mRecvAns.GetMediaType() != SdpMediaSection::MediaType::kApplication) {
       mRecvAns.UpdateRecvTrack(*mOffer, GetOffer());
     }
 
-    mSendAns.AddToAnswer(GetOffer(), mSsrcGenerator, true, &GetAnswer());
-    mRecvAns.AddToAnswer(GetOffer(), mSsrcGenerator, true, &GetAnswer());
+    mSendAns.AddToAnswer(GetOffer(), mSsrcGenerator, &GetAnswer());
+    mRecvAns.AddToAnswer(GetOffer(), mSsrcGenerator, &GetAnswer());
   }
 
   void Negotiate() {
     std::cerr << "Offer SDP: " << std::endl;
     mOffer->Serialize(std::cerr);
 
     std::cerr << "Answer SDP: " << std::endl;
     mAnswer->Serialize(std::cerr);
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -102,17 +102,17 @@ nsresult JsepSessionImpl::AddTransceiver
 
   if (transceiver->GetMediaType() != SdpMediaSection::kApplication) {
     // Make sure we have an ssrc. Might already be set.
     transceiver->mSendTrack.EnsureSsrcs(mSsrcGenerator);
     transceiver->mSendTrack.SetCNAME(mCNAME);
 
     // Make sure we have identifiers for send track, just in case.
     // (man I hate this)
-    if (transceiver->mSendTrack.GetTrackId().empty()) {
+    if (mEncodeTrackId) {
       std::string trackId;
       if (!mUuidGen->Generate(&trackId)) {
         JSEP_SET_ERROR("Failed to generate UUID for JsepTrack");
         return NS_ERROR_FAILURE;
       }
 
       transceiver->mSendTrack.SetTrackId(trackId);
     }
@@ -252,18 +252,18 @@ nsresult JsepSessionImpl::CreateOfferMse
     // Set RTCP-MUX.
     msection->GetAttributeList().SetAttribute(
         new SdpFlagAttribute(SdpAttribute::kRtcpMuxAttribute));
   }
 
   nsresult rv = AddTransportAttributes(msection, SdpSetupAttribute::kActpass);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  transceiver.mSendTrack.AddToOffer(mSsrcGenerator, mEncodeTrackId, msection);
-  transceiver.mRecvTrack.AddToOffer(mSsrcGenerator, mEncodeTrackId, msection);
+  transceiver.mSendTrack.AddToOffer(mSsrcGenerator, msection);
+  transceiver.mRecvTrack.AddToOffer(mSsrcGenerator, msection);
 
   AddExtmap(msection);
 
   std::string mid;
   // We do not set the mid on the transceiver, that happens when a description
   // is set.
   if (transceiver.IsAssociated()) {
     mid = transceiver.GetMid();
@@ -533,20 +533,18 @@ nsresult JsepSessionImpl::CreateAnswerMs
   } else {
     rv = DetermineAnswererSetupRole(remoteMsection, &role);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   rv = AddTransportAttributes(&msection, role);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  transceiver.mSendTrack.AddToAnswer(remoteMsection, mSsrcGenerator,
-                                     mEncodeTrackId, &msection);
-  transceiver.mRecvTrack.AddToAnswer(remoteMsection, mSsrcGenerator,
-                                     mEncodeTrackId, &msection);
+  transceiver.mSendTrack.AddToAnswer(remoteMsection, mSsrcGenerator, &msection);
+  transceiver.mRecvTrack.AddToAnswer(remoteMsection, mSsrcGenerator, &msection);
 
   // Add extmap attributes. This logic will probably be moved to the track,
   // since it can be specified on a per-sender basis in JS.
   // We will need some validation to ensure that the ids are identical for
   // RTP streams that are bundled together, though (bug 1406529).
   AddCommonExtmaps(remoteMsection, &msection);
 
   if (msection.GetFormats().empty()) {
--- a/media/webrtc/signaling/src/jsep/JsepTrack.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.cpp
@@ -101,41 +101,41 @@ void JsepTrack::PopulateCodecs(
       mPrototypeCodecs.emplace_back(prototypeCodec->Clone());
       mPrototypeCodecs.back()->mDirection = mDirection;
     }
   }
 
   EnsureNoDuplicatePayloadTypes(&mPrototypeCodecs);
 }
 
-void JsepTrack::AddToOffer(SsrcGenerator& ssrcGenerator, bool encodeTrackId,
+void JsepTrack::AddToOffer(SsrcGenerator& ssrcGenerator,
                            SdpMediaSection* offer) {
-  AddToMsection(mPrototypeCodecs, encodeTrackId, offer);
+  AddToMsection(mPrototypeCodecs, offer);
 
   if (mDirection == sdp::kSend) {
     std::vector<JsConstraints> constraints;
     if (offer->IsSending()) {
       constraints = mJsEncodeConstraints;
     }
     AddToMsection(constraints, sdp::kSend, ssrcGenerator, offer);
   }
 }
 
 void JsepTrack::AddToAnswer(const SdpMediaSection& offer,
-                            SsrcGenerator& ssrcGenerator, bool encodeTrackId,
+                            SsrcGenerator& ssrcGenerator,
                             SdpMediaSection* answer) {
   // We do not modify mPrototypeCodecs here, since we're only creating an
   // answer. Once offer/answer concludes, we will update mPrototypeCodecs.
   std::vector<UniquePtr<JsepCodecDescription>> codecs =
       NegotiateCodecs(offer, true);
   if (codecs.empty()) {
     return;
   }
 
-  AddToMsection(codecs, encodeTrackId, answer);
+  AddToMsection(codecs, answer);
 
   if (mDirection == sdp::kSend) {
     std::vector<JsConstraints> constraints;
     if (answer->IsSending()) {
       constraints = mJsEncodeConstraints;
       std::vector<SdpRidAttributeList::Rid> rids;
       GetRids(offer, sdp::kRecv, &rids);
       NegotiateRids(rids, &constraints);
@@ -164,31 +164,31 @@ bool JsepTrack::SetJsConstraints(
     }
   }
 
   return constraintsChanged;
 }
 
 void JsepTrack::AddToMsection(
     const std::vector<UniquePtr<JsepCodecDescription>>& codecs,
-    bool encodeTrackId, SdpMediaSection* msection) {
+    SdpMediaSection* msection) {
   MOZ_ASSERT(msection->GetMediaType() == mType);
   MOZ_ASSERT(!codecs.empty());
 
   for (const auto& codec : codecs) {
     codec->AddToMediaSection(*msection);
   }
 
   if ((mDirection == sdp::kSend) && (mType != SdpMediaSection::kApplication) &&
       msection->IsSending()) {
     if (mStreamIds.empty()) {
-      msection->AddMsid("-", encodeTrackId ? mTrackId : "");
+      msection->AddMsid("-", mTrackId);
     } else {
       for (const std::string& streamId : mStreamIds) {
-        msection->AddMsid(streamId, encodeTrackId ? mTrackId : "");
+        msection->AddMsid(streamId, mTrackId);
       }
     }
   }
 }
 
 // Updates the |id| values in |constraintsList| with the rid values in |rids|,
 // where necessary.
 void JsepTrack::NegotiateRids(
--- a/media/webrtc/signaling/src/jsep/JsepTrack.h
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.h
@@ -181,20 +181,19 @@ class JsepTrack {
   }
 
   template <class BinaryPredicate>
   void SortCodecs(BinaryPredicate sorter) {
     std::stable_sort(mPrototypeCodecs.begin(), mPrototypeCodecs.end(), sorter);
   }
 
   // These two are non-const because this is where ssrcs are chosen.
-  virtual void AddToOffer(SsrcGenerator& ssrcGenerator, bool encodeTrackId,
-                          SdpMediaSection* offer);
+  virtual void AddToOffer(SsrcGenerator& ssrcGenerator, SdpMediaSection* offer);
   virtual void AddToAnswer(const SdpMediaSection& offer,
-                           SsrcGenerator& ssrcGenerator, bool encodeTrackId,
+                           SsrcGenerator& ssrcGenerator,
                            SdpMediaSection* answer);
 
   virtual void Negotiate(const SdpMediaSection& answer,
                          const SdpMediaSection& remote);
   static void SetUniquePayloadTypes(std::vector<JsepTrack*>& tracks);
   virtual void GetNegotiatedPayloadTypes(
       std::vector<uint16_t>* payloadTypes) const;
 
@@ -238,17 +237,17 @@ class JsepTrack {
  private:
   std::vector<UniquePtr<JsepCodecDescription>> GetCodecClones() const;
   static void EnsureNoDuplicatePayloadTypes(
       std::vector<UniquePtr<JsepCodecDescription>>* codecs);
   static void GetPayloadTypes(
       const std::vector<UniquePtr<JsepCodecDescription>>& codecs,
       std::vector<uint16_t>* pts);
   void AddToMsection(const std::vector<UniquePtr<JsepCodecDescription>>& codecs,
-                     bool encodeTrackId, SdpMediaSection* msection);
+                     SdpMediaSection* msection);
   void GetRids(const SdpMediaSection& msection, sdp::Direction direction,
                std::vector<SdpRidAttributeList::Rid>* rids) const;
   void CreateEncodings(
       const SdpMediaSection& remote,
       const std::vector<UniquePtr<JsepCodecDescription>>& negotiatedCodecs,
       JsepTrackNegotiatedDetails* details);
 
   virtual std::vector<UniquePtr<JsepCodecDescription>> NegotiateCodecs(