Bug 1141652: Simplify RTCP bundle filtering to only filter for receive pipelines, based only on the SR SSRC. r=jesup
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 12 Mar 2015 09:08:13 -0700
changeset 233556 8940a7b2bef3d8dcc8c2e73299a92129bdbca784
parent 233555 8878cee7f4fc96a13bcab63b6972e5a16bd69f75
child 233557 906c7ac5ac4017dcae027554380e6281c4b5eafc
push id28417
push userryanvm@gmail.com
push dateFri, 13 Mar 2015 19:52:44 +0000
treeherdermozilla-central@977add19414a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1141652
milestone39.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 1141652: Simplify RTCP bundle filtering to only filter for receive pipelines, based only on the SR SSRC. r=jesup
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h
media/webrtc/signaling/test/mediapipeline_unittest.cpp
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -538,20 +538,20 @@ void MediaPipeline::RtcpPacketReceived(T
   nsresult res = rtcp_.recv_srtp_->UnprotectRtcp(inner_data,
                                                  len,
                                                  len,
                                                  &out_len);
 
   if (!NS_SUCCEEDED(res))
     return;
 
-  MediaPipelineFilter::Result filter_result = MediaPipelineFilter::PASS;
-  if (filter_) {
-    filter_result = filter_->FilterRTCP(inner_data, out_len);
-    if (filter_result == MediaPipelineFilter::FAIL) {
+  // We do not filter RTCP for send pipelines, since the webrtc.org code for
+  // senders already has logic to ignore RRs that do not apply.
+  if (filter_ && direction_ == RECEIVE) {
+    if (!filter_->FilterSenderReport(inner_data, out_len)) {
       MOZ_MTLOG(ML_NOTICE, "Dropping rtcp packet");
       return;
     }
   }
 
   MOZ_MTLOG(ML_DEBUG, description_ << " received RTCP packet.");
   increment_rtcp_packets_received();
 
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
@@ -2,25 +2,20 @@
 /* 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 "logging.h"
-
 #include "MediaPipelineFilter.h"
 
 #include "webrtc/modules/interface/module_common_types.h"
 
-// Logging context
-MOZ_MTLOG_MODULE("mediapipeline")
-
 namespace mozilla {
 
 MediaPipelineFilter::MediaPipelineFilter() : correlator_(0) {
 }
 
 bool MediaPipelineFilter::Filter(const webrtc::RTPHeader& header,
                                  uint32_t correlator) {
   if (correlator) {
@@ -76,107 +71,32 @@ void MediaPipelineFilter::Update(const M
     remote_ssrc_set_ = filter_update.remote_ssrc_set_;
   }
 
   local_ssrc_set_ = filter_update.local_ssrc_set_;
   payload_type_set_ = filter_update.payload_type_set_;
   correlator_ = filter_update.correlator_;
 }
 
-MediaPipelineFilter::Result
-MediaPipelineFilter::FilterRTCP(const unsigned char* data,
-                                size_t len) const {
+bool
+MediaPipelineFilter::FilterSenderReport(const unsigned char* data,
+                                        size_t len) const {
   if (len < FIRST_SSRC_OFFSET) {
-    return FAIL;
+    return false;
   }
 
   uint8_t payload_type = data[PT_OFFSET];
 
-  switch (payload_type) {
-    case SENDER_REPORT_T:
-      return CheckRtcpReport(data, len, RECEIVER_REPORT_START_SR) ? PASS : FAIL;
-    case RECEIVER_REPORT_T:
-      return CheckRtcpReport(data, len, SENDER_REPORT_START_RR) ? PASS : FAIL;
-    default:
-      return UNSUPPORTED;
-  }
-
-  return UNSUPPORTED;
-}
-
-bool MediaPipelineFilter::CheckRtcpSsrc(const unsigned char* data,
-                                        size_t len,
-                                        size_t ssrc_offset,
-                                        uint8_t flags) const {
-  if (ssrc_offset + 4 > len) {
+  if (payload_type != SENDER_REPORT_T) {
     return false;
   }
 
   uint32_t ssrc = 0;
-  ssrc += (uint32_t)data[ssrc_offset++] << 24;
-  ssrc += (uint32_t)data[ssrc_offset++] << 16;
-  ssrc += (uint32_t)data[ssrc_offset++] << 8;
-  ssrc += (uint32_t)data[ssrc_offset++];
-
-  if (flags | MAYBE_LOCAL_SSRC) {
-    if (local_ssrc_set_.count(ssrc)) {
-      return true;
-    }
-  }
-
-  if (flags | MAYBE_REMOTE_SSRC) {
-    if (remote_ssrc_set_.count(ssrc)) {
-      return true;
-    }
-  }
-  return false;
-}
-
-static uint8_t GetCount(const unsigned char* data, size_t len) {
-  // This might be a count of rr reports, or might be a count of stuff like
-  // SDES reports, or what-have-you. They all live in bits 3-7.
-  return data[0] & 0x1F;
-}
+  ssrc += (uint32_t)data[FIRST_SSRC_OFFSET] << 24;
+  ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 1] << 16;
+  ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 2] << 8;
+  ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 3];
 
-bool MediaPipelineFilter::CheckRtcpReport(const unsigned char* data,
-                                          size_t len,
-                                          size_t first_rr_offset) const {
-  bool remote_ssrc_matched = CheckRtcpSsrc(data,
-                                           len,
-                                           FIRST_SSRC_OFFSET,
-                                           MAYBE_REMOTE_SSRC);
-
-  uint8_t rr_count = GetCount(data, len);
-
-  // We need to check for consistency. If the remote ssrc matched, the local
-  // ssrcs must too. If it did not, this may just be because our filter is
-  // incomplete, so the local ssrcs could go either way.
-  bool ssrcs_must_match = remote_ssrc_matched;
-  bool ssrcs_must_not_match = false;
-
-  for (uint8_t rr_num = 0; rr_num < rr_count; ++rr_num) {
-    size_t ssrc_offset = first_rr_offset + (rr_num * RECEIVER_REPORT_SIZE);
-
-    if (!CheckRtcpSsrc(data, len, ssrc_offset, MAYBE_LOCAL_SSRC)) {
-      ssrcs_must_not_match = true;
-      if (ssrcs_must_match) {
-        break;
-      }
-    } else {
-      ssrcs_must_match = true;
-      if (ssrcs_must_not_match) {
-        break;
-      }
-    }
-  }
-
-  if (ssrcs_must_match && ssrcs_must_not_match) {
-    MOZ_MTLOG(ML_ERROR, "Received an RTCP packet with SSRCs from "
-              "multiple m-lines! This is broken.");
-  }
-
-  // This is set if any ssrc matched
-  return ssrcs_must_match;
+  return !!remote_ssrc_set_.count(ssrc);
 }
 
 } // end namespace mozilla
 
-
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h
@@ -37,72 +37,48 @@ namespace mozilla {
 // 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).
+//    sender reports later).
 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);
 
-  typedef enum {
-    FAIL,
-    PASS,
-    UNSUPPORTED
-  } Result;
-
   // RTCP doesn't have things like the RTP correlator, and uses its own
   // payload types too.
-  Result FilterRTCP(const unsigned char* data, size_t len) const;
+  bool FilterSenderReport(const unsigned char* data, size_t len) const;
 
   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 Update(const MediaPipelineFilter& filter_update);
 
   // Some payload types
   static const uint8_t SENDER_REPORT_T = 200;
-  static const uint8_t RECEIVER_REPORT_T = 201;
 
  private:
-  static const uint8_t MAYBE_LOCAL_SSRC = 1;
-  static const uint8_t MAYBE_REMOTE_SSRC = 2;
-
   // Payload type is always in the second byte
   static const size_t PT_OFFSET = 1;
   // First SSRC always starts at the fifth byte.
   static const size_t FIRST_SSRC_OFFSET = 4;
 
-  static const size_t RECEIVER_REPORT_START_SR = 7*4;
-  static const size_t SENDER_REPORT_START_RR = 2*4;
-  static const size_t RECEIVER_REPORT_SIZE = 6*4;
-
-  bool CheckRtcpSsrc(const unsigned char* data,
-                     size_t len,
-                     size_t ssrc_offset,
-                     uint8_t flags) const;
-
-
-  bool CheckRtcpReport(const unsigned char* data,
-                        size_t len,
-                        size_t first_rr_offset) const;
-
   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_;
 };
 
--- a/media/webrtc/signaling/test/mediapipeline_unittest.cpp
+++ b/media/webrtc/signaling/test/mediapipeline_unittest.cpp
@@ -510,291 +510,129 @@ TEST_F(MediaPipelineFilterTest, TestSSRC
   0,0,0,0, \
   0,0,0,0, \
   0,0,0,0, \
   0,0,0,0
 
 #define RTCP_TYPEINFO(num_rrs, type, size) \
   0x80 + num_rrs, type, 0, size
 
-const unsigned char rtcp_rr_s16[] = {
-  // zero rrs, size 1 words
-  RTCP_TYPEINFO(0, MediaPipelineFilter::RECEIVER_REPORT_T, 1),
-  SSRC(16)
-};
-
-const unsigned char rtcp_rr_s16_r17[] = {
-  // one rr, 7 words
-  RTCP_TYPEINFO(1, MediaPipelineFilter::RECEIVER_REPORT_T, 7),
-  SSRC(16),
-  REPORT_FRAGMENT(17)
-};
-
-const unsigned char rtcp_rr_s16_r17_18[] = {
-  // two rrs, size 13 words
-  RTCP_TYPEINFO(2, MediaPipelineFilter::RECEIVER_REPORT_T, 13),
-  SSRC(16),
-  REPORT_FRAGMENT(17),
-  REPORT_FRAGMENT(18)
-};
-
 const unsigned char rtcp_sr_s16[] = {
   // zero rrs, size 6 words
   RTCP_TYPEINFO(0, MediaPipelineFilter::SENDER_REPORT_T, 6),
   REPORT_FRAGMENT(16)
 };
 
 const unsigned char rtcp_sr_s16_r17[] = {
   // one rr, size 12 words
   RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12),
   REPORT_FRAGMENT(16),
   REPORT_FRAGMENT(17)
 };
 
-const unsigned char rtcp_sr_s16_r17_18[] = {
-  // two rrs, size 18 words
-  RTCP_TYPEINFO(2, MediaPipelineFilter::SENDER_REPORT_T, 18),
-  REPORT_FRAGMENT(16),
-  REPORT_FRAGMENT(17),
-  REPORT_FRAGMENT(18)
-};
-
 const unsigned char unknown_type[] = {
   RTCP_TYPEINFO(1, 222, 0)
 };
 
 TEST_F(MediaPipelineFilterTest, TestEmptyFilterReport0) {
   MediaPipelineFilter filter;
-  ASSERT_EQ(MediaPipelineFilter::FAIL,
-            filter.FilterRTCP(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
-  ASSERT_EQ(MediaPipelineFilter::FAIL,
-            filter.FilterRTCP(rtcp_rr_s16, sizeof(rtcp_rr_s16)));
+  ASSERT_FALSE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterReport0) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(16);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16, sizeof(rtcp_rr_s16)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport0SSRCTruncated) {
-  MediaPipelineFilter filter;
-  filter.AddRemoteSSRC(16);
-  const unsigned char data[] = {
-    RTCP_TYPEINFO(0, MediaPipelineFilter::RECEIVER_REPORT_T, 1),
-    0,0,0
-  };
-  ASSERT_EQ(MediaPipelineFilter::FAIL,
-            filter.FilterRTCP(data, sizeof(data)));
+  ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterReport0PTTruncated) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(16);
   const unsigned char data[] = {0x80};
-  ASSERT_EQ(MediaPipelineFilter::FAIL,
-            filter.FilterRTCP(data, sizeof(data)));
+  ASSERT_FALSE(filter.FilterSenderReport(data, sizeof(data)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterReport0CountTruncated) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(16);
   const unsigned char data[] = {};
-  ASSERT_EQ(MediaPipelineFilter::FAIL,
-            filter.FilterRTCP(data, sizeof(data)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport1BothMatch) {
-  MediaPipelineFilter filter;
-  filter.AddRemoteSSRC(16);
-  filter.AddLocalSSRC(17);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17, sizeof(rtcp_sr_s16_r17)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17, sizeof(rtcp_rr_s16_r17)));
+  ASSERT_FALSE(filter.FilterSenderReport(data, sizeof(data)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterReport1SSRCTruncated) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(16);
   filter.AddLocalSSRC(17);
-  const unsigned char rr[] = {
-    RTCP_TYPEINFO(1, MediaPipelineFilter::RECEIVER_REPORT_T, 7),
-    SSRC(16),
-    0,0,0
-  };
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rr, sizeof(rr)));
   const unsigned char sr[] = {
-    RTCP_TYPEINFO(1, MediaPipelineFilter::RECEIVER_REPORT_T, 12),
+    RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12),
     REPORT_FRAGMENT(16),
     0,0,0
   };
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(sr, sizeof(rr)));
+  ASSERT_TRUE(filter.FilterSenderReport(sr, sizeof(sr)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterReport1BigSSRC) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(0x01020304);
   filter.AddLocalSSRC(0x11121314);
-  const unsigned char rr[] = {
-    RTCP_TYPEINFO(1, MediaPipelineFilter::RECEIVER_REPORT_T, 7),
-    SSRC(0x01020304),
-    REPORT_FRAGMENT(0x11121314)
-  };
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rr, sizeof(rr)));
   const unsigned char sr[] = {
-    RTCP_TYPEINFO(1, MediaPipelineFilter::RECEIVER_REPORT_T, 12),
+    RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12),
     SSRC(0x01020304),
     REPORT_FRAGMENT(0x11121314)
   };
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(sr, sizeof(rr)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport1LocalMatch) {
-  MediaPipelineFilter filter;
-  filter.AddLocalSSRC(17);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17, sizeof(rtcp_sr_s16_r17)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17, sizeof(rtcp_rr_s16_r17)));
+  ASSERT_TRUE(filter.FilterSenderReport(sr, sizeof(sr)));
 }
 
-TEST_F(MediaPipelineFilterTest, TestFilterReport1Inconsistent) {
-  MediaPipelineFilter filter;
-  filter.AddRemoteSSRC(16);
-  // We assume that the filter is exactly correct in terms of local ssrcs.
-  // So, when RTCP shows up with a remote SSRC that matches, and a local
-  // ssrc that doesn't, we assume the other end has messed up and put ssrcs
-  // from more than one m-line in the packet.
-  // TODO: Currently, the webrtc.org code will continue putting old ssrcs
-  // in RTCP that have been negotiated away, causing the RTCP to be dropped
-  // if we leave this checking in. Not sure how we're supposed to prompt
-  // the webrtc.org code to stop doing this.
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17, sizeof(rtcp_sr_s16_r17)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17, sizeof(rtcp_rr_s16_r17)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_sr_s16_r17, sizeof(rtcp_sr_s16_r17)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_rr_s16_r17, sizeof(rtcp_rr_s16_r17)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport1NeitherMatch) {
-  MediaPipelineFilter filter;
-  filter.AddRemoteSSRC(17);
-  filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17, sizeof(rtcp_sr_s16_r17)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17, sizeof(rtcp_rr_s16_r17)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport2AllMatch) {
+TEST_F(MediaPipelineFilterTest, TestFilterReportMatch) {
   MediaPipelineFilter filter;
   filter.AddRemoteSSRC(16);
-  filter.AddLocalSSRC(17);
-  filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17_18,
-                              sizeof(rtcp_sr_s16_r17_18)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport2LocalMatch) {
-  MediaPipelineFilter filter;
-  filter.AddLocalSSRC(17);
-  filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17_18,
-                              sizeof(rtcp_sr_s16_r17_18)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17_18,
-                              sizeof(rtcp_rr_s16_r17_18)));
+  ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16_r17,
+                                        sizeof(rtcp_sr_s16_r17)));
 }
 
-TEST_F(MediaPipelineFilterTest, TestFilterReport2Inconsistent101) {
+TEST_F(MediaPipelineFilterTest, TestFilterReportNoMatch) {
   MediaPipelineFilter filter;
-  filter.AddRemoteSSRC(16);
-  filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17_18,
-                              sizeof(rtcp_sr_s16_r17_18)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17_18,
-                              sizeof(rtcp_rr_s16_r17_18)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_sr_s16_r17_18,
-  //                            sizeof(rtcp_sr_s16_r17_18)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_rr_s16_r17_18,
-  //                            sizeof(rtcp_rr_s16_r17_18)));
-}
-
-TEST_F(MediaPipelineFilterTest, TestFilterReport2Inconsistent001) {
-  MediaPipelineFilter filter;
-  filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16_r17_18,
-                              sizeof(rtcp_sr_s16_r17_18)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16_r17_18,
-                              sizeof(rtcp_rr_s16_r17_18)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_sr_s16_r17_18,
-  //                            sizeof(rtcp_sr_s16_r17_18)));
-  //ASSERT_EQ(MediaPipelineFilter::FAIL,
-  //          filter.FilterRTCP(rtcp_rr_s16_r17_18,
-  //                            sizeof(rtcp_rr_s16_r17_18)));
+  filter.AddRemoteSSRC(17);
+  ASSERT_FALSE(filter.FilterSenderReport(rtcp_sr_s16_r17,
+                                         sizeof(rtcp_sr_s16_r17)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestFilterUnknownRTCPType) {
   MediaPipelineFilter filter;
   filter.AddLocalSSRC(18);
-  ASSERT_EQ(MediaPipelineFilter::UNSUPPORTED,
-            filter.FilterRTCP(unknown_type, sizeof(unknown_type)));
+  ASSERT_FALSE(filter.FilterSenderReport(unknown_type, sizeof(unknown_type)));
 }
 
 TEST_F(MediaPipelineFilterTest, TestCorrelatorFilter) {
   MediaPipelineFilter filter;
   filter.SetCorrelator(7777);
   ASSERT_TRUE(Filter(filter, 7777, 16, 110));
   ASSERT_FALSE(Filter(filter, 7778, 17, 110));
   // This should also have resulted in the SSRC 16 being added to the filter
   ASSERT_TRUE(Filter(filter, 0, 16, 110));
   ASSERT_FALSE(Filter(filter, 0, 17, 110));
 
   // rtcp_sr_s16 has 16 as an SSRC
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_rr_s16, sizeof(rtcp_rr_s16)));
+  ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
 }
 
 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, 16, 110));
 
   // rtcp_sr_s16 has 16 as an SSRC
-  ASSERT_EQ(MediaPipelineFilter::PASS,
-            filter.FilterRTCP(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
+  ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16)));
 }
 
 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));