diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index eacbeaedc3..ee359271c2 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -444,12 +444,10 @@ bool ConvertRtcOptionsForOffer( } session_options->vad_enabled = rtc_options.voice_activity_detection; - session_options->audio_transport_options.ice_restart = - rtc_options.ice_restart; - session_options->video_transport_options.ice_restart = - rtc_options.ice_restart; - session_options->data_transport_options.ice_restart = rtc_options.ice_restart; session_options->bundle_enabled = rtc_options.use_rtp_mux; + for (auto& kv : session_options->transport_options) { + kv.second.ice_restart = rtc_options.ice_restart; + } return true; } @@ -492,16 +490,14 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints, session_options->bundle_enabled = true; } + bool ice_restart = false; if (FindConstraint(constraints, MediaConstraintsInterface::kIceRestart, &value, &mandatory_constraints_satisfied)) { - session_options->audio_transport_options.ice_restart = value; - session_options->video_transport_options.ice_restart = value; - session_options->data_transport_options.ice_restart = value; - } else { // kIceRestart defaults to false according to spec. - session_options->audio_transport_options.ice_restart = false; - session_options->video_transport_options.ice_restart = false; - session_options->data_transport_options.ice_restart = false; + ice_restart = true; + } + for (auto& kv : session_options->transport_options) { + kv.second.ice_restart = ice_restart; } if (!constraints) { @@ -1500,6 +1496,15 @@ void PeerConnection::PostCreateSessionDescriptionFailure( bool PeerConnection::GetOptionsForOffer( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, cricket::MediaSessionOptions* session_options) { + // TODO(deadbeef): Once we have transceivers, enumerate them here instead of + // ContentInfos. + if (session_->local_description()) { + for (const cricket::ContentInfo& content : + session_->local_description()->description()->contents()) { + session_options->transport_options[content.name] = + cricket::TransportOptions(); + } + } if (!ConvertRtcOptionsForOffer(rtc_options, session_options)) { return false; } @@ -1533,6 +1538,16 @@ bool PeerConnection::GetOptionsForAnswer( cricket::MediaSessionOptions* session_options) { session_options->recv_audio = false; session_options->recv_video = false; + // TODO(deadbeef): Once we have transceivers, enumerate them here instead of + // ContentInfos. + if (session_->remote_description()) { + // Initialize the transport_options map. + for (const cricket::ContentInfo& content : + session_->remote_description()->description()->contents()) { + session_options->transport_options[content.name] = + cricket::TransportOptions(); + } + } if (!ParseConstraintsForAnswer(constraints, session_options)) { return false; } diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index 7ad1940f77..a63dd43423 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -30,12 +30,15 @@ class RemoteMediaStreamFactory; // Populates |session_options| from |rtc_options|, and returns true if options // are valid. +// |session_options|->transport_options map entries must exist in order for +// them to be populated from |rtc_options|. bool ConvertRtcOptionsForOffer( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, cricket::MediaSessionOptions* session_options); // Populates |session_options| from |constraints|, and returns true if all // mandatory constraints are satisfied. +// Assumes that |session_options|->transport_options map entries exist. bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints, cricket::MediaSessionOptions* session_options); diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index 2c7cc128ad..701ab3c001 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -2489,14 +2489,15 @@ TEST(CreateSessionOptionsTest, GetDefaultMediaSessionOptionsForOffer) { RTCOfferAnswerOptions rtc_options; cricket::MediaSessionOptions options; + options.transport_options["audio"] = cricket::TransportOptions(); + options.transport_options["video"] = cricket::TransportOptions(); EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options)); EXPECT_TRUE(options.has_audio()); EXPECT_FALSE(options.has_video()); EXPECT_TRUE(options.bundle_enabled); EXPECT_TRUE(options.vad_enabled); - EXPECT_FALSE(options.audio_transport_options.ice_restart); - EXPECT_FALSE(options.video_transport_options.ice_restart); - EXPECT_FALSE(options.data_transport_options.ice_restart); + EXPECT_FALSE(options.transport_options["audio"].ice_restart); + EXPECT_FALSE(options.transport_options["video"].ice_restart); } // Test that a correct MediaSessionOptions is created for an offer if @@ -2537,16 +2538,16 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithIceRestart) { rtc_options.ice_restart = true; cricket::MediaSessionOptions options; + options.transport_options["audio"] = cricket::TransportOptions(); + options.transport_options["video"] = cricket::TransportOptions(); EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options)); - EXPECT_TRUE(options.audio_transport_options.ice_restart); - EXPECT_TRUE(options.video_transport_options.ice_restart); - EXPECT_TRUE(options.data_transport_options.ice_restart); + EXPECT_TRUE(options.transport_options["audio"].ice_restart); + EXPECT_TRUE(options.transport_options["video"].ice_restart); rtc_options = RTCOfferAnswerOptions(); EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options)); - EXPECT_FALSE(options.audio_transport_options.ice_restart); - EXPECT_FALSE(options.video_transport_options.ice_restart); - EXPECT_FALSE(options.data_transport_options.ice_restart); + EXPECT_FALSE(options.transport_options["audio"].ice_restart); + EXPECT_FALSE(options.transport_options["video"].ice_restart); } // Test that the MediaConstraints in an answer don't affect if audio and video diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index c693cb1077..b2494140b4 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -457,69 +457,37 @@ uint32_t ConvertIceTransportTypeToCandidateFilter( return cricket::CF_NONE; } -// Help class used to remember if a a remote peer has requested ice restart by -// by sending a description with new ice ufrag and password. -class IceRestartAnswerLatch { - public: - IceRestartAnswerLatch() : ice_restart_(false) { } - - // Returns true if CheckForRemoteIceRestart has been called with a new session - // description where ice password and ufrag has changed since last time - // Reset() was called. - bool Get() const { - return ice_restart_; - } - - void Reset() { - if (ice_restart_) { - ice_restart_ = false; - } - } - - // This method has two purposes: 1. Return whether |new_desc| requests - // an ICE restart (i.e., new ufrag/pwd). 2. If it requests an ICE restart - // and it is an OFFER, remember this in |ice_restart_| so that the next - // Local Answer will be created with new ufrag and pwd. - bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc, - const SessionDescriptionInterface* new_desc) { - if (!old_desc) { - return false; - } - const SessionDescription* new_sd = new_desc->description(); - const SessionDescription* old_sd = old_desc->description(); - const ContentInfos& contents = new_sd->contents(); - for (size_t index = 0; index < contents.size(); ++index) { - const ContentInfo* cinfo = &contents[index]; - if (cinfo->rejected) { - continue; - } - // If the content isn't rejected, check if ufrag and password has - // changed. - const cricket::TransportDescription* new_transport_desc = - new_sd->GetTransportDescriptionByName(cinfo->name); - const cricket::TransportDescription* old_transport_desc = - old_sd->GetTransportDescriptionByName(cinfo->name); - if (!new_transport_desc || !old_transport_desc) { - // No transport description exist. This is not an ice restart. - continue; - } - if (cricket::IceCredentialsChanged(old_transport_desc->ice_ufrag, - old_transport_desc->ice_pwd, - new_transport_desc->ice_ufrag, - new_transport_desc->ice_pwd)) { - LOG(LS_INFO) << "Remote peer request ice restart."; - if (new_desc->type() == SessionDescriptionInterface::kOffer) { - ice_restart_ = true; - } - return true; - } - } +// Returns true if |new_desc| requests an ICE restart (i.e., new ufrag/pwd). +bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc, + const SessionDescriptionInterface* new_desc, + const std::string& content_name) { + if (!old_desc) { return false; } - - private: - bool ice_restart_; -}; + const SessionDescription* new_sd = new_desc->description(); + const SessionDescription* old_sd = old_desc->description(); + const ContentInfo* cinfo = new_sd->GetContentByName(content_name); + if (!cinfo || cinfo->rejected) { + return false; + } + // If the content isn't rejected, check if ufrag and password has changed. + const cricket::TransportDescription* new_transport_desc = + new_sd->GetTransportDescriptionByName(content_name); + const cricket::TransportDescription* old_transport_desc = + old_sd->GetTransportDescriptionByName(content_name); + if (!new_transport_desc || !old_transport_desc) { + // No transport description exists. This is not an ICE restart. + return false; + } + if (cricket::IceCredentialsChanged( + old_transport_desc->ice_ufrag, old_transport_desc->ice_pwd, + new_transport_desc->ice_ufrag, new_transport_desc->ice_pwd)) { + LOG(LS_INFO) << "Remote peer requests ICE restart for " << content_name + << "."; + return true; + } + return false; +} WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller, rtc::Thread* signaling_thread, @@ -543,7 +511,6 @@ WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller, older_version_remote_peer_(false), dtls_enabled_(false), data_channel_type_(cricket::DCT_NONE), - ice_restart_latch_(new IceRestartAnswerLatch), metrics_observer_(NULL) { transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLED); transport_controller_->SignalConnectionState.connect( @@ -692,6 +659,20 @@ void WebRtcSession::Close() { ASSERT(!data_channel_); } +cricket::BaseChannel* WebRtcSession::GetChannel( + const std::string& content_name) { + if (voice_channel() && voice_channel()->content_name() == content_name) { + return voice_channel(); + } + if (video_channel() && video_channel()->content_name() == content_name) { + return video_channel(); + } + if (data_channel() && data_channel()->content_name() == content_name) { + return data_channel(); + } + return nullptr; +} + void WebRtcSession::SetSdesPolicy(cricket::SecurePolicy secure_policy) { webrtc_session_desc_factory_->SetSdesPolicy(secure_policy); } @@ -779,6 +760,8 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, UseCandidatesInSessionDescription(remote_desc_.get()); } + pending_ice_restarts_.clear(); + if (error() != ERROR_NONE) { return BadLocalSdp(desc->type(), GetSessionErrorMsg(), err_desc); } @@ -822,20 +805,30 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, return BadRemoteSdp(desc->type(), kInvalidCandidates, err_desc); } - // Check if this new SessionDescription contains new ice ufrag and password - // that indicates the remote peer requests ice restart. - bool ice_restart = - ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(), desc); - // We retain all received candidates only if ICE is not restarted. - // When ICE is restarted, all previous candidates belong to an old generation - // and should not be kept. - // TODO(deadbeef): This goes against the W3C spec which says the remote - // description should only contain candidates from the last set remote - // description plus any candidates added since then. We should remove this - // once we're sure it won't break anything. - if (!ice_restart) { - WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( - old_remote_desc.get(), desc); + if (old_remote_desc) { + for (const cricket::ContentInfo& content : + old_remote_desc->description()->contents()) { + // Check if this new SessionDescription contains new ICE ufrag and + // password that indicates the remote peer requests an ICE restart. + // TODO(deadbeef): When we start storing both the current and pending + // remote description, this should reset pending_ice_restarts and compare + // against the current description. + if (CheckForRemoteIceRestart(old_remote_desc.get(), desc, content.name)) { + if (action == kOffer) { + pending_ice_restarts_.insert(content.name); + } + } else { + // We retain all received candidates only if ICE is not restarted. + // When ICE is restarted, all previous candidates belong to an old + // generation and should not be kept. + // TODO(deadbeef): This goes against the W3C spec which says the remote + // description should only contain candidates from the last set remote + // description plus any candidates added since then. We should remove + // this once we're sure it won't break anything. + WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( + old_remote_desc.get(), content.name, desc); + } + } } if (error() != ERROR_NONE) { @@ -1094,20 +1087,6 @@ bool WebRtcSession::GetRemoteSSLCertificate(const std::string& transport_name, return transport_controller_->GetRemoteSSLCertificate(transport_name, cert); } -cricket::BaseChannel* WebRtcSession::GetChannel( - const std::string& content_name) { - if (voice_channel() && voice_channel()->content_name() == content_name) { - return voice_channel(); - } - if (video_channel() && video_channel()->content_name() == content_name) { - return video_channel(); - } - if (data_channel() && data_channel()->content_name() == content_name) { - return data_channel(); - } - return nullptr; -} - bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) { const std::string* first_content_name = bundle.FirstContentName(); if (!first_content_name) { @@ -1432,12 +1411,9 @@ cricket::DataChannelType WebRtcSession::data_channel_type() const { return data_channel_type_; } -bool WebRtcSession::IceRestartPending() const { - return ice_restart_latch_->Get(); -} - -void WebRtcSession::ResetIceRestartLatch() { - ice_restart_latch_->Reset(); +bool WebRtcSession::IceRestartPending(const std::string& content_name) const { + return pending_ice_restarts_.find(content_name) != + pending_ice_restarts_.end(); } void WebRtcSession::OnCertificateReady( diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index e8b2a05af6..70265d3043 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -11,6 +11,7 @@ #ifndef WEBRTC_API_WEBRTCSESSION_H_ #define WEBRTC_API_WEBRTCSESSION_H_ +#include #include #include @@ -179,6 +180,8 @@ class WebRtcSession : public AudioProviderInterface, return data_channel_.get(); } + cricket::BaseChannel* GetChannel(const std::string& content_name); + void SetSdesPolicy(cricket::SecurePolicy secure_policy); cricket::SecurePolicy SdesPolicy() const; @@ -280,9 +283,7 @@ class WebRtcSession : public AudioProviderInterface, cricket::DataChannelType data_channel_type() const; - bool IceRestartPending() const; - - void ResetIceRestartLatch(); + bool IceRestartPending(const std::string& content_name) const; // Called when an RTCCertificate is generated or retrieved by // WebRTCSessionDescriptionFactory. Should happen before setLocalDescription. @@ -364,7 +365,6 @@ class WebRtcSession : public AudioProviderInterface, const std::string& content_name, cricket::TransportDescription* info); - cricket::BaseChannel* GetChannel(const std::string& content_name); // Cause all the BaseChannels in the bundle group to have the same // transport channel. bool EnableBundle(const cricket::ContentGroup& bundle); @@ -482,7 +482,8 @@ class WebRtcSession : public AudioProviderInterface, // 2. If constraint kEnableRtpDataChannels is true, RTP is allowed (DCT_RTP); // 3. If both 1&2 are false, data channel is not allowed (DCT_NONE). cricket::DataChannelType data_channel_type_; - rtc::scoped_ptr ice_restart_latch_; + // List of content names for which the remote side triggered an ICE restart. + std::set pending_ice_restarts_; rtc::scoped_ptr webrtc_session_desc_factory_; diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 16ce621eb8..8062fe5db7 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -731,12 +731,10 @@ class WebRtcSessionTest VerifyCryptoParams(answer->description()); } - void CompareIceUfragAndPassword(const cricket::SessionDescription* desc1, - const cricket::SessionDescription* desc2, - bool expect_equal) { + bool IceUfragPwdEqual(const cricket::SessionDescription* desc1, + const cricket::SessionDescription* desc2) { if (desc1->contents().size() != desc2->contents().size()) { - EXPECT_FALSE(expect_equal); - return; + return false; } const cricket::ContentInfos& contents = desc1->contents(); @@ -748,16 +746,38 @@ class WebRtcSessionTest const cricket::TransportDescription* transport_desc2 = desc2->GetTransportDescriptionByName(it->name); if (!transport_desc1 || !transport_desc2) { - EXPECT_FALSE(expect_equal); - return; + return false; } if (transport_desc1->ice_pwd != transport_desc2->ice_pwd || transport_desc1->ice_ufrag != transport_desc2->ice_ufrag) { - EXPECT_FALSE(expect_equal); - return; + return false; } } - EXPECT_TRUE(expect_equal); + return true; + } + + // Compares ufrag/password only for the specified |media_type|. + bool IceUfragPwdEqual(const cricket::SessionDescription* desc1, + const cricket::SessionDescription* desc2, + cricket::MediaType media_type) { + if (desc1->contents().size() != desc2->contents().size()) { + return false; + } + + const cricket::ContentInfo* cinfo = + cricket::GetFirstMediaContent(desc1->contents(), media_type); + const cricket::TransportDescription* transport_desc1 = + desc1->GetTransportDescriptionByName(cinfo->name); + const cricket::TransportDescription* transport_desc2 = + desc2->GetTransportDescriptionByName(cinfo->name); + if (!transport_desc1 || !transport_desc2) { + return false; + } + if (transport_desc1->ice_pwd != transport_desc2->ice_pwd || + transport_desc1->ice_ufrag != transport_desc2->ice_ufrag) { + return false; + } + return true; } void RemoveIceUfragPwdLines(const SessionDescriptionInterface* current_desc, @@ -784,35 +804,33 @@ class WebRtcSessionTest } } - void ModifyIceUfragPwdLines(const SessionDescriptionInterface* current_desc, - const std::string& modified_ice_ufrag, - const std::string& modified_ice_pwd, - std::string* sdp) { - const cricket::SessionDescription* desc = current_desc->description(); - EXPECT_TRUE(current_desc->ToString(sdp)); - - const cricket::ContentInfos& contents = desc->contents(); - cricket::ContentInfos::const_iterator it = contents.begin(); - // Replace ufrag and pwd lines with |modified_ice_ufrag| and - // |modified_ice_pwd| strings. - for (; it != contents.end(); ++it) { - const cricket::TransportDescription* transport_desc = - desc->GetTransportDescriptionByName(it->name); - std::string ufrag_line = "a=ice-ufrag:" + transport_desc->ice_ufrag - + "\r\n"; - std::string pwd_line = "a=ice-pwd:" + transport_desc->ice_pwd - + "\r\n"; - std::string mod_ufrag = "a=ice-ufrag:" + modified_ice_ufrag + "\r\n"; - std::string mod_pwd = "a=ice-pwd:" + modified_ice_pwd + "\r\n"; - rtc::replace_substrs(ufrag_line.c_str(), ufrag_line.length(), - mod_ufrag.c_str(), mod_ufrag.length(), - sdp); - rtc::replace_substrs(pwd_line.c_str(), pwd_line.length(), - mod_pwd.c_str(), mod_pwd.length(), - sdp); + void SetIceUfragPwd(SessionDescriptionInterface* current_desc, + const std::string& ufrag, + const std::string& pwd) { + cricket::SessionDescription* desc = current_desc->description(); + for (TransportInfo& transport_info : desc->transport_infos()) { + cricket::TransportDescription& transport_desc = + transport_info.description; + transport_desc.ice_ufrag = ufrag; + transport_desc.ice_pwd = pwd; } } + // Sets ufrag/pwd for specified |media_type|. + void SetIceUfragPwd(SessionDescriptionInterface* current_desc, + cricket::MediaType media_type, + const std::string& ufrag, + const std::string& pwd) { + cricket::SessionDescription* desc = current_desc->description(); + const cricket::ContentInfo* cinfo = + cricket::GetFirstMediaContent(desc->contents(), media_type); + TransportInfo* transport_info = desc->GetTransportInfoByName(cinfo->name); + cricket::TransportDescription* transport_desc = + &transport_info->description; + transport_desc->ice_ufrag = ufrag; + transport_desc->ice_pwd = pwd; + } + // Creates a remote offer and and applies it as a remote description, // creates a local answer and applies is as a local description. // Call SendAudioVideoStreamX() before this function @@ -2775,23 +2793,16 @@ TEST_F(WebRtcSessionTest, TestSetLocalDescriptionInvalidIceCredentials) { Init(); SendAudioVideoStream1(); rtc::scoped_ptr offer(CreateOffer()); - - std::string sdp; // Modifying ice ufrag and pwd in local offer with strings smaller than the // recommended values of 4 and 22 bytes respectively. - ModifyIceUfragPwdLines(offer.get(), "ice", "icepwd", &sdp); - SessionDescriptionInterface* modified_offer = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + SetIceUfragPwd(offer.get(), "ice", "icepwd"); std::string error; - EXPECT_FALSE(session_->SetLocalDescription(modified_offer, &error)); + EXPECT_FALSE(session_->SetLocalDescription(offer.release(), &error)); // Test with string greater than 256. - sdp.clear(); - ModifyIceUfragPwdLines(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd, - &sdp); - modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, - NULL); - EXPECT_FALSE(session_->SetLocalDescription(modified_offer, &error)); + offer.reset(CreateOffer()); + SetIceUfragPwd(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd); + EXPECT_FALSE(session_->SetLocalDescription(offer.release(), &error)); } // This test verifies that setRemoteDescription fails if remote offer has @@ -2799,76 +2810,57 @@ TEST_F(WebRtcSessionTest, TestSetLocalDescriptionInvalidIceCredentials) { TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) { Init(); rtc::scoped_ptr offer(CreateRemoteOffer()); - std::string sdp; // Modifying ice ufrag and pwd in remote offer with strings smaller than the // recommended values of 4 and 22 bytes respectively. - ModifyIceUfragPwdLines(offer.get(), "ice", "icepwd", &sdp); - SessionDescriptionInterface* modified_offer = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + SetIceUfragPwd(offer.get(), "ice", "icepwd"); std::string error; - EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error)); + EXPECT_FALSE(session_->SetRemoteDescription(offer.release(), &error)); - sdp.clear(); - ModifyIceUfragPwdLines(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd, - &sdp); - modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, - NULL); - EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error)); + offer.reset(CreateRemoteOffer()); + SetIceUfragPwd(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd); + EXPECT_FALSE(session_->SetRemoteDescription(offer.release(), &error)); } // Test that if the remote offer indicates the peer requested ICE restart (via // a new ufrag or pwd), the old ICE candidates are not copied, and vice versa. TEST_F(WebRtcSessionTest, TestSetRemoteOfferWithIceRestart) { Init(); - scoped_ptr offer(CreateRemoteOffer()); // Create the first offer. - std::string sdp; - ModifyIceUfragPwdLines(offer.get(), "0123456789012345", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* offer1 = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + scoped_ptr offer(CreateRemoteOffer()); + SetIceUfragPwd(offer.get(), "0123456789012345", "abcdefghijklmnopqrstuvwx"); cricket::Candidate candidate1(1, "udp", rtc::SocketAddress("1.1.1.1", 5000), 0, "", "", "relay", 0, ""); JsepIceCandidate ice_candidate1(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(offer1->AddCandidate(&ice_candidate1)); - SetRemoteDescriptionWithoutError(offer1); + EXPECT_TRUE(offer->AddCandidate(&ice_candidate1)); + SetRemoteDescriptionWithoutError(offer.release()); EXPECT_EQ(1, session_->remote_description()->candidates(0)->count()); // The second offer has the same ufrag and pwd but different address. - sdp.clear(); - ModifyIceUfragPwdLines(offer.get(), "0123456789012345", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* offer2 = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + offer.reset(CreateRemoteOffer()); + SetIceUfragPwd(offer.get(), "0123456789012345", "abcdefghijklmnopqrstuvwx"); candidate1.set_address(rtc::SocketAddress("1.1.1.1", 6000)); JsepIceCandidate ice_candidate2(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(offer2->AddCandidate(&ice_candidate2)); - SetRemoteDescriptionWithoutError(offer2); + EXPECT_TRUE(offer->AddCandidate(&ice_candidate2)); + SetRemoteDescriptionWithoutError(offer.release()); EXPECT_EQ(2, session_->remote_description()->candidates(0)->count()); // The third offer has a different ufrag and different address. - sdp.clear(); - ModifyIceUfragPwdLines(offer.get(), "0123456789012333", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* offer3 = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + offer.reset(CreateRemoteOffer()); + SetIceUfragPwd(offer.get(), "0123456789012333", "abcdefghijklmnopqrstuvwx"); candidate1.set_address(rtc::SocketAddress("1.1.1.1", 7000)); JsepIceCandidate ice_candidate3(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(offer3->AddCandidate(&ice_candidate3)); - SetRemoteDescriptionWithoutError(offer3); + EXPECT_TRUE(offer->AddCandidate(&ice_candidate3)); + SetRemoteDescriptionWithoutError(offer.release()); EXPECT_EQ(1, session_->remote_description()->candidates(0)->count()); // The fourth offer has no candidate but a different ufrag/pwd. - sdp.clear(); - ModifyIceUfragPwdLines(offer.get(), "0123456789012444", - "abcdefghijklmnopqrstuvyz", &sdp); - SessionDescriptionInterface* offer4 = - CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); - SetRemoteDescriptionWithoutError(offer4); + offer.reset(CreateRemoteOffer()); + SetIceUfragPwd(offer.get(), "0123456789012444", "abcdefghijklmnopqrstuvyz"); + SetRemoteDescriptionWithoutError(offer.release()); EXPECT_EQ(0, session_->remote_description()->candidates(0)->count()); } @@ -2878,55 +2870,46 @@ TEST_F(WebRtcSessionTest, TestSetRemoteAnswerWithIceRestart) { Init(); SessionDescriptionInterface* offer = CreateOffer(); SetLocalDescriptionWithoutError(offer); - scoped_ptr answer(CreateRemoteAnswer(offer)); // Create the first answer. - std::string sdp; - ModifyIceUfragPwdLines(answer.get(), "0123456789012345", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* answer1 = - CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL); + scoped_ptr answer(CreateRemoteAnswer(offer)); + answer->set_type(JsepSessionDescription::kPrAnswer); + SetIceUfragPwd(answer.get(), "0123456789012345", "abcdefghijklmnopqrstuvwx"); cricket::Candidate candidate1(1, "udp", rtc::SocketAddress("1.1.1.1", 5000), 0, "", "", "relay", 0, ""); JsepIceCandidate ice_candidate1(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(answer1->AddCandidate(&ice_candidate1)); - SetRemoteDescriptionWithoutError(answer1); + EXPECT_TRUE(answer->AddCandidate(&ice_candidate1)); + SetRemoteDescriptionWithoutError(answer.release()); EXPECT_EQ(1, session_->remote_description()->candidates(0)->count()); // The second answer has the same ufrag and pwd but different address. - sdp.clear(); - ModifyIceUfragPwdLines(answer.get(), "0123456789012345", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* answer2 = - CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL); + answer.reset(CreateRemoteAnswer(offer)); + answer->set_type(JsepSessionDescription::kPrAnswer); + SetIceUfragPwd(answer.get(), "0123456789012345", "abcdefghijklmnopqrstuvwx"); candidate1.set_address(rtc::SocketAddress("1.1.1.1", 6000)); JsepIceCandidate ice_candidate2(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(answer2->AddCandidate(&ice_candidate2)); - SetRemoteDescriptionWithoutError(answer2); + EXPECT_TRUE(answer->AddCandidate(&ice_candidate2)); + SetRemoteDescriptionWithoutError(answer.release()); EXPECT_EQ(2, session_->remote_description()->candidates(0)->count()); // The third answer has a different ufrag and different address. - sdp.clear(); - ModifyIceUfragPwdLines(answer.get(), "0123456789012333", - "abcdefghijklmnopqrstuvwx", &sdp); - SessionDescriptionInterface* answer3 = - CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL); + answer.reset(CreateRemoteAnswer(offer)); + answer->set_type(JsepSessionDescription::kPrAnswer); + SetIceUfragPwd(answer.get(), "0123456789012333", "abcdefghijklmnopqrstuvwx"); candidate1.set_address(rtc::SocketAddress("1.1.1.1", 7000)); JsepIceCandidate ice_candidate3(kMediaContentName0, kMediaContentIndex0, candidate1); - EXPECT_TRUE(answer3->AddCandidate(&ice_candidate3)); - SetRemoteDescriptionWithoutError(answer3); + EXPECT_TRUE(answer->AddCandidate(&ice_candidate3)); + SetRemoteDescriptionWithoutError(answer.release()); EXPECT_EQ(1, session_->remote_description()->candidates(0)->count()); // The fourth answer has no candidate but a different ufrag/pwd. - sdp.clear(); - ModifyIceUfragPwdLines(answer.get(), "0123456789012444", - "abcdefghijklmnopqrstuvyz", &sdp); - SessionDescriptionInterface* offer4 = - CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL); - SetRemoteDescriptionWithoutError(offer4); + answer.reset(CreateRemoteAnswer(offer)); + answer->set_type(JsepSessionDescription::kPrAnswer); + SetIceUfragPwd(answer.get(), "0123456789012444", "abcdefghijklmnopqrstuvyz"); + SetRemoteDescriptionWithoutError(answer.release()); EXPECT_EQ(0, session_->remote_description()->candidates(0)->count()); } @@ -3643,9 +3626,10 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) { SetLocalDescriptionWithoutError(answer.release()); // Receive an offer with new ufrag and password. - options.audio_transport_options.ice_restart = true; - options.video_transport_options.ice_restart = true; - options.data_transport_options.ice_restart = true; + for (const cricket::ContentInfo& content : + session_->local_description()->description()->contents()) { + options.transport_options[content.name].ice_restart = true; + } rtc::scoped_ptr updated_offer1( CreateRemoteOffer(options, session_->remote_description())); SetRemoteDescriptionWithoutError(updated_offer1.release()); @@ -3653,11 +3637,64 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) { rtc::scoped_ptr updated_answer1( CreateAnswer(NULL)); - CompareIceUfragAndPassword(updated_answer1->description(), - session_->local_description()->description(), - false); + EXPECT_FALSE(IceUfragPwdEqual(updated_answer1->description(), + session_->local_description()->description())); + // Even a second answer (created before the description is set) should have + // a new ufrag/password. + rtc::scoped_ptr updated_answer2( + CreateAnswer(NULL)); + + EXPECT_FALSE(IceUfragPwdEqual(updated_answer2->description(), + session_->local_description()->description())); + + SetLocalDescriptionWithoutError(updated_answer2.release()); +} + +// This test verifies that an answer contains new ufrag and password if an offer +// that changes either the ufrag or password (but not both) is received. +// RFC 5245 says: "If the offer contained a change in the a=ice-ufrag or +// a=ice-pwd attributes compared to the previous SDP from the peer, it +// indicates that ICE is restarting for this media stream." +TEST_F(WebRtcSessionTest, TestOfferChangingOnlyUfragOrPassword) { + Init(); + cricket::MediaSessionOptions options; + options.recv_audio = true; + options.recv_video = true; + // Create an offer with audio and video. + rtc::scoped_ptr offer(CreateRemoteOffer(options)); + SetIceUfragPwd(offer.get(), "original_ufrag", "original_password12345"); + SetRemoteDescriptionWithoutError(offer.release()); + + SendAudioVideoStream1(); + rtc::scoped_ptr answer(CreateAnswer(nullptr)); + SetLocalDescriptionWithoutError(answer.release()); + + // Receive an offer with a new ufrag but stale password. + rtc::scoped_ptr ufrag_changed_offer( + CreateRemoteOffer(options, session_->remote_description())); + SetIceUfragPwd(ufrag_changed_offer.get(), "modified_ufrag", + "original_password12345"); + SetRemoteDescriptionWithoutError(ufrag_changed_offer.release()); + + rtc::scoped_ptr updated_answer1( + CreateAnswer(nullptr)); + EXPECT_FALSE(IceUfragPwdEqual(updated_answer1->description(), + session_->local_description()->description())); SetLocalDescriptionWithoutError(updated_answer1.release()); + + // Receive an offer with a new password but stale ufrag. + rtc::scoped_ptr password_changed_offer( + CreateRemoteOffer(options, session_->remote_description())); + SetIceUfragPwd(password_changed_offer.get(), "modified_ufrag", + "modified_password12345"); + SetRemoteDescriptionWithoutError(password_changed_offer.release()); + + rtc::scoped_ptr updated_answer2( + CreateAnswer(nullptr)); + EXPECT_FALSE(IceUfragPwdEqual(updated_answer2->description(), + session_->local_description()->description())); + SetLocalDescriptionWithoutError(updated_answer2.release()); } // This test verifies that an answer contains old ufrag and password if an offer @@ -3676,9 +3713,6 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithOldUfragAndPassword) { SetLocalDescriptionWithoutError(answer.release()); // Receive an offer without changed ufrag or password. - options.audio_transport_options.ice_restart = false; - options.video_transport_options.ice_restart = false; - options.data_transport_options.ice_restart = false; rtc::scoped_ptr updated_offer2( CreateRemoteOffer(options, session_->remote_description())); SetRemoteDescriptionWithoutError(updated_offer2.release()); @@ -3686,13 +3720,55 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithOldUfragAndPassword) { rtc::scoped_ptr updated_answer2( CreateAnswer(NULL)); - CompareIceUfragAndPassword(updated_answer2->description(), - session_->local_description()->description(), - true); + EXPECT_TRUE(IceUfragPwdEqual(updated_answer2->description(), + session_->local_description()->description())); SetLocalDescriptionWithoutError(updated_answer2.release()); } +// This test verifies that if an offer does an ICE restart on some, but not all +// media sections, the answer will change the ufrag/password in the correct +// media sections. +TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewAndOldUfragAndPassword) { + Init(); + cricket::MediaSessionOptions options; + options.recv_video = true; + options.recv_audio = true; + options.bundle_enabled = false; + rtc::scoped_ptr offer(CreateRemoteOffer(options)); + + SetIceUfragPwd(offer.get(), cricket::MEDIA_TYPE_AUDIO, "aaaa", + "aaaaaaaaaaaaaaaaaaaaaa"); + SetIceUfragPwd(offer.get(), cricket::MEDIA_TYPE_VIDEO, "bbbb", + "bbbbbbbbbbbbbbbbbbbbbb"); + SetRemoteDescriptionWithoutError(offer.release()); + + SendAudioVideoStream1(); + rtc::scoped_ptr answer(CreateAnswer(nullptr)); + SetLocalDescriptionWithoutError(answer.release()); + + // Receive an offer with new ufrag and password, but only for the video media + // section. + rtc::scoped_ptr updated_offer( + CreateRemoteOffer(options, session_->remote_description())); + SetIceUfragPwd(updated_offer.get(), cricket::MEDIA_TYPE_VIDEO, "cccc", + "cccccccccccccccccccccc"); + SetRemoteDescriptionWithoutError(updated_offer.release()); + + rtc::scoped_ptr updated_answer( + CreateAnswer(nullptr)); + + EXPECT_TRUE(IceUfragPwdEqual(updated_answer->description(), + session_->local_description()->description(), + cricket::MEDIA_TYPE_AUDIO)); + + EXPECT_FALSE(IceUfragPwdEqual(updated_answer->description(), + session_->local_description()->description(), + cricket::MEDIA_TYPE_VIDEO)); + + SetLocalDescriptionWithoutError(updated_answer.release()); +} + TEST_F(WebRtcSessionTest, TestSessionContentError) { Init(); SendAudioVideoStream1(); diff --git a/webrtc/api/webrtcsessiondescriptionfactory.cc b/webrtc/api/webrtcsessiondescriptionfactory.cc index 19ee1e6e03..aa929d3347 100644 --- a/webrtc/api/webrtcsessiondescriptionfactory.cc +++ b/webrtc/api/webrtcsessiondescriptionfactory.cc @@ -95,18 +95,27 @@ void WebRtcIdentityRequestObserver::OnSuccess( // static void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( const SessionDescriptionInterface* source_desc, + const std::string& content_name, SessionDescriptionInterface* dest_desc) { - if (!source_desc) + if (!source_desc) { return; - for (size_t m = 0; m < source_desc->number_of_mediasections() && - m < dest_desc->number_of_mediasections(); ++m) { - const IceCandidateCollection* source_candidates = - source_desc->candidates(m); - const IceCandidateCollection* dest_candidates = dest_desc->candidates(m); - for (size_t n = 0; n < source_candidates->count(); ++n) { - const IceCandidateInterface* new_candidate = source_candidates->at(n); - if (!dest_candidates->HasCandidate(new_candidate)) - dest_desc->AddCandidate(source_candidates->at(n)); + } + const cricket::ContentInfos& contents = + source_desc->description()->contents(); + const cricket::ContentInfo* cinfo = + source_desc->description()->GetContentByName(content_name); + if (!cinfo) { + return; + } + size_t mediasection_index = static_cast(cinfo - &contents[0]); + const IceCandidateCollection* source_candidates = + source_desc->candidates(mediasection_index); + const IceCandidateCollection* dest_candidates = + dest_desc->candidates(mediasection_index); + for (size_t n = 0; n < source_candidates->count(); ++n) { + const IceCandidateInterface* new_candidate = source_candidates->at(n); + if (!dest_candidates->HasCandidate(new_candidate)) { + dest_desc->AddCandidate(source_candidates->at(n)); } } } @@ -374,42 +383,38 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer( "Failed to initialize the offer."); return; } - if (session_->local_description() && - !request.options.audio_transport_options.ice_restart && - !request.options.video_transport_options.ice_restart && - !request.options.data_transport_options.ice_restart) { - // Include all local ice candidates in the SessionDescription unless - // the an ice restart has been requested. - CopyCandidatesFromSessionDescription(session_->local_description(), offer); + if (session_->local_description()) { + for (const cricket::ContentInfo& content : + session_->local_description()->description()->contents()) { + // Include all local ICE candidates in the SessionDescription unless + // the remote peer has requested an ICE restart. + if (!request.options.transport_options[content.name].ice_restart) { + CopyCandidatesFromSessionDescription(session_->local_description(), + content.name, offer); + } + } } PostCreateSessionDescriptionSucceeded(request.observer, offer); } void WebRtcSessionDescriptionFactory::InternalCreateAnswer( CreateSessionDescriptionRequest request) { - // According to http://tools.ietf.org/html/rfc5245#section-9.2.1.1 - // an answer should also contain new ice ufrag and password if an offer has - // been received with new ufrag and password. - request.options.audio_transport_options.ice_restart = - session_->IceRestartPending(); - request.options.video_transport_options.ice_restart = - session_->IceRestartPending(); - request.options.data_transport_options.ice_restart = - session_->IceRestartPending(); - // We should pass current ssl role to the transport description factory, if - // there is already an existing ongoing session. - rtc::SSLRole ssl_role; - if (session_->GetSslRole(session_->voice_channel(), &ssl_role)) { - request.options.audio_transport_options.prefer_passive_role = - (rtc::SSL_SERVER == ssl_role); - } - if (session_->GetSslRole(session_->video_channel(), &ssl_role)) { - request.options.video_transport_options.prefer_passive_role = - (rtc::SSL_SERVER == ssl_role); - } - if (session_->GetSslRole(session_->data_channel(), &ssl_role)) { - request.options.data_transport_options.prefer_passive_role = - (rtc::SSL_SERVER == ssl_role); + if (session_->remote_description()) { + for (const cricket::ContentInfo& content : + session_->remote_description()->description()->contents()) { + // According to http://tools.ietf.org/html/rfc5245#section-9.2.1.1 + // an answer should also contain new ICE ufrag and password if an offer + // has been received with new ufrag and password. + request.options.transport_options[content.name].ice_restart = + session_->IceRestartPending(content.name); + // We should pass the current SSL role to the transport description + // factory, if there is already an existing ongoing session. + rtc::SSLRole ssl_role; + if (session_->GetSslRole(session_->GetChannel(content.name), &ssl_role)) { + request.options.transport_options[content.name].prefer_passive_role = + (rtc::SSL_SERVER == ssl_role); + } + } } cricket::SessionDescription* desc(session_desc_factory_.CreateAnswer( @@ -436,15 +441,17 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer( "Failed to initialize the answer."); return; } - if (session_->local_description() && - !request.options.audio_transport_options.ice_restart && - !request.options.video_transport_options.ice_restart && - !request.options.data_transport_options.ice_restart) { - // Include all local ice candidates in the SessionDescription unless - // the remote peer has requested an ice restart. - CopyCandidatesFromSessionDescription(session_->local_description(), answer); + if (session_->local_description()) { + for (const cricket::ContentInfo& content : + session_->local_description()->description()->contents()) { + // Include all local ICE candidates in the SessionDescription unless + // the remote peer has requested an ICE restart. + if (!request.options.transport_options[content.name].ice_restart) { + CopyCandidatesFromSessionDescription(session_->local_description(), + content.name, answer); + } + } } - session_->ResetIceRestartLatch(); PostCreateSessionDescriptionSucceeded(request.observer, answer); } diff --git a/webrtc/api/webrtcsessiondescriptionfactory.h b/webrtc/api/webrtcsessiondescriptionfactory.h index 77771687a3..b58b55ddd2 100644 --- a/webrtc/api/webrtcsessiondescriptionfactory.h +++ b/webrtc/api/webrtcsessiondescriptionfactory.h @@ -97,8 +97,9 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, virtual ~WebRtcSessionDescriptionFactory(); static void CopyCandidatesFromSessionDescription( - const SessionDescriptionInterface* source_desc, - SessionDescriptionInterface* dest_desc); + const SessionDescriptionInterface* source_desc, + const std::string& content_name, + SessionDescriptionInterface* dest_desc); void CreateOffer( CreateSessionDescriptionObserver* observer, diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index bac695d6b3..673ce7fa0b 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -50,10 +50,10 @@ bool IceCredentialsChanged(const std::string& old_ufrag, const std::string& old_pwd, const std::string& new_ufrag, const std::string& new_pwd) { - // TODO(jiayl): The standard (RFC 5245 Section 9.1.1.1) says that ICE should - // restart when both the ufrag and password are changed, but we do restart - // when either ufrag or passwrod is changed to keep compatible with GICE. We - // should clean this up when GICE is no longer used. + // The standard (RFC 5245 Section 9.1.1.1) says that ICE restarts MUST change + // both the ufrag and password. However, section 9.2.1.1 says changing the + // ufrag OR password indicates an ICE restart. So, to keep compatibility with + // endpoints that only change one, we'll treat this as an ICE restart. return (old_ufrag != new_ufrag) || (old_pwd != new_pwd); } diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 34fc6a1c2d..9ec9169779 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -721,6 +721,15 @@ static bool IsRtxCodec(const C& codec) { return stricmp(codec.name.c_str(), kRtxCodecName) == 0; } +static TransportOptions GetTransportOptions(const MediaSessionOptions& options, + const std::string& content_name) { + auto it = options.transport_options.find(content_name); + if (it == options.transport_options.end()) { + return TransportOptions(); + } + return it->second; +} + // Create a media content to be offered in a session-initiate, // according to the given options.rtcp_mux, options.is_muc, // options.streams, codecs, secure_transport, crypto, and streams. If we don't @@ -1586,7 +1595,8 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( } desc->AddContent(content_name, NS_JINGLE_RTP, audio.release()); - if (!AddTransportOffer(content_name, options.audio_transport_options, + if (!AddTransportOffer(content_name, + GetTransportOptions(options, content_name), current_description, desc)) { return false; } @@ -1646,7 +1656,8 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( } desc->AddContent(content_name, NS_JINGLE_RTP, video.release()); - if (!AddTransportOffer(content_name, options.video_transport_options, + if (!AddTransportOffer(content_name, + GetTransportOptions(options, content_name), current_description, desc)) { return false; } @@ -1710,7 +1721,8 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer( SetMediaProtocol(secure_transport, data.get()); desc->AddContent(content_name, NS_JINGLE_RTP, data.release()); } - if (!AddTransportOffer(content_name, options.data_transport_options, + if (!AddTransportOffer(content_name, + GetTransportOptions(options, content_name), current_description, desc)) { return false; } @@ -1726,8 +1738,8 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( const ContentInfo* audio_content = GetFirstAudioContent(offer); scoped_ptr audio_transport(CreateTransportAnswer( - audio_content->name, offer, options.audio_transport_options, - current_description)); + audio_content->name, offer, + GetTransportOptions(options, audio_content->name), current_description)); if (!audio_transport) { return false; } @@ -1784,8 +1796,8 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( SessionDescription* answer) const { const ContentInfo* video_content = GetFirstVideoContent(offer); scoped_ptr video_transport(CreateTransportAnswer( - video_content->name, offer, options.video_transport_options, - current_description)); + video_content->name, offer, + GetTransportOptions(options, video_content->name), current_description)); if (!video_transport) { return false; } @@ -1839,8 +1851,8 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer( SessionDescription* answer) const { const ContentInfo* data_content = GetFirstDataContent(offer); scoped_ptr data_transport(CreateTransportAnswer( - data_content->name, offer, options.data_transport_options, - current_description)); + data_content->name, offer, + GetTransportOptions(options, data_content->name), current_description)); if (!data_transport) { return false; } @@ -1908,15 +1920,15 @@ bool IsDataContent(const ContentInfo* content) { return IsMediaContentOfType(content, MEDIA_TYPE_DATA); } -static const ContentInfo* GetFirstMediaContent(const ContentInfos& contents, - MediaType media_type) { +const ContentInfo* GetFirstMediaContent(const ContentInfos& contents, + MediaType media_type) { for (ContentInfos::const_iterator content = contents.begin(); content != contents.end(); content++) { if (IsMediaContentOfType(&*content, media_type)) { return &*content; } } - return NULL; + return nullptr; } const ContentInfo* GetFirstAudioContent(const ContentInfos& contents) { @@ -1933,8 +1945,9 @@ const ContentInfo* GetFirstDataContent(const ContentInfos& contents) { static const ContentInfo* GetFirstMediaContent(const SessionDescription* sdesc, MediaType media_type) { - if (sdesc == NULL) - return NULL; + if (sdesc == nullptr) { + return nullptr; + } return GetFirstMediaContent(sdesc->contents(), media_type); } diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index 7a634b5e3e..60ded556ae 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -14,6 +14,7 @@ #define TALK_SESSION_MEDIA_MEDIASESSION_H_ #include +#include #include #include @@ -131,9 +132,8 @@ struct MediaSessionOptions { // bps. -1 == auto. int video_bandwidth; int data_bandwidth; - TransportOptions audio_transport_options; - TransportOptions video_transport_options; - TransportOptions data_transport_options; + // content name ("mid") => options. + std::map transport_options; struct Stream { Stream(MediaType type, @@ -519,6 +519,8 @@ bool IsMediaContent(const ContentInfo* content); bool IsAudioContent(const ContentInfo* content); bool IsVideoContent(const ContentInfo* content); bool IsDataContent(const ContentInfo* content); +const ContentInfo* GetFirstMediaContent(const ContentInfos& contents, + MediaType media_type); const ContentInfo* GetFirstAudioContent(const ContentInfos& contents); const ContentInfo* GetFirstVideoContent(const ContentInfos& contents); const ContentInfo* GetFirstDataContent(const ContentInfos& contents);