Move ready to send logic from BaseChannel to RtpTransport.

webrtc/pc/channel.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 158 matching lines @@
       signaling_thread_(signaling_thread),
       content_name_(content_name),
       rtp_transport_(rtcp_mux_required),
       srtp_required_(srtp_required),
       media_channel_(media_channel),
       selected_candidate_pair_(nullptr) {
   RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
 #if defined(ENABLE_EXTERNAL_AUTH)
   srtp_filter_.EnableExternalAuth();
 #endif
+  rtp_transport_.SignalReadyToSend.connect(
+      this, &BaseChannel::OnTransportReadyToSend);
   LOG(LS_INFO) << "Created channel for " << content_name;
 }
 
 BaseChannel::~BaseChannel() {
   TRACE_EVENT0("webrtc", "BaseChannel::~BaseChannel");
   RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
   Deinit();
   StopConnectionMonitor();
   // Eats any outstanding messages or packets.
   worker_thread_->Clear(&invoker_);
@@ skipping 181 matching lines @@
            : rtp_transport_.rtp_packet_transport();
 
   if (!old_packet_transport && !new_packet_transport) {
     // Nothing to do.
     return;
   }
 
   RTC_DCHECK(old_packet_transport != new_packet_transport);
   if (old_dtls_transport) {
     DisconnectFromDtlsTransport(old_dtls_transport);
+    rtp_transport_.Disconnect(rtcp);
   } else if (old_packet_transport) {
     DisconnectFromPacketTransport(old_packet_transport);
+    rtp_transport_.Disconnect(rtcp);

[Taylor Brandstetter, 2017/04/14 18:05:33]

Could the "disconnect"/"connect" happen implicitly in
"set_rt[c]p_packet_transport"? If you do this, they should be changed to use
camel case naming, since they'd no longer be trivial setters.

[Zach Stein, 2017/04/18 23:39:29]

Done.

   }
 
   if (rtcp) {
     rtp_transport_.set_rtcp_packet_transport(new_packet_transport);
   } else {
     rtp_transport_.set_rtp_packet_transport(new_packet_transport);
   }
   old_dtls_transport = new_dtls_transport;
 
   // If there's no new transport, we're done after disconnecting from old one.
   if (!new_packet_transport) {
     return;
   }
 
   if (rtcp && new_dtls_transport) {
     RTC_CHECK(!(ShouldSetupDtlsSrtp_n() && srtp_filter_.IsActive()))
         << "Setting RTCP for DTLS/SRTP after SrtpFilter is active "
         << "should never happen.";
   }
+
+  rtp_transport_.Connect(rtcp);
   if (new_dtls_transport) {
     ConnectToDtlsTransport(new_dtls_transport);
   } else {
     ConnectToPacketTransport(new_packet_transport);
   }
   auto& socket_options = rtcp ? rtcp_socket_options_ : socket_options_;
   for (const auto& pair : socket_options) {
     new_packet_transport->SetOption(pair.first, pair.second);
   }
 }
 
 void BaseChannel::ConnectToDtlsTransport(DtlsTransportInternal* transport) {
   RTC_DCHECK(network_thread_->IsCurrent());
 
+  // TODO(zstein): de-dup with ConnectToPacketTransport
   transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead);
-  transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
   transport->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState);
   transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n);
   transport->ice_transport()->SignalSelectedCandidatePairChanged.connect(
       this, &BaseChannel::OnSelectedCandidatePairChanged);
 }
 
 void BaseChannel::DisconnectFromDtlsTransport(
     DtlsTransportInternal* transport) {
   RTC_DCHECK(network_thread_->IsCurrent());
   OnSelectedCandidatePairChanged(transport->ice_transport(), nullptr, -1,
                                  false);
 
   transport->SignalWritableState.disconnect(this);
   transport->SignalReadPacket.disconnect(this);
-  transport->SignalReadyToSend.disconnect(this);
   transport->SignalDtlsState.disconnect(this);
   transport->SignalSentPacket.disconnect(this);
   transport->ice_transport()->SignalSelectedCandidatePairChanged.disconnect(
       this);
 }
 
 void BaseChannel::ConnectToPacketTransport(
     rtc::PacketTransportInternal* transport) {
   RTC_DCHECK_RUN_ON(network_thread_);
   transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead);
-  transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
   transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n);
 }
 
 void BaseChannel::DisconnectFromPacketTransport(
     rtc::PacketTransportInternal* transport) {
   RTC_DCHECK_RUN_ON(network_thread_);
   transport->SignalWritableState.disconnect(this);
   transport->SignalReadPacket.disconnect(this);
-  transport->SignalReadyToSend.disconnect(this);
   transport->SignalSentPacket.disconnect(this);
 }
 
 bool BaseChannel::Enable(bool enable) {
   worker_thread_->Invoke<void>(
       RTC_FROM_HERE,
       Bind(enable ? &BaseChannel::EnableMedia_w : &BaseChannel::DisableMedia_w,
            this));
   return true;
 }
@@ skipping 147 matching lines @@
   // OnPacketRead gets called from P2PSocket; now pass data to MediaEngine
   RTC_DCHECK(network_thread_->IsCurrent());
 
   // When using RTCP multiplexing we might get RTCP packets on the RTP
   // transport. We feed RTP traffic into the demuxer to determine if it is RTCP.
   bool rtcp = PacketIsRtcp(transport, data, len);
   rtc::CopyOnWriteBuffer packet(data, len);
   HandlePacket(rtcp, &packet, packet_time);
 }
 
-void BaseChannel::OnReadyToSend(rtc::PacketTransportInternal* transport) {
-  RTC_DCHECK(transport == rtp_transport_.rtp_packet_transport() ||
-             transport == rtp_transport_.rtcp_packet_transport());
-  SetTransportChannelReadyToSend(
-      transport == rtp_transport_.rtcp_packet_transport(), true);
-}
-
 void BaseChannel::OnDtlsState(DtlsTransportInternal* transport,
                               DtlsTransportState state) {
   if (!ShouldSetupDtlsSrtp_n()) {
     return;
   }
 
   // Reset the srtp filter if it's not the CONNECTED state. For the CONNECTED
   // state, setting up DTLS-SRTP context is deferred to ChannelWritable_w to
   // cover other scenarios like the whole transport is writable (not just this
   // TransportChannel) or when TransportChannel is attached after DTLS is
@@ skipping 23 matching lines @@
         last_sent_packet_id);
 
     UpdateTransportOverhead();
   }
   invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, worker_thread_,
       Bind(&MediaChannel::OnNetworkRouteChanged, media_channel_, transport_name,
            network_route));
 }
 
 void BaseChannel::SetTransportChannelReadyToSend(bool rtcp, bool ready) {

[Taylor Brandstetter, 2017/04/14 18:05:33]

Can this method be removed, and the necessary logic moved to RtpTransport?
Currently this is called either:

1. When the transport channel changes
2. When sending a packet fails

RtpTransport already knows about #1. So you'd just need to move the
responsibility of sending packets to RtpTransport to handle #2. Meaning, add
"SendRtpPacket"/"SendRtcpPacket" methods that take an RTP or SRTP packet as
input and just send it on the right transport.

[Zach Stein, 2017/04/18 23:39:29]

Done.

   RTC_DCHECK(network_thread_->IsCurrent());
-  if (rtcp) {
-    rtcp_ready_to_send_ = ready;
-  } else {
-    rtp_ready_to_send_ = ready;
-  }
+  rtp_transport_.SetReadyToSend(rtcp, ready);
+}
 
-  bool ready_to_send =
-      (rtp_ready_to_send_ &&
-       // In the case of rtcp mux |rtcp_packet_transport_| will be null.
-       (rtcp_ready_to_send_ || !rtp_transport_.rtcp_packet_transport()));
-
+void BaseChannel::OnTransportReadyToSend(bool ready) {
   invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, worker_thread_,
-      Bind(&MediaChannel::OnReadyToSend, media_channel_, ready_to_send));
+      Bind(&MediaChannel::OnReadyToSend, media_channel_, ready));
 }
 
 bool BaseChannel::PacketIsRtcp(const rtc::PacketTransportInternal* transport,
                                const char* data,
                                size_t len) {
   return (transport == rtp_transport_.rtcp_packet_transport() ||
           rtcp_mux_filter_.DemuxRtcp(data, static_cast<int>(len)));
 }
 
 bool BaseChannel::SendPacket(bool rtcp,
@@ skipping 1789 matching lines @@
   signaling_thread()->Post(RTC_FROM_HERE, this, MSG_READYTOSENDDATA,
                            new DataChannelReadyToSendMessageData(writable));
 }
 
 void RtpDataChannel::GetSrtpCryptoSuites_n(
     std::vector<int>* crypto_suites) const {
   GetSupportedDataCryptoSuites(crypto_options(), crypto_suites);
 }
 
 }  // namespace cricket