Only generate one CNAME per PeerConnection.
The CNAME is generated in the PeerConnection constructor and is populated through the MediaSessionOptions.
A default cname will be set in the MediaSessionOptions constructor.
BUG=webrtc:3431
Review-Url: https://codereview.webrtc.org/1871993002
Cr-Commit-Position: refs/heads/master@{#12650}
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 7f1f452..506a215 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -66,6 +66,9 @@
// 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 @@
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 @@
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 @@
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 @@
if (!ParseConstraintsForAnswer(constraints, session_options)) {
return false;
}
+ session_options->rtcp_cname = rtcp_cname_;
+
FinishOptionsForAnswer(session_options);
return true;
}
@@ -1552,6 +1570,8 @@
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 b557715..862c6fb 100644
--- a/webrtc/api/peerconnection.h
+++ b/webrtc/api/peerconnection.h
@@ -369,6 +369,10 @@
std::unique_ptr<cricket::PortAllocator> port_allocator_;
std::unique_ptr<MediaControllerInterface> 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<StreamCollection> local_streams_;
// Streams created as a result of SetRemoteDescription.
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 1a8dd57..2594b6c 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -934,6 +934,34 @@
ASSERT_TRUE(stream->AddTrack(video_track));
}
+ rtc::scoped_ptr<SessionDescriptionInterface> CreateOfferWithOneAudioStream() {
+ CreatePeerConnection();
+ AddVoiceStream(kStreamLabel1);
+ rtc::scoped_ptr<SessionDescriptionInterface> offer;
+ EXPECT_TRUE(DoCreateOffer(&offer, nullptr));
+ return offer;
+ }
+
+ rtc::scoped_ptr<SessionDescriptionInterface>
+ CreateAnswerWithOneAudioStream() {
+ rtc::scoped_ptr<SessionDescriptionInterface> offer =
+ CreateOfferWithOneAudioStream();
+ EXPECT_TRUE(DoSetRemoteDescription(offer.release()));
+ rtc::scoped_ptr<SessionDescriptionInterface> 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<const cricket::AudioContentDescription*>(
+ audio_content->description);
+ return audio_desc->streams()[0].cname;
+ }
+
cricket::FakePortAllocator* port_allocator_ = nullptr;
scoped_refptr<webrtc::PeerConnectionFactoryInterface> pc_factory_;
scoped_refptr<PeerConnectionInterface> pc_;
@@ -941,6 +969,27 @@
rtc::scoped_refptr<StreamCollection> 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<SessionDescriptionInterface> offer1 =
+ CreateOfferWithOneAudioStream();
+ rtc::scoped_ptr<SessionDescriptionInterface> offer2 =
+ CreateOfferWithOneAudioStream();
+ EXPECT_NE(GetFirstAudioStreamCname(offer1.get()),
+ GetFirstAudioStreamCname(offer2.get()));
+}
+
+TEST_F(PeerConnectionInterfaceTest, CnameGenerationInAnswer) {
+ rtc::scoped_ptr<SessionDescriptionInterface> answer1 =
+ CreateAnswerWithOneAudioStream();
+ rtc::scoped_ptr<SessionDescriptionInterface> 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 6d8138d..52abfe8 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -211,55 +211,6 @@
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 @@
// media_type to content_description.
// |current_params| - All currently known StreamParams of any media type.
template <class C>
-static bool AddStreamParams(
- MediaType media_type,
- const MediaSessionOptions::Streams& streams,
- StreamParamsVec* current_streams,
- MediaContentDescriptionImpl<C>* content_description,
- const bool add_legacy_stream) {
+static bool AddStreamParams(MediaType media_type,
+ const MediaSessionOptions& options,
+ StreamParamsVec* current_streams,
+ MediaContentDescriptionImpl<C>* 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<uint32_t> ssrcs;
@@ -483,13 +434,6 @@
// 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<uint32_t> ssrcs;
if (IsSctp(content_description)) {
GenerateSctpSids(*current_streams, &ssrcs);
@@ -517,7 +461,7 @@
}
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 @@
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 @@
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 6ac74f2..39ac26b 100644
--- a/webrtc/pc/mediasession.h
+++ b/webrtc/pc/mediasession.h
@@ -76,18 +76,21 @@
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 @@
int data_bandwidth;
// content name ("mid") => options.
std::map<std::string, TransportOptions> 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 3a1a1c8..b1c4044 100644
--- a/webrtc/pc/mediasession_unittest.cc
+++ b/webrtc/pc/mediasession_unittest.cc
@@ -1312,7 +1312,8 @@
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 @@
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 @@
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 @@
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());
