From e7d47a1473e885a57986dcdbf06e7e1d25226ca6 Mon Sep 17 00:00:00 2001 From: "jiayl@webrtc.org" Date: Tue, 5 Aug 2014 19:19:05 +0000 Subject: [PATCH] Maintain the order of the m-lines in CreateOffer and CreateAnswer. The order in the offer follows the order in the current local description. The order in the answer follows the order in the current offer. BUG=2395 R=wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/12799004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6828 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/peerconnection_unittest.cc | 1 - talk/app/webrtc/webrtcsdp_unittest.cc | 78 +++ talk/session/media/mediasession.cc | 648 ++++++++++++-------- talk/session/media/mediasession.h | 48 ++ talk/session/media/mediasession_unittest.cc | 79 +++ 5 files changed, 584 insertions(+), 270 deletions(-) diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index 44009c09aa..3776809b41 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -1314,7 +1314,6 @@ TEST_F(JsepPeerConnectionP2PTestClient, RegisterDataChannelObserver) { // Unregister the existing observer. receiving_client()->data_channel()->UnregisterObserver(); - std::string data = "hello world"; SendRtpData(initializing_client()->data_channel(), data); diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index e018034fc8..71ebe0d85a 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -312,6 +312,39 @@ static const char kSdpConferenceString[] = "c=IN IP4 0.0.0.0\r\n" "a=x-google-flag:conference\r\n"; +static const char kSdpSessionString[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=msid-semantic: WMS local_stream\r\n"; + +static const char kSdpAudioString[] = + "m=audio 1 RTP/SAVPF 111\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:1 IN IP4 0.0.0.0\r\n" + "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n" + "a=mid:audio_content_name\r\n" + "a=sendrecv\r\n" + "a=rtpmap:111 opus/48000/2\r\n" + "a=ssrc:1 cname:stream_1_cname\r\n" + "a=ssrc:1 msid:local_stream audio_track_id_1\r\n" + "a=ssrc:1 mslabel:local_stream\r\n" + "a=ssrc:1 label:audio_track_id_1\r\n"; + +static const char kSdpVideoString[] = + "m=video 1 RTP/SAVPF 120\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:1 IN IP4 0.0.0.0\r\n" + "a=ice-ufrag:ufrag_video\r\na=ice-pwd:pwd_video\r\n" + "a=mid:video_content_name\r\n" + "a=sendrecv\r\n" + "a=rtpmap:120 VP8/90000\r\n" + "a=ssrc:2 cname:stream_1_cname\r\n" + "a=ssrc:2 msid:local_stream video_track_id_1\r\n" + "a=ssrc:2 mslabel:local_stream\r\n" + "a=ssrc:2 label:video_track_id_1\r\n"; + // One candidate reference string as per W3c spec. // candidate: not a=candidate:CRLF @@ -2351,3 +2384,48 @@ TEST_F(WebRtcSdpTest, DeserializeDtlsSetupAttribute) { EXPECT_EQ(cricket::CONNECTIONROLE_ACTPASS, vtinfo->description.connection_role); } + +// Verifies that the order of the serialized m-lines follows the order of the +// ContentInfo in SessionDescription, and vise versa for deserialization. +TEST_F(WebRtcSdpTest, MediaContentOrderMaintainedRoundTrip) { + JsepSessionDescription jdesc(kDummyString); + const std::string media_content_sdps[3] = { + kSdpAudioString, + kSdpVideoString, + kSdpSctpDataChannelString + }; + const cricket::MediaType media_types[3] = { + cricket::MEDIA_TYPE_AUDIO, + cricket::MEDIA_TYPE_VIDEO, + cricket::MEDIA_TYPE_DATA + }; + + // Verifies all 6 permutations. + for (size_t i = 0; i < 6; ++i) { + size_t media_content_in_sdp[3]; + // The index of the first media content. + media_content_in_sdp[0] = i / 2; + // The index of the second media content. + media_content_in_sdp[1] = (media_content_in_sdp[0] + i % 2 + 1) % 3; + // The index of the third media content. + media_content_in_sdp[2] = (media_content_in_sdp[0] + (i + 1) % 2 + 1) % 3; + + std::string sdp_string = kSdpSessionString; + for (size_t i = 0; i < 3; ++i) + sdp_string += media_content_sdps[media_content_in_sdp[i]]; + + EXPECT_TRUE(SdpDeserialize(sdp_string, &jdesc)); + cricket::SessionDescription* desc = jdesc.description(); + EXPECT_EQ(3u, desc->contents().size()); + + for (size_t i = 0; i < 3; ++i) { + const cricket::MediaContentDescription* mdesc = + static_cast( + desc->contents()[i].description); + EXPECT_EQ(media_types[media_content_in_sdp[i]], mdesc->type()); + } + + std::string serialized_sdp = webrtc::SdpSerialize(jdesc); + EXPECT_EQ(sdp_string, serialized_sdp); + } +} diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index a250632284..d4ab1f6a9a 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -1140,8 +1140,6 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( SessionDescription* MediaSessionDescriptionFactory::CreateOffer( const MediaSessionOptions& options, const SessionDescription* current_description) const { - bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); - scoped_ptr offer(new SessionDescription()); StreamParamsVec current_streams; @@ -1163,117 +1161,57 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer( GetRtpHdrExtsToOffer(current_description, &audio_rtp_extensions, &video_rtp_extensions); - // Handle m=audio. - if (options.has_audio) { - cricket::SecurePolicy sdes_policy = - IsDtlsActive(CN_AUDIO, current_description) ? - cricket::SEC_DISABLED : secure(); + bool audio_added = false; + bool video_added = false; + bool data_added = false; - scoped_ptr audio(new AudioContentDescription()); - std::vector crypto_suites; - GetSupportedAudioCryptoSuites(&crypto_suites); - if (!CreateMediaContentOffer( - options, - audio_codecs, - sdes_policy, - GetCryptos(GetFirstAudioContentDescription(current_description)), - crypto_suites, - audio_rtp_extensions, - add_legacy_, - ¤t_streams, - audio.get())) { - return NULL; - } - - audio->set_lang(lang_); - SetMediaProtocol(secure_transport, audio.get()); - offer->AddContent(CN_AUDIO, NS_JINGLE_RTP, audio.release()); - if (!AddTransportOffer(CN_AUDIO, options.transport_options, - current_description, offer.get())) { - return NULL; + // Iterate through the contents of |current_description| to maintain the order + // of the m-lines in the new offer. + if (current_description) { + ContentInfos::const_iterator it = current_description->contents().begin(); + for (; it != current_description->contents().end(); ++it) { + if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO) && options.has_audio) { + if (!AddAudioContentForOffer(options, current_description, + audio_rtp_extensions, audio_codecs, + ¤t_streams, offer.get())) { + return NULL; + } + audio_added = true; + } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO) && + options.has_video) { + if (!AddVideoContentForOffer(options, current_description, + video_rtp_extensions, video_codecs, + ¤t_streams, offer.get())) { + return NULL; + } + video_added = true; + } else if (options.has_data()) { + ASSERT(IsMediaContentOfType(&*it, MEDIA_TYPE_DATA)); + if (!AddDataContentForOffer(options, current_description, &data_codecs, + ¤t_streams, offer.get())) { + return NULL; + } + data_added = true; + } } } - - // Handle m=video. - if (options.has_video) { - cricket::SecurePolicy sdes_policy = - IsDtlsActive(CN_VIDEO, current_description) ? - cricket::SEC_DISABLED : secure(); - - scoped_ptr video(new VideoContentDescription()); - std::vector crypto_suites; - GetSupportedVideoCryptoSuites(&crypto_suites); - if (!CreateMediaContentOffer( - options, - video_codecs, - sdes_policy, - GetCryptos(GetFirstVideoContentDescription(current_description)), - crypto_suites, - video_rtp_extensions, - add_legacy_, - ¤t_streams, - video.get())) { - return NULL; - } - - video->set_bandwidth(options.video_bandwidth); - SetMediaProtocol(secure_transport, video.get()); - offer->AddContent(CN_VIDEO, NS_JINGLE_RTP, video.release()); - if (!AddTransportOffer(CN_VIDEO, options.transport_options, - current_description, offer.get())) { - return NULL; - } + // Append contents that are not in |current_description|. + if (!audio_added && options.has_audio && + !AddAudioContentForOffer(options, current_description, + audio_rtp_extensions, audio_codecs, + ¤t_streams, offer.get())) { + return NULL; } - - // Handle m=data. - if (options.has_data()) { - scoped_ptr data(new DataContentDescription()); - bool is_sctp = (options.data_channel_type == DCT_SCTP); - - FilterDataCodecs(&data_codecs, is_sctp); - - cricket::SecurePolicy sdes_policy = - IsDtlsActive(CN_DATA, current_description) ? - cricket::SEC_DISABLED : secure(); - std::vector crypto_suites; - if (is_sctp) { - // SDES doesn't make sense for SCTP, so we disable it, and we only - // get SDES crypto suites for RTP-based data channels. - sdes_policy = cricket::SEC_DISABLED; - // Unlike SetMediaProtocol below, we need to set the protocol - // before we call CreateMediaContentOffer. Otherwise, - // CreateMediaContentOffer won't know this is SCTP and will - // generate SSRCs rather than SIDs. - data->set_protocol( - secure_transport ? kMediaProtocolDtlsSctp : kMediaProtocolSctp); - } else { - GetSupportedDataCryptoSuites(&crypto_suites); - } - - if (!CreateMediaContentOffer( - options, - data_codecs, - sdes_policy, - GetCryptos(GetFirstDataContentDescription(current_description)), - crypto_suites, - RtpHeaderExtensions(), - add_legacy_, - ¤t_streams, - data.get())) { - return NULL; - } - - if (is_sctp) { - offer->AddContent(CN_DATA, NS_JINGLE_DRAFT_SCTP, data.release()); - } else { - data->set_bandwidth(options.data_bandwidth); - SetMediaProtocol(secure_transport, data.get()); - offer->AddContent(CN_DATA, NS_JINGLE_RTP, data.release()); - } - if (!AddTransportOffer(CN_DATA, options.transport_options, - current_description, offer.get())) { - return NULL; - } + if (!video_added && options.has_video && + !AddVideoContentForOffer(options, current_description, + video_rtp_extensions, video_codecs, + ¤t_streams, offer.get())) { + return NULL; + } + if (!data_added && options.has_data() && + !AddDataContentForOffer(options, current_description, &data_codecs, + ¤t_streams, offer.get())) { + return NULL; } // Bundle the contents together, if we've been asked to do so, and update any @@ -1309,168 +1247,27 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( StreamParamsVec current_streams; GetCurrentStreamParams(current_description, ¤t_streams); - bool bundle_enabled = - offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled; - - // Handle m=audio. - const ContentInfo* audio_content = GetFirstAudioContent(offer); - if (audio_content) { - scoped_ptr audio_transport( - CreateTransportAnswer(audio_content->name, offer, - options.transport_options, - current_description)); - if (!audio_transport) { - return NULL; - } - - AudioCodecs audio_codecs = audio_codecs_; - if (!options.vad_enabled) { - StripCNCodecs(&audio_codecs); - } - - scoped_ptr audio_answer( - new AudioContentDescription()); - // Do not require or create SDES cryptos if DTLS is used. - cricket::SecurePolicy sdes_policy = - audio_transport->secure() ? cricket::SEC_DISABLED : secure(); - if (!CreateMediaContentAnswer( - static_cast( - audio_content->description), - options, - audio_codecs, - sdes_policy, - GetCryptos(GetFirstAudioContentDescription(current_description)), - audio_rtp_extensions_, - ¤t_streams, - add_legacy_, - bundle_enabled, - audio_answer.get())) { - return NULL; // Fails the session setup. - } - - bool rejected = !options.has_audio || audio_content->rejected || - !IsMediaProtocolSupported(MEDIA_TYPE_AUDIO, - audio_answer->protocol(), - audio_transport->secure()); - if (!rejected) { - AddTransportAnswer(audio_content->name, *(audio_transport.get()), - answer.get()); - } else { - // RFC 3264 - // The answer MUST contain the same number of m-lines as the offer. - LOG(LS_INFO) << "Audio is not supported in the answer."; - } - - answer->AddContent(audio_content->name, audio_content->type, rejected, - audio_answer.release()); - } else { - LOG(LS_INFO) << "Audio is not available in the offer."; - } - - // Handle m=video. - const ContentInfo* video_content = GetFirstVideoContent(offer); - if (video_content) { - scoped_ptr video_transport( - CreateTransportAnswer(video_content->name, offer, - options.transport_options, - current_description)); - if (!video_transport) { - return NULL; - } - - scoped_ptr video_answer( - new VideoContentDescription()); - // Do not require or create SDES cryptos if DTLS is used. - cricket::SecurePolicy sdes_policy = - video_transport->secure() ? cricket::SEC_DISABLED : secure(); - if (!CreateMediaContentAnswer( - static_cast( - video_content->description), - options, - video_codecs_, - sdes_policy, - GetCryptos(GetFirstVideoContentDescription(current_description)), - video_rtp_extensions_, - ¤t_streams, - add_legacy_, - bundle_enabled, - video_answer.get())) { - return NULL; - } - bool rejected = !options.has_video || video_content->rejected || - !IsMediaProtocolSupported(MEDIA_TYPE_VIDEO, - video_answer->protocol(), - video_transport->secure()); - if (!rejected) { - if (!AddTransportAnswer(video_content->name, *(video_transport.get()), - answer.get())) { - return NULL; + if (offer) { + ContentInfos::const_iterator it = offer->contents().begin(); + for (; it != offer->contents().end(); ++it) { + if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO)) { + if (!AddAudioContentForAnswer(offer, options, current_description, + ¤t_streams, answer.get())) { + return NULL; + } + } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO)) { + if (!AddVideoContentForAnswer(offer, options, current_description, + ¤t_streams, answer.get())) { + return NULL; + } + } else { + ASSERT(IsMediaContentOfType(&*it, MEDIA_TYPE_DATA)); + if (!AddDataContentForAnswer(offer, options, current_description, + ¤t_streams, answer.get())) { + return NULL; + } } - video_answer->set_bandwidth(options.video_bandwidth); - } else { - // RFC 3264 - // The answer MUST contain the same number of m-lines as the offer. - LOG(LS_INFO) << "Video is not supported in the answer."; } - answer->AddContent(video_content->name, video_content->type, rejected, - video_answer.release()); - } else { - LOG(LS_INFO) << "Video is not available in the offer."; - } - - // Handle m=data. - const ContentInfo* data_content = GetFirstDataContent(offer); - if (data_content) { - scoped_ptr data_transport( - CreateTransportAnswer(data_content->name, offer, - options.transport_options, - current_description)); - if (!data_transport) { - return NULL; - } - bool is_sctp = (options.data_channel_type == DCT_SCTP); - std::vector data_codecs(data_codecs_); - FilterDataCodecs(&data_codecs, is_sctp); - - scoped_ptr data_answer( - new DataContentDescription()); - // Do not require or create SDES cryptos if DTLS is used. - cricket::SecurePolicy sdes_policy = - data_transport->secure() ? cricket::SEC_DISABLED : secure(); - if (!CreateMediaContentAnswer( - static_cast( - data_content->description), - options, - data_codecs_, - sdes_policy, - GetCryptos(GetFirstDataContentDescription(current_description)), - RtpHeaderExtensions(), - ¤t_streams, - add_legacy_, - bundle_enabled, - data_answer.get())) { - return NULL; // Fails the session setup. - } - - bool rejected = !options.has_data() || data_content->rejected || - !IsMediaProtocolSupported(MEDIA_TYPE_DATA, - data_answer->protocol(), - data_transport->secure()); - if (!rejected) { - data_answer->set_bandwidth(options.data_bandwidth); - if (!AddTransportAnswer(data_content->name, *(data_transport.get()), - answer.get())) { - return NULL; - } - } else { - // RFC 3264 - // The answer MUST contain the same number of m-lines as the offer. - LOG(LS_INFO) << "Data is not supported in the answer."; - } - answer->AddContent(data_content->name, data_content->type, rejected, - data_answer.release()); - } else { - LOG(LS_INFO) << "Data is not available in the offer."; } // If the offer supports BUNDLE, and we want to use it too, create a BUNDLE @@ -1632,6 +1429,319 @@ bool MediaSessionDescriptionFactory::AddTransportAnswer( return true; } +bool MediaSessionDescriptionFactory::AddAudioContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + const RtpHeaderExtensions& audio_rtp_extensions, + const AudioCodecs& audio_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const { + cricket::SecurePolicy sdes_policy = + IsDtlsActive(CN_AUDIO, current_description) ? + cricket::SEC_DISABLED : secure(); + + scoped_ptr audio(new AudioContentDescription()); + std::vector crypto_suites; + GetSupportedAudioCryptoSuites(&crypto_suites); + if (!CreateMediaContentOffer( + options, + audio_codecs, + sdes_policy, + GetCryptos(GetFirstAudioContentDescription(current_description)), + crypto_suites, + audio_rtp_extensions, + add_legacy_, + current_streams, + audio.get())) { + return false; + } + audio->set_lang(lang_); + + bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); + SetMediaProtocol(secure_transport, audio.get()); + desc->AddContent(CN_AUDIO, NS_JINGLE_RTP, audio.release()); + if (!AddTransportOffer(CN_AUDIO, options.transport_options, + current_description, desc)) { + return false; + } + + return true; +} + +bool MediaSessionDescriptionFactory::AddVideoContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + const RtpHeaderExtensions& video_rtp_extensions, + const VideoCodecs& video_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const { + cricket::SecurePolicy sdes_policy = + IsDtlsActive(CN_VIDEO, current_description) ? + cricket::SEC_DISABLED : secure(); + + scoped_ptr video(new VideoContentDescription()); + std::vector crypto_suites; + GetSupportedVideoCryptoSuites(&crypto_suites); + if (!CreateMediaContentOffer( + options, + video_codecs, + sdes_policy, + GetCryptos(GetFirstVideoContentDescription(current_description)), + crypto_suites, + video_rtp_extensions, + add_legacy_, + current_streams, + video.get())) { + return false; + } + + video->set_bandwidth(options.video_bandwidth); + + bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); + SetMediaProtocol(secure_transport, video.get()); + desc->AddContent(CN_VIDEO, NS_JINGLE_RTP, video.release()); + if (!AddTransportOffer(CN_VIDEO, options.transport_options, + current_description, desc)) { + return false; + } + + return true; +} + +bool MediaSessionDescriptionFactory::AddDataContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + DataCodecs* data_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const { + bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); + + scoped_ptr data(new DataContentDescription()); + bool is_sctp = (options.data_channel_type == DCT_SCTP); + + FilterDataCodecs(data_codecs, is_sctp); + + cricket::SecurePolicy sdes_policy = + IsDtlsActive(CN_DATA, current_description) ? + cricket::SEC_DISABLED : secure(); + std::vector crypto_suites; + if (is_sctp) { + // SDES doesn't make sense for SCTP, so we disable it, and we only + // get SDES crypto suites for RTP-based data channels. + sdes_policy = cricket::SEC_DISABLED; + // Unlike SetMediaProtocol below, we need to set the protocol + // before we call CreateMediaContentOffer. Otherwise, + // CreateMediaContentOffer won't know this is SCTP and will + // generate SSRCs rather than SIDs. + data->set_protocol( + secure_transport ? kMediaProtocolDtlsSctp : kMediaProtocolSctp); + } else { + GetSupportedDataCryptoSuites(&crypto_suites); + } + + if (!CreateMediaContentOffer( + options, + *data_codecs, + sdes_policy, + GetCryptos(GetFirstDataContentDescription(current_description)), + crypto_suites, + RtpHeaderExtensions(), + add_legacy_, + current_streams, + data.get())) { + return false; + } + + if (is_sctp) { + desc->AddContent(CN_DATA, NS_JINGLE_DRAFT_SCTP, data.release()); + } else { + data->set_bandwidth(options.data_bandwidth); + SetMediaProtocol(secure_transport, data.get()); + desc->AddContent(CN_DATA, NS_JINGLE_RTP, data.release()); + } + if (!AddTransportOffer(CN_DATA, options.transport_options, + current_description, desc)) { + return false; + } + return true; +} + +bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const { + const ContentInfo* audio_content = GetFirstAudioContent(offer); + + scoped_ptr audio_transport( + CreateTransportAnswer(audio_content->name, offer, + options.transport_options, + current_description)); + if (!audio_transport) { + return false; + } + + AudioCodecs audio_codecs = audio_codecs_; + if (!options.vad_enabled) { + StripCNCodecs(&audio_codecs); + } + + bool bundle_enabled = + offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled; + scoped_ptr audio_answer( + new AudioContentDescription()); + // Do not require or create SDES cryptos if DTLS is used. + cricket::SecurePolicy sdes_policy = + audio_transport->secure() ? cricket::SEC_DISABLED : secure(); + if (!CreateMediaContentAnswer( + static_cast( + audio_content->description), + options, + audio_codecs, + sdes_policy, + GetCryptos(GetFirstAudioContentDescription(current_description)), + audio_rtp_extensions_, + current_streams, + add_legacy_, + bundle_enabled, + audio_answer.get())) { + return false; // Fails the session setup. + } + + bool rejected = !options.has_audio || audio_content->rejected || + !IsMediaProtocolSupported(MEDIA_TYPE_AUDIO, + audio_answer->protocol(), + audio_transport->secure()); + if (!rejected) { + AddTransportAnswer(audio_content->name, *(audio_transport.get()), answer); + } else { + // RFC 3264 + // The answer MUST contain the same number of m-lines as the offer. + LOG(LS_INFO) << "Audio is not supported in the answer."; + } + + answer->AddContent(audio_content->name, audio_content->type, rejected, + audio_answer.release()); + return true; +} + +bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const { + const ContentInfo* video_content = GetFirstVideoContent(offer); + scoped_ptr video_transport( + CreateTransportAnswer(video_content->name, offer, + options.transport_options, + current_description)); + if (!video_transport) { + return false; + } + + scoped_ptr video_answer( + new VideoContentDescription()); + // Do not require or create SDES cryptos if DTLS is used. + cricket::SecurePolicy sdes_policy = + video_transport->secure() ? cricket::SEC_DISABLED : secure(); + bool bundle_enabled = + offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled; + if (!CreateMediaContentAnswer( + static_cast( + video_content->description), + options, + video_codecs_, + sdes_policy, + GetCryptos(GetFirstVideoContentDescription(current_description)), + video_rtp_extensions_, + current_streams, + add_legacy_, + bundle_enabled, + video_answer.get())) { + return false; + } + bool rejected = !options.has_video || video_content->rejected || + !IsMediaProtocolSupported(MEDIA_TYPE_VIDEO, + video_answer->protocol(), + video_transport->secure()); + if (!rejected) { + if (!AddTransportAnswer(video_content->name, *(video_transport.get()), + answer)) { + return false; + } + video_answer->set_bandwidth(options.video_bandwidth); + } else { + // RFC 3264 + // The answer MUST contain the same number of m-lines as the offer. + LOG(LS_INFO) << "Video is not supported in the answer."; + } + answer->AddContent(video_content->name, video_content->type, rejected, + video_answer.release()); + return true; +} + +bool MediaSessionDescriptionFactory::AddDataContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const { + const ContentInfo* data_content = GetFirstDataContent(offer); + scoped_ptr data_transport( + CreateTransportAnswer(data_content->name, offer, + options.transport_options, + current_description)); + if (!data_transport) { + return false; + } + bool is_sctp = (options.data_channel_type == DCT_SCTP); + std::vector data_codecs(data_codecs_); + FilterDataCodecs(&data_codecs, is_sctp); + + scoped_ptr data_answer( + new DataContentDescription()); + // Do not require or create SDES cryptos if DTLS is used. + cricket::SecurePolicy sdes_policy = + data_transport->secure() ? cricket::SEC_DISABLED : secure(); + bool bundle_enabled = + offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled; + if (!CreateMediaContentAnswer( + static_cast( + data_content->description), + options, + data_codecs_, + sdes_policy, + GetCryptos(GetFirstDataContentDescription(current_description)), + RtpHeaderExtensions(), + current_streams, + add_legacy_, + bundle_enabled, + data_answer.get())) { + return false; // Fails the session setup. + } + + bool rejected = !options.has_data() || data_content->rejected || + !IsMediaProtocolSupported(MEDIA_TYPE_DATA, + data_answer->protocol(), + data_transport->secure()); + if (!rejected) { + data_answer->set_bandwidth(options.data_bandwidth); + if (!AddTransportAnswer(data_content->name, *(data_transport.get()), + answer)) { + return false; + } + } else { + // RFC 3264 + // The answer MUST contain the same number of m-lines as the offer. + LOG(LS_INFO) << "Data is not supported in the answer."; + } + answer->AddContent(data_content->name, data_content->type, rejected, + data_answer.release()); + return true; +} + bool IsMediaContent(const ContentInfo* content) { return (content && (content->type == NS_JINGLE_RTP || diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h index 6abee3a179..514cf28d42 100644 --- a/talk/session/media/mediasession.h +++ b/talk/session/media/mediasession.h @@ -460,6 +460,54 @@ class MediaSessionDescriptionFactory { const TransportDescription& transport_desc, SessionDescription* answer_desc) const; + // Helpers for adding media contents to the SessionDescription. Returns true + // it succeeds or the media content is not needed, or false if there is any + // error. + + bool AddAudioContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + const RtpHeaderExtensions& audio_rtp_extensions, + const AudioCodecs& audio_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const; + + bool AddVideoContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + const RtpHeaderExtensions& video_rtp_extensions, + const VideoCodecs& video_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const; + + bool AddDataContentForOffer( + const MediaSessionOptions& options, + const SessionDescription* current_description, + DataCodecs* data_codecs, + StreamParamsVec* current_streams, + SessionDescription* desc) const; + + bool AddAudioContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const; + + bool AddVideoContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const; + + bool AddDataContentForAnswer( + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description, + StreamParamsVec* current_streams, + SessionDescription* answer) const; + AudioCodecs audio_codecs_; RtpHeaderExtensions audio_rtp_extensions_; VideoCodecs video_codecs_; diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc index 78c162f809..5505fbb509 100644 --- a/talk/session/media/mediasession_unittest.cc +++ b/talk/session/media/mediasession_unittest.cc @@ -183,6 +183,13 @@ static const char kDataTrack1[] = "data_1"; static const char kDataTrack2[] = "data_2"; static const char kDataTrack3[] = "data_3"; +static bool IsMediaContentOfType(const ContentInfo* content, + MediaType media_type) { + const MediaContentDescription* mdesc = + static_cast(content->description); + return mdesc && mdesc->type() == media_type; +} + class MediaSessionDescriptionFactoryTest : public testing::Test { public: MediaSessionDescriptionFactoryTest() @@ -644,6 +651,46 @@ TEST_F(MediaSessionDescriptionFactoryTest, EXPECT_FALSE(acd->has_ssrcs()); // No StreamParams. } +// Verifies that the order of the media contents in the current +// SessionDescription is preserved in the new SessionDescription. +TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferContentOrder) { + MediaSessionOptions opts; + opts.has_audio = false; + opts.has_video = false; + opts.data_channel_type = cricket::DCT_SCTP; + + rtc::scoped_ptr offer1(f1_.CreateOffer(opts, NULL)); + ASSERT_TRUE(offer1.get() != NULL); + EXPECT_EQ(1u, offer1->contents().size()); + EXPECT_TRUE(IsMediaContentOfType(&offer1->contents()[0], MEDIA_TYPE_DATA)); + + opts.has_video = true; + rtc::scoped_ptr offer2( + f1_.CreateOffer(opts, offer1.get())); + ASSERT_TRUE(offer2.get() != NULL); + EXPECT_EQ(2u, offer2->contents().size()); + EXPECT_TRUE(IsMediaContentOfType(&offer2->contents()[0], MEDIA_TYPE_DATA)); + EXPECT_TRUE(IsMediaContentOfType(&offer2->contents()[1], MEDIA_TYPE_VIDEO)); + + opts.has_audio = true; + rtc::scoped_ptr offer3( + f1_.CreateOffer(opts, offer2.get())); + ASSERT_TRUE(offer3.get() != NULL); + EXPECT_EQ(3u, offer3->contents().size()); + EXPECT_TRUE(IsMediaContentOfType(&offer3->contents()[0], MEDIA_TYPE_DATA)); + EXPECT_TRUE(IsMediaContentOfType(&offer3->contents()[1], MEDIA_TYPE_VIDEO)); + EXPECT_TRUE(IsMediaContentOfType(&offer3->contents()[2], MEDIA_TYPE_AUDIO)); + + // Verifies the default order is audio-video-data, so that the previous checks + // didn't pass by accident. + rtc::scoped_ptr offer4(f1_.CreateOffer(opts, NULL)); + ASSERT_TRUE(offer4.get() != NULL); + EXPECT_EQ(3u, offer4->contents().size()); + EXPECT_TRUE(IsMediaContentOfType(&offer4->contents()[0], MEDIA_TYPE_AUDIO)); + EXPECT_TRUE(IsMediaContentOfType(&offer4->contents()[1], MEDIA_TYPE_VIDEO)); + EXPECT_TRUE(IsMediaContentOfType(&offer4->contents()[2], MEDIA_TYPE_DATA)); +} + // Create a typical audio answer, and ensure it matches what we expect. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswer) { f1_.set_secure(SEC_ENABLED); @@ -736,6 +783,38 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswer) { EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), vcd->protocol()); } +// Verifies that the order of the media contents in the offer is preserved in +// the answer. +TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAnswerContentOrder) { + MediaSessionOptions opts; + + // Creates a data only offer. + opts.has_audio = false; + opts.data_channel_type = cricket::DCT_SCTP; + rtc::scoped_ptr offer1(f1_.CreateOffer(opts, NULL)); + ASSERT_TRUE(offer1.get() != NULL); + + // Appends audio to the offer. + opts.has_audio = true; + rtc::scoped_ptr offer2( + f1_.CreateOffer(opts, offer1.get())); + ASSERT_TRUE(offer2.get() != NULL); + + // Appends video to the offer. + opts.has_video = true; + rtc::scoped_ptr offer3( + f1_.CreateOffer(opts, offer2.get())); + ASSERT_TRUE(offer3.get() != NULL); + + rtc::scoped_ptr answer( + f2_.CreateAnswer(offer3.get(), opts, NULL)); + ASSERT_TRUE(answer.get() != NULL); + EXPECT_EQ(3u, answer->contents().size()); + EXPECT_TRUE(IsMediaContentOfType(&answer->contents()[0], MEDIA_TYPE_DATA)); + EXPECT_TRUE(IsMediaContentOfType(&answer->contents()[1], MEDIA_TYPE_AUDIO)); + EXPECT_TRUE(IsMediaContentOfType(&answer->contents()[2], MEDIA_TYPE_VIDEO)); +} + // This test that the media direction is set to send/receive in an answer if // the offer is send receive. TEST_F(MediaSessionDescriptionFactoryTest, CreateAnswerToSendReceiveOffer) {