Bug 786234 - Part 2: Implementation of the filtering logic itself plus a unit-test. r=abr
☠☠ backed out by 69602085b4f0 ☠ ☠
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 19 Dec 2013 16:19:05 -0800
changeset 187412 4f7eac2b16ca293cbbfab61c1220b65990467186
parent 187411 724240c068509996b17a47e7d1ddb7917b2ae9ed
child 187413 1dea344fa608897709f2e674ee726ec858c21da2
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersabr
bugs786234
milestone30.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 786234 - Part 2: Implementation of the filtering logic itself plus a unit-test. r=abr
media/webrtc/signaling/signaling.gyp
media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h
media/webrtc/signaling/test/mediapipeline_unittest.cpp
media/webrtc/signaling/test/moz.build
--- a/media/webrtc/signaling/signaling.gyp
+++ b/media/webrtc/signaling/signaling.gyp
@@ -150,16 +150,18 @@
         './src/peerconnection/PeerConnectionImpl.cpp',
         './src/peerconnection/PeerConnectionImpl.h',
         './src/peerconnection/PeerConnectionMedia.cpp',
         './src/peerconnection/PeerConnectionMedia.h',
 
         # Media pipeline
         './src/mediapipeline/MediaPipeline.h',
         './src/mediapipeline/MediaPipeline.cpp',
+        './src/mediapipeline/MediaPipelineFilter.h',
+        './src/mediapipeline/MediaPipelineFilter.cpp',
         './src/mediapipeline/SrtpFlow.h',
         './src/mediapipeline/SrtpFlow.cpp',
       ],
 
       #
       # DEFINES
       #
 
new file mode 100644
--- /dev/null
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
@@ -0,0 +1,88 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: softtabstop=2:shiftwidth=2:expandtab
+ * */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Original author: bcampen@mozilla.com
+
+#include "MediaPipelineFilter.h"
+
+#include "webrtc/modules/interface/module_common_types.h"
+
+namespace mozilla {
+
+MediaPipelineFilter::MediaPipelineFilter() : correlator_(0) {
+}
+
+bool MediaPipelineFilter::Filter(const webrtc::RTPHeader& header,
+                                 uint32_t correlator) {
+  if (correlator) {
+    // This special correlator header takes precedence. It also lets us learn
+    // about SSRC mappings if we don't know about them yet.
+    if (correlator == correlator_) {
+      AddRemoteSSRC(header.ssrc);
+      return true;
+    } else {
+      // Some other stream; it is possible that an SSRC has moved, so make sure
+      // we don't have that SSRC in our filter any more.
+      remote_ssrc_set_.erase(header.ssrc);
+      return false;
+    }
+  }
+
+  if (remote_ssrc_set_.count(header.ssrc)) {
+    return true;
+  }
+
+  // Last ditch effort...
+  if (payload_type_set_.count(header.payloadType)) {
+    // Actual match. We need to update the ssrc map so we can route rtcp
+    // sender reports correctly (these use a different payload-type field)
+    AddRemoteSSRC(header.ssrc);
+    return true;
+  }
+
+  return false;
+}
+
+bool MediaPipelineFilter::FilterRTCP(uint32_t ssrc) {
+  return remote_ssrc_set_.count(ssrc) != 0;
+}
+
+bool MediaPipelineFilter::FilterRTCPReceiverReport(uint32_t ssrc) {
+  return local_ssrc_set_.count(ssrc) != 0;
+}
+
+void MediaPipelineFilter::AddLocalSSRC(uint32_t ssrc) {
+  local_ssrc_set_.insert(ssrc);
+}
+
+void MediaPipelineFilter::AddRemoteSSRC(uint32_t ssrc) {
+  remote_ssrc_set_.insert(ssrc);
+}
+
+void MediaPipelineFilter::AddUniquePT(uint8_t payload_type) {
+  payload_type_set_.insert(payload_type);
+}
+
+void MediaPipelineFilter::SetCorrelator(uint32_t correlator) {
+  correlator_ = correlator;
+}
+
+void MediaPipelineFilter::IncorporateRemoteDescription(
+    const MediaPipelineFilter& remote_filter) {
+  // Update SSRCs; we completely replace the remote SSRCs, since this could be
+  // renegotiation. We leave our SSRCs alone, though.
+  if (!remote_filter.remote_ssrc_set_.empty()) {
+    remote_ssrc_set_ = remote_filter.remote_ssrc_set_;
+  }
+
+  // We do not mess with the payload types or correlator here, since the remote
+  // SDP doesn't tell us anything about what we will be receiving.
+}
+
+} // end namespace mozilla
+
+
new file mode 100644
--- /dev/null
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h
@@ -0,0 +1,83 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: softtabstop=2:shiftwidth=2:expandtab
+ * */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Original author: bcampen@mozilla.com
+
+#ifndef mediapipelinefilter_h__
+#define mediapipelinefilter_h__
+
+#include <cstddef>
+#include <stdint.h>
+
+#include <set>
+
+namespace webrtc {
+class RTPHeader;
+}
+
+namespace mozilla {
+
+// A class that handles the work of filtering RTP packets that arrive at a
+// MediaPipeline. This is primarily important for the use of BUNDLE (ie;
+// multiple m-lines share the same RTP stream). There are three ways that this
+// can work;
+//
+// 1) In our SDP, we include a media-level extmap parameter with a unique
+//    integer of our choosing, with the hope that the other side will include
+//    this value in a header in the first few RTP packets it sends us. This
+//    allows us to perform correlation in cases where the other side has not
+//    informed us of the ssrcs it will be sending (either because it did not
+//    include them in its SDP, or their SDP has not arrived yet)
+//    and also gives us the opportunity to learn SSRCs from packets so adorned.
+//
+// 2) If the remote endpoint includes SSRC media-level attributes in its SDP,
+//    we can simply use this information to populate the filter. The only
+//    shortcoming here is when RTP packets arrive before the answer does. See
+//    above.
+//
+// 3) As a fallback, we can try to use payload type IDs to perform correlation,
+//    but only when the type id is unique to this media section.
+//    This too allows us to learn about SSRCs (mostly useful for filtering
+//    any RTCP packets that follow).
+class MediaPipelineFilter {
+ public:
+  MediaPipelineFilter();
+
+  // Checks whether this packet passes the filter, possibly updating the filter
+  // in the process (if the correlator or payload types are used, they can teach
+  // the filter about ssrcs)
+  bool Filter(const webrtc::RTPHeader& header, uint32_t correlator = 0);
+
+  // RTCP doesn't have things like the RTP correlator, and uses its own
+  // payload types too.
+  // RTCP receiver reports use _our_ ssrcs
+  bool FilterRTCPReceiverReport(uint32_t ssrc);
+  // The rest of RTCP uses their ssrcs
+  bool FilterRTCP(uint32_t ssrc);
+
+  void AddLocalSSRC(uint32_t ssrc);
+  void AddRemoteSSRC(uint32_t ssrc);
+
+  // When a payload type id is unique to our media section, add it here.
+  void AddUniquePT(uint8_t payload_type);
+  void SetCorrelator(uint32_t correlator);
+
+  void IncorporateRemoteDescription(const MediaPipelineFilter& remote_filter);
+
+ private:
+  uint32_t correlator_;
+  // The number of filters we manage here is quite small, so I am optimizing
+  // for readability.
+  std::set<uint32_t> remote_ssrc_set_;
+  std::set<uint32_t> local_ssrc_set_;
+  std::set<uint8_t> payload_type_set_;
+};
+
+} // end namespace mozilla
+
+#endif // mediapipelinefilter_h__
+
--- a/media/webrtc/signaling/test/mediapipeline_unittest.cpp
+++ b/media/webrtc/signaling/test/mediapipeline_unittest.cpp
@@ -17,26 +17,29 @@
 
 #include "dtlsidentity.h"
 #include "mozilla/RefPtr.h"
 #include "FakeMediaStreams.h"
 #include "FakeMediaStreamsImpl.h"
 #include "MediaConduitErrors.h"
 #include "MediaConduitInterface.h"
 #include "MediaPipeline.h"
+#include "MediaPipelineFilter.h"
 #include "runnable_utils.h"
 #include "transportflow.h"
 #include "transportlayerprsock.h"
 #include "transportlayerdtls.h"
 #include "mozilla/SyncRunnable.h"
 
 
 #include "mtransport_test_utils.h"
 #include "runnable_utils.h"
 
+#include "webrtc/modules/interface/module_common_types.h"
+
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
 
 using namespace mozilla;
 MOZ_MTLOG_MODULE("mediapipeline")
 
 MtransportTestUtils *test_utils;
@@ -337,16 +340,169 @@ class MediaPipelineTest : public ::testi
 
 protected:
   PRFileDesc *rtp_fds_[2];
   PRFileDesc *rtcp_fds_[2];
   TestAgentSend p1_;
   TestAgentReceive p2_;
 };
 
+class MediaPipelineFilterTest : public ::testing::Test {
+  public:
+    bool Filter(MediaPipelineFilter& filter,
+                int32_t correlator,
+                uint32_t ssrc,
+                uint8_t payload_type) {
+
+      webrtc::RTPHeader header;
+      header.ssrc = ssrc;
+      header.payloadType = payload_type;
+      return filter.Filter(header, correlator);
+    }
+};
+
+TEST_F(MediaPipelineFilterTest, TestConstruct) {
+  MediaPipelineFilter filter;
+}
+
+TEST_F(MediaPipelineFilterTest, TestDefault) {
+  MediaPipelineFilter filter;
+  ASSERT_FALSE(Filter(filter, 0, 233, 110));
+}
+
+TEST_F(MediaPipelineFilterTest, TestSSRCFilter) {
+  MediaPipelineFilter filter;
+  filter.AddRemoteSSRC(555);
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_FALSE(Filter(filter, 0, 556, 110));
+}
+
+TEST_F(MediaPipelineFilterTest, TestSSRCFilterRTCP) {
+  MediaPipelineFilter filter;
+  filter.AddRemoteSSRC(555);
+  ASSERT_TRUE(filter.FilterRTCP(555));
+  ASSERT_FALSE(filter.FilterRTCP(556));
+  ASSERT_FALSE(filter.FilterRTCPReceiverReport(555));
+}
+
+TEST_F(MediaPipelineFilterTest, TestSSRCFilterReceiverReport) {
+  MediaPipelineFilter filter;
+  filter.AddLocalSSRC(555);
+  ASSERT_TRUE(filter.FilterRTCPReceiverReport(555));
+  ASSERT_FALSE(filter.FilterRTCPReceiverReport(556));
+  ASSERT_FALSE(filter.FilterRTCP(555));
+}
+
+TEST_F(MediaPipelineFilterTest, TestCorrelatorFilter) {
+  MediaPipelineFilter filter;
+  filter.SetCorrelator(7777);
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+  ASSERT_FALSE(Filter(filter, 7778, 556, 110));
+  // This should also have resulted in the SSRC 555 being added to the filter
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_FALSE(Filter(filter, 0, 556, 110));
+
+  ASSERT_TRUE(filter.FilterRTCP(555));
+}
+
+TEST_F(MediaPipelineFilterTest, TestPayloadTypeFilter) {
+  MediaPipelineFilter filter;
+  filter.AddUniquePT(110);
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_FALSE(Filter(filter, 0, 556, 111));
+}
+
+TEST_F(MediaPipelineFilterTest, TestPayloadTypeFilterSSRCUpdate) {
+  MediaPipelineFilter filter;
+  filter.AddUniquePT(110);
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_TRUE(filter.FilterRTCP(555));
+  ASSERT_FALSE(filter.FilterRTCP(556));
+  ASSERT_FALSE(filter.FilterRTCPReceiverReport(555));
+}
+
+TEST_F(MediaPipelineFilterTest, TestAnswerAddsSSRCs) {
+  MediaPipelineFilter filter;
+  filter.SetCorrelator(7777);
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+  ASSERT_FALSE(Filter(filter, 7778, 556, 110));
+  // This should also have resulted in the SSRC 555 being added to the filter
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_FALSE(Filter(filter, 0, 556, 110));
+
+  // This sort of thing can happen when getting an answer with SSRC attrs
+  // The answer will not contain the correlator.
+  MediaPipelineFilter filter2;
+  filter2.AddRemoteSSRC(555);
+  filter2.AddRemoteSSRC(556);
+  filter2.AddRemoteSSRC(557);
+
+  filter.IncorporateRemoteDescription(filter2);
+
+  // Ensure that the old SSRC still works.
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+
+  // Ensure that the new SSRCs work.
+  ASSERT_TRUE(Filter(filter, 0, 556, 110));
+  ASSERT_TRUE(Filter(filter, 0, 557, 110));
+
+  // Ensure that the correlator continues to work
+  ASSERT_TRUE(Filter(filter, 7777, 558, 110));
+}
+
+TEST_F(MediaPipelineFilterTest, TestSSRCMovedWithSDP) {
+  MediaPipelineFilter filter;
+  filter.SetCorrelator(7777);
+  filter.AddUniquePT(111);
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+
+  MediaPipelineFilter filter2;
+  filter2.AddRemoteSSRC(556);
+
+  filter.IncorporateRemoteDescription(filter2);
+
+  // Ensure that the old SSRC has been removed.
+  ASSERT_FALSE(Filter(filter, 0, 555, 110));
+
+  // Ensure that the new SSRC works.
+  ASSERT_TRUE(Filter(filter, 0, 556, 110));
+
+  // Ensure that the correlator continues to work
+  ASSERT_TRUE(Filter(filter, 7777, 558, 110));
+
+  // Ensure that the payload type mapping continues to work
+  ASSERT_TRUE(Filter(filter, 0, 559, 111));
+}
+
+TEST_F(MediaPipelineFilterTest, TestSSRCMovedWithCorrelator) {
+  MediaPipelineFilter filter;
+  filter.SetCorrelator(7777);
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+  ASSERT_TRUE(Filter(filter, 0, 555, 110));
+  ASSERT_FALSE(Filter(filter, 7778, 555, 110));
+  ASSERT_FALSE(Filter(filter, 0, 555, 110));
+}
+
+TEST_F(MediaPipelineFilterTest, TestRemoteSDPNoSSRCs) {
+  // If the remote SDP doesn't have SSRCs, right now this is a no-op and
+  // there is no point of even incorporating a filter, but we make the behavior
+  // consistent to avoid confusion.
+  MediaPipelineFilter filter;
+  filter.SetCorrelator(7777);
+  filter.AddUniquePT(111);
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+
+  MediaPipelineFilter filter2;
+
+  filter.IncorporateRemoteDescription(filter2);
+
+  // Ensure that the old SSRC still works.
+  ASSERT_TRUE(Filter(filter, 7777, 555, 110));
+}
+
 TEST_F(MediaPipelineTest, TestAudioSendNoMux) {
   TestAudioSend(false);
 }
 
 TEST_F(MediaPipelineTest, TestAudioSendMux) {
   TestAudioSend(true);
 }
 
--- a/media/webrtc/signaling/test/moz.build
+++ b/media/webrtc/signaling/test/moz.build
@@ -44,16 +44,17 @@ LOCAL_INCLUDES += [
     '/media/webrtc/signaling/src/media',
     '/media/webrtc/signaling/src/media-conduit',
     '/media/webrtc/signaling/src/mediapipeline',
     '/media/webrtc/signaling/src/peerconnection',
     '/media/webrtc/signaling/src/sipcc/core/includes',
     '/media/webrtc/signaling/src/sipcc/core/sdp',
     '/media/webrtc/signaling/src/sipcc/cpr/include',
     '/media/webrtc/signaling/src/sipcc/include',
+    '/media/webrtc/trunk',
     '/media/webrtc/trunk/testing/gtest/include',
     '/media/webrtc/trunk/third_party/libjingle/source',
     '/xpcom/base',
 ]
 
 if CONFIG['OS_TARGET'] == 'Android':
     LOCAL_INCLUDES += [
         '/media/mtransport/third_party/nrappkit/src/port/android/include',