Use separate rtp module lists for send and receive in PacketRouter.

webrtc/voice_engine/channel.cc

 /*
  *  Copyright (c) 2012 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 107 matching lines @@
  public:
   RtpPacketSenderProxy() : rtp_packet_sender_(nullptr) {
   }
 
   void SetPacketSender(RtpPacketSender* rtp_packet_sender) {
     RTC_DCHECK(thread_checker_.CalledOnValidThread());
     rtc::CritScope lock(&crit_);
     rtp_packet_sender_ = rtp_packet_sender;
   }
 
+  bool HasPacketSender() const {
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    return rtp_packet_sender_ != nullptr;
+  }
+
   // Implements RtpPacketSender.
   void InsertPacket(Priority priority,
                     uint32_t ssrc,
                     uint16_t sequence_number,
                     int64_t capture_time_ms,
                     size_t bytes,
                     bool retransmission) override {
     rtc::CritScope lock(&crit_);
     if (rtp_packet_sender_) {
       rtp_packet_sender_->InsertPacket(priority, ssrc, sequence_number,
@@ skipping 2777 matching lines @@
       kRtpExtensionTransportSequenceNumber);
   bool ret = rtp_header_parser_->RegisterRtpHeaderExtension(
       kRtpExtensionTransportSequenceNumber, id);
   RTC_DCHECK(ret);
 }
 
 void Channel::SetCongestionControlObjects(
     RtpPacketSender* rtp_packet_sender,
     TransportFeedbackObserver* transport_feedback_observer,
     PacketRouter* packet_router) {
+  bool sender = rtp_packet_sender != nullptr ||
+                (rtp_packet_sender_proxy_.get() &&
+                 rtp_packet_sender_proxy_->HasPacketSender());

[the sun, 2016/01/25 15:42:01]

Can this property change as a consequence of this call to
SetCongestionControlObjects? Does it need to be evaluated twice - once before
SetPacketSender and once after?

[stefan-webrtc, 2016/01/25 16:00:26]

It can change to null as the stream is being teared down, but a channel can only
be a sender or a receiver and this expression makes sure we can figure out which
it is. 

If rtp_packet_sender isn't null, we know that the channel (stream) is either
being teared down or that it is a receiving channel. If it's true we know that
it must be a sending channel since no receivers have an rtp_packet_sender.

To check the former case, we check if the rtp_packet_sender_proxy_ has already
been configured with an rtp_packet_sender, since in that case we know that it
already is a sending channel (and that it is actually being teared down).

Because of this I don't think we need to evaluate it twice.

[the sun, 2016/01/26 09:17:03]

Very well, but I think this code is becoming hard to follow. How about we split
it into three functions instead?

SetSenderCongestionControlObjects(x, y, z)
SetReceiverCongestionControlObjects(z)
ResetCongestionControlObjects()

I think the logic would be simpler.

[stefan-webrtc, 2016/01/26 10:50:38]

Agree, done.

   RTC_DCHECK(packet_router != nullptr || packet_router_ != nullptr);
   if (transport_feedback_observer) {
     RTC_DCHECK(feedback_observer_proxy_.get());
     feedback_observer_proxy_->SetTransportFeedbackObserver(
         transport_feedback_observer);
   }
   if (rtp_packet_sender) {
     RTC_DCHECK(rtp_packet_sender_proxy_.get());
     rtp_packet_sender_proxy_->SetPacketSender(rtp_packet_sender);
   }
   if (seq_num_allocator_proxy_.get()) {
     seq_num_allocator_proxy_->SetSequenceNumberAllocator(packet_router);
   }
   _rtpRtcpModule->SetStorePacketsStatus(rtp_packet_sender != nullptr, 600);
   if (packet_router != nullptr) {
-    packet_router->AddRtpModule(_rtpRtcpModule.get());
+    packet_router->AddRtpModule(_rtpRtcpModule.get(), sender);
   } else {
-    packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
+    packet_router_->RemoveRtpModule(_rtpRtcpModule.get(), sender);
   }
   packet_router_ = packet_router;
 }
 
 void Channel::SetRTCPStatus(bool enable) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetRTCPStatus()");
   _rtpRtcpModule->SetRTCPStatus(enable ? RtcpMode::kCompound : RtcpMode::kOff);
 }
 
@@ skipping 1151 matching lines @@
   int64_t min_rtt = 0;
   if (_rtpRtcpModule->RTT(remoteSSRC, &rtt, &avg_rtt, &min_rtt, &max_rtt)
       != 0) {
     return 0;
   }
   return rtt;
 }
 
 }  // namespace voe
 }  // namespace webrtc