Bug 1287006 - Make JsepTrackPair::mBundleLevel size_t instead of Maybe<size_t>, with SIZE_MAX encoding the previous not-size_t state. (mBundleLevel counts things in memory, so SIZE_MAX is excluded from the typical semantics.) r=bwc
authorJeff Walden <jwalden@mit.edu>
Tue, 21 Feb 2017 23:57:56 -0800
changeset 373491 c34603e73ded3e732e3a9423ca3ba0e157775cd5
parent 373490 bb5d3a227ee88805e1f5d555643eb500d444da55
child 373492 e0f2e64261cc3e1a535b1500f13cefe7e32be2dd
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1287006
milestone54.0a1
Bug 1287006 - Make JsepTrackPair::mBundleLevel size_t instead of Maybe<size_t>, with SIZE_MAX encoding the previous not-size_t state. (mBundleLevel counts things in memory, so SIZE_MAX is excluded from the typical semantics.) r=bwc
media/webrtc/signaling/gtest/jsep_session_unittest.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepTrack.h
media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -403,17 +403,17 @@ protected:
   }
 
   bool Equals(const JsepTrackPair& p1,
               const JsepTrackPair& p2) const {
     if (p1.mLevel != p2.mLevel) {
       return false;
     }
 
-    // We don't check things like mBundleLevel, since that can change without
+    // We don't check things like BundleLevel(), since that can change without
     // any changes to the transport, which is what we're really interested in.
 
     if (p1.mSending.get() != p2.mSending.get()) {
       return false;
     }
 
     if (p1.mReceiving.get() != p2.mReceiving.get()) {
       return false;
@@ -917,20 +917,20 @@ protected:
   }
 
   void CheckPairs(const JsepSession& session, const std::string& context)
   {
     auto pairs = session.GetNegotiatedTrackPairs();
 
     for (JsepTrackPair& pair : pairs) {
       if (types.size() == 1) {
-        ASSERT_FALSE(pair.mBundleLevel.isSome()) << context;
+        ASSERT_FALSE(pair.HasBundleLevel()) << context;
       } else {
-        ASSERT_TRUE(pair.mBundleLevel.isSome()) << context;
-        ASSERT_EQ(0U, *pair.mBundleLevel) << context;
+        ASSERT_TRUE(pair.HasBundleLevel()) << context;
+        ASSERT_EQ(0U, pair.BundleLevel()) << context;
       }
     }
   }
 
   void
   DisableMsid(std::string* sdp) const {
     size_t pos = sdp->find("a=msid-semantic");
     ASSERT_NE(std::string::npos, pos);
@@ -1632,34 +1632,34 @@ TEST_P(JsepSessionTest, RenegotiationBot
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size() + 1);
 
   for (size_t i = 0; i < newOffererPairs.size(); ++i) {
     JsepTrackPair oldPair(offererPairs[i + 1]);
     JsepTrackPair newPair(newOffererPairs[i]);
     ASSERT_EQ(oldPair.mLevel, newPair.mLevel);
     ASSERT_EQ(oldPair.mSending.get(), newPair.mSending.get());
     ASSERT_EQ(oldPair.mReceiving.get(), newPair.mReceiving.get());
-    ASSERT_TRUE(oldPair.mBundleLevel.isSome());
-    ASSERT_TRUE(newPair.mBundleLevel.isSome());
-    ASSERT_EQ(0U, *oldPair.mBundleLevel);
-    ASSERT_EQ(1U, *newPair.mBundleLevel);
+    ASSERT_TRUE(oldPair.HasBundleLevel());
+    ASSERT_TRUE(newPair.HasBundleLevel());
+    ASSERT_EQ(0U, oldPair.BundleLevel());
+    ASSERT_EQ(1U, newPair.BundleLevel());
   }
 
   ASSERT_EQ(answererPairs.size(), newAnswererPairs.size() + 1);
 
   for (size_t i = 0; i < newAnswererPairs.size(); ++i) {
     JsepTrackPair oldPair(answererPairs[i + 1]);
     JsepTrackPair newPair(newAnswererPairs[i]);
     ASSERT_EQ(oldPair.mLevel, newPair.mLevel);
     ASSERT_EQ(oldPair.mSending.get(), newPair.mSending.get());
     ASSERT_EQ(oldPair.mReceiving.get(), newPair.mReceiving.get());
-    ASSERT_TRUE(oldPair.mBundleLevel.isSome());
-    ASSERT_TRUE(newPair.mBundleLevel.isSome());
-    ASSERT_EQ(0U, *oldPair.mBundleLevel);
-    ASSERT_EQ(1U, *newPair.mBundleLevel);
+    ASSERT_TRUE(oldPair.HasBundleLevel());
+    ASSERT_TRUE(newPair.BundleLevel());
+    ASSERT_EQ(0U, oldPair.BundleLevel());
+    ASSERT_EQ(1U, newPair.BundleLevel());
   }
 }
 
 TEST_P(JsepSessionTest, RenegotiationBothRemoveThenAddTrack)
 {
   AddTracks(mSessionOff);
   AddTracks(mSessionAns);
   if (types.front() == SdpMediaSection::kApplication) {
@@ -2161,36 +2161,36 @@ TEST_P(JsepSessionTest, RenegotiationOff
   auto newAnswererPairs = GetTrackPairsByLevel(mSessionAns);
 
   ASSERT_EQ(newOffererPairs.size(), newAnswererPairs.size());
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size());
   ASSERT_EQ(answererPairs.size(), newAnswererPairs.size());
 
   for (size_t i = 0; i < newOffererPairs.size(); ++i) {
     // No bundle initially
-    ASSERT_FALSE(offererPairs[i].mBundleLevel.isSome());
-    ASSERT_FALSE(answererPairs[i].mBundleLevel.isSome());
+    ASSERT_FALSE(offererPairs[i].HasBundleLevel());
+    ASSERT_FALSE(answererPairs[i].HasBundleLevel());
     if (i != 0) {
       ASSERT_NE(offererPairs[0].mRtpTransport.get(),
                 offererPairs[i].mRtpTransport.get());
       if (offererPairs[0].mRtcpTransport) {
         ASSERT_NE(offererPairs[0].mRtcpTransport.get(),
                   offererPairs[i].mRtcpTransport.get());
       }
       ASSERT_NE(answererPairs[0].mRtpTransport.get(),
                 answererPairs[i].mRtpTransport.get());
       if (answererPairs[0].mRtcpTransport) {
         ASSERT_NE(answererPairs[0].mRtcpTransport.get(),
                   answererPairs[i].mRtcpTransport.get());
       }
     }
 
     // Verify that bundle worked after renegotiation
-    ASSERT_TRUE(newOffererPairs[i].mBundleLevel.isSome());
-    ASSERT_TRUE(newAnswererPairs[i].mBundleLevel.isSome());
+    ASSERT_TRUE(newOffererPairs[i].HasBundleLevel());
+    ASSERT_TRUE(newAnswererPairs[i].HasBundleLevel());
     ASSERT_EQ(newOffererPairs[0].mRtpTransport.get(),
               newOffererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newOffererPairs[0].mRtcpTransport.get(),
               newOffererPairs[i].mRtcpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtpTransport.get(),
               newAnswererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtcpTransport.get(),
               newAnswererPairs[i].mRtcpTransport.get());
@@ -2224,20 +2224,20 @@ TEST_P(JsepSessionTest, RenegotiationOff
   auto newOffererPairs = GetTrackPairsByLevel(mSessionOff);
   auto newAnswererPairs = GetTrackPairsByLevel(mSessionAns);
 
   ASSERT_EQ(newOffererPairs.size(), newAnswererPairs.size());
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size() + 1);
   ASSERT_EQ(answererPairs.size(), newAnswererPairs.size() + 1);
 
   for (size_t i = 0; i < newOffererPairs.size(); ++i) {
-    ASSERT_TRUE(newOffererPairs[i].mBundleLevel.isSome());
-    ASSERT_TRUE(newAnswererPairs[i].mBundleLevel.isSome());
-    ASSERT_EQ(1U, *newOffererPairs[i].mBundleLevel);
-    ASSERT_EQ(1U, *newAnswererPairs[i].mBundleLevel);
+    ASSERT_TRUE(newOffererPairs[i].HasBundleLevel());
+    ASSERT_TRUE(newAnswererPairs[i].HasBundleLevel());
+    ASSERT_EQ(1U, newOffererPairs[i].BundleLevel());
+    ASSERT_EQ(1U, newAnswererPairs[i].BundleLevel());
     ASSERT_EQ(newOffererPairs[0].mRtpTransport.get(),
               newOffererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newOffererPairs[0].mRtcpTransport.get(),
               newOffererPairs[i].mRtcpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtpTransport.get(),
               newAnswererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtcpTransport.get(),
               newAnswererPairs[i].mRtcpTransport.get());
@@ -2282,20 +2282,20 @@ TEST_P(JsepSessionTest, RenegotiationAns
   auto newOffererPairs = GetTrackPairsByLevel(mSessionOff);
   auto newAnswererPairs = GetTrackPairsByLevel(mSessionAns);
 
   ASSERT_EQ(newOffererPairs.size(), newAnswererPairs.size());
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size() + 1);
   ASSERT_EQ(answererPairs.size(), newAnswererPairs.size() + 1);
 
   for (size_t i = 0; i < newOffererPairs.size(); ++i) {
-    ASSERT_TRUE(newOffererPairs[i].mBundleLevel.isSome());
-    ASSERT_TRUE(newAnswererPairs[i].mBundleLevel.isSome());
-    ASSERT_EQ(1U, *newOffererPairs[i].mBundleLevel);
-    ASSERT_EQ(1U, *newAnswererPairs[i].mBundleLevel);
+    ASSERT_TRUE(newOffererPairs[i].HasBundleLevel());
+    ASSERT_TRUE(newAnswererPairs[i].HasBundleLevel());
+    ASSERT_EQ(1U, newOffererPairs[i].BundleLevel());
+    ASSERT_EQ(1U, newAnswererPairs[i].BundleLevel());
     ASSERT_EQ(newOffererPairs[0].mRtpTransport.get(),
               newOffererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newOffererPairs[0].mRtcpTransport.get(),
               newOffererPairs[i].mRtcpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtpTransport.get(),
               newAnswererPairs[i].mRtpTransport.get());
     ASSERT_EQ(newAnswererPairs[0].mRtcpTransport.get(),
               newAnswererPairs[i].mRtcpTransport.get());
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -1469,17 +1469,17 @@ JsepSessionImpl::MakeNegotiatedTrackPair
 
   trackPairOut->mLevel = local.GetLevel();
 
   MOZ_ASSERT(mRecvonlySsrcs.size() > local.GetLevel(),
              "Failed to set the default ssrc for an active m-section");
   trackPairOut->mRecvonlySsrc = mRecvonlySsrcs[local.GetLevel()];
 
   if (usingBundle) {
-    trackPairOut->mBundleLevel = Some(transportLevel);
+    trackPairOut->SetBundleLevel(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) {
@@ -1492,17 +1492,17 @@ JsepSessionImpl::MakeNegotiatedTrackPair
 
   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() &&
+        trackPairOut->HasBundleLevel() &&
         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()
--- a/media/webrtc/signaling/src/jsep/JsepTrack.h
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.h
@@ -294,19 +294,33 @@ private:
   std::vector<uint32_t> mSsrcs;
   bool mActive;
 };
 
 // Need a better name for this.
 struct JsepTrackPair {
   size_t mLevel;
   // Is this track pair sharing a transport with another?
-  Maybe<size_t> mBundleLevel;
+  size_t mBundleLevel = SIZE_MAX; // SIZE_MAX if no bundle level
   uint32_t mRecvonlySsrc;
   RefPtr<JsepTrack> mSending;
   RefPtr<JsepTrack> mReceiving;
   RefPtr<JsepTransport> mRtpTransport;
   RefPtr<JsepTransport> mRtcpTransport;
+
+  bool HasBundleLevel() const {
+    return mBundleLevel != SIZE_MAX;
+  }
+
+  size_t BundleLevel() const {
+    MOZ_ASSERT(HasBundleLevel());
+    return mBundleLevel;
+  }
+
+  void SetBundleLevel(size_t aBundleLevel) {
+    MOZ_ASSERT(aBundleLevel != SIZE_MAX);
+    mBundleLevel = aBundleLevel;
+  }
 };
 
 } // namespace mozilla
 
 #endif
--- a/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
@@ -323,18 +323,18 @@ MediaPipelineFactory::GetTransportParame
     const JsepTrack& aTrack,
     size_t* aLevelOut,
     RefPtr<TransportFlow>* aRtpOut,
     RefPtr<TransportFlow>* aRtcpOut,
     nsAutoPtr<MediaPipelineFilter>* aFilterOut)
 {
   *aLevelOut = aTrackPair.mLevel;
 
-  size_t transportLevel = aTrackPair.mBundleLevel.isSome() ?
-                          *aTrackPair.mBundleLevel :
+  size_t transportLevel = aTrackPair.HasBundleLevel() ?
+                          aTrackPair.BundleLevel() :
                           aTrackPair.mLevel;
 
   nsresult rv = CreateOrGetTransportFlow(
       transportLevel, false, *aTrackPair.mRtpTransport, aRtpOut);
   if (NS_FAILED(rv)) {
     return rv;
   }
   MOZ_ASSERT(aRtpOut);
@@ -343,17 +343,17 @@ MediaPipelineFactory::GetTransportParame
     rv = CreateOrGetTransportFlow(
         transportLevel, true, *aTrackPair.mRtcpTransport, aRtcpOut);
     if (NS_FAILED(rv)) {
       return rv;
     }
     MOZ_ASSERT(aRtcpOut);
   }
 
-  if (aTrackPair.mBundleLevel.isSome()) {
+  if (aTrackPair.HasBundleLevel()) {
     bool receiving = aTrack.GetDirection() == sdp::kRecv;
 
     *aFilterOut = new MediaPipelineFilter;
 
     if (receiving) {
       // Add remote SSRCs so we can distinguish which RTP packets actually
       // belong to this pipeline (also RTCP sender reports).
       for (unsigned int ssrc : aTrack.GetSsrcs()) {
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -1154,18 +1154,18 @@ PeerConnectionImpl::GetDatachannelParame
           *channels = codec->mChannels;
         } else {
           *channels = WEBRTC_DATACHANNEL_STREAMS_DEFAULT;
         }
         *localport =
           static_cast<const JsepApplicationCodecDescription*>(codec)->mLocalPort;
         *remoteport =
           static_cast<const JsepApplicationCodecDescription*>(codec)->mRemotePort;
-        if (trackPair.mBundleLevel.isSome()) {
-          *level = static_cast<uint16_t>(*trackPair.mBundleLevel);
+        if (trackPair.HasBundleLevel()) {
+          *level = static_cast<uint16_t>(trackPair.BundleLevel());
         } else {
           *level = static_cast<uint16_t>(trackPair.mLevel);
         }
         return NS_OK;
       }
     }
   }
 
@@ -2462,18 +2462,18 @@ PeerConnectionImpl::InsertDTMF(mozilla::
     MOZ_ASSERT(state->mSendTimer);
   }
   MOZ_ASSERT(state);
 
   auto trackPairs = mJsepSession->GetNegotiatedTrackPairs();
   state->mLevel = -1;
   for (auto& trackPair : trackPairs) {
     if (state->mTrackId.EqualsASCII(trackPair.mSending->GetTrackId().c_str())) {
-      if (trackPair.mBundleLevel.isSome()) {
-        state->mLevel = *trackPair.mBundleLevel;
+      if (trackPair.HasBundleLevel()) {
+        state->mLevel = trackPair.BundleLevel();
       } else {
         state->mLevel = trackPair.mLevel;
       }
       break;
     }
   }
 
   state->mTones = tones;