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 170539 4f7eac2b16ca293cbbfab61c1220b65990467186
parent 170538 724240c068509996b17a47e7d1ddb7917b2ae9ed
child 170540 1dea344fa608897709f2e674ee726ec858c21da2
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersabr
bugs786234
milestone30.0a1
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',