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 2773 matching lines @@
 }
 
 void Channel::EnableReceiveTransportSequenceNumber(int id) {
   rtp_header_parser_->DeregisterRtpHeaderExtension(
       kRtpExtensionTransportSequenceNumber);
   bool ret = rtp_header_parser_->RegisterRtpHeaderExtension(
       kRtpExtensionTransportSequenceNumber, id);
   RTC_DCHECK(ret);
 }
 
-void Channel::SetCongestionControlObjects(
+void Channel::RegisterSenderCongestionControlObjects(
     RtpPacketSender* rtp_packet_sender,
     TransportFeedbackObserver* transport_feedback_observer,
     PacketRouter* packet_router) {
-  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);
-  }
+  RTC_DCHECK(packet_router && !packet_router_);
+  RTC_DCHECK(transport_feedback_observer && feedback_observer_proxy_.get());
+  RTC_DCHECK(seq_num_allocator_proxy_.get());
+  RTC_DCHECK(rtp_packet_sender && rtp_packet_sender_proxy_.get());
+  feedback_observer_proxy_->SetTransportFeedbackObserver(
+      transport_feedback_observer);
+  seq_num_allocator_proxy_->SetSequenceNumberAllocator(packet_router);
+  rtp_packet_sender_proxy_->SetPacketSender(rtp_packet_sender);
   _rtpRtcpModule->SetStorePacketsStatus(rtp_packet_sender != nullptr, 600);
-  if (packet_router != nullptr) {
-    packet_router->AddRtpModule(_rtpRtcpModule.get());
-  } else {
-    packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
-  }
+  packet_router->AddRtpModule(_rtpRtcpModule.get(), true);

[the sun, 2016/01/28 15:33:45]

Could we call _rtpRtcpModule->SetSendingMediaStatus(true) here? and ...(false)
in the method below?

[stefan-webrtc, 2016/01/28 15:52:00]

I think we can, but as I'm not likely to be able to make use of it right now due
to how video code is written, I'd prefer to wait with that.

   packet_router_ = packet_router;
 }
 
+void Channel::RegisterReceiverCongestionControlObjects(
+    PacketRouter* packet_router) {
+  RTC_DCHECK(packet_router && !packet_router_);
+  packet_router->AddRtpModule(_rtpRtcpModule.get(), false);
+  packet_router_ = packet_router;
+}
+
+void Channel::ResetCongestionControlObjects() {
+  _rtpRtcpModule->SetStorePacketsStatus(false, 600);
+  if (feedback_observer_proxy_) {

[the sun, 2016/01/28 15:33:45]

This is always true? Do an RTC_DCHECK() like in RegisterSender...?
and for the ones below

[stefan-webrtc, 2016/01/28 15:52:00]

No, it's not true if pacing hasn't been enabled for audio, which could be the
case for a receive stream. We could require that pacing is enabled for receive
streams too, but i think it would be a bit strange.

[the sun, 2016/01/29 10:07:07]

Ah ok, very well. Another option would be to always initialize these members,
but leave them out from the RtpRtcp::Configuration unless pacing_enabled_. The
state of the object becomes slightly simpler then.

[stefan-webrtc, 2016/01/29 10:59:55]

That's an option. I'll make it so.

+    feedback_observer_proxy_->SetTransportFeedbackObserver(nullptr);
+  }
+  if (seq_num_allocator_proxy_) {
+    seq_num_allocator_proxy_->SetSequenceNumberAllocator(nullptr);
+  }
+  bool sender = false;
+  if (rtp_packet_sender_proxy_) {
+    sender = rtp_packet_sender_proxy_->HasPacketSender();
+    rtp_packet_sender_proxy_->SetPacketSender(nullptr);

[the sun, 2016/01/28 15:33:45]

Always just SetPacketSender(nullptr)? Remove HasPacketSender()

[stefan-webrtc, 2016/01/28 15:52:00]

I need HasPacketSender() to determine if it's a sender or not. Or would you like
me to get that status from the rtp module instead? That should work for voice I
think.

[the sun, 2016/01/29 10:07:07]

I think a single point of decision making is preferable, but best would be if we
didn't need the sender flag when calling RemoveRtpModule(). :)

[stefan-webrtc, 2016/01/29 10:59:55]

Ack. I think it will be hard to do at the moment unfortunately. Do you want me
to make a change here?

+  }
+  packet_router_->RemoveRtpModule(_rtpRtcpModule.get(), sender);
+  packet_router_ = nullptr;
+}
+
 void Channel::SetRTCPStatus(bool enable) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetRTCPStatus()");
   _rtpRtcpModule->SetRTCPStatus(enable ? RtcpMode::kCompound : RtcpMode::kOff);
 }
 
 int
 Channel::GetRTCPStatus(bool& enabled)
 {
   RtcpMode method = _rtpRtcpModule->RTCP();
@@ skipping 1147 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