diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp index 96288fe70..143e96b6e 100755 --- a/talk/libjingle.gyp +++ b/talk/libjingle.gyp @@ -1114,6 +1114,8 @@ 'session/tunnel/securetunnelsessionclient.h', 'session/media/audiomonitor.cc', 'session/media/audiomonitor.h', + 'session/media/bundlefilter.cc', + 'session/media/bundlefilter.h', 'session/media/call.cc', 'session/media/call.h', 'session/media/channel.cc', @@ -1139,8 +1141,6 @@ 'session/media/soundclip.h', 'session/media/srtpfilter.cc', 'session/media/srtpfilter.h', - 'session/media/ssrcmuxfilter.cc', - 'session/media/ssrcmuxfilter.h', 'session/media/typingmonitor.cc', 'session/media/typingmonitor.h', 'session/media/voicechannel.h', diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp index fd85451da..f700261bc 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -357,6 +357,7 @@ 'p2p/client/connectivitychecker_unittest.cc', 'p2p/client/fakeportallocator.h', 'p2p/client/portallocator_unittest.cc', + 'session/media/bundlefilter_unittest.cc', 'session/media/channel_unittest.cc', 'session/media/channelmanager_unittest.cc', 'session/media/currentspeakermonitor_unittest.cc', @@ -366,7 +367,6 @@ 'session/media/mediasessionclient_unittest.cc', 'session/media/rtcpmuxfilter_unittest.cc', 'session/media/srtpfilter_unittest.cc', - 'session/media/ssrcmuxfilter_unittest.cc', ], 'conditions': [ ['OS=="win"', { diff --git a/talk/session/media/ssrcmuxfilter.cc b/talk/session/media/bundlefilter.cc old mode 100644 new mode 100755 similarity index 52% rename from talk/session/media/ssrcmuxfilter.cc rename to talk/session/media/bundlefilter.cc index 638167d18..0d7927c2e --- a/talk/session/media/ssrcmuxfilter.cc +++ b/talk/session/media/bundlefilter.cc @@ -25,9 +25,7 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "talk/session/media/ssrcmuxfilter.h" - -#include +#include "talk/session/media/bundlefilter.h" #include "talk/base/logging.h" #include "talk/media/base/rtputils.h" @@ -36,41 +34,52 @@ namespace cricket { static const uint32 kSsrc01 = 0x01; -SsrcMuxFilter::SsrcMuxFilter() { +BundleFilter::BundleFilter() { } -SsrcMuxFilter::~SsrcMuxFilter() { +BundleFilter::~BundleFilter() { } -bool SsrcMuxFilter::IsActive() const { - return !streams_.empty(); -} - -bool SsrcMuxFilter::DemuxPacket(const char* data, size_t len, bool rtcp) { - uint32 ssrc = 0; +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) { - GetRtpSsrc(data, len, &ssrc); + int payload_type = 0; + if (!GetRtpPayloadType(data, len, &payload_type)) { + return false; + } + return FindPayloadType(payload_type); + } + + // Rtcp packets using ssrc filter. + int pl_type = 0; + uint32 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 { - int pl_type = 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."; + 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; - } 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; - } } } - return FindStream(ssrc); + // Pass through if |streams_| is empty to allow early rtcp packets in. + return !HasStreams() || FindStream(ssrc); } -bool SsrcMuxFilter::AddStream(const StreamParams& stream) { +void BundleFilter::AddPayloadType(int payload_type) { + payload_types_.insert(payload_type); +} + +bool BundleFilter::AddStream(const StreamParams& stream) { if (GetStreamBySsrc(streams_, stream.first_ssrc(), NULL)) { LOG(LS_WARNING) << "Stream already added to filter"; return false; @@ -79,15 +88,27 @@ bool SsrcMuxFilter::AddStream(const StreamParams& stream) { return true; } -bool SsrcMuxFilter::RemoveStream(uint32 ssrc) { +bool BundleFilter::RemoveStream(uint32 ssrc) { return RemoveStreamBySsrc(&streams_, ssrc); } -bool SsrcMuxFilter::FindStream(uint32 ssrc) const { +bool BundleFilter::HasStreams() const { + return !streams_.empty(); +} + +bool BundleFilter::FindStream(uint32 ssrc) const { if (ssrc == 0) { return false; } return (GetStreamBySsrc(streams_, ssrc, NULL)); } +bool BundleFilter::FindPayloadType(int pl_type) const { + return payload_types_.find(pl_type) != payload_types_.end(); +} + +void BundleFilter::ClearAllPayloadTypes() { + payload_types_.clear(); +} + } // namespace cricket diff --git a/talk/session/media/ssrcmuxfilter.h b/talk/session/media/bundlefilter.h old mode 100644 new mode 100755 similarity index 76% rename from talk/session/media/ssrcmuxfilter.h rename to talk/session/media/bundlefilter.h index 9420f54cc..34bc33073 --- a/talk/session/media/ssrcmuxfilter.h +++ b/talk/session/media/bundlefilter.h @@ -25,9 +25,10 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef TALK_SESSION_MEDIA_SSRCMUXFILTER_H_ -#define TALK_SESSION_MEDIA_SSRCMUXFILTER_H_ +#ifndef TALK_SESSION_MEDIA_BUNDLEFILTER_H_ +#define TALK_SESSION_MEDIA_BUNDLEFILTER_H_ +#include #include #include "talk/base/basictypes.h" @@ -35,33 +36,45 @@ namespace cricket { -// This class maintains list of recv SSRC's destined for cricket::BaseChannel. // In case of single RTP session and single transport channel, all session // ( 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. -class SsrcMuxFilter { +// +// 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. +class BundleFilter { public: - SsrcMuxFilter(); - ~SsrcMuxFilter(); + BundleFilter(); + ~BundleFilter(); - // Whether the rtp mux is active for a sdp session. - // Returns true if the filter contains a stream. - bool IsActive() const; // Determines packet belongs to valid cricket::BaseChannel. bool DemuxPacket(const char* data, size_t len, bool rtcp); + + // 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 ssrc); - // Utility method added for unitest. + + // Utility methods added for unitest. + // True if |streams_| is not empty. + bool HasStreams() const; bool FindStream(uint32 ssrc) const; + bool FindPayloadType(int pl_type) const; + void ClearAllPayloadTypes(); + private: + std::set payload_types_; std::vector streams_; }; } // namespace cricket -#endif // TALK_SESSION_MEDIA_SSRCMUXFILTER_H_ +#endif // TALK_SESSION_MEDIA_BUNDLEFILTER_H_ diff --git a/talk/session/media/ssrcmuxfilter_unittest.cc b/talk/session/media/bundlefilter_unittest.cc old mode 100644 new mode 100755 similarity index 57% rename from talk/session/media/ssrcmuxfilter_unittest.cc rename to talk/session/media/bundlefilter_unittest.cc index 85a4dbe50..0386666c0 --- a/talk/session/media/ssrcmuxfilter_unittest.cc +++ b/talk/session/media/bundlefilter_unittest.cc @@ -25,34 +25,34 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ - #include "talk/base/gunit.h" -#include "talk/session/media/ssrcmuxfilter.h" +#include "talk/session/media/bundlefilter.h" + +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; -using cricket::StreamParams; - -// SSRC = 0x1111 -static const unsigned char kRtpPacketSsrc1[] = { - 0x80, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, +// SSRC = 0x1111, Payload type = 0x11 +static const unsigned char kRtpPacketPt1Ssrc1[] = { + 0x80, kPayloadType1, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x11, + 0x11, }; -// SSRC = 0x2222 -static const unsigned char kRtpPacketSsrc2[] = { - 0x80, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x22, 0x22, +// SSRC = 0x2222, Payload type = 0x22 +static const unsigned char kRtpPacketPt2Ssrc2[] = { + 0x80, 0x80 + kPayloadType2, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x22, 0x22, }; -// SSRC = 0 -static const unsigned char kRtpPacketInvalidSsrc[] = { - 0x80, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, -}; - -// invalid size -static const unsigned char kRtpPacketTooSmall[] = { - 0x80, 0x80, 0x00, 0x00, +// SSRC = 0x2222, Payload type = 0x33 +static const unsigned char kRtpPacketPt3Ssrc2[] = { + 0x80, kPayloadType3, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x22, + 0x22, }; // PT = 200 = SR, len = 28, SSRC of sender = 0x0001 @@ -105,80 +105,92 @@ static const unsigned char kRtcpPacketNonCompoundRtcpPliFeedback[] = { 0x81, 0xCE, 0x00, 0x0C, 0x00, 0x00, 0x11, 0x11, 0x00, 0x00, 0x11, 0x11, }; -TEST(SsrcMuxFilterTest, AddRemoveStreamTest) { - cricket::SsrcMuxFilter ssrc_filter; - EXPECT_FALSE(ssrc_filter.IsActive()); - EXPECT_TRUE(ssrc_filter.AddStream(StreamParams::CreateLegacy(kSsrc1))); +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(ssrc_filter.AddStream(stream2)); + EXPECT_TRUE(bundle_filter.AddStream(stream2)); - EXPECT_TRUE(ssrc_filter.IsActive()); - EXPECT_TRUE(ssrc_filter.FindStream(kSsrc1)); - EXPECT_TRUE(ssrc_filter.FindStream(kSsrc2)); - EXPECT_TRUE(ssrc_filter.FindStream(kSsrc3)); - EXPECT_TRUE(ssrc_filter.RemoveStream(kSsrc1)); - EXPECT_FALSE(ssrc_filter.FindStream(kSsrc1)); - EXPECT_TRUE(ssrc_filter.RemoveStream(kSsrc3)); - EXPECT_FALSE(ssrc_filter.RemoveStream(kSsrc2)); // Already removed. - EXPECT_FALSE(ssrc_filter.IsActive()); + 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(SsrcMuxFilterTest, RtpPacketTest) { - cricket::SsrcMuxFilter ssrc_filter; - EXPECT_TRUE(ssrc_filter.AddStream(StreamParams::CreateLegacy(kSsrc1))); - EXPECT_TRUE(ssrc_filter.DemuxPacket( - reinterpret_cast(kRtpPacketSsrc1), - sizeof(kRtpPacketSsrc1), false)); - EXPECT_TRUE(ssrc_filter.AddStream(StreamParams::CreateLegacy(kSsrc2))); - EXPECT_TRUE(ssrc_filter.DemuxPacket( - reinterpret_cast(kRtpPacketSsrc2), - sizeof(kRtpPacketSsrc2), false)); - EXPECT_TRUE(ssrc_filter.RemoveStream(kSsrc2)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( - reinterpret_cast(kRtpPacketSsrc2), - sizeof(kRtpPacketSsrc2), false)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( - reinterpret_cast(kRtpPacketInvalidSsrc), - sizeof(kRtpPacketInvalidSsrc), false)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( - reinterpret_cast(kRtpPacketTooSmall), - sizeof(kRtpPacketTooSmall), false)); +TEST(BundleFilterTest, RtpPacketTest) { + cricket::BundleFilter bundle_filter; + bundle_filter.AddPayloadType(kPayloadType1); + EXPECT_TRUE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt1Ssrc1), + sizeof(kRtpPacketPt1Ssrc1), false)); + bundle_filter.AddPayloadType(kPayloadType2); + EXPECT_TRUE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt2Ssrc2), + sizeof(kRtpPacketPt2Ssrc2), false)); + + // Payload type 0x33 is not added. + EXPECT_FALSE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt3Ssrc2), + sizeof(kRtpPacketPt3Ssrc2), false)); + // Size is too small. + EXPECT_FALSE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt1Ssrc1), 11, false)); + + bundle_filter.ClearAllPayloadTypes(); + EXPECT_FALSE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt1Ssrc1), + sizeof(kRtpPacketPt1Ssrc1), false)); + EXPECT_FALSE(bundle_filter.DemuxPacket( + reinterpret_cast(kRtpPacketPt2Ssrc2), + sizeof(kRtpPacketPt2Ssrc2), false)); } -TEST(SsrcMuxFilterTest, RtcpPacketTest) { - cricket::SsrcMuxFilter ssrc_filter; - EXPECT_TRUE(ssrc_filter.AddStream(StreamParams::CreateLegacy(kSsrc1))); - EXPECT_TRUE(ssrc_filter.DemuxPacket( +TEST(BundleFilterTest, RtcpPacketTest) { + cricket::BundleFilter bundle_filter; + EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc1))); + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketCompoundSrSdesSsrc1), sizeof(kRtcpPacketCompoundSrSdesSsrc1), true)); - EXPECT_TRUE(ssrc_filter.AddStream(StreamParams::CreateLegacy(kSsrc2))); - EXPECT_TRUE(ssrc_filter.DemuxPacket( + EXPECT_TRUE(bundle_filter.AddStream(StreamParams::CreateLegacy(kSsrc2))); + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketSrSsrc2), sizeof(kRtcpPacketSrSsrc2), true)); - EXPECT_TRUE(ssrc_filter.DemuxPacket( + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketSdesSsrc2), sizeof(kRtcpPacketSdesSsrc2), true)); - EXPECT_TRUE(ssrc_filter.RemoveStream(kSsrc2)); + EXPECT_TRUE(bundle_filter.RemoveStream(kSsrc2)); // RTCP Packets other than SR and RR are demuxed regardless of SSRC. - EXPECT_TRUE(ssrc_filter.DemuxPacket( + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketSdesSsrc2), sizeof(kRtcpPacketSdesSsrc2), true)); // RTCP Packets with 'special' SSRC 0x01 are demuxed also - EXPECT_TRUE(ssrc_filter.DemuxPacket( + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketSrSsrc01), sizeof(kRtcpPacketSrSsrc01), true)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( + EXPECT_FALSE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketSrSsrc2), sizeof(kRtcpPacketSrSsrc2), true)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( + EXPECT_FALSE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketFixedHeaderOnly), sizeof(kRtcpPacketFixedHeaderOnly), true)); - EXPECT_FALSE(ssrc_filter.DemuxPacket( + EXPECT_FALSE(bundle_filter.DemuxPacket( reinterpret_cast(kRtcpPacketTooSmall), sizeof(kRtcpPacketTooSmall), true)); - EXPECT_TRUE(ssrc_filter.DemuxPacket( + EXPECT_TRUE(bundle_filter.DemuxPacket( reinterpret_cast(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(kRtcpPacketSrSsrc2), + sizeof(kRtcpPacketSrSsrc2), true)); } diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 9440e173f..b13d5e169 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -38,7 +38,6 @@ #include "talk/p2p/base/transportchannel.h" #include "talk/session/media/channelmanager.h" #include "talk/session/media/mediamessages.h" -#include "talk/session/media/rtcpmuxfilter.h" #include "talk/session/media/typingmonitor.h" @@ -559,15 +558,9 @@ bool BaseChannel::WantsPacket(bool rtcp, talk_base::Buffer* packet) { << packet->length(); return false; } - // If this channel is suppose to handle RTP data, that is determined by - // checking against ssrc filter. This is necessary to do it here to avoid - // double decryption. - if (ssrc_filter_.IsActive() && - !ssrc_filter_.DemuxPacket(packet->data(), packet->length(), rtcp)) { - return false; - } - return true; + // Bundle filter handles both rtp and rtcp packets. + return bundle_filter_.DemuxPacket(packet->data(), packet->length(), rtcp); } void BaseChannel::HandlePacket(bool rtcp, talk_base::Buffer* packet, @@ -996,12 +989,12 @@ bool BaseChannel::AddRecvStream_w(const StreamParams& sp) { if (!media_channel()->AddRecvStream(sp)) return false; - return ssrc_filter_.AddStream(sp); + return bundle_filter_.AddStream(sp); } bool BaseChannel::RemoveRecvStream_w(uint32 ssrc) { ASSERT(worker_thread() == talk_base::Thread::Current()); - ssrc_filter_.RemoveStream(ssrc); + bundle_filter_.RemoveStream(ssrc); return media_channel()->RemoveRecvStream(ssrc); } @@ -1479,6 +1472,10 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, // If everything worked, see if we can start receiving. if (ret) { + std::vector::const_iterator it = audio->codecs().begin(); + for (; it != audio->codecs().end(); ++it) { + bundle_filter()->AddPayloadType(it->id); + } ChangeState(); } else { LOG(LS_WARNING) << "Failed to set local voice description"; @@ -1844,6 +1841,10 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, // If everything worked, see if we can start receiving. if (ret) { + std::vector::const_iterator it = video->codecs().begin(); + for (; it != video->codecs().end(); ++it) { + bundle_filter()->AddPayloadType(it->id); + } ChangeState(); } else { LOG(LS_WARNING) << "Failed to set local video description"; @@ -2249,7 +2250,6 @@ bool DataChannel::SetLocalContent_w(const MediaContentDescription* content, } } else { ret = SetBaseLocalContent_w(content, action, error_desc); - if (action != CA_UPDATE || data->has_codecs()) { if (!media_channel()->SetRecvCodecs(data->codecs())) { SafeSetError("Failed to set data receive codecs.", error_desc); @@ -2260,6 +2260,10 @@ bool DataChannel::SetLocalContent_w(const MediaContentDescription* content, // If everything worked, see if we can start receiving. if (ret) { + std::vector::const_iterator it = data->codecs().begin(); + for (; it != data->codecs().end(); ++it) { + bundle_filter()->AddPayloadType(it->id); + } ChangeState(); } else { LOG(LS_WARNING) << "Failed to set local data description"; diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index 2896bcede..2aec552f7 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -44,11 +44,11 @@ #include "talk/p2p/base/session.h" #include "talk/p2p/client/socketmonitor.h" #include "talk/session/media/audiomonitor.h" +#include "talk/session/media/bundlefilter.h" #include "talk/session/media/mediamonitor.h" #include "talk/session/media/mediasession.h" #include "talk/session/media/rtcpmuxfilter.h" #include "talk/session/media/srtpfilter.h" -#include "talk/session/media/ssrcmuxfilter.h" namespace cricket { @@ -213,7 +213,7 @@ class BaseChannel } } - SsrcMuxFilter* ssrc_filter() { return &ssrc_filter_; } + BundleFilter* bundle_filter() { return &bundle_filter_; } const std::vector& local_streams() const { return local_streams_; @@ -372,7 +372,7 @@ class BaseChannel TransportChannel* rtcp_transport_channel_; SrtpFilter srtp_filter_; RtcpMuxFilter rtcp_mux_filter_; - SsrcMuxFilter ssrc_filter_; + BundleFilter bundle_filter_; talk_base::scoped_ptr socket_monitor_; bool enabled_; bool writable_; diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index cb2bd6997..6e88fd284 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -71,6 +71,8 @@ static const cricket::DataCodec kGoogleDataCodec(101, "google-data", 0); static const uint32 kSsrc1 = 0x1111; static const uint32 kSsrc2 = 0x2222; static const uint32 kSsrc3 = 0x3333; +static const int kAudioPts[] = {0, 8}; +static const int kVideoPts[] = {97, 99}; template { static_cast(rtcp_packet_.size())); } // Methods to send custom data. - bool SendCustomRtp1(uint32 ssrc, int sequence_number) { - std::string data(CreateRtpData(ssrc, sequence_number)); + bool SendCustomRtp1(uint32 ssrc, int sequence_number, int pl_type = -1) { + std::string data(CreateRtpData(ssrc, sequence_number, pl_type)); return media_channel1_->SendRtp(data.c_str(), static_cast(data.size())); } - bool SendCustomRtp2(uint32 ssrc, int sequence_number) { - std::string data(CreateRtpData(ssrc, sequence_number)); + bool SendCustomRtp2(uint32 ssrc, int sequence_number, int pl_type = -1) { + std::string data(CreateRtpData(ssrc, sequence_number, pl_type)); return media_channel2_->SendRtp(data.c_str(), static_cast(data.size())); } @@ -445,13 +447,13 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { static_cast(rtcp_packet_.size())); } // Methods to check custom data. - bool CheckCustomRtp1(uint32 ssrc, int sequence_number) { - std::string data(CreateRtpData(ssrc, sequence_number)); + bool CheckCustomRtp1(uint32 ssrc, int sequence_number, int pl_type = -1 ) { + std::string data(CreateRtpData(ssrc, sequence_number, pl_type)); return media_channel1_->CheckRtp(data.c_str(), static_cast(data.size())); } - bool CheckCustomRtp2(uint32 ssrc, int sequence_number) { - std::string data(CreateRtpData(ssrc, sequence_number)); + bool CheckCustomRtp2(uint32 ssrc, int sequence_number, int pl_type = -1) { + std::string data(CreateRtpData(ssrc, sequence_number, pl_type)); return media_channel2_->CheckRtp(data.c_str(), static_cast(data.size())); } @@ -465,11 +467,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { return media_channel2_->CheckRtcp(data.c_str(), static_cast(data.size())); } - std::string CreateRtpData(uint32 ssrc, int sequence_number) { + std::string CreateRtpData(uint32 ssrc, int sequence_number, int pl_type) { std::string data(rtp_packet_); // Set SSRC in the rtp packet copy. talk_base::SetBE32(const_cast(data.c_str()) + 8, ssrc); talk_base::SetBE16(const_cast(data.c_str()) + 2, sequence_number); + if (pl_type >= 0) { + talk_base::Set8(const_cast(data.c_str()), 1, pl_type); + } return data; } std::string CreateRtcpData(uint32 ssrc) { @@ -1438,60 +1443,63 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(CheckNoRtp2()); } - void SendSsrcMuxToSsrcMuxWithRtcpMux() { + void SendBundleToBundle( + const int* pl_types, int len, bool rtcp_mux, bool secure) { + ASSERT_EQ(2, len); int sequence_number1_1 = 0, sequence_number2_2 = 0; - CreateChannels(SSRC_MUX | RTCP | RTCP_MUX, SSRC_MUX | RTCP | RTCP_MUX); + // Only pl_type1 was added to the bundle filter for both |channel1_| + // and |channel2_|. + int pl_type1 = pl_types[0]; + int pl_type2 = pl_types[1]; + int flags = SSRC_MUX | RTCP; + if (secure) flags |= SECURE; + uint32 expected_channels = 2U; + if (rtcp_mux) { + flags |= RTCP_MUX; + expected_channels = 1U; + } + CreateChannels(flags, flags); EXPECT_TRUE(SendInitiate()); EXPECT_EQ(2U, GetTransport1()->channels().size()); - EXPECT_EQ(1U, GetTransport2()->channels().size()); + EXPECT_EQ(expected_channels, GetTransport2()->channels().size()); EXPECT_TRUE(SendAccept()); - EXPECT_EQ(1U, GetTransport1()->channels().size()); - EXPECT_EQ(1U, GetTransport2()->channels().size()); - EXPECT_TRUE(channel1_->ssrc_filter()->IsActive()); - // channel1 - should have media_content2 as remote. i.e. kSsrc2 - EXPECT_TRUE(channel1_->ssrc_filter()->FindStream(kSsrc2)); - EXPECT_TRUE(channel2_->ssrc_filter()->IsActive()); - // channel2 - should have media_content1 as remote. i.e. kSsrc1 - EXPECT_TRUE(channel2_->ssrc_filter()->FindStream(kSsrc1)); - EXPECT_TRUE(SendCustomRtp1(kSsrc1, ++sequence_number1_1)); - EXPECT_TRUE(SendCustomRtp2(kSsrc2, ++sequence_number2_2)); + EXPECT_EQ(expected_channels, GetTransport1()->channels().size()); + EXPECT_EQ(expected_channels, GetTransport2()->channels().size()); + EXPECT_TRUE(channel1_->bundle_filter()->FindPayloadType(pl_type1)); + 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)); + EXPECT_TRUE(CheckCustomRtp2(kSsrc1, sequence_number1_1, pl_type1)); + EXPECT_TRUE(SendCustomRtp2(kSsrc2, ++sequence_number2_2, pl_type1)); + EXPECT_TRUE(CheckCustomRtp1(kSsrc2, sequence_number2_2, pl_type1)); + EXPECT_TRUE(CheckNoRtp1()); + EXPECT_TRUE(CheckNoRtp2()); + + // RTCP test + EXPECT_TRUE(SendCustomRtp1(kSsrc1, ++sequence_number1_1, pl_type2)); + EXPECT_FALSE(CheckCustomRtp2(kSsrc1, sequence_number1_1, pl_type2)); + EXPECT_TRUE(SendCustomRtp2(kSsrc2, ++sequence_number2_2, pl_type2)); + EXPECT_FALSE(CheckCustomRtp1(kSsrc2, sequence_number2_2, pl_type2)); + EXPECT_TRUE(SendCustomRtcp1(kSsrc1)); EXPECT_TRUE(SendCustomRtcp2(kSsrc2)); - EXPECT_TRUE(CheckCustomRtp1(kSsrc2, sequence_number2_2)); - EXPECT_TRUE(CheckNoRtp1()); - EXPECT_TRUE(CheckCustomRtp2(kSsrc1, sequence_number1_1)); - EXPECT_TRUE(CheckNoRtp2()); EXPECT_TRUE(CheckCustomRtcp1(kSsrc2)); EXPECT_TRUE(CheckNoRtcp1()); EXPECT_TRUE(CheckCustomRtcp2(kSsrc1)); EXPECT_TRUE(CheckNoRtcp2()); - } - void SendSsrcMuxToSsrcMux() { - int sequence_number1_1 = 0, sequence_number2_2 = 0; - CreateChannels(SSRC_MUX | RTCP, SSRC_MUX | RTCP); - EXPECT_TRUE(SendInitiate()); - EXPECT_EQ(2U, GetTransport1()->channels().size()); - EXPECT_EQ(2U, GetTransport2()->channels().size()); - EXPECT_TRUE(SendAccept()); - EXPECT_EQ(2U, GetTransport1()->channels().size()); - EXPECT_EQ(2U, GetTransport2()->channels().size()); - EXPECT_TRUE(channel1_->ssrc_filter()->IsActive()); - // channel1 - should have media_content2 as remote. i.e. kSsrc2 - EXPECT_TRUE(channel1_->ssrc_filter()->FindStream(kSsrc2)); - EXPECT_TRUE(channel2_->ssrc_filter()->IsActive()); - // channel2 - should have media_content1 as remote. i.e. kSsrc1 - EXPECT_TRUE(SendCustomRtp1(kSsrc1, ++sequence_number1_1)); - EXPECT_TRUE(SendCustomRtp2(kSsrc2, ++sequence_number2_2)); - EXPECT_TRUE(SendCustomRtcp1(kSsrc1)); - EXPECT_TRUE(SendCustomRtcp2(kSsrc2)); - EXPECT_TRUE(CheckCustomRtp1(kSsrc2, sequence_number2_2)); - EXPECT_FALSE(CheckCustomRtp1(kSsrc1, sequence_number2_2)); - EXPECT_TRUE(CheckCustomRtp2(kSsrc1, sequence_number1_1)); - EXPECT_FALSE(CheckCustomRtp2(kSsrc2, sequence_number1_1)); - EXPECT_TRUE(CheckCustomRtcp1(kSsrc2)); + EXPECT_TRUE(SendCustomRtcp1(kSsrc2)); + EXPECT_TRUE(SendCustomRtcp2(kSsrc1)); EXPECT_FALSE(CheckCustomRtcp1(kSsrc1)); - EXPECT_TRUE(CheckCustomRtcp2(kSsrc1)); EXPECT_FALSE(CheckCustomRtcp2(kSsrc2)); } @@ -1753,9 +1761,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { error_); } - void TestSrtpError() { - static const unsigned char kBadPacket[] = { - 0x84, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 + void TestSrtpError(int pl_type) { + // For Audio, only pl_type 0 is added to the bundle filter. + // For Video, only pl_type 97 is added to the bundle filter. + // So we need to pass in pl_type so that the packet can pass through + // the bundle filter before it can be processed by the srtp filter. + // The packet is not a valid srtp packet because it is too short. + unsigned const char kBadPacket[] = { + 0x84, pl_type, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }; CreateChannels(RTCP | SECURE, RTCP | SECURE); EXPECT_FALSE(channel1_->secure()); @@ -2277,7 +2290,7 @@ TEST_F(VoiceChannelTest, TestChangeStateError) { } TEST_F(VoiceChannelTest, TestSrtpError) { - Base::TestSrtpError(); + Base::TestSrtpError(kAudioPts[0]); } TEST_F(VoiceChannelTest, TestOnReadyToSend) { @@ -2389,12 +2402,22 @@ TEST_F(VoiceChannelTest, TestScaleVolumeMultiwayCall) { EXPECT_DOUBLE_EQ(0.0, right); } -TEST_F(VoiceChannelTest, SendSsrcMuxToSsrcMux) { - Base::SendSsrcMuxToSsrcMux(); +TEST_F(VoiceChannelTest, SendBundleToBundle) { + Base::SendBundleToBundle(kAudioPts, ARRAY_SIZE(kAudioPts), false, false); } -TEST_F(VoiceChannelTest, SendSsrcMuxToSsrcMuxWithRtcpMux) { - Base::SendSsrcMuxToSsrcMuxWithRtcpMux(); +TEST_F(VoiceChannelTest, SendBundleToBundleSecure) { + Base::SendBundleToBundle(kAudioPts, ARRAY_SIZE(kAudioPts), false, true); +} + +TEST_F(VoiceChannelTest, SendBundleToBundleWithRtcpMux) { + Base::SendBundleToBundle( + kAudioPts, ARRAY_SIZE(kAudioPts), true, false); +} + +TEST_F(VoiceChannelTest, SendBundleToBundleWithRtcpMuxSecure) { + Base::SendBundleToBundle( + kAudioPts, ARRAY_SIZE(kAudioPts), true, true); } TEST_F(VoiceChannelTest, TestSetChannelOptions) { @@ -2604,18 +2627,28 @@ TEST_F(VideoChannelTest, TestFlushRtcp) { Base::TestFlushRtcp(); } -TEST_F(VideoChannelTest, SendSsrcMuxToSsrcMux) { - Base::SendSsrcMuxToSsrcMux(); +TEST_F(VideoChannelTest, SendBundleToBundle) { + Base::SendBundleToBundle(kVideoPts, ARRAY_SIZE(kVideoPts), false, false); } -TEST_F(VideoChannelTest, SendSsrcMuxToSsrcMuxWithRtcpMux) { - Base::SendSsrcMuxToSsrcMuxWithRtcpMux(); +TEST_F(VideoChannelTest, SendBundleToBundleSecure) { + Base::SendBundleToBundle(kVideoPts, ARRAY_SIZE(kVideoPts), false, true); +} + +TEST_F(VideoChannelTest, SendBundleToBundleWithRtcpMux) { + Base::SendBundleToBundle( + kVideoPts, ARRAY_SIZE(kVideoPts), true, false); +} + +TEST_F(VideoChannelTest, SendBundleToBundleWithRtcpMuxSecure) { + Base::SendBundleToBundle( + kVideoPts, ARRAY_SIZE(kVideoPts), true, true); } // TODO(gangji): Add VideoChannelTest.TestChangeStateError. TEST_F(VideoChannelTest, TestSrtpError) { - Base::TestSrtpError(); + Base::TestSrtpError(kVideoPts[0]); } TEST_F(VideoChannelTest, TestOnReadyToSend) {