Remove a dependency of BaseChannel on WebRtcSession by having WebRtcSession push down new media descriptions to BaseChannel rather than having BaseChannel listen to the description changes from WebRtcSession.
This is a part of the big BUNDLE implementation at https://webrtc-codereview.appspot.com/45519004/
R=tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/47599004
Cr-Commit-Position: refs/heads/master@{#8743}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8743 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index c2ce02c..1af0e19 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -874,6 +874,9 @@
}
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 @@
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 @@
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 @@
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::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 @@
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 @@
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 @@
// 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<int>(index);
@@ -1468,8 +1496,7 @@
const IceCandidateInterface* candidate) {
size_t mediacontent_index = static_cast<size_t>(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 @@
}
cricket::ContentInfo content =
- BaseSession::remote_description()->contents()[mediacontent_index];
+ base_remote_description()->contents()[mediacontent_index];
std::vector<cricket::Candidate> 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 14e4414..ce0fb0c 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -175,6 +175,15 @@
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 @@
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 @@
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 3c2c64f..eb06aef 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -688,30 +688,48 @@
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 bc4b6f7..aebb41f 100644
--- a/talk/session/media/channel.h
+++ b/talk/session/media/channel.h
@@ -108,6 +108,12 @@
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 8dffca7..cdee801 100644
--- a/webrtc/p2p/base/session.h
+++ b/webrtc/p2p/base/session.h
@@ -409,6 +409,9 @@
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 @@
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);
