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

webrtc/voice_engine/channel.cc

Index: webrtc/voice_engine/channel.cc
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 25f44c1c66fcbaccc7543121971f0c9cda6dc4e1..36bb7e42673f47bb4d595bf7b1db18b9a6717c47 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -125,6 +125,12 @@ class RtpPacketSenderProxy : public RtpPacketSender {
     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,
@@ -2918,30 +2924,45 @@ void Channel::EnableReceiveTransportSequenceNumber(int 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);
+  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);
+  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_.get()) {
-    seq_num_allocator_proxy_->SetSequenceNumberAllocator(packet_router);
+  if (seq_num_allocator_proxy_) {
+    seq_num_allocator_proxy_->SetSequenceNumberAllocator(nullptr);
   }
-  _rtpRtcpModule->SetStorePacketsStatus(rtp_packet_sender != nullptr, 600);
-  if (packet_router != nullptr) {
-    packet_router->AddRtpModule(_rtpRtcpModule.get());
-  } else {
-    packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
+  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_ = packet_router;
+  packet_router_->RemoveRtpModule(_rtpRtcpModule.get(), sender);
+  packet_router_ = nullptr;
 }
 
 void Channel::SetRTCPStatus(bool enable) {