diff --git a/talk/app/webrtc/java/jni/classreferenceholder.cc b/talk/app/webrtc/java/jni/classreferenceholder.cc index 7ff48b972..f07492133 100644 --- a/talk/app/webrtc/java/jni/classreferenceholder.cc +++ b/talk/app/webrtc/java/jni/classreferenceholder.cc @@ -93,6 +93,7 @@ ClassReferenceHolder::ClassReferenceHolder(JNIEnv* jni) { LoadClass(jni, "org/webrtc/MediaStream"); LoadClass(jni, "org/webrtc/MediaStreamTrack$State"); LoadClass(jni, "org/webrtc/PeerConnection$BundlePolicy"); + LoadClass(jni, "org/webrtc/PeerConnection$RtcpMuxPolicy"); LoadClass(jni, "org/webrtc/PeerConnection$IceConnectionState"); LoadClass(jni, "org/webrtc/PeerConnection$IceGatheringState"); LoadClass(jni, "org/webrtc/PeerConnection$IceTransportsType"); @@ -143,4 +144,3 @@ jclass FindClass(JNIEnv* jni, const char* name) { } } // namespace webrtc_jni - diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index a6cba440c..9e1ce74e1 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -1216,6 +1216,22 @@ JavaBundlePolicyToNativeType(JNIEnv* jni, jobject j_bundle_policy) { return PeerConnectionInterface::kBundlePolicyBalanced; } +static PeerConnectionInterface::RtcpMuxPolicy +JavaRtcpMuxPolicyToNativeType(JNIEnv* jni, jobject j_rtcp_mux_policy) { + std::string enum_name = GetJavaEnumName( + jni, "org/webrtc/PeerConnection$RtcpMuxPolicy", + j_rtcp_mux_policy); + + if (enum_name == "NEGOTIATE") + return PeerConnectionInterface::kRtcpMuxPolicyNegotiate; + + if (enum_name == "REQUIRE") + return PeerConnectionInterface::kRtcpMuxPolicyRequire; + + CHECK(false) << "Unexpected RtcpMuxPolicy enum_name " << enum_name; + return PeerConnectionInterface::kRtcpMuxPolicyNegotiate; +} + static PeerConnectionInterface::TcpCandidatePolicy JavaTcpCandidatePolicyToNativeType( JNIEnv* jni, jobject j_tcp_candidate_policy) { @@ -1292,6 +1308,12 @@ JOW(jlong, PeerConnectionFactory_nativeCreatePeerConnection)( jobject j_bundle_policy = GetObjectField( jni, j_rtc_config, j_bundle_policy_id); + jfieldID j_rtcp_mux_policy_id = GetFieldID( + jni, j_rtc_config_class, "rtcpMuxPolicy", + "Lorg/webrtc/PeerConnection$RtcpMuxPolicy;"); + jobject j_rtcp_mux_policy = GetObjectField( + jni, j_rtc_config, j_rtcp_mux_policy_id); + jfieldID j_tcp_candidate_policy_id = GetFieldID( jni, j_rtc_config_class, "tcpCandidatePolicy", "Lorg/webrtc/PeerConnection$TcpCandidatePolicy;"); @@ -1311,6 +1333,8 @@ JOW(jlong, PeerConnectionFactory_nativeCreatePeerConnection)( rtc_config.type = JavaIceTransportsTypeToNativeType(jni, j_ice_transports_type); rtc_config.bundle_policy = JavaBundlePolicyToNativeType(jni, j_bundle_policy); + rtc_config.rtcp_mux_policy = + JavaRtcpMuxPolicyToNativeType(jni, j_rtcp_mux_policy); rtc_config.tcp_candidate_policy = JavaTcpCandidatePolicyToNativeType(jni, j_tcp_candidate_policy); JavaIceServersToJsepIceServers(jni, j_ice_servers, &rtc_config.servers); diff --git a/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java b/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java index 80e7bfe6c..a229e17d0 100644 --- a/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java +++ b/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java @@ -117,7 +117,11 @@ public class PeerConnection { BALANCED, MAXBUNDLE, MAXCOMPAT }; - /** Java version of PeerConnectionInterface.BundlePolicy */ + /** Java version of PeerConnectionInterface.RtcpMuxPolicy */ + public enum RtcpMuxPolicy { + NEGOTIATE, REQUIRE + }; + /** Java version of PeerConnectionInterface.TcpCandidatePolicy */ public enum TcpCandidatePolicy { ENABLED, DISABLED }; @@ -127,12 +131,14 @@ public class PeerConnection { public IceTransportsType iceTransportsType; public List iceServers; public BundlePolicy bundlePolicy; + public RtcpMuxPolicy rtcpMuxPolicy; public TcpCandidatePolicy tcpCandidatePolicy; public int audioJitterBufferMaxPackets; public RTCConfiguration(List iceServers) { iceTransportsType = IceTransportsType.ALL; bundlePolicy = BundlePolicy.BALANCED; + rtcpMuxPolicy = RtcpMuxPolicy.NEGOTIATE; tcpCandidatePolicy = TcpCandidatePolicy.ENABLED; this.iceServers = iceServers; audioJitterBufferMaxPackets = 50; diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index e32676e25..960e286f7 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -197,6 +197,12 @@ class PeerConnectionInterface : public rtc::RefCountInterface { kBundlePolicyMaxCompat }; + // https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-09#section-4.1.1 + enum RtcpMuxPolicy { + kRtcpMuxPolicyNegotiate, + kRtcpMuxPolicyRequire, + }; + enum TcpCandidatePolicy { kTcpCandidatePolicyEnabled, kTcpCandidatePolicyDisabled @@ -210,12 +216,14 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // at the same time. IceServers servers; BundlePolicy bundle_policy; + RtcpMuxPolicy rtcp_mux_policy; TcpCandidatePolicy tcp_candidate_policy; int audio_jitter_buffer_max_packets; RTCConfiguration() : type(kAll), bundle_policy(kBundlePolicyBalanced), + rtcp_mux_policy(kRtcpMuxPolicyNegotiate), tcp_candidate_policy(kTcpCandidatePolicyEnabled), audio_jitter_buffer_max_packets(50) {} }; diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index e2a9d60f2..e4613650d 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -523,6 +523,7 @@ bool WebRtcSession::Initialize( DTLSIdentityServiceInterface* dtls_identity_service, const PeerConnectionInterface::RTCConfiguration& rtc_configuration) { bundle_policy_ = rtc_configuration.bundle_policy; + rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy; // TODO(perkj): Take |constraints| into consideration. Return false if not all // mandatory constraints can be fulfilled. Note that |constraints| @@ -1600,6 +1601,18 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) { } } + if (rtcp_mux_policy_ == PeerConnectionInterface::kRtcpMuxPolicyRequire) { + if (voice_channel()) { + voice_channel()->ActivateRtcpMux(); + } + if (video_channel()) { + video_channel()->ActivateRtcpMux(); + } + if (data_channel()) { + data_channel()->ActivateRtcpMux(); + } + } + // Enable bundle before when kMaxBundle policy is in effect. if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle) { const cricket::ContentGroup* bundle_group = desc->GetGroupByName( diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index aa1deb523..570d32f03 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -413,6 +413,9 @@ class WebRtcSession : public cricket::BaseSession, // Declares the bundle policy for the WebRTCSession. PeerConnectionInterface::BundlePolicy bundle_policy_; + // Declares the RTCP mux policy for the WebRTCSession. + PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy_; + DISALLOW_COPY_AND_ASSIGN(WebRtcSession); }; } // namespace webrtc diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index e4f39f822..2b6f207e2 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -156,8 +156,6 @@ static const char kSdpWithRtx[] = "a=rtpmap:96 rtx/90000\r\n" "a=fmtp:96 apt=0\r\n"; -static const int kAudioJitterBufferMaxPackets = 50; - // Add some extra |newlines| to the |message| after |line|. static void InjectAfter(const std::string& line, const std::string& newlines, @@ -405,11 +403,6 @@ class WebRtcSessionTest : public testing::Test { void Init() { PeerConnectionInterface::RTCConfiguration configuration; - configuration.type = PeerConnectionInterface::kAll; - configuration.bundle_policy = - PeerConnectionInterface::kBundlePolicyBalanced; - configuration.audio_jitter_buffer_max_packets = - kAudioJitterBufferMaxPackets; Init(NULL, configuration); } @@ -417,20 +410,20 @@ class WebRtcSessionTest : public testing::Test { PeerConnectionInterface::IceTransportsType ice_transport_type) { PeerConnectionInterface::RTCConfiguration configuration; configuration.type = ice_transport_type; - configuration.bundle_policy = - PeerConnectionInterface::kBundlePolicyBalanced; - configuration.audio_jitter_buffer_max_packets = - kAudioJitterBufferMaxPackets; Init(NULL, configuration); } void InitWithBundlePolicy( PeerConnectionInterface::BundlePolicy bundle_policy) { PeerConnectionInterface::RTCConfiguration configuration; - configuration.type = PeerConnectionInterface::kAll; configuration.bundle_policy = bundle_policy; - configuration.audio_jitter_buffer_max_packets = - kAudioJitterBufferMaxPackets; + Init(NULL, configuration); + } + + void InitWithRtcpMuxPolicy( + PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { + PeerConnectionInterface::RTCConfiguration configuration; + configuration.rtcp_mux_policy = rtcp_mux_policy; Init(NULL, configuration); } @@ -438,11 +431,6 @@ class WebRtcSessionTest : public testing::Test { FakeIdentityService* identity_service = new FakeIdentityService(); identity_service->set_should_fail(identity_request_should_fail); PeerConnectionInterface::RTCConfiguration configuration; - configuration.type = PeerConnectionInterface::kAll; - configuration.bundle_policy = - PeerConnectionInterface::kBundlePolicyBalanced; - configuration.audio_jitter_buffer_max_packets = - kAudioJitterBufferMaxPackets; Init(identity_service, configuration); } @@ -2789,6 +2777,46 @@ TEST_F(WebRtcSessionTest, TestMaxBundleWithSetRemoteDescriptionFirst) { session_->GetTransportProxy("video")->impl()); } +TEST_F(WebRtcSessionTest, TestRequireRtcpMux) { + InitWithRtcpMuxPolicy(PeerConnectionInterface::kRtcpMuxPolicyRequire); + mediastream_signaling_.SendAudioVideoStream1(); + + PeerConnectionInterface::RTCOfferAnswerOptions options; + SessionDescriptionInterface* offer = CreateOffer(options); + SetLocalDescriptionWithoutError(offer); + + EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2)); + EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2)); + + mediastream_signaling_.SendAudioVideoStream2(); + SessionDescriptionInterface* answer = + CreateRemoteAnswer(session_->local_description()); + SetRemoteDescriptionWithoutError(answer); + + EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2)); + EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2)); +} + +TEST_F(WebRtcSessionTest, TestNegotiateRtcpMux) { + InitWithRtcpMuxPolicy(PeerConnectionInterface::kRtcpMuxPolicyNegotiate); + mediastream_signaling_.SendAudioVideoStream1(); + + PeerConnectionInterface::RTCOfferAnswerOptions options; + SessionDescriptionInterface* offer = CreateOffer(options); + SetLocalDescriptionWithoutError(offer); + + EXPECT_TRUE(session_->GetTransportProxy("audio")->impl()->HasChannel(2)); + EXPECT_TRUE(session_->GetTransportProxy("video")->impl()->HasChannel(2)); + + mediastream_signaling_.SendAudioVideoStream2(); + SessionDescriptionInterface* answer = + CreateRemoteAnswer(session_->local_description()); + SetRemoteDescriptionWithoutError(answer); + + EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2)); + EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2)); +} + // This test verifies that SetLocalDescription and SetRemoteDescription fails // if BUNDLE is enabled but rtcp-mux is disabled in m-lines. TEST_F(WebRtcSessionTest, TestDisabledRtcpMuxWithBundleEnabled) { diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 0034f15d9..95f5bc1e8 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -1042,6 +1042,18 @@ bool BaseChannel::SetSrtp_w(const std::vector& cryptos, return true; } +void BaseChannel::ActivateRtcpMux() { + worker_thread_->Invoke(Bind( + &BaseChannel::ActivateRtcpMux_w, this)); +} + +void BaseChannel::ActivateRtcpMux_w() { + if (!rtcp_mux_filter_.IsActive()) { + rtcp_mux_filter_.SetActive(); + set_rtcp_transport_channel(NULL); + } +} + bool BaseChannel::SetRtcpMux_w(bool enable, ContentAction action, ContentSource src, std::string* error_desc) { diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index 441fe64b6..904777e17 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -108,6 +108,11 @@ class BaseChannel bool writable() const { return writable_; } bool IsStreamMuted(uint32 ssrc); + // Activate RTCP mux, regardless of the state so far. Once + // activated, it can not be deactivated, and if the remote + // description doesn't support RTCP mux, setting the remote + // description will fail. + void ActivateRtcpMux(); bool PushdownLocalDescription(const SessionDescription* local_desc, ContentAction action, std::string* error_desc); @@ -350,6 +355,7 @@ class BaseChannel ContentAction action, ContentSource src, std::string* error_desc); + void ActivateRtcpMux_w(); bool SetRtcpMux_w(bool enable, ContentAction action, ContentSource src, diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index a55f6e8cb..8128190a5 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -1140,6 +1140,89 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(CheckNoRtcp2()); } + // Check that RTP and RTCP are transmitted ok when both sides + // support mux and one the offerer requires mux. + void SendRequireRtcpMuxToRtcpMux() { + CreateChannels(RTCP | RTCP_MUX, RTCP | RTCP_MUX); + channel1_->ActivateRtcpMux(); + EXPECT_TRUE(SendInitiate()); + EXPECT_EQ(1U, GetTransport1()->channels().size()); + EXPECT_EQ(1U, GetTransport2()->channels().size()); + EXPECT_TRUE(SendAccept()); + EXPECT_TRUE(SendRtp1()); + EXPECT_TRUE(SendRtp2()); + EXPECT_TRUE(SendRtcp1()); + EXPECT_TRUE(SendRtcp2()); + EXPECT_TRUE(CheckRtp1()); + EXPECT_TRUE(CheckRtp2()); + EXPECT_TRUE(CheckNoRtp1()); + EXPECT_TRUE(CheckNoRtp2()); + EXPECT_TRUE(CheckRtcp1()); + EXPECT_TRUE(CheckRtcp2()); + EXPECT_TRUE(CheckNoRtcp1()); + EXPECT_TRUE(CheckNoRtcp2()); + } + + // Check that RTP and RTCP are transmitted ok when both sides + // support mux and one the answerer requires rtcp mux. + void SendRtcpMuxToRequireRtcpMux() { + CreateChannels(RTCP | RTCP_MUX, RTCP | RTCP_MUX); + channel2_->ActivateRtcpMux(); + EXPECT_TRUE(SendInitiate()); + EXPECT_EQ(2U, GetTransport1()->channels().size()); + EXPECT_EQ(1U, GetTransport2()->channels().size()); + EXPECT_TRUE(SendAccept()); + EXPECT_EQ(1U, GetTransport1()->channels().size()); + EXPECT_TRUE(SendRtp1()); + EXPECT_TRUE(SendRtp2()); + EXPECT_TRUE(SendRtcp1()); + EXPECT_TRUE(SendRtcp2()); + EXPECT_TRUE(CheckRtp1()); + EXPECT_TRUE(CheckRtp2()); + EXPECT_TRUE(CheckNoRtp1()); + EXPECT_TRUE(CheckNoRtp2()); + EXPECT_TRUE(CheckRtcp1()); + EXPECT_TRUE(CheckRtcp2()); + EXPECT_TRUE(CheckNoRtcp1()); + EXPECT_TRUE(CheckNoRtcp2()); + } + + // Check that RTP and RTCP are transmitted ok when both sides + // require mux. + void SendRequireRtcpMuxToRequireRtcpMux() { + CreateChannels(RTCP | RTCP_MUX, RTCP | RTCP_MUX); + channel1_->ActivateRtcpMux(); + channel2_->ActivateRtcpMux(); + EXPECT_TRUE(SendInitiate()); + EXPECT_EQ(1U, GetTransport1()->channels().size()); + EXPECT_EQ(1U, GetTransport2()->channels().size()); + EXPECT_TRUE(SendAccept()); + EXPECT_EQ(1U, GetTransport1()->channels().size()); + EXPECT_TRUE(SendRtp1()); + EXPECT_TRUE(SendRtp2()); + EXPECT_TRUE(SendRtcp1()); + EXPECT_TRUE(SendRtcp2()); + EXPECT_TRUE(CheckRtp1()); + EXPECT_TRUE(CheckRtp2()); + EXPECT_TRUE(CheckNoRtp1()); + EXPECT_TRUE(CheckNoRtp2()); + EXPECT_TRUE(CheckRtcp1()); + EXPECT_TRUE(CheckRtcp2()); + EXPECT_TRUE(CheckNoRtcp1()); + EXPECT_TRUE(CheckNoRtcp2()); + } + + // Check that SendAccept fails if the answerer doesn't support mux + // and the offerer requires it. + void SendRequireRtcpMuxToNoRtcpMux() { + CreateChannels(RTCP | RTCP_MUX, RTCP); + channel1_->ActivateRtcpMux(); + EXPECT_TRUE(SendInitiate()); + EXPECT_EQ(1U, GetTransport1()->channels().size()); + EXPECT_EQ(2U, GetTransport2()->channels().size()); + EXPECT_FALSE(SendAccept()); + } + // Check that RTCP data sent by the initiator before the accept is not muxed. void SendEarlyRtcpMuxToRtcp() { CreateChannels(RTCP | RTCP_MUX, RTCP); @@ -2085,6 +2168,22 @@ TEST_F(VoiceChannelTest, SendRtcpMuxToRtcpMux) { Base::SendRtcpMuxToRtcpMux(); } +TEST_F(VoiceChannelTest, SendRequireRtcpMuxToRtcpMux) { + Base::SendRequireRtcpMuxToRtcpMux(); +} + +TEST_F(VoiceChannelTest, SendRtcpMuxToRequireRtcpMux) { + Base::SendRtcpMuxToRequireRtcpMux(); +} + +TEST_F(VoiceChannelTest, SendRequireRtcpMuxToRequireRtcpMux) { + Base::SendRequireRtcpMuxToRequireRtcpMux(); +} + +TEST_F(VoiceChannelTest, SendRequireRtcpMuxToNoRtcpMux) { + Base::SendRequireRtcpMuxToNoRtcpMux(); +} + TEST_F(VoiceChannelTest, SendEarlyRtcpMuxToRtcp) { Base::SendEarlyRtcpMuxToRtcp(); } @@ -2507,6 +2606,22 @@ TEST_F(VideoChannelTest, SendRtcpMuxToRtcpMux) { Base::SendRtcpMuxToRtcpMux(); } +TEST_F(VideoChannelTest, SendRequireRtcpMuxToRtcpMux) { + Base::SendRequireRtcpMuxToRtcpMux(); +} + +TEST_F(VideoChannelTest, SendRtcpMuxToRequireRtcpMux) { + Base::SendRtcpMuxToRequireRtcpMux(); +} + +TEST_F(VideoChannelTest, SendRequireRtcpMuxToRequireRtcpMux) { + Base::SendRequireRtcpMuxToRequireRtcpMux(); +} + +TEST_F(VideoChannelTest, SendRequireRtcpMuxToNoRtcpMux) { + Base::SendRequireRtcpMuxToNoRtcpMux(); +} + TEST_F(VideoChannelTest, SendEarlyRtcpMuxToRtcp) { Base::SendEarlyRtcpMuxToRtcp(); } diff --git a/talk/session/media/rtcpmuxfilter.cc b/talk/session/media/rtcpmuxfilter.cc index f95199241..f21e0eeb5 100644 --- a/talk/session/media/rtcpmuxfilter.cc +++ b/talk/session/media/rtcpmuxfilter.cc @@ -40,7 +40,16 @@ bool RtcpMuxFilter::IsActive() const { state_ == ST_ACTIVE; } +void RtcpMuxFilter::SetActive() { + state_ = ST_ACTIVE; +} + bool RtcpMuxFilter::SetOffer(bool offer_enable, ContentSource src) { + if (state_ == ST_ACTIVE) { + // Fail if we try to deactivate and no-op if we try and activate. + return offer_enable; + } + if (!ExpectOffer(offer_enable, src)) { LOG(LS_ERROR) << "Invalid state for change of RTCP mux offer"; return false; @@ -53,6 +62,11 @@ bool RtcpMuxFilter::SetOffer(bool offer_enable, ContentSource src) { bool RtcpMuxFilter::SetProvisionalAnswer(bool answer_enable, ContentSource src) { + if (state_ == ST_ACTIVE) { + // Fail if we try to deactivate and no-op if we try and activate. + return answer_enable; + } + if (!ExpectAnswer(src)) { LOG(LS_ERROR) << "Invalid state for RTCP mux provisional answer"; return false; @@ -83,6 +97,11 @@ bool RtcpMuxFilter::SetProvisionalAnswer(bool answer_enable, } bool RtcpMuxFilter::SetAnswer(bool answer_enable, ContentSource src) { + if (state_ == ST_ACTIVE) { + // Fail if we try to deactivate and no-op if we try and activate. + return answer_enable; + } + if (!ExpectAnswer(src)) { LOG(LS_ERROR) << "Invalid state for RTCP mux answer"; return false; @@ -100,19 +119,24 @@ bool RtcpMuxFilter::SetAnswer(bool answer_enable, ContentSource src) { return true; } -bool RtcpMuxFilter::DemuxRtcp(const char* data, int len) { - // If we're muxing RTP/RTCP, we must inspect each packet delivered and - // determine whether it is RTP or RTCP. We do so by checking the packet type, - // and assuming RTP if type is 0-63 or 96-127. For additional details, see - // http://tools.ietf.org/html/rfc5761. - // Note that if we offer RTCP mux, we may receive muxed RTCP before we - // receive the answer, so we operate in that state too. - if (!offer_enable_ || state_ < ST_SENTOFFER) { +// Check the RTP payload type. If 63 < payload type < 96, it's RTCP. +// For additional details, see http://tools.ietf.org/html/rfc5761. +bool IsRtcp(const char* data, int len) { + if (len < 2) { return false; } + char pt = data[1] & 0x7F; + return (63 < pt) && (pt < 96); +} - int type = (len >= 2) ? (static_cast(data[1]) & 0x7F) : 0; - return (type >= 64 && type < 96); +bool RtcpMuxFilter::DemuxRtcp(const char* data, int len) { + // If we're muxing RTP/RTCP, we must inspect each packet delivered + // and determine whether it is RTP or RTCP. We do so by looking at + // the RTP payload type (see IsRtcp). Note that if we offer RTCP + // mux, we may receive muxed RTCP before we receive the answer, so + // we operate in that state too. + bool offered_mux = ((state_ == ST_SENTOFFER) && offer_enable_); + return (IsActive() || offered_mux) && IsRtcp(data, len); } bool RtcpMuxFilter::ExpectOffer(bool offer_enable, ContentSource source) { diff --git a/talk/session/media/rtcpmuxfilter.h b/talk/session/media/rtcpmuxfilter.h index 948b3c33f..8888fd4ff 100644 --- a/talk/session/media/rtcpmuxfilter.h +++ b/talk/session/media/rtcpmuxfilter.h @@ -41,6 +41,9 @@ class RtcpMuxFilter { // Whether the filter is active, i.e. has RTCP mux been properly negotiated. bool IsActive() const; + // Make the filter active, regardless of the current state. + void SetActive(); + // Specifies whether the offer indicates the use of RTCP mux. bool SetOffer(bool offer_enable, ContentSource src); diff --git a/talk/session/media/rtcpmuxfilter_unittest.cc b/talk/session/media/rtcpmuxfilter_unittest.cc index 1305c891a..d4e6376e6 100644 --- a/talk/session/media/rtcpmuxfilter_unittest.cc +++ b/talk/session/media/rtcpmuxfilter_unittest.cc @@ -212,3 +212,44 @@ TEST(RtcpMuxFilterTest, KeepFilterDisabledDuringUpdate) { EXPECT_TRUE(filter.SetAnswer(false, cricket::CS_LOCAL)); EXPECT_FALSE(filter.IsActive()); } + +// Test that we can SetActive and then can't deactivate. +TEST(RtcpMuxFilterTest, SetActiveCantDeactivate) { + cricket::RtcpMuxFilter filter; + const char data[] = { 0, 73, 0, 0 }; + const int len = 4; + + filter.SetActive(); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.DemuxRtcp(data, len)); + + EXPECT_FALSE(filter.SetOffer(false, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetOffer(true, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); + + EXPECT_FALSE(filter.SetProvisionalAnswer(false, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetProvisionalAnswer(true, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + + EXPECT_FALSE(filter.SetAnswer(false, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetAnswer(true, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + + EXPECT_FALSE(filter.SetOffer(false, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetOffer(true, cricket::CS_REMOTE)); + EXPECT_TRUE(filter.IsActive()); + + EXPECT_FALSE(filter.SetProvisionalAnswer(false, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetProvisionalAnswer(true, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); + + EXPECT_FALSE(filter.SetAnswer(false, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); + EXPECT_TRUE(filter.SetAnswer(true, cricket::CS_LOCAL)); + EXPECT_TRUE(filter.IsActive()); +}