diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index c2ce02c21a..1af0e19563 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -874,6 +874,9 @@ bool WebRtcSession::UpdateSessionState( } SetState(source == cricket::CS_LOCAL ? STATE_SENTINITIATE : STATE_RECEIVEDINITIATE); + if (!PushdownMediaDescription(cricket::CA_OFFER, source, err_desc)) { + SetError(BaseSession::ERROR_CONTENT, *err_desc); + } if (error() != cricket::BaseSession::ERROR_NONE) { return BadOfferSdp(source, GetSessionErrorMsg(), err_desc); } @@ -884,6 +887,9 @@ bool WebRtcSession::UpdateSessionState( EnableChannels(); SetState(source == cricket::CS_LOCAL ? STATE_SENTPRACCEPT : STATE_RECEIVEDPRACCEPT); + if (!PushdownMediaDescription(cricket::CA_PRANSWER, source, err_desc)) { + SetError(BaseSession::ERROR_CONTENT, *err_desc); + } if (error() != cricket::BaseSession::ERROR_NONE) { return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc); } @@ -895,6 +901,9 @@ bool WebRtcSession::UpdateSessionState( EnableChannels(); SetState(source == cricket::CS_LOCAL ? STATE_SENTACCEPT : STATE_RECEIVEDACCEPT); + if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) { + SetError(BaseSession::ERROR_CONTENT, *err_desc); + } if (error() != cricket::BaseSession::ERROR_NONE) { return BadAnswerSdp(source, GetSessionErrorMsg(), err_desc); } @@ -902,6 +911,27 @@ bool WebRtcSession::UpdateSessionState( return true; } +bool WebRtcSession::PushdownMediaDescription( + cricket::ContentAction action, + cricket::ContentSource source, + std::string* err) { + auto set_content = [this, action, source, err](cricket::BaseChannel* ch) { + if (!ch) { + return true; + } else if (source == cricket::CS_LOCAL) { + return ch->PushdownLocalDescription( + base_local_description(), action, err); + } else { + return ch->PushdownRemoteDescription( + base_remote_description(), action, err); + } + }; + + return (set_content(voice_channel()) && + set_content(video_channel()) && + set_content(data_channel())); +} + WebRtcSession::Action WebRtcSession::GetAction(const std::string& type) { if (type == SessionDescriptionInterface::kOffer) { return WebRtcSession::kOffer; @@ -986,17 +1016,15 @@ bool WebRtcSession::SetIceTransports( } bool WebRtcSession::GetLocalTrackIdBySsrc(uint32 ssrc, std::string* track_id) { - if (!BaseSession::local_description()) + if (!base_local_description()) return false; - return webrtc::GetTrackIdBySsrc( - BaseSession::local_description(), ssrc, track_id); + return webrtc::GetTrackIdBySsrc(base_local_description(), ssrc, track_id); } bool WebRtcSession::GetRemoteTrackIdBySsrc(uint32 ssrc, std::string* track_id) { - if (!BaseSession::remote_description()) + if (!base_remote_description()) return false; - return webrtc::GetTrackIdBySsrc( - BaseSession::remote_description(), ssrc, track_id); + return webrtc::GetTrackIdBySsrc(base_remote_description(), ssrc, track_id); } std::string WebRtcSession::BadStateErrMsg(State state) { @@ -1124,7 +1152,7 @@ bool WebRtcSession::CanInsertDtmf(const std::string& track_id) { uint32 send_ssrc = 0; // The Dtmf is negotiated per channel not ssrc, so we only check if the ssrc // exists. - if (!GetAudioSsrcByTrackId(BaseSession::local_description(), track_id, + if (!GetAudioSsrcByTrackId(base_local_description(), track_id, &send_ssrc)) { LOG(LS_ERROR) << "CanInsertDtmf: Track does not exist: " << track_id; return false; @@ -1140,7 +1168,7 @@ bool WebRtcSession::InsertDtmf(const std::string& track_id, return false; } uint32 send_ssrc = 0; - if (!VERIFY(GetAudioSsrcByTrackId(BaseSession::local_description(), + if (!VERIFY(GetAudioSsrcByTrackId(base_local_description(), track_id, &send_ssrc))) { LOG(LS_ERROR) << "InsertDtmf: Track does not exist: " << track_id; return false; @@ -1419,11 +1447,11 @@ void WebRtcSession::ProcessNewLocalCandidate( // Returns the media index for a local ice candidate given the content name. bool WebRtcSession::GetLocalCandidateMediaIndex(const std::string& content_name, int* sdp_mline_index) { - if (!BaseSession::local_description() || !sdp_mline_index) + if (!base_local_description() || !sdp_mline_index) return false; bool content_found = false; - const ContentInfos& contents = BaseSession::local_description()->contents(); + const ContentInfos& contents = base_local_description()->contents(); for (size_t index = 0; index < contents.size(); ++index) { if (contents[index].name == content_name) { *sdp_mline_index = static_cast(index); @@ -1468,8 +1496,7 @@ bool WebRtcSession::UseCandidate( const IceCandidateInterface* candidate) { size_t mediacontent_index = static_cast(candidate->sdp_mline_index()); - size_t remote_content_size = - BaseSession::remote_description()->contents().size(); + size_t remote_content_size = base_remote_description()->contents().size(); if (mediacontent_index >= remote_content_size) { LOG(LS_ERROR) << "UseRemoteCandidateInSession: Invalid candidate media index."; @@ -1477,7 +1504,7 @@ bool WebRtcSession::UseCandidate( } cricket::ContentInfo content = - BaseSession::remote_description()->contents()[mediacontent_index]; + base_remote_description()->contents()[mediacontent_index]; std::vector candidates; candidates.push_back(candidate->candidate()); // Invoking BaseSession method to handle remote candidates. diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 14e4414053..ce0fb0cb0f 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -175,6 +175,15 @@ class WebRtcSession : public cricket::BaseSession, const SessionDescriptionInterface* remote_description() const { return remote_desc_.get(); } + // TODO(pthatcher): Cleanup the distinction between + // SessionDescription and SessionDescriptionInterface and remove + // these if possible. + const cricket::SessionDescription* base_local_description() const { + return BaseSession::local_description(); + } + const cricket::SessionDescription* base_remote_description() const { + return BaseSession::remote_description(); + } // Get the id used as a media stream track's "id" field from ssrc. virtual bool GetLocalTrackIdBySsrc(uint32 ssrc, std::string* track_id); @@ -244,6 +253,19 @@ class WebRtcSession : public cricket::BaseSession, metrics_observer_ = metrics_observer; } + protected: + // Don't fire a new description. The only thing it's used for is to + // push new media descriptions to the BaseChannels. But in + // WebRtcSession, we just push to the BaseChannels directly, so we + // don't need this (and it would cause the descriptions to be pushed + // down twice). + // TODO(pthatcher): Remove this method and signal completely from + // BaseSession once all the subclasses of BaseSession push to + // BaseChannels directly rather than relying on the signal, or once + // BaseChannel no longer listens to the event and requires + // descriptions to be pushed down. + virtual void SignalNewDescription() override {} + private: // Indicates the type of SessionDescription in a call to SetLocalDescription // and SetRemoteDescription. @@ -259,6 +281,12 @@ class WebRtcSession : public cricket::BaseSession, bool UpdateSessionState(Action action, cricket::ContentSource source, std::string* err_desc); static Action GetAction(const std::string& type); + // Push the media parts of the local or remote session description + // down to all of the channels. + bool PushdownMediaDescription(cricket::ContentAction action, + cricket::ContentSource source, + std::string* error_desc); + // Transport related callbacks, override from cricket::BaseSession. virtual void OnTransportRequestSignaling(cricket::Transport* transport); diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 3c2c64f880..eb06aef366 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -688,30 +688,48 @@ void BaseChannel::HandlePacket(bool rtcp, rtc::Buffer* packet, void BaseChannel::OnNewLocalDescription( BaseSession* session, ContentAction action) { - const ContentInfo* content_info = - GetFirstContent(session->local_description()); - const MediaContentDescription* content_desc = - GetContentDescription(content_info); std::string error_desc; - if (content_desc && content_info && !content_info->rejected && - !SetLocalContent(content_desc, action, &error_desc)) { + if (!PushdownLocalDescription( + session->local_description(), action, &error_desc)) { SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc); - LOG(LS_ERROR) << "Failure in SetLocalContent with action " << action; } } void BaseChannel::OnNewRemoteDescription( BaseSession* session, ContentAction action) { - const ContentInfo* content_info = - GetFirstContent(session->remote_description()); + std::string error_desc; + if (!PushdownRemoteDescription( + session->remote_description(), action, &error_desc)) { + SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc); + } +} + +bool BaseChannel::PushdownLocalDescription( + const SessionDescription* local_desc, ContentAction action, + std::string* error_desc) { + const ContentInfo* content_info = GetFirstContent(local_desc); const MediaContentDescription* content_desc = GetContentDescription(content_info); - std::string error_desc; if (content_desc && content_info && !content_info->rejected && - !SetRemoteContent(content_desc, action, &error_desc)) { - SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc); - LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action; + !SetLocalContent(content_desc, action, error_desc)) { + LOG(LS_ERROR) << "Failure in SetLocalContent with action " << action; + return false; } + return true; +} + +bool BaseChannel::PushdownRemoteDescription( + const SessionDescription* remote_desc, ContentAction action, + std::string* error_desc) { + const ContentInfo* content_info = GetFirstContent(remote_desc); + const MediaContentDescription* content_desc = + GetContentDescription(content_info); + if (content_desc && content_info && !content_info->rejected && + !SetRemoteContent(content_desc, action, error_desc)) { + LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action; + return false; + } + return true; } void BaseChannel::EnableMedia_w() { diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index bc4b6f76aa..aebb41fa83 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -108,6 +108,12 @@ class BaseChannel bool writable() const { return writable_; } bool IsStreamMuted(uint32 ssrc); + bool PushdownLocalDescription(const SessionDescription* local_desc, + ContentAction action, + std::string* error_desc); + bool PushdownRemoteDescription(const SessionDescription* remote_desc, + ContentAction action, + std::string* error_desc); // Channel control bool SetLocalContent(const MediaContentDescription* content, ContentAction action, diff --git a/webrtc/p2p/base/session.h b/webrtc/p2p/base/session.h index 8dffca7771..cdee8014c0 100644 --- a/webrtc/p2p/base/session.h +++ b/webrtc/p2p/base/session.h @@ -409,6 +409,9 @@ class BaseSession : public sigslot::has_slots<>, Error error_; std::string error_desc_; + // Fires the new description signal according to the current state. + virtual void SignalNewDescription(); + private: // Helper methods to push local and remote transport descriptions. bool PushdownLocalTransportDescription( @@ -435,9 +438,6 @@ class BaseSession : public sigslot::has_slots<>, const std::string& content_name, TransportDescription* info); - // Fires the new description signal according to the current state. - void SignalNewDescription(); - // Gets the ContentAction and ContentSource according to the session state. bool GetContentAction(ContentAction* action, ContentSource* source);