From 4dfb8cef514bf030f08d61289543e32127366f12 Mon Sep 17 00:00:00 2001 From: zhihuang Date: Wed, 23 Nov 2016 10:30:12 -0800 Subject: [PATCH] Make the default value of rtcp-mux policy to required. Change the default value of rtcp-mux policy in RTCConfiguration. Refactor the peerconnectioninterface and webrtcsession unit tests. BUG=webrtc:6030 Review-Url: https://codereview.webrtc.org/2043193003 Cr-Commit-Position: refs/heads/master@{#15217} --- .../java/src/org/webrtc/PeerConnection.java | 2 +- webrtc/api/peerconnectioninterface.h | 2 +- .../api/peerconnectioninterface_unittest.cc | 16 +++++++++++- webrtc/api/test/testsdpstrings.h | 2 ++ webrtc/api/webrtcsession_unittest.cc | 25 ++++++++++++------- 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/webrtc/api/android/java/src/org/webrtc/PeerConnection.java b/webrtc/api/android/java/src/org/webrtc/PeerConnection.java index 359b52ead7..ab08598519 100644 --- a/webrtc/api/android/java/src/org/webrtc/PeerConnection.java +++ b/webrtc/api/android/java/src/org/webrtc/PeerConnection.java @@ -146,7 +146,7 @@ public class PeerConnection { public RTCConfiguration(List iceServers) { iceTransportsType = IceTransportsType.ALL; bundlePolicy = BundlePolicy.BALANCED; - rtcpMuxPolicy = RtcpMuxPolicy.NEGOTIATE; + rtcpMuxPolicy = RtcpMuxPolicy.REQUIRE; tcpCandidatePolicy = TcpCandidatePolicy.ENABLED; candidateNetworkPolicy = candidateNetworkPolicy.ALL; this.iceServers = iceServers; diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index b304b144e8..e14b79ca6c 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -295,7 +295,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // at the same time. IceServers servers; BundlePolicy bundle_policy = kBundlePolicyBalanced; - RtcpMuxPolicy rtcp_mux_policy = kRtcpMuxPolicyNegotiate; + RtcpMuxPolicy rtcp_mux_policy = kRtcpMuxPolicyRequire; TcpCandidatePolicy tcp_candidate_policy = kTcpCandidatePolicyEnabled; CandidateNetworkPolicy candidate_network_policy = kCandidateNetworkPolicyAll; diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index c35141224c..0726c75d4d 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -79,6 +79,7 @@ static const char kSdpStringWithStream1[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n" "a=ssrc:1 cname:stream1\r\n" "a=ssrc:1 mslabel:stream1\r\n" @@ -86,6 +87,7 @@ static const char kSdpStringWithStream1[] = "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n" "a=ssrc:2 cname:stream1\r\n" "a=ssrc:2 mslabel:stream1\r\n" @@ -108,7 +110,8 @@ static const char kSdpStringWithStream1AudioTrackOnly[] = "a=rtpmap:103 ISAC/16000\r\n" "a=ssrc:1 cname:stream1\r\n" "a=ssrc:1 mslabel:stream1\r\n" - "a=ssrc:1 label:audiotrack0\r\n"; + "a=ssrc:1 label:audiotrack0\r\n" + "a=rtcp-mux\r\n"; // Reference SDP with two MediaStreams with label "stream1" and "stream2. Each // MediaStreams have one audio track and one video track. @@ -126,6 +129,7 @@ static const char kSdpStringWithStream1And2[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n" "a=ssrc:1 cname:stream1\r\n" "a=ssrc:1 msid:stream1 audiotrack0\r\n" @@ -134,6 +138,7 @@ static const char kSdpStringWithStream1And2[] = "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/0\r\n" "a=ssrc:2 cname:stream1\r\n" "a=ssrc:2 msid:stream1 videotrack0\r\n" @@ -153,10 +158,12 @@ static const char kSdpStringWithoutStreams[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n" "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n"; // Reference SDP without MediaStreams. Msid is supported. @@ -173,10 +180,12 @@ static const char kSdpStringWithMsidWithoutStreams[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n" "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n"; // Reference SDP without MediaStreams and audio only. @@ -192,6 +201,7 @@ static const char kSdpStringWithoutStreamsAudioOnly[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n"; // Reference SENDONLY SDP without MediaStreams. Msid is not supported. @@ -208,11 +218,13 @@ static const char kSdpStringSendOnlyWithoutStreams[] = "a=mid:audio\r\n" "a=sendrecv\r\n" "a=sendonly\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n" "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" "a=sendonly\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n"; static const char kSdpStringInit[] = @@ -230,12 +242,14 @@ static const char kSdpStringAudio[] = "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:103 ISAC/16000\r\n"; static const char kSdpStringVideo[] = "m=video 1 RTP/AVPF 120\r\n" "a=mid:video\r\n" "a=sendrecv\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n"; static const char kSdpStringMs1Audio0[] = diff --git a/webrtc/api/test/testsdpstrings.h b/webrtc/api/test/testsdpstrings.h index 158b16c03e..b203aad6a9 100644 --- a/webrtc/api/test/testsdpstrings.h +++ b/webrtc/api/test/testsdpstrings.h @@ -29,6 +29,7 @@ static const char kFireFoxSdpOffer[] = "c=IN IP4 74.95.2.170\r\n" "a=rtpmap:109 opus/48000/2\r\n" "a=ptime:20\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:0 PCMU/8000\r\n" "a=rtpmap:8 PCMA/8000\r\n" "a=rtpmap:101 telephone-event/8000\r\n" @@ -46,6 +47,7 @@ static const char kFireFoxSdpOffer[] = " 10.0.254.2 rport 50962\r\n" "m=video 38826 RTP/SAVPF 120\r\n" "c=IN IP4 74.95.2.170\r\n" + "a=rtcp-mux\r\n" "a=rtpmap:120 VP8/90000\r\n" "a=sendrecv\r\n" "a=candidate:0 1 UDP 2112946431 172.16.191.1 62017 typ host\r\n" diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 6efb996f76..aa1159bdee 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -383,7 +383,8 @@ class WebRtcSessionTest // options. When DTLS is enabled a certificate will be used if provided, // otherwise one will be generated using the |cert_generator|. void Init( - std::unique_ptr cert_generator) { + std::unique_ptr cert_generator, + PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { ASSERT_TRUE(session_.get() == NULL); session_.reset(new WebRtcSessionForTest( media_controller_.get(), rtc::Thread::Current(), rtc::Thread::Current(), @@ -397,6 +398,7 @@ class WebRtcSessionTest session_->GetOnDestroyedSignal()->connect( this, &WebRtcSessionTest::OnSessionDestroyed); + configuration_.rtcp_mux_policy = rtcp_mux_policy; EXPECT_EQ(PeerConnectionInterface::kIceConnectionNew, observer_.ice_connection_state_); EXPECT_EQ(PeerConnectionInterface::kIceGatheringNew, @@ -415,7 +417,13 @@ class WebRtcSessionTest void OnSessionDestroyed() { session_destroyed_ = true; } - void Init() { Init(nullptr); } + void Init() { + Init(nullptr, PeerConnectionInterface::kRtcpMuxPolicyNegotiate); + } + + void Init(PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { + Init(nullptr, rtcp_mux_policy); + } void InitWithBundlePolicy( PeerConnectionInterface::BundlePolicy bundle_policy) { @@ -426,8 +434,7 @@ class WebRtcSessionTest void InitWithRtcpMuxPolicy( PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { PeerConnectionInterface::RTCConfiguration configuration; - configuration_.rtcp_mux_policy = rtcp_mux_policy; - Init(); + Init(rtcp_mux_policy); } // Successfully init with DTLS; with a certificate generated and supplied or @@ -443,7 +450,8 @@ class WebRtcSessionTest } else { RTC_CHECK(false); } - Init(std::move(cert_generator)); + Init(std::move(cert_generator), + PeerConnectionInterface::kRtcpMuxPolicyNegotiate); } // Init with DTLS with a store that will fail to generate a certificate. @@ -451,7 +459,8 @@ class WebRtcSessionTest std::unique_ptr cert_generator( new FakeRTCCertificateGenerator()); cert_generator->set_should_fail(true); - Init(std::move(cert_generator)); + Init(std::move(cert_generator), + PeerConnectionInterface::kRtcpMuxPolicyNegotiate); } void InitWithDtmfCodec() { @@ -3347,10 +3356,8 @@ TEST_F(WebRtcSessionTest, TestAddChannelToConnectedBundle) { // answer to find out if more transports are needed. configuration_.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - configuration_.rtcp_mux_policy = - PeerConnectionInterface::kRtcpMuxPolicyRequire; options_.disable_encryption = true; - Init(); + Init(PeerConnectionInterface::kRtcpMuxPolicyRequire); // Negotiate an audio channel with MAX_BUNDLE enabled. SendAudioOnlyStream2();