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
This commit is contained in:
pthatcher@webrtc.org 2015-03-16 21:15:37 +00:00
parent 12e7951bf2
commit 592470b4ff
5 changed files with 108 additions and 29 deletions

View File

@ -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<int>(index);
@ -1468,8 +1496,7 @@ bool WebRtcSession::UseCandidate(
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 @@ bool WebRtcSession::UseCandidate(
}
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.

View File

@ -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);

View File

@ -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() {

View File

@ -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,

View File

@ -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);