Bug 1358224 - pt 2 - change to RTP stream id filtering on simulcast mochitests. r=drno

The simulcast mochitests setup the receiving PeerConnection to receive
simulcast video streams which Firefox doesn't really support.  Without
a test media server, this is about the best we can do and still test
simulcast.

Unfortunately the two simulcast streams arriving with different ssrcs
(as expect) exercises code we have to deal with some services switching
ssrcs midstream.  In the tests, this causes intermittent failures
because the test is waiting to receive a certain ssrc, and the receiving
VideoConduit has switched to the other ssrc.

This change adds the ability to filter on RID at the MediaPipeline level,
which we can setup prior to media flowing.  This avoids the ssrc switching
issue since the VideoConduit only receives one ssrc until we change the
RID filter to the second RID.  At that point, the VideoConduit sees a new
ssrc and the switching code works as intended.

The modified mochitests setup the RTP stream id header extension, and then
filter on each of the RTP stream ids in turn.

MozReview-Commit-ID: KApfaxMX8rl

--HG--
extra : rebase_source : d7ae88d9675acd7b3700f342ca6a68d0bbb0ced5

dom/media/tests/mochitest/mochitest.ini

@@ -192,12 +192,10 @@ skip-if = toolkit == 'android'
 skip-if = toolkit == 'android'
 [test_peerConnection_scaleResolution.html]
 skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator)
-# disable test_peerConnection_simulcastOffer.html for Bug 1351590
-#[test_peerConnection_simulcastOffer.html]
-#skip-if = android_version # no simulcast support on android
-# disable test_peerConnection_simulcastAnswer.html for Bug 1351531
-#[test_peerConnection_simulcastAnswer.html]
-#skip-if = android_version # no simulcast support on android
+[test_peerConnection_simulcastOffer.html]
+skip-if = android_version # no simulcast support on android
+[test_peerConnection_simulcastAnswer.html]
+skip-if = android_version # no simulcast support on android
 #[test_peerConnection_relayOnly.html]
 [test_peerConnection_callbacks.html]
 skip-if = toolkit == 'android' # android(Bug 1189784, timeouts on 4.3 emulator)

dom/media/tests/mochitest/test_peerConnection_simulcastAnswer.html

@@ -16,12 +16,20 @@
   var test;
   var pushPrefs = (...p) => SpecialPowers.pushPrefEnv({set: p});
 
-  function selectRecvSsrc(pc, index) {
+  function addRIDExtension(pc, extensionId) {
     var receivers = pc._pc.getReceivers();
     is(receivers.length, 1, "We have exactly one RTP receiver");
     var receiver = receivers[0];
 
-    SpecialPowers.wrap(pc._pc).mozSelectSsrc(receiver, index);
+    SpecialPowers.wrap(pc._pc).mozAddRIDExtension(receiver, extensionId);
+  }
+
+  function selectRecvRID(pc, rid) {
+    var receivers = pc._pc.getReceivers();
+    is(receivers.length, 1, "We have exactly one RTP receiver");
+    var receiver = receivers[0];
+
+    SpecialPowers.wrap(pc._pc).mozAddRIDFilter(receiver, rid);
   }
 
   runNetworkTest(() =>
@@ -76,11 +84,17 @@
         }
       ]);
 
-      test.chain.insertAfter('PC_REMOTE_WAIT_FOR_MEDIA_FLOW',[
-        function PC_REMOTE_SET_RTP_FIRST_RID(test) {
-          // Cause pcLocal to filter out everything but the first SSRC. This
-          // lets only one of the simulcast streams through.
-          selectRecvSsrc(test.pcLocal, 0);
+      // do this after set remote description so the MediaPipeline
+      // has been created.
+      test.chain.insertAfter('PC_LOCAL_SET_REMOTE_DESCRIPTION',[
+        function PC_LOCAL_SET_RTP_FIRST_RID(test) {
+          var extmap_id = test._local_offer.sdp.match(
+              "a=extmap:([0-9+])/recvonly urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id");
+          ok(extmap_id, "Local offer has extmap id for simulcast: " + extmap_id[1]);
+          // Cause pcLocal to filter out everything but RID "bar", only
+          // allowing one of the simulcast streams through.
+          addRIDExtension(test.pcLocal, extmap_id[1]);
+          selectRecvRID(test.pcLocal, "bar");
         }
       ]);
 
@@ -101,9 +115,9 @@
           is(vremote.videoHeight, vlocal.videoHeight / 2, "sink is 1/2 height of source");
         },
         function PC_LOCAL_SET_RTP_SECOND_RID(test) {
-          // Now, cause pcLocal to filter out everything but the second SSRC.
-          // This lets only the other simulcast stream through.
-          selectRecvSsrc(test.pcLocal, 1);
+          // Now, cause pcLocal to filter out everything but RID "foo", only
+          // allowing the other simulcast stream through.
+          selectRecvRID(test.pcLocal, "foo");
         },
         function PC_LOCAL_WAIT_FOR_SECOND_MEDIA_FLOW(test) {
           return test.pcLocal.waitForMediaFlow();

dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html

@@ -16,12 +16,20 @@
   var test;
   var pushPrefs = (...p) => SpecialPowers.pushPrefEnv({set: p});
 
-  function selectRecvSsrc(pc, index) {
+  function addRIDExtension(pc, extensionId) {
     var receivers = pc._pc.getReceivers();
     is(receivers.length, 1, "We have exactly one RTP receiver");
     var receiver = receivers[0];
 
-    SpecialPowers.wrap(pc._pc).mozSelectSsrc(receiver, index);
+    SpecialPowers.wrap(pc._pc).mozAddRIDExtension(receiver, extensionId);
+  }
+
+  function selectRecvRID(pc, rid) {
+    var receivers = pc._pc.getReceivers();
+    is(receivers.length, 1, "We have exactly one RTP receiver");
+    var receiver = receivers[0];
+
+    SpecialPowers.wrap(pc._pc).mozAddRIDFilter(receiver, rid);
   }
 
   runNetworkTest(() =>
@@ -68,11 +76,17 @@
         }
       ]);
 
-      test.chain.insertAfter('PC_REMOTE_WAIT_FOR_MEDIA_FLOW',[
+      // do this after set local description so the MediaPipeline
+      // has been created.
+      test.chain.insertAfter('PC_REMOTE_SET_LOCAL_DESCRIPTION',[
         function PC_REMOTE_SET_RTP_FIRST_RID(test) {
-          // Cause pcRemote to filter out everything but the first SSRC. This
-          // lets only one of the simulcast streams through.
-          selectRecvSsrc(test.pcRemote, 0);
+          var extmap_id = test.originalOffer.sdp.match(
+              "a=extmap:([0-9+])/sendonly urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id");
+          ok(extmap_id, "Original offer has extmap id for simulcast: " + extmap_id[1]);
+          // Cause pcRemote to filter out everything but RID "foo", only
+          // allowing one of the simulcast streams through.
+          addRIDExtension(test.pcRemote, extmap_id[1]);
+          selectRecvRID(test.pcRemote, "foo");
         }
       ]);
 
@@ -89,13 +103,13 @@
           ok(vremote, "Should have remote video element for pcRemote");
           ok(vlocal.videoWidth > 0, "source width is positive");
           ok(vlocal.videoHeight > 0, "source height is positive");
-          is(vremote.videoWidth, vlocal.videoWidth / 2, "sink is 1/2 width of source");
-          is(vremote.videoHeight, vlocal.videoHeight / 2, "sink is 1/2 height of source");
+          is(vremote.videoWidth, vlocal.videoWidth, "sink is same width as source");
+          is(vremote.videoHeight, vlocal.videoHeight,  "sink is same height as source");
         },
         function PC_REMOTE_SET_RTP_SECOND_RID(test) {
-          // Now, cause pcRemote to filter out everything but the second SSRC.
-          // This lets only the other simulcast stream through.
-          selectRecvSsrc(test.pcRemote, 1);
+          // Now, cause pcRemote to filter out everything but RID "bar", only
+          // allowing the other simulcast stream through.
+          selectRecvRID(test.pcRemote, "bar");
         },
         function PC_REMOTE_WAIT_FOR_SECOND_MEDIA_FLOW(test) {
           return test.pcRemote.waitForMediaFlow();
@@ -119,8 +133,8 @@
           ok(vremote, "Should have remote video element for pcRemote");
           ok(vlocal.videoWidth > 0, "source width is positive");
           ok(vlocal.videoHeight > 0, "source height is positive");
-          is(vremote.videoWidth, vlocal.videoWidth, "sink is same width as source");
-          is(vremote.videoHeight, vlocal.videoHeight,  "sink is same height as source");
+          is(vremote.videoWidth, vlocal.videoWidth / 2, "sink is 1/2 width of source");
+          is(vremote.videoHeight, vlocal.videoHeight / 2, "sink is 1/2 height of source");
         },
       ]);
 

media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp

@@ -735,6 +735,40 @@ MediaPipeline::UpdateTransport_s(int level,
   }
 }
 
+void
+MediaPipeline::AddRIDExtension_m(size_t extension_id)
+{
+  RUN_ON_THREAD(sts_thread_,
+                WrapRunnable(this,
+                             &MediaPipeline::AddRIDExtension_s,
+                             extension_id),
+                NS_DISPATCH_NORMAL);
+}
+
+void
+MediaPipeline::AddRIDExtension_s(size_t extension_id)
+{
+  rtp_parser_->RegisterRtpHeaderExtension(webrtc::kRtpExtensionRtpStreamId,
+                                          extension_id);
+}
+
+void
+MediaPipeline::AddRIDFilter_m(const std::string& rid)
+{
+  RUN_ON_THREAD(sts_thread_,
+                WrapRunnable(this,
+                             &MediaPipeline::AddRIDFilter_s,
+                             rid),
+                NS_DISPATCH_NORMAL);
+}
+
+void
+MediaPipeline::AddRIDFilter_s(const std::string& rid)
+{
+  filter_ = new MediaPipelineFilter;
+  filter_->AddRemoteRtpStreamId(rid);
+}
+
 void
 MediaPipeline::SelectSsrc_m(size_t ssrc_index)
 {

media/webrtc/signaling/src/mediapipeline/MediaPipeline.h

@@ -118,6 +118,14 @@ class MediaPipeline : public sigslot::has_slots<> {
   // everything but the nth ssrc
   void SelectSsrc_m(size_t ssrc_index);
   void SelectSsrc_s(uint32_t ssrc);
+  // Used only for testing; adds RTP header extension for RTP Stream Id with
+  // the given id.
+  void AddRIDExtension_m(size_t extension_id);
+  void AddRIDExtension_s(size_t extension_id);
+  // Used only for testing; installs a MediaPipelineFilter that filters
+  // everything but the given RID
+  void AddRIDFilter_m(const std::string& rid);
+  void AddRIDFilter_s(const std::string& rid);
 
   virtual Direction direction() const { return direction_; }
   virtual const std::string& trackid() const { return track_id_; }

media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp

@@ -31,6 +31,12 @@ bool MediaPipelineFilter::Filter(const webrtc::RTPHeader& header,
     return false;
   }
 
+  if (header.extension.hasRID &&
+      remote_rid_set_.size() &&
+      remote_rid_set_.count(header.extension.rid)) {
+    return true;
+  }
+
   if (remote_ssrc_set_.count(header.ssrc)) {
     return true;
   }
@@ -50,6 +56,10 @@ void MediaPipelineFilter::AddRemoteSSRC(uint32_t ssrc) {
   remote_ssrc_set_.insert(ssrc);
 }
 
+void MediaPipelineFilter::AddRemoteRtpStreamId(const std::string& rtp_strm_id) {
+  remote_rid_set_.insert(rtp_strm_id);
+}
+
 void MediaPipelineFilter::AddUniquePT(uint8_t payload_type) {
   payload_type_set_.insert(payload_type);
 }

media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h

@@ -12,6 +12,7 @@
 
 #include <cstddef>
 #include <stdint.h>
+#include <string>
 
 #include <set>
 
@@ -57,6 +58,7 @@ class MediaPipelineFilter {
   bool FilterSenderReport(const unsigned char* data, size_t len) const;
 
   void AddRemoteSSRC(uint32_t ssrc);
+  void AddRemoteRtpStreamId(const std::string& rtp_strm_id);
 
   // When a payload type id is unique to our media section, add it here.
   void AddUniquePT(uint8_t payload_type);
@@ -78,6 +80,7 @@ class MediaPipelineFilter {
   // for readability.
   std::set<uint32_t> remote_ssrc_set_;
   std::set<uint8_t> payload_type_set_;
+  std::set<std::string> remote_rid_set_;
 };
 
 } // end namespace mozilla

media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp

@@ -2348,9 +2348,8 @@ PeerConnectionImpl::AddTrack(MediaStreamTrack& aTrack,
   return NS_OK;
 }
 
-nsresult
-PeerConnectionImpl::SelectSsrc(MediaStreamTrack& aRecvTrack,
-                               unsigned short aSsrcIndex)
+RefPtr<MediaPipeline>
+PeerConnectionImpl::GetMediaPipelineForTrack(MediaStreamTrack& aRecvTrack)
 {
   for (size_t i = 0; i < mMedia->RemoteStreamsLength(); ++i) {
     if (mMedia->GetRemoteStreamByIndex(i)->GetMediaStream()->
@@ -2359,10 +2358,22 @@ PeerConnectionImpl::SelectSsrc(MediaStreamTrack& aRecvTrack,
       std::string trackId = PeerConnectionImpl::GetTrackId(aRecvTrack);
       auto it = pipelines.find(trackId);
       if (it != pipelines.end()) {
-        it->second->SelectSsrc_m(aSsrcIndex);
+        return it->second;
       }
     }
   }
+
+  return nullptr;
+}
+
+nsresult
+PeerConnectionImpl::SelectSsrc(MediaStreamTrack& aRecvTrack,
+                               unsigned short aSsrcIndex)
+{
+  RefPtr<MediaPipeline> pipeline = GetMediaPipelineForTrack(aRecvTrack);
+  if (pipeline) {
+    pipeline->SelectSsrc_m(aSsrcIndex);
+  }
   return NS_OK;
 }
 
@@ -2370,6 +2381,10 @@ nsresult
 PeerConnectionImpl::AddRIDExtension(MediaStreamTrack& aRecvTrack,
                                     unsigned short aExtensionId)
 {
+  RefPtr<MediaPipeline> pipeline = GetMediaPipelineForTrack(aRecvTrack);
+  if (pipeline) {
+    pipeline->AddRIDExtension_m(aExtensionId);
+  }
   return NS_OK;
 }
 
@@ -2377,6 +2392,10 @@ nsresult
 PeerConnectionImpl::AddRIDFilter(MediaStreamTrack& aRecvTrack,
                                  const nsAString& aRid)
 {
+  RefPtr<MediaPipeline> pipeline = GetMediaPipelineForTrack(aRecvTrack);
+  if (pipeline) {
+    pipeline->AddRIDFilter_m(NS_ConvertUTF16toUTF8(aRid).get());
+  }
   return NS_OK;
 }
 

media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h

@@ -665,6 +665,9 @@ private:
     return true;
   }
 
+  RefPtr<MediaPipeline> GetMediaPipelineForTrack(
+      dom::MediaStreamTrack& aRecvTrack);
+
   nsresult GetTimeSinceEpoch(DOMHighResTimeStamp *result);
 
   // Shut down media - called on main thread only