Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(139)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: talk/session/media/channel.cc

Issue 1246913005: TransportController refactoring (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: More renaming, formatting and other polish Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « talk/session/media/channel.h ('k') | talk/session/media/channel_unittest.cc » ('j') | webrtc/p2p/base/transport.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: talk/session/media/channel.cc
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index f65579f98b221b2caac5e0feea8a0d3a395bc0f0..db89e4bfbb4f6a16c95caa23ce382d393c1c1a0a 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -171,15 +171,17 @@ void RtpSendParametersFromMediaDescription(
}
BaseChannel::BaseChannel(rtc::Thread* thread,
- MediaChannel* media_channel, BaseSession* session,
- const std::string& content_name, bool rtcp)
+ MediaChannel* media_channel,
+ TransportController* transport_controller,
+ const std::string& content_name,
+ bool rtcp)
: worker_thread_(thread),
- session_(session),
+ transport_controller_(transport_controller),
media_channel_(media_channel),
content_name_(content_name),
- rtcp_(rtcp),
- transport_channel_(NULL),
- rtcp_transport_channel_(NULL),
+ rtcp_transport_enabled_(rtcp),
+ transport_channel_(nullptr),
+ rtcp_transport_channel_(nullptr),
enabled_(false),
writable_(false),
rtp_ready_to_send_(false),
@@ -205,20 +207,31 @@ BaseChannel::~BaseChannel() {
// the media channel may try to send on the dead transport channel. NULLing
// is not an effective strategy since the sends will come on another thread.
delete media_channel_;
- set_transport_channel(nullptr);
- set_rtcp_transport_channel(nullptr);
+ // Note that we don't just call set_transport_channel(nullptr) because that
+ // would call a pure virtual method which we can't do from a destructor.
+ if (transport_channel_) {
+ DisconnectFromTransportChannel(transport_channel_);
+ transport_controller_->DestroyTransportChannel_w(
+ transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ }
+ if (rtcp_transport_channel_) {
+ DisconnectFromTransportChannel(rtcp_transport_channel_);
+ transport_controller_->DestroyTransportChannel_w(
+ transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
+ }
LOG(LS_INFO) << "Destroyed channel";
}
bool BaseChannel::Init() {
- if (!SetTransportChannels(session(), rtcp())) {
+ if (!SetTransport(content_name())) {
return false;
}
if (!SetDtlsSrtpCiphers(transport_channel(), false)) {
return false;
}
- if (rtcp() && !SetDtlsSrtpCiphers(rtcp_transport_channel(), true)) {
+ if (rtcp_transport_enabled() &&
+ !SetDtlsSrtpCiphers(rtcp_transport_channel(), true)) {
return false;
}
@@ -232,29 +245,35 @@ void BaseChannel::Deinit() {
media_channel_->SetInterface(NULL);
}
-bool BaseChannel::SetTransportChannels(BaseSession* session, bool rtcp) {
- return worker_thread_->Invoke<bool>(Bind(
- &BaseChannel::SetTransportChannels_w, this, session, rtcp));
+bool BaseChannel::SetTransport(const std::string& transport_name) {
+ return worker_thread_->Invoke<bool>(
+ Bind(&BaseChannel::SetTransport_w, this, transport_name));
}
-bool BaseChannel::SetTransportChannels_w(BaseSession* session, bool rtcp) {
+bool BaseChannel::SetTransport_w(const std::string& transport_name) {
ASSERT(worker_thread_ == rtc::Thread::Current());
- set_transport_channel(session->CreateChannel(
- content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ if (transport_name == transport_name_) {
+ // Nothing to do if transport name isn't changing
+ return true;
+ }
+
+ set_transport_channel(transport_controller_->CreateTransportChannel_w(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP));
if (!transport_channel()) {
return false;
}
- if (rtcp) {
- set_rtcp_transport_channel(session->CreateChannel(
- content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTCP));
+ if (rtcp_transport_enabled()) {
+ LOG(LS_INFO) << "Create RTCP TransportChannel for " << content_name()
+ << " on " << transport_name << " transport ";
+ set_rtcp_transport_channel(transport_controller_->CreateTransportChannel_w(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP));
if (!rtcp_transport_channel()) {
return false;
}
- } else {
- set_rtcp_transport_channel(nullptr);
}
+ transport_name_ = transport_name;
return true;
}
@@ -262,42 +281,62 @@ void BaseChannel::set_transport_channel(TransportChannel* new_tc) {
ASSERT(worker_thread_ == rtc::Thread::Current());
TransportChannel* old_tc = transport_channel_;
-
- if (old_tc == new_tc) {
+ if (!old_tc && !new_tc) {
+ // Nothing to do
return;
}
+ ASSERT(old_tc != new_tc);
+
if (old_tc) {
DisconnectFromTransportChannel(old_tc);
- session()->DestroyChannel(
- content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ transport_controller_->DestroyTransportChannel_w(
+ transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTP);
}
transport_channel_ = new_tc;
if (new_tc) {
ConnectToTransportChannel(new_tc);
+ for (const auto& pair : socket_options_) {
+ new_tc->SetOption(pair.first, pair.second);
+ }
}
+
+ // Update aggregate writable/ready-to-send state between RTP and RTCP upon
+ // setting new channel
+ UpdateWritableState_w();
+ SetReadyToSend(false, new_tc && new_tc->writable());
}
void BaseChannel::set_rtcp_transport_channel(TransportChannel* new_tc) {
ASSERT(worker_thread_ == rtc::Thread::Current());
TransportChannel* old_tc = rtcp_transport_channel_;
-
- if (old_tc == new_tc) {
+ if (!old_tc && !new_tc) {
+ // Nothing to do
return;
}
+ ASSERT(old_tc != new_tc);
+
if (old_tc) {
DisconnectFromTransportChannel(old_tc);
- session()->DestroyChannel(
- content_name(), cricket::ICE_CANDIDATE_COMPONENT_RTCP);
+ transport_controller_->DestroyTransportChannel_w(
+ transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
}
rtcp_transport_channel_ = new_tc;
if (new_tc) {
ConnectToTransportChannel(new_tc);
+ for (const auto& pair : rtcp_socket_options_) {
+ new_tc->SetOption(pair.first, pair.second);
+ }
}
+
+ // Update aggregate writable/ready-to-send state between RTP and RTCP upon
+ // setting new channel
+ UpdateWritableState_w();
+ SetReadyToSend(true, new_tc && new_tc->writable());
}
void BaseChannel::ConnectToTransportChannel(TransportChannel* tc) {
@@ -416,9 +455,13 @@ int BaseChannel::SetOption(SocketType type, rtc::Socket::Option opt,
switch (type) {
case ST_RTP:
channel = transport_channel_;
+ socket_options_.push_back(
+ std::pair<rtc::Socket::Option, int>(opt, value));
break;
case ST_RTCP:
channel = rtcp_transport_channel_;
+ rtcp_socket_options_.push_back(
+ std::pair<rtc::Socket::Option, int>(opt, value));
break;
}
return channel ? channel->SetOption(opt, value) : -1;
@@ -426,12 +469,7 @@ int BaseChannel::SetOption(SocketType type, rtc::Socket::Option opt,
void BaseChannel::OnWritableState(TransportChannel* channel) {
ASSERT(channel == transport_channel_ || channel == rtcp_transport_channel_);
- if (transport_channel_->writable()
- && (!rtcp_transport_channel_ || rtcp_transport_channel_->writable())) {
- ChannelWritable_w();
- } else {
- ChannelNotWritable_w();
- }
+ UpdateWritableState_w();
}
void BaseChannel::OnChannelRead(TransportChannel* channel,
@@ -449,26 +487,25 @@ void BaseChannel::OnChannelRead(TransportChannel* channel,
}
void BaseChannel::OnReadyToSend(TransportChannel* channel) {
- SetReadyToSend(channel, true);
+ ASSERT(channel == transport_channel_ || channel == rtcp_transport_channel_);
+ SetReadyToSend(channel == rtcp_transport_channel_, true);
}
-void BaseChannel::SetReadyToSend(TransportChannel* channel, bool ready) {
- ASSERT(channel == transport_channel_ || channel == rtcp_transport_channel_);
- if (channel == transport_channel_) {
- rtp_ready_to_send_ = ready;
- }
- if (channel == rtcp_transport_channel_) {
+void BaseChannel::SetReadyToSend(bool rtcp, bool ready) {
+ if (rtcp) {
rtcp_ready_to_send_ = ready;
+ } else {
+ rtp_ready_to_send_ = ready;
}
- if (!ready) {
- // Notify the MediaChannel when either rtp or rtcp channel can't send.
- media_channel_->OnReadyToSend(false);
- } else if (rtp_ready_to_send_ &&
- // In the case of rtcp mux |rtcp_transport_channel_| will be null.
- (rtcp_ready_to_send_ || !rtcp_transport_channel_)) {
+ if (rtp_ready_to_send_ &&
+ // In the case of rtcp mux |rtcp_transport_channel_| will be null.
+ (rtcp_ready_to_send_ || !rtcp_transport_channel_)) {
// Notify the MediaChannel when both rtp and rtcp channel can send.
media_channel_->OnReadyToSend(true);
+ } else {
+ // Notify the MediaChannel when either rtp or rtcp channel can't send.
+ media_channel_->OnReadyToSend(false);
}
}
@@ -590,7 +627,7 @@ bool BaseChannel::SendPacket(bool rtcp, rtc::Buffer* packet,
if (ret != static_cast<int>(packet->size())) {
if (channel->GetError() == EWOULDBLOCK) {
LOG(LS_WARNING) << "Got EWOULDBLOCK from socket.";
- SetReadyToSend(channel, false);
+ SetReadyToSend(rtcp, false);
}
return false;
}
@@ -741,14 +778,21 @@ bool BaseChannel::IsStreamMuted_w(uint32 ssrc) {
return muted_streams_.find(ssrc) != muted_streams_.end();
}
+void BaseChannel::UpdateWritableState_w() {
+ if (transport_channel_ && transport_channel_->writable() &&
+ (!rtcp_transport_channel_ || rtcp_transport_channel_->writable())) {
+ ChannelWritable_w();
+ } else {
+ ChannelNotWritable_w();
+ }
+}
+
void BaseChannel::ChannelWritable_w() {
ASSERT(worker_thread_ == rtc::Thread::Current());
if (writable_)
return;
- LOG(LS_INFO) << "Channel socket writable ("
- << transport_channel_->content_name() << ", "
- << transport_channel_->component() << ")"
+ LOG(LS_INFO) << "Channel writable (" << content_name_ << ")"
<< (was_ever_writable_ ? "" : " for the first time");
std::vector<ConnectionInfo> infos;
@@ -765,13 +809,13 @@ void BaseChannel::ChannelWritable_w() {
// If we're doing DTLS-SRTP, now is the time.
if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) {
if (!SetupDtlsSrtp(false)) {
- SignalDtlsSetupFailure(this, false);
+ SignalDtlsSetupFailure_w(false);
return;
}
if (rtcp_transport_channel_) {
if (!SetupDtlsSrtp(true)) {
- SignalDtlsSetupFailure(this, true);
+ SignalDtlsSetupFailure_w(true);
return;
}
}
@@ -814,8 +858,8 @@ bool BaseChannel::ShouldSetupDtlsSrtp() const {
bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) {
bool ret = false;
- TransportChannel *channel = rtcp_channel ?
- rtcp_transport_channel_ : transport_channel_;
+ TransportChannel* channel =
+ rtcp_channel ? rtcp_transport_channel_ : transport_channel_;
// No DTLS
if (!channel->IsDtlsActive())
@@ -910,9 +954,7 @@ void BaseChannel::ChannelNotWritable_w() {
if (!writable_)
return;
- LOG(LS_INFO) << "Channel socket not writable ("
- << transport_channel_->content_name() << ", "
- << transport_channel_->component() << ")";
+ LOG(LS_INFO) << "Channel not writable (" << content_name_ << ")";
writable_ = false;
ChangeState();
}
@@ -1011,7 +1053,8 @@ void BaseChannel::ActivateRtcpMux() {
void BaseChannel::ActivateRtcpMux_w() {
if (!rtcp_mux_filter_.IsActive()) {
rtcp_mux_filter_.SetActive();
- set_rtcp_transport_channel(NULL);
+ set_rtcp_transport_channel(nullptr);
+ rtcp_transport_enabled_ = false;
}
}
@@ -1030,7 +1073,11 @@ bool BaseChannel::SetRtcpMux_w(bool enable, ContentAction action,
ret = rtcp_mux_filter_.SetAnswer(enable, src);
if (ret && rtcp_mux_filter_.IsActive()) {
// We activated RTCP mux, close down the RTCP transport.
- set_rtcp_transport_channel(NULL);
+ LOG(LS_INFO) << "Enabling rtcp-mux for " << content_name()
+ << " by destroying RTCP transport channel for "
+ << transport_name();
+ set_rtcp_transport_channel(nullptr);
+ rtcp_transport_enabled_ = false;
}
break;
case CA_UPDATE:
@@ -1257,10 +1304,13 @@ void BaseChannel::FlushRtcpMessages() {
VoiceChannel::VoiceChannel(rtc::Thread* thread,
MediaEngineInterface* media_engine,
VoiceMediaChannel* media_channel,
- BaseSession* session,
+ TransportController* transport_controller,
const std::string& content_name,
bool rtcp)
- : BaseChannel(thread, media_channel, session, content_name,
+ : BaseChannel(thread,
+ media_channel,
+ transport_controller,
+ content_name,
rtcp),
media_engine_(media_engine),
received_media_(false) {
@@ -1656,10 +1706,13 @@ void VoiceChannel::GetSrtpCiphers(std::vector<std::string>* ciphers) const {
VideoChannel::VideoChannel(rtc::Thread* thread,
VideoMediaChannel* media_channel,
- BaseSession* session,
+ TransportController* transport_controller,
const std::string& content_name,
bool rtcp)
- : BaseChannel(thread, media_channel, session, content_name,
+ : BaseChannel(thread,
+ media_channel,
+ transport_controller,
+ content_name,
rtcp),
renderer_(NULL),
previous_we_(rtc::WE_CLOSE) {
@@ -2094,10 +2147,14 @@ void VideoChannel::GetSrtpCiphers(std::vector<std::string>* ciphers) const {
DataChannel::DataChannel(rtc::Thread* thread,
DataMediaChannel* media_channel,
- BaseSession* session,
+ TransportController* transport_controller,
const std::string& content_name,
bool rtcp)
- : BaseChannel(thread, media_channel, session, content_name, rtcp),
+ : BaseChannel(thread,
+ media_channel,
+ transport_controller,
+ content_name,
+ rtcp),
data_channel_type_(cricket::DCT_NONE),
ready_to_send_data_(false) {
}
« no previous file with comments | « talk/session/media/channel.h ('k') | talk/session/media/channel_unittest.cc » ('j') | webrtc/p2p/base/transport.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698