diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index 7f1f452378..506a21582f 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -66,6 +66,9 @@ static const char kTransport[] = "transport"; // NOTE: Must be in the same order as the ServiceType enum. static const char* kValidIceServiceTypes[] = {"stun", "stuns", "turn", "turns"}; +// The length of RTCP CNAMEs. +static const int kRtcpCnameLength = 16; + // NOTE: A loop below assumes that the first value of this enum is 0 and all // other values are incremental. enum ServiceType { @@ -377,6 +380,16 @@ void AddSendStreams( namespace webrtc { +// Generate a RTCP CNAME when a PeerConnection is created. +std::string GenerateRtcpCname() { + std::string cname; + if (!rtc::CreateRandomString(kRtcpCnameLength, &cname)) { + LOG(LS_ERROR) << "Failed to generate CNAME."; + RTC_DCHECK(false); + } + return cname; +} + bool ExtractMediaSessionOptions( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, bool is_offer, @@ -508,6 +521,7 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory) ice_state_(kIceNew), ice_connection_state_(kIceConnectionNew), ice_gathering_state_(kIceGatheringNew), + rtcp_cname_(GenerateRtcpCname()), local_streams_(StreamCollection::Create()), remote_streams_(StreamCollection::Create()) {} @@ -1503,6 +1517,8 @@ bool PeerConnection::GetOptionsForOffer( if (session_->data_channel_type() == cricket::DCT_SCTP && HasDataChannels()) { session_options->data_channel_type = cricket::DCT_SCTP; } + + session_options->rtcp_cname = rtcp_cname_; return true; } @@ -1540,6 +1556,8 @@ bool PeerConnection::GetOptionsForAnswer( if (!ParseConstraintsForAnswer(constraints, session_options)) { return false; } + session_options->rtcp_cname = rtcp_cname_; + FinishOptionsForAnswer(session_options); return true; } @@ -1552,6 +1570,8 @@ bool PeerConnection::GetOptionsForAnswer( if (!ExtractMediaSessionOptions(options, false, session_options)) { return false; } + session_options->rtcp_cname = rtcp_cname_; + FinishOptionsForAnswer(session_options); return true; } diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index b5577157a6..862c6fb630 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -369,6 +369,10 @@ class PeerConnection : public PeerConnectionInterface, std::unique_ptr port_allocator_; std::unique_ptr media_controller_; + // One PeerConnection has only one RTCP CNAME. + // https://tools.ietf.org/html/draft-ietf-rtcweb-rtp-usage-26#section-4.9 + std::string rtcp_cname_; + // Streams added via AddStream. rtc::scoped_refptr local_streams_; // Streams created as a result of SetRemoteDescription. diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index 1a8dd57f08..2594b6c106 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -934,6 +934,34 @@ class PeerConnectionInterfaceTest : public testing::Test { ASSERT_TRUE(stream->AddTrack(video_track)); } + rtc::scoped_ptr CreateOfferWithOneAudioStream() { + CreatePeerConnection(); + AddVoiceStream(kStreamLabel1); + rtc::scoped_ptr offer; + EXPECT_TRUE(DoCreateOffer(&offer, nullptr)); + return offer; + } + + rtc::scoped_ptr + CreateAnswerWithOneAudioStream() { + rtc::scoped_ptr offer = + CreateOfferWithOneAudioStream(); + EXPECT_TRUE(DoSetRemoteDescription(offer.release())); + rtc::scoped_ptr answer; + EXPECT_TRUE(DoCreateAnswer(&answer, nullptr)); + return answer; + } + + const std::string& GetFirstAudioStreamCname( + const SessionDescriptionInterface* desc) { + const cricket::ContentInfo* audio_content = + cricket::GetFirstAudioContent(desc->description()); + const cricket::AudioContentDescription* audio_desc = + static_cast( + audio_content->description); + return audio_desc->streams()[0].cname; + } + cricket::FakePortAllocator* port_allocator_ = nullptr; scoped_refptr pc_factory_; scoped_refptr pc_; @@ -941,6 +969,27 @@ class PeerConnectionInterfaceTest : public testing::Test { rtc::scoped_refptr reference_collection_; }; +// Generate different CNAMEs when PeerConnections are created. +// The CNAMEs are expected to be generated randomly. It is possible +// that the test fails, though the possibility is very low. +TEST_F(PeerConnectionInterfaceTest, CnameGenerationInOffer) { + rtc::scoped_ptr offer1 = + CreateOfferWithOneAudioStream(); + rtc::scoped_ptr offer2 = + CreateOfferWithOneAudioStream(); + EXPECT_NE(GetFirstAudioStreamCname(offer1.get()), + GetFirstAudioStreamCname(offer2.get())); +} + +TEST_F(PeerConnectionInterfaceTest, CnameGenerationInAnswer) { + rtc::scoped_ptr answer1 = + CreateAnswerWithOneAudioStream(); + rtc::scoped_ptr answer2 = + CreateAnswerWithOneAudioStream(); + EXPECT_NE(GetFirstAudioStreamCname(answer1.get()), + GetFirstAudioStreamCname(answer2.get())); +} + TEST_F(PeerConnectionInterfaceTest, CreatePeerConnectionWithDifferentConfigurations) { CreatePeerConnectionWithDifferentConfigurations(); diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 6d8138daac..52abfe855f 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -211,55 +211,6 @@ static bool SelectCrypto(const MediaContentDescription* offer, return false; } -static const StreamParams* FindFirstStreamParamsByCname( - const StreamParamsVec& params_vec, - const std::string& cname) { - for (StreamParamsVec::const_iterator it = params_vec.begin(); - it != params_vec.end(); ++it) { - if (cname == it->cname) - return &*it; - } - return NULL; -} - -// Generates a new CNAME or the CNAME of an already existing StreamParams -// if a StreamParams exist for another Stream in streams with sync_label -// sync_label. -static bool GenerateCname(const StreamParamsVec& params_vec, - const MediaSessionOptions::Streams& streams, - const std::string& synch_label, - std::string* cname) { - ASSERT(cname != NULL); - if (!cname) - return false; - - // Check if a CNAME exist for any of the other synched streams. - for (MediaSessionOptions::Streams::const_iterator stream_it = streams.begin(); - stream_it != streams.end() ; ++stream_it) { - if (synch_label != stream_it->sync_label) - continue; - - // groupid is empty for StreamParams generated using - // MediaSessionDescriptionFactory. - const StreamParams* param = GetStreamByIds(params_vec, "", stream_it->id); - if (param) { - *cname = param->cname; - return true; - } - } - // No other stream seems to exist that we should sync with. - // Generate a random string for the RTCP CNAME, as stated in RFC 6222. - // This string is only used for synchronization, and therefore is opaque. - do { - if (!rtc::CreateRandomString(16, cname)) { - ASSERT(false); - return false; - } - } while (FindFirstStreamParamsByCname(params_vec, *cname)); - - return true; -} - // Generate random SSRC values that are not already present in |params_vec|. // The generated values are added to |ssrcs|. // |num_ssrcs| is the number of the SSRC will be generated. @@ -444,15 +395,15 @@ static bool IsSctp(const MediaContentDescription* desc) { // media_type to content_description. // |current_params| - All currently known StreamParams of any media type. template -static bool AddStreamParams( - MediaType media_type, - const MediaSessionOptions::Streams& streams, - StreamParamsVec* current_streams, - MediaContentDescriptionImpl* content_description, - const bool add_legacy_stream) { +static bool AddStreamParams(MediaType media_type, + const MediaSessionOptions& options, + StreamParamsVec* current_streams, + MediaContentDescriptionImpl* content_description, + const bool add_legacy_stream) { const bool include_rtx_streams = ContainsRtxCodec(content_description->codecs()); + const MediaSessionOptions::Streams& streams = options.streams; if (streams.empty() && add_legacy_stream) { // TODO(perkj): Remove this legacy stream when all apps use StreamParams. std::vector ssrcs; @@ -483,13 +434,6 @@ static bool AddStreamParams( // MediaSessionDescriptionFactory. if (!param) { // This is a new stream. - // Get a CNAME. Either new or same as one of the other synched streams. - std::string cname; - if (!GenerateCname(*current_streams, streams, stream_it->sync_label, - &cname)) { - return false; - } - std::vector ssrcs; if (IsSctp(content_description)) { GenerateSctpSids(*current_streams, &ssrcs); @@ -517,7 +461,7 @@ static bool AddStreamParams( } content_description->set_multistream(true); } - stream_param.cname = cname; + stream_param.cname = options.rtcp_cname; stream_param.sync_label = stream_it->sync_label; content_description->AddStream(stream_param); @@ -761,9 +705,8 @@ static bool CreateMediaContentOffer( offer->set_multistream(options.is_muc); offer->set_rtp_header_extensions(rtp_extensions); - if (!AddStreamParams( - offer->type(), options.streams, current_streams, - offer, add_legacy_stream)) { + if (!AddStreamParams(offer->type(), options, current_streams, offer, + add_legacy_stream)) { return false; } @@ -1080,9 +1023,8 @@ static bool CreateMediaContentAnswer( return false; } - if (!AddStreamParams( - answer->type(), options.streams, current_streams, - answer, add_legacy_stream)) { + if (!AddStreamParams(answer->type(), options, current_streams, answer, + add_legacy_stream)) { return false; // Something went seriously wrong. } diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index 6ac74f2d33..39ac26bd8d 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -76,18 +76,21 @@ extern const char kMediaProtocolTcpDtlsSctp[]; const int kAutoBandwidth = -1; const int kBufferedModeDisabled = 0; +// Default RTCP CNAME for unit tests. +const char kDefaultRtcpCname[] = "DefaultRtcpCname"; + struct MediaSessionOptions { - MediaSessionOptions() : - recv_audio(true), - recv_video(false), - data_channel_type(DCT_NONE), - is_muc(false), - vad_enabled(true), // When disabled, removes all CN codecs from SDP. - rtcp_mux_enabled(true), - bundle_enabled(false), - video_bandwidth(kAutoBandwidth), - data_bandwidth(kDataMaxBandwidth) { - } + MediaSessionOptions() + : recv_audio(true), + recv_video(false), + data_channel_type(DCT_NONE), + is_muc(false), + vad_enabled(true), // When disabled, removes all CN codecs from SDP. + rtcp_mux_enabled(true), + bundle_enabled(false), + video_bandwidth(kAutoBandwidth), + data_bandwidth(kDataMaxBandwidth), + rtcp_cname(kDefaultRtcpCname) {} bool has_audio() const { return recv_audio || HasSendMediaStream(MEDIA_TYPE_AUDIO); @@ -133,6 +136,7 @@ struct MediaSessionOptions { int data_bandwidth; // content name ("mid") => options. std::map transport_options; + std::string rtcp_cname; struct Stream { Stream(MediaType type, diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index 3a1a1c865e..b1c4044b70 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -1312,7 +1312,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { ASSERT_EQ(2U, updated_video_streams.size()); EXPECT_EQ(video_streams[0], updated_video_streams[0]); EXPECT_EQ(kVideoTrack2, updated_video_streams[1].id); - EXPECT_NE(updated_video_streams[1].cname, updated_video_streams[0].cname); + // All the media streams in one PeerConnection share one RTCP CNAME. + EXPECT_EQ(updated_video_streams[1].cname, updated_video_streams[0].cname); const StreamParamsVec& updated_data_streams = updated_dcd->streams(); ASSERT_EQ(2U, updated_data_streams.size()); @@ -1321,6 +1322,10 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { ASSERT_EQ(1U, updated_data_streams[1].ssrcs.size()); EXPECT_NE(0U, updated_data_streams[1].ssrcs[0]); EXPECT_EQ(updated_data_streams[0].cname, updated_data_streams[1].cname); + // The stream correctly got the CNAME from the MediaSessionOptions. + // The Expected RTCP CNAME is the default one as we are using the default + // MediaSessionOptions. + EXPECT_EQ(updated_data_streams[0].cname, cricket::kDefaultRtcpCname); } // Create an offer with simulcast video stream. @@ -1431,7 +1436,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) { EXPECT_TRUE(dcd->rtcp_mux()); // rtcp-mux defaults on // Update the answer. Add a new video track that is not synched to the - // other traacks and remove 1 audio track. + // other tracks and remove 1 audio track. opts.AddSendStream(MEDIA_TYPE_VIDEO, kVideoTrack2, kMediaStream2); opts.RemoveSendStream(MEDIA_TYPE_AUDIO, kAudioTrack2); opts.RemoveSendStream(MEDIA_TYPE_DATA, kDataTrack2); @@ -1474,7 +1479,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) { ASSERT_EQ(2U, updated_video_streams.size()); EXPECT_EQ(video_streams[0], updated_video_streams[0]); EXPECT_EQ(kVideoTrack2, updated_video_streams[1].id); - EXPECT_NE(updated_video_streams[1].cname, updated_video_streams[0].cname); + // All media streams in one PeerConnection share one CNAME. + EXPECT_EQ(updated_video_streams[1].cname, updated_video_streams[0].cname); const StreamParamsVec& updated_data_streams = updated_dcd->streams(); ASSERT_EQ(1U, updated_data_streams.size());