Combining "SetTransportChannel" and "SetRtcpTransportChannel".
The only real difference between the two is that SetRtcpTransportChannel
had a workaround to prevent a signal from being emitted early.
Basically, in SetTransport, we want to switch the transport channels and
*then* update the state, rather than updating the state after changing
only one transport channel.
But this can be accomplished more easily by simply updating the state in
SetTransport directly.
Review-Url: https://codereview.webrtc.org/2274283004
Cr-Commit-Position: refs/heads/master@{#13945}
diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc
index 7ec536d..98838dd 100644
--- a/webrtc/pc/channel.cc
+++ b/webrtc/pc/channel.cc
@@ -300,61 +300,30 @@
if (rtcp_enabled_ && !rtcp_mux_filter_.IsFullyActive()) {
LOG(LS_INFO) << "Create RTCP TransportChannel for " << content_name()
<< " on " << transport_name << " transport ";
- // TODO(deadbeef): Remove this grossness when we remove non-muxed RTCP.
- SetRtcpTransportChannel_n(
- transport_controller_->CreateTransportChannel_n(
- transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP),
- false /* update_writablity */);
+ SetTransportChannel_n(
+ true, transport_controller_->CreateTransportChannel_n(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP));
if (!rtcp_transport_channel_) {
return false;
}
}
- // We're not updating the writablity during the transition state.
- SetTransportChannel_n(transport_controller_->CreateTransportChannel_n(
- transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ LOG(LS_INFO) << "Create non-RTCP TransportChannel for " << content_name()
+ << " on " << transport_name << " transport ";
+ SetTransportChannel_n(
+ false, transport_controller_->CreateTransportChannel_n(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP));
if (!transport_channel_) {
return false;
}
- // TODO(deadbeef): Remove this grossness when we remove non-muxed RTCP.
- if (rtcp_transport_channel_) {
- // We can only update the RTCP ready to send after set_transport_channel has
- // handled channel writability.
- SetTransportChannelReadyToSend(true, rtcp_transport_channel_->writable());
- }
transport_name_ = transport_name;
- return true;
-}
-
-void BaseChannel::SetTransportChannel_n(TransportChannel* new_tc) {
- RTC_DCHECK(network_thread_->IsCurrent());
-
- TransportChannel* old_tc = transport_channel_;
- if (!old_tc && !new_tc) {
- // Nothing to do.
- return;
- }
- RTC_DCHECK(old_tc != new_tc);
-
- if (old_tc) {
- DisconnectFromTransportChannel(old_tc);
- transport_controller_->DestroyTransportChannel_n(
- 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 transport channels.
UpdateWritableState_n();
+ // We can only update ready-to-send after updating writability.
+ //
// On setting a new channel, assume it's ready to send if it's writable,
// because we have no way of knowing otherwise (the channel doesn't give us
// "was last send successful?").
@@ -362,50 +331,45 @@
// This won't always be accurate (the last SendPacket call from another
// BaseChannel could have resulted in an error), but even so, we'll just
// encounter the error again and update "ready to send" accordingly.
- SetTransportChannelReadyToSend(false, new_tc && new_tc->writable());
+ SetTransportChannelReadyToSend(
+ false, transport_channel_ && transport_channel_->writable());
+ SetTransportChannelReadyToSend(
+ true, rtcp_transport_channel_ && rtcp_transport_channel_->writable());
+ return true;
}
-void BaseChannel::SetRtcpTransportChannel_n(TransportChannel* new_tc,
- bool update_writablity) {
+void BaseChannel::SetTransportChannel_n(bool rtcp,
+ TransportChannel* new_channel) {
RTC_DCHECK(network_thread_->IsCurrent());
+ TransportChannel*& old_channel =
+ rtcp ? rtcp_transport_channel_ : transport_channel_;
- TransportChannel* old_tc = rtcp_transport_channel_;
- if (!old_tc && !new_tc) {
+ if (!old_channel && !new_channel) {
// Nothing to do.
return;
}
- RTC_DCHECK(old_tc != new_tc);
+ RTC_DCHECK(old_channel != new_channel);
- if (old_tc) {
- DisconnectFromTransportChannel(old_tc);
+ if (old_channel) {
+ DisconnectFromTransportChannel(old_channel);
transport_controller_->DestroyTransportChannel_n(
- transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
+ transport_name_, rtcp ? cricket::ICE_CANDIDATE_COMPONENT_RTCP
+ : cricket::ICE_CANDIDATE_COMPONENT_RTP);
}
- rtcp_transport_channel_ = new_tc;
+ old_channel = new_channel;
- if (new_tc) {
- RTC_CHECK(!(ShouldSetupDtlsSrtp_n() && srtp_filter_.IsActive()))
- << "Setting RTCP for DTLS/SRTP after SrtpFilter is active "
- << "should never happen.";
- ConnectToTransportChannel(new_tc);
- for (const auto& pair : rtcp_socket_options_) {
- new_tc->SetOption(pair.first, pair.second);
+ if (new_channel) {
+ if (rtcp) {
+ RTC_CHECK(!(ShouldSetupDtlsSrtp_n() && srtp_filter_.IsActive()))
+ << "Setting RTCP for DTLS/SRTP after SrtpFilter is active "
+ << "should never happen.";
}
- }
-
- if (update_writablity) {
- // Update aggregate writable/ready-to-send state between RTP and RTCP upon
- // setting new channel
- UpdateWritableState_n();
- // On setting a new channel, assume it's ready to send if it's writable,
- // because we have no way of knowing otherwise (the channel doesn't give us
- // "was last send successful?").
- //
- // This won't always be accurate (the last SendPacket call from another
- // BaseChannel could have resulted in an error), but even so, we'll just
- // encounter the error again and update "ready to send" accordingly.
- SetTransportChannelReadyToSend(true, new_tc && new_tc->writable());
+ ConnectToTransportChannel(new_channel);
+ auto& socket_options = rtcp ? rtcp_socket_options_ : socket_options_;
+ for (const auto& pair : socket_options) {
+ new_channel->SetOption(pair.first, pair.second);
+ }
}
}
@@ -1212,7 +1176,11 @@
void BaseChannel::ActivateRtcpMux_n() {
if (!rtcp_mux_filter_.IsActive()) {
rtcp_mux_filter_.SetActive();
- SetRtcpTransportChannel_n(nullptr, true);
+ SetTransportChannel_n(true, nullptr);
+ // Update aggregate writable/ready-to-send state between RTP and RTCP upon
+ // removing channel.
+ UpdateWritableState_n();
+ SetTransportChannelReadyToSend(true, false);
}
}
@@ -1237,7 +1205,9 @@
LOG(LS_INFO) << "Enabling rtcp-mux for " << content_name()
<< " by destroying RTCP transport channel for "
<< transport_name();
- SetRtcpTransportChannel_n(nullptr, true);
+ SetTransportChannel_n(true, nullptr);
+ UpdateWritableState_n();
+ SetTransportChannelReadyToSend(true, false);
}
break;
case CA_UPDATE:
diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h
index 2a5d3a5..7d3bc94 100644
--- a/webrtc/pc/channel.h
+++ b/webrtc/pc/channel.h
@@ -48,15 +48,16 @@
struct CryptoParams;
class MediaContentDescription;
-// BaseChannel contains logic common to voice and video, including
-// enable, marshaling calls to a worker and network threads, and
-// connection and media monitors.
+// BaseChannel contains logic common to voice and video, including enable,
+// marshaling calls to a worker and network threads, and connection and media
+// monitors.
+//
// BaseChannel assumes signaling and other threads are allowed to make
// synchronous calls to the worker thread, the worker thread makes synchronous
// calls only to the network thread, and the network thread can't be blocked by
// other threads.
// All methods with _n suffix must be called on network thread,
-// methods with _w suffix - on worker thread
+// methods with _w suffix on worker thread
// and methods with _s suffix on signaling thread.
// Network and worker threads may be the same thread.
//
@@ -187,11 +188,12 @@
// Sets the |transport_channel_| (and |rtcp_transport_channel_|, if
// |rtcp_enabled_| is true). Gets the transport channels from
// |transport_controller_|.
+ // This method also updates writability and "ready-to-send" state.
bool SetTransport_n(const std::string& transport_name);
- void SetTransportChannel_n(TransportChannel* transport);
- void SetRtcpTransportChannel_n(TransportChannel* transport,
- bool update_writablity);
+ // This does not update writability or "ready-to-send" state; it just
+ // disconnects from the old channel and connects to the new one.
+ void SetTransportChannel_n(bool rtcp, TransportChannel* new_channel);
bool was_ever_writable() const { return was_ever_writable_; }
void set_local_content_direction(MediaContentDirection direction) {
