Bug 1353028: Fix broken extmap validation logic. r?drno draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Mon, 03 Apr 2017 15:05:31 -0500
changeset 555243 14b92d02debe778ac40c4bee9e3e63fa7b9096ee
parent 553172 6ea713ccc9abea93126423fefb855d0e051c95e2
child 622577 4cf5349dddf4867d292d1d6069f876f0a70ec0f1
push id52204
push userbcampen@mozilla.com
push dateMon, 03 Apr 2017 22:21:46 +0000
reviewersdrno
bugs1353028
milestone55.0a1
Bug 1353028: Fix broken extmap validation logic. r?drno MozReview-Commit-ID: Egvr4ppNZ5Q
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepTrack.cpp
media/webrtc/signaling/src/sdp/SdpAttribute.h
media/webrtc/signaling/src/sdp/SdpHelper.cpp
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -308,18 +308,18 @@ JsepSessionImpl::SetParameters(const std
   // Add RtpStreamId Extmap
   // SdpDirectionAttribute::Direction is a bitmask
   SdpDirectionAttribute::Direction addVideoExt = SdpDirectionAttribute::kInactive;
   SdpDirectionAttribute::Direction addAudioExt = SdpDirectionAttribute::kInactive;
   for (auto constraintEntry: constraints) {
     if (constraintEntry.rid != "") {
       switch (it->mTrack->GetMediaType()) {
         case SdpMediaSection::kVideo: {
-           addVideoExt = static_cast<SdpDirectionAttribute::Direction>(addVideoExt
-                                                                       | it->mTrack->GetDirection());
+          addVideoExt = static_cast<SdpDirectionAttribute::Direction>(addVideoExt
+                                                                      | it->mTrack->GetDirection());
           break;
         }
         case SdpMediaSection::kAudio: {
           addAudioExt = static_cast<SdpDirectionAttribute::Direction>(addAudioExt
                                                                       | it->mTrack->GetDirection());
           break;
         }
         default: {
@@ -2066,17 +2066,18 @@ JsepSessionImpl::ValidateAnswer(const Sd
         JSEP_SET_ERROR("Answer adds extmap attributes to level " << i);
         return NS_ERROR_INVALID_ARG;
       }
 
       for (const auto& ansExt : answerAttrs.GetExtmap().mExtmaps) {
         bool found = false;
         for (const auto& offExt : offerAttrs.GetExtmap().mExtmaps) {
           if (ansExt.extensionname == offExt.extensionname) {
-            if ((ansExt.direction | ~offExt.direction) != ansExt.direction) {
+            if ((ansExt.direction & reverse(offExt.direction))
+                  != ansExt.direction) {
               JSEP_SET_ERROR("Answer has inconsistent direction on extmap "
                              "attribute at level " << i << " ("
                              << ansExt.extensionname << "). Offer had "
                              << offExt.direction << ", answer had "
                              << ansExt.direction << ".");
               return NS_ERROR_INVALID_ARG;
             }
 
--- a/media/webrtc/signaling/src/jsep/JsepTrack.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepTrack.cpp
@@ -464,17 +464,17 @@ JsepTrack::Negotiate(const SdpMediaSecti
 
   CreateEncodings(remote, negotiatedCodecs.values, negotiatedDetails.get());
 
   if (answer.GetAttributeList().HasAttribute(SdpAttribute::kExtmapAttribute)) {
     for (auto& extmapAttr : answer.GetAttributeList().GetExtmap().mExtmaps) {
       SdpDirectionAttribute::Direction direction = extmapAttr.direction;
       if (&remote == &answer) {
         // Answer is remote, we need to flip this.
-        direction = ~direction;
+        direction = reverse(direction);
       }
 
       if (direction & mDirection) {
         negotiatedDetails->mExtmap[extmapAttr.extensionname] = extmapAttr;
       }
     }
   }
 
--- a/media/webrtc/signaling/src/sdp/SdpAttribute.h
+++ b/media/webrtc/signaling/src/sdp/SdpAttribute.h
@@ -219,17 +219,17 @@ inline std::ostream& operator<<(std::ost
     default:
       MOZ_ASSERT(false);
       os << "?";
   }
   return os;
 }
 
 inline SdpDirectionAttribute::Direction
-operator~(SdpDirectionAttribute::Direction d)
+reverse(SdpDirectionAttribute::Direction d)
 {
   switch (d) {
     case SdpDirectionAttribute::Direction::kInactive:
       return SdpDirectionAttribute::Direction::kInactive;
     case SdpDirectionAttribute::Direction::kSendonly:
       return SdpDirectionAttribute::Direction::kRecvonly;
     case SdpDirectionAttribute::Direction::kRecvonly:
       return SdpDirectionAttribute::Direction::kSendonly;
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -750,17 +750,18 @@ SdpHelper::AddCommonExtmaps(
   for (const auto& theirExt : theirExtmap) {
     for (const auto& ourExt : localExtensions) {
       if (theirExt.extensionname != ourExt.extensionname) {
         continue;
       }
 
       auto negotiatedExt = theirExt;
 
-      negotiatedExt.direction = ~negotiatedExt.direction & ourExt.direction;
+      negotiatedExt.direction =
+        reverse(negotiatedExt.direction) & ourExt.direction;
       if (negotiatedExt.direction ==
             SdpDirectionAttribute::Direction::kInactive) {
         continue;
       }
 
       // RFC 5285 says that ids >= 4096 can be used by the offerer to
       // force the answerer to pick, otherwise the value in the offer is
       // used.