Bug 1353028: Fix broken extmap validation logic. r=drno
authorByron Campen [:bwc] <docfaraday@gmail.com>
Mon, 03 Apr 2017 15:05:31 -0500
changeset 351335 9a66f1e840d09cb9419d86425fd18bcb6e06393a
parent 351334 aa550815a5ec19a3ab2d94a702a1e05f73a4bf50
child 351336 f659d86aa293982f335d4b11c8ab99f0089c5789
push id40139
push userbcampen@mozilla.com
push dateWed, 05 Apr 2017 18:25:54 +0000
treeherderautoland@9a66f1e840d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno
bugs1353028
milestone55.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 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.