zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove BundleFilter filtering of RTCP.

Browse Source 
BundleFilter may not know the remote SSRC for all incoming RTCP packets,
so there's no point in filtering them.

BUG=webrtc:4740
R=hta@webrtc.org, juberti@webrtc.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1437683005

Cr-Commit-Position: refs/heads/master@{#10655}
This commit is contained in:
pbos
2015-11-16 10:19:58 -08:00
committed by Commit bot
parent 8b85de2ba1
commit 482b12e2c3
5 changed files with 38 additions and 226 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
talk/session/media/bundlefilter.cc
View File

@ -32,78 +32,29 @@
namespace cricket {
static const uint32_t kSsrc01 = 0x01;
BundleFilter::BundleFilter() {
}
BundleFilter::~BundleFilter() {
}
bool BundleFilter::DemuxPacket(const char* data, size_t len, bool rtcp) {
// For rtp packets, we check whether the payload type can be found.
// For rtcp packets, we check whether the ssrc can be found or is the special
// value 1 except for SDES packets which always pass through. Plus, if
// |streams_| is empty, we will allow all rtcp packets pass through provided
// that they are valid rtcp packets in case that they are for early media.
if (!rtcp) {
// It may not be a RTP packet (e.g. SCTP).
if (!IsRtpPacket(data, len))
return false;
int payload_type = 0;
if (!GetRtpPayloadType(data, len, &payload_type)) {
return false;
}
return FindPayloadType(payload_type);
bool BundleFilter::DemuxPacket(const uint8_t* data, size_t len) {
// For RTP packets, we check whether the payload type can be found.
if (!IsRtpPacket(data, len)) {
return false;
}
// Rtcp packets using ssrc filter.
int pl_type = 0;
uint32_t ssrc = 0;
if (!GetRtcpType(data, len, &pl_type)) return false;
if (pl_type == kRtcpTypeSDES) {
// SDES packet parsing not supported.
LOG(LS_INFO) << "SDES packet received for demux.";
return true;
} else {
if (!GetRtcpSsrc(data, len, &ssrc)) return false;
if (ssrc == kSsrc01) {
// SSRC 1 has a special meaning and indicates generic feedback on
// some systems and should never be dropped. If it is forwarded
// incorrectly it will be ignored by lower layers anyway.
return true;
}
int payload_type = 0;
if (!GetRtpPayloadType(data, len, &payload_type)) {
return false;
}
// Pass through if |streams_| is empty to allow early rtcp packets in.
return !HasStreams() || FindStream(ssrc);
return FindPayloadType(payload_type);
}
void BundleFilter::AddPayloadType(int payload_type) {
payload_types_.insert(payload_type);
}
bool BundleFilter::AddStream(const StreamParams& stream) {
if (GetStreamBySsrc(streams_, stream.first_ssrc())) {
LOG(LS_WARNING) << "Stream already added to filter";
return false;
}
streams_.push_back(stream);
return true;
}
bool BundleFilter::RemoveStream(uint32_t ssrc) {
return RemoveStreamBySsrc(&streams_, ssrc);
}
bool BundleFilter::HasStreams() const {
return !streams_.empty();
}
bool BundleFilter::FindStream(uint32_t ssrc) const {
return ssrc == 0 ? false : GetStreamBySsrc(streams_, ssrc) != nullptr;
}
bool BundleFilter::FindPayloadType(int pl_type) const {
return payload_types_.find(pl_type) != payload_types_.end();
}

25
talk/session/media/bundlefilter.h
View File

@ -28,6 +28,8 @@
#ifndef TALK_SESSION_MEDIA_BUNDLEFILTER_H_
#define TALK_SESSION_MEDIA_BUNDLEFILTER_H_
#include <stdint.h>
#include <set>
#include <vector>
@ -37,42 +39,31 @@
namespace cricket {
// In case of single RTP session and single transport channel, all session
// ( or media) channels share a common transport channel. Hence they all get
// (or media) channels share a common transport channel. Hence they all get
// SignalReadPacket when packet received on transport channel. This requires
// cricket::BaseChannel to know all the valid sources, else media channel
// will decode invalid packets.
//
// This class determines whether a packet is destined for cricket::BaseChannel.
// For rtp packets, this is decided based on the payload type. For rtcp packets,
// this is decided based on the sender ssrc values.
// This is only to be used for RTP packets as RTCP packets are not filtered.
// For RTP packets, this is decided based on the payload type.
class BundleFilter {
public:
BundleFilter();
~BundleFilter();
// Determines packet belongs to valid cricket::BaseChannel.
bool DemuxPacket(const char* data, size_t len, bool rtcp);
// Determines if a RTP packet belongs to valid cricket::BaseChannel.
bool DemuxPacket(const uint8_t* data, size_t len);
// Adds the supported payload type.
void AddPayloadType(int payload_type);
// Adding a valid source to the filter.
bool AddStream(const StreamParams& stream);
// Removes source from the filter.
bool RemoveStream(uint32_t ssrc);
// Utility methods added for unitest.
// True if |streams_| is not empty.
bool HasStreams() const;
bool FindStream(uint32_t ssrc) const;
// Public for unittests.
bool FindPayloadType(int pl_type) const;
void ClearAllPayloadTypes();
private:
std::set<int> payload_types_;
std::vector<StreamParams> streams_;
};
} // namespace cricket

148
talk/session/media/bundlefilter_unittest.cc
View File

@ -30,9 +30,6 @@
using cricket::StreamParams;
static const int kSsrc1 = 0x1111;
static const int kSsrc2 = 0x2222;
static const int kSsrc3 = 0x3333;
static const int kPayloadType1 = 0x11;
static const int kPayloadType2 = 0x22;
static const int kPayloadType3 = 0x33;
@ -55,56 +52,6 @@ static const unsigned char kRtpPacketPt3Ssrc2[] = {
0x22,
};
// PT = 200 = SR, len = 28, SSRC of sender = 0x0001
// NTP TS = 0, RTP TS = 0, packet count = 0
static const unsigned char kRtcpPacketSrSsrc01[] = {
0x80, 0xC8, 0x00, 0x1B, 0x00, 0x00, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
};
// PT = 200 = SR, len = 28, SSRC of sender = 0x2222
// NTP TS = 0, RTP TS = 0, packet count = 0
static const unsigned char kRtcpPacketSrSsrc2[] = {
0x80, 0xC8, 0x00, 0x1B, 0x00, 0x00, 0x22, 0x22,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
};
// First packet - SR = PT = 200, len = 0, SSRC of sender = 0x1111
// NTP TS = 0, RTP TS = 0, packet count = 0
// second packet - SDES = PT = 202, count = 0, SSRC = 0x1111, cname len = 0
static const unsigned char kRtcpPacketCompoundSrSdesSsrc1[] = {
0x80, 0xC8, 0x00, 0x01, 0x00, 0x00, 0x11, 0x11,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x81, 0xCA, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x01, 0x00,
};
// SDES = PT = 202, count = 0, SSRC = 0x2222, cname len = 0
static const unsigned char kRtcpPacketSdesSsrc2[] = {
0x81, 0xCA, 0x00, 0x00, 0x00, 0x00, 0x22, 0x22, 0x01, 0x00,
};
// Packet has only mandatory fixed RTCP header
static const unsigned char kRtcpPacketFixedHeaderOnly[] = {
0x80, 0xC8, 0x00, 0x00,
};
// Small packet for SSRC demux.
static const unsigned char kRtcpPacketTooSmall[] = {
0x80, 0xC8, 0x00, 0x00, 0x00, 0x00,
};
// PT = 206, FMT = 1, Sender SSRC = 0x1111, Media SSRC = 0x1111
// No FCI information is needed for PLI.
static const unsigned char kRtcpPacketNonCompoundRtcpPliFeedback[] = {
0x81, 0xCE, 0x00, 0x0C, 0x00, 0x00, 0x11, 0x11, 0x00, 0x00, 0x11, 0x11,
};
// An SCTP packet.
static const unsigned char kSctpPacket[] = {
0x00, 0x01, 0x00, 0x01,
@ -114,100 +61,29 @@ static const unsigned char kSctpPacket[] = {
0x00, 0x00, 0x00, 0x00,
};
TEST(BundleFilterTest, AddRemoveStreamTest) {
cricket::BundleFilter bundle_filter;
EXPECT_FALSE(bundle_filter.HasStreams());
EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc1)));
StreamParams stream2;
stream2.ssrcs.push_back(kSsrc2);
stream2.ssrcs.push_back(kSsrc3);
EXPECT_TRUE(bundle_filter.AddStream(stream2));
EXPECT_TRUE(bundle_filter.HasStreams());
EXPECT_TRUE(bundle_filter.FindStream(kSsrc1));
EXPECT_TRUE(bundle_filter.FindStream(kSsrc2));
EXPECT_TRUE(bundle_filter.FindStream(kSsrc3));
EXPECT_TRUE(bundle_filter.RemoveStream(kSsrc1));
EXPECT_FALSE(bundle_filter.FindStream(kSsrc1));
EXPECT_TRUE(bundle_filter.RemoveStream(kSsrc3));
EXPECT_FALSE(bundle_filter.RemoveStream(kSsrc2)); // Already removed.
EXPECT_FALSE(bundle_filter.HasStreams());
}
TEST(BundleFilterTest, RtpPacketTest) {
cricket::BundleFilter bundle_filter;
bundle_filter.AddPayloadType(kPayloadType1);
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt1Ssrc1),
sizeof(kRtpPacketPt1Ssrc1), false));
EXPECT_TRUE(bundle_filter.DemuxPacket(kRtpPacketPt1Ssrc1,
sizeof(kRtpPacketPt1Ssrc1)));
bundle_filter.AddPayloadType(kPayloadType2);
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt2Ssrc2),
sizeof(kRtpPacketPt2Ssrc2), false));
EXPECT_TRUE(bundle_filter.DemuxPacket(kRtpPacketPt2Ssrc2,
sizeof(kRtpPacketPt2Ssrc2)));
// Payload type 0x33 is not added.
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt3Ssrc2),
sizeof(kRtpPacketPt3Ssrc2), false));
EXPECT_FALSE(bundle_filter.DemuxPacket(kRtpPacketPt3Ssrc2,
sizeof(kRtpPacketPt3Ssrc2)));
// Size is too small.
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt1Ssrc1), 11, false));
EXPECT_FALSE(bundle_filter.DemuxPacket(kRtpPacketPt1Ssrc1, 11));
bundle_filter.ClearAllPayloadTypes();
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt1Ssrc1),
sizeof(kRtpPacketPt1Ssrc1), false));
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtpPacketPt2Ssrc2),
sizeof(kRtpPacketPt2Ssrc2), false));
}
TEST(BundleFilterTest, RtcpPacketTest) {
cricket::BundleFilter bundle_filter;
EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc1)));
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketCompoundSrSdesSsrc1),
sizeof(kRtcpPacketCompoundSrSdesSsrc1), true));
EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc2)));
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSrSsrc2),
sizeof(kRtcpPacketSrSsrc2), true));
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSdesSsrc2),
sizeof(kRtcpPacketSdesSsrc2), true));
EXPECT_TRUE(bundle_filter.RemoveStream(kSsrc2));
// RTCP Packets other than SR and RR are demuxed regardless of SSRC.
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSdesSsrc2),
sizeof(kRtcpPacketSdesSsrc2), true));
// RTCP Packets with 'special' SSRC 0x01 are demuxed also
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSrSsrc01),
sizeof(kRtcpPacketSrSsrc01), true));
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSrSsrc2),
sizeof(kRtcpPacketSrSsrc2), true));
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketFixedHeaderOnly),
sizeof(kRtcpPacketFixedHeaderOnly), true));
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketTooSmall),
sizeof(kRtcpPacketTooSmall), true));
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketNonCompoundRtcpPliFeedback),
sizeof(kRtcpPacketNonCompoundRtcpPliFeedback), true));
// If the streams_ is empty, rtcp packet passes through
EXPECT_TRUE(bundle_filter.RemoveStream(kSsrc1));
EXPECT_FALSE(bundle_filter.HasStreams());
EXPECT_TRUE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kRtcpPacketSrSsrc2),
sizeof(kRtcpPacketSrSsrc2), true));
EXPECT_FALSE(bundle_filter.DemuxPacket(kRtpPacketPt1Ssrc1,
sizeof(kRtpPacketPt1Ssrc1)));
EXPECT_FALSE(bundle_filter.DemuxPacket(kRtpPacketPt2Ssrc2,
sizeof(kRtpPacketPt2Ssrc2)));
}
TEST(BundleFilterTest, InvalidRtpPacket) {
cricket::BundleFilter bundle_filter;
EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc1)));
EXPECT_FALSE(bundle_filter.DemuxPacket(
reinterpret_cast<const char*>(kSctpPacket),
sizeof(kSctpPacket), false));
EXPECT_FALSE(bundle_filter.DemuxPacket(kSctpPacket, sizeof(kSctpPacket)));
}

15
talk/session/media/channel.cc
View File

@ -628,9 +628,12 @@ bool BaseChannel::WantsPacket(bool rtcp, rtc::Buffer* packet) {
<< " packet: wrong size=" << packet->size();
return false;
}
// Bundle filter handles both rtp and rtcp packets.
return bundle_filter_.DemuxPacket(packet->data<char>(), packet->size(), rtcp);
if (rtcp) {
// Permit all (seemingly valid) RTCP packets.
return true;
}
// Check whether we handle this payload.
return bundle_filter_.DemuxPacket(packet->data<uint8_t>(), packet->size());
}
void BaseChannel::HandlePacket(bool rtcp, rtc::Buffer* packet,
@ -1075,15 +1078,11 @@ bool BaseChannel::SetRtcpMux_w(bool enable, ContentAction action,
bool BaseChannel::AddRecvStream_w(const StreamParams& sp) {
ASSERT(worker_thread() == rtc::Thread::Current());
if (!media_channel()->AddRecvStream(sp))
return false;
return bundle_filter_.AddStream(sp);
return media_channel()->AddRecvStream(sp);
}
bool BaseChannel::RemoveRecvStream_w(uint32_t ssrc) {
ASSERT(worker_thread() == rtc::Thread::Current());
bundle_filter_.RemoveStream(ssrc);
return media_channel()->RemoveRecvStream(ssrc);
}

11
talk/session/media/channel_unittest.cc
View File

@ -1474,12 +1474,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
EXPECT_TRUE(channel2_->bundle_filter()->FindPayloadType(pl_type1));
EXPECT_FALSE(channel1_->bundle_filter()->FindPayloadType(pl_type2));
EXPECT_FALSE(channel2_->bundle_filter()->FindPayloadType(pl_type2));
// channel1 - should only have media_content2 as remote. i.e. kSsrc2
EXPECT_TRUE(channel1_->bundle_filter()->FindStream(kSsrc2));
EXPECT_FALSE(channel1_->bundle_filter()->FindStream(kSsrc1));
// channel2 - should only have media_content1 as remote. i.e. kSsrc1
EXPECT_TRUE(channel2_->bundle_filter()->FindStream(kSsrc1));
EXPECT_FALSE(channel2_->bundle_filter()->FindStream(kSsrc2));
// Both channels can receive pl_type1 only.
EXPECT_TRUE(SendCustomRtp1(kSsrc1, ++sequence_number1_1, pl_type1));
@ -1504,8 +1498,9 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
EXPECT_TRUE(SendCustomRtcp1(kSsrc2));
EXPECT_TRUE(SendCustomRtcp2(kSsrc1));
EXPECT_FALSE(CheckCustomRtcp1(kSsrc1));
EXPECT_FALSE(CheckCustomRtcp2(kSsrc2));
// Bundle filter shouldn't filter out any RTCP.
EXPECT_TRUE(CheckCustomRtcp1(kSsrc1));
EXPECT_TRUE(CheckCustomRtcp2(kSsrc2));
}
// Test that the media monitor can be run and gives timely callbacks.