Let PacketRouter separate send and receive modules.

This is in preparation for merging the ViERemb logic in packet_router,
to send REMB feedback as sender reports if possible, otherwise as
receiver reports.

BUG=webrtc:7135

Review-Url: https://codereview.webrtc.org/2774623006
Cr-Commit-Position: refs/heads/master@{#17489}

webrtc/audio/audio_receive_stream.cc

@@ -117,7 +117,7 @@ AudioReceiveStream::~AudioReceiveStream() {
   }
   channel_proxy_->DisassociateSendChannel();
   channel_proxy_->DeRegisterExternalTransport();
-  channel_proxy_->ResetCongestionControlObjects();
+  channel_proxy_->ResetReceiverCongestionControlObjects();
   channel_proxy_->SetRtcEventLog(nullptr);
 }
 

webrtc/audio/audio_receive_stream_unittest.cc

@@ -97,7 +97,7 @@ struct ConfigHelper {
           EXPECT_CALL(*channel_proxy_,
               RegisterReceiverCongestionControlObjects(&packet_router_))
                   .Times(1);
-          EXPECT_CALL(*channel_proxy_, ResetCongestionControlObjects())
+          EXPECT_CALL(*channel_proxy_, ResetReceiverCongestionControlObjects())
               .Times(1);
           EXPECT_CALL(*channel_proxy_, RegisterExternalTransport(nullptr))
               .Times(1);

webrtc/audio/audio_send_stream.cc

@@ -111,7 +111,7 @@ AudioSendStream::~AudioSendStream() {
   LOG(LS_INFO) << "~AudioSendStream: " << config_.ToString();
   transport_->send_side_cc()->DeRegisterPacketFeedbackObserver(this);
   channel_proxy_->DeRegisterExternalTransport();
-  channel_proxy_->ResetCongestionControlObjects();
+  channel_proxy_->ResetSenderCongestionControlObjects();
   channel_proxy_->SetRtcEventLog(nullptr);
   channel_proxy_->SetRtcpRttStats(nullptr);
 }

webrtc/audio/audio_send_stream_unittest.cc

@@ -176,7 +176,8 @@ struct ConfigHelper {
                                        &fake_transport_, Eq(nullptr)))
           .Times(1);
     }
-    EXPECT_CALL(*channel_proxy_, ResetCongestionControlObjects()).Times(1);
+    EXPECT_CALL(*channel_proxy_, ResetSenderCongestionControlObjects())
+        .Times(1);
     EXPECT_CALL(*channel_proxy_, RegisterExternalTransport(nullptr)).Times(1);
     EXPECT_CALL(*channel_proxy_, DeRegisterExternalTransport()).Times(1);
     EXPECT_CALL(*channel_proxy_, SetRtcEventLog(testing::NotNull())).Times(1);

webrtc/modules/pacing/packet_router.cc

@@ -23,27 +23,43 @@ PacketRouter::PacketRouter() : transport_seq_(0) {
 }
 
 PacketRouter::~PacketRouter() {
-  RTC_DCHECK(rtp_modules_.empty());
+  RTC_DCHECK(rtp_send_modules_.empty());
+  RTC_DCHECK(rtp_receive_modules_.empty());
 }
 
-void PacketRouter::AddRtpModule(RtpRtcp* rtp_module) {
+void PacketRouter::AddSendRtpModule(RtpRtcp* rtp_module) {
   rtc::CritScope cs(&modules_crit_);
-  RTC_DCHECK(std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module) ==
-             rtp_modules_.end());
+  RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(),
+                       rtp_module) == rtp_send_modules_.end());
   // Put modules which can use regular payload packets (over rtx) instead of
   // padding first as it's less of a waste
   if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) > 0) {
-    rtp_modules_.push_front(rtp_module);
+    rtp_send_modules_.push_front(rtp_module);
   } else {
-    rtp_modules_.push_back(rtp_module);
+    rtp_send_modules_.push_back(rtp_module);
   }
 }
 
-void PacketRouter::RemoveRtpModule(RtpRtcp* rtp_module) {
+void PacketRouter::RemoveSendRtpModule(RtpRtcp* rtp_module) {
   rtc::CritScope cs(&modules_crit_);
-  RTC_DCHECK(std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module) !=
-             rtp_modules_.end());
-  rtp_modules_.remove(rtp_module);
+  RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(),
+                       rtp_module) != rtp_send_modules_.end());
+  rtp_send_modules_.remove(rtp_module);
+}
+
+void PacketRouter::AddReceiveRtpModule(RtpRtcp* rtp_module) {
+  rtc::CritScope cs(&modules_crit_);
+  RTC_DCHECK(std::find(rtp_receive_modules_.begin(), rtp_receive_modules_.end(),
+                       rtp_module) == rtp_receive_modules_.end());
+  rtp_receive_modules_.push_back(rtp_module);
+}
+
+void PacketRouter::RemoveReceiveRtpModule(RtpRtcp* rtp_module) {
+  rtc::CritScope cs(&modules_crit_);
+  const auto& it = std::find(rtp_receive_modules_.begin(),
+                             rtp_receive_modules_.end(), rtp_module);
+  RTC_DCHECK(it != rtp_receive_modules_.end());
+  rtp_receive_modules_.erase(it);
 }
 
 bool PacketRouter::TimeToSendPacket(uint32_t ssrc,
@@ -53,7 +69,7 @@ bool PacketRouter::TimeToSendPacket(uint32_t ssrc,
                                     const PacedPacketInfo& pacing_info) {
   RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread());
   rtc::CritScope cs(&modules_crit_);
-  for (auto* rtp_module : rtp_modules_) {
+  for (auto* rtp_module : rtp_send_modules_) {
     if (!rtp_module->SendingMedia())
       continue;
     if (ssrc == rtp_module->SSRC() || ssrc == rtp_module->FlexfecSsrc()) {
@@ -71,7 +87,7 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send,
   size_t total_bytes_sent = 0;
   rtc::CritScope cs(&modules_crit_);
   // Rtp modules are ordered by which stream can most benefit from padding.
-  for (RtpRtcp* module : rtp_modules_) {
+  for (RtpRtcp* module : rtp_send_modules_) {
     if (module->SendingMedia() && module->HasBweExtensions()) {
       size_t bytes_sent = module->TimeToSendPadding(
           bytes_to_send - total_bytes_sent, pacing_info);
@@ -106,8 +122,15 @@ uint16_t PacketRouter::AllocateSequenceNumber() {
 }
 
 bool PacketRouter::SendFeedback(rtcp::TransportFeedback* packet) {
+  RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread());
   rtc::CritScope cs(&modules_crit_);
-  for (auto* rtp_module : rtp_modules_) {
+  // Prefer send modules.
+  for (auto* rtp_module : rtp_send_modules_) {
+    packet->SetSenderSsrc(rtp_module->SSRC());
+    if (rtp_module->SendFeedbackPacket(*packet))
+      return true;
+  }
+  for (auto* rtp_module : rtp_receive_modules_) {
     packet->SetSenderSsrc(rtp_module->SSRC());
     if (rtp_module->SendFeedbackPacket(*packet))
       return true;

webrtc/modules/pacing/packet_router.h

@@ -12,6 +12,7 @@
 #define WEBRTC_MODULES_PACING_PACKET_ROUTER_H_
 
 #include <list>
+#include <vector>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/criticalsection.h"
@@ -28,16 +29,29 @@ namespace rtcp {
 class TransportFeedback;
 }  // namespace rtcp
 
-// PacketRouter routes outgoing data to the correct sending RTP module, based
-// on the simulcast layer in RTPVideoHeader.
+// PacketRouter keeps track of rtp send modules to support the pacer.
+// In addition, it handles feedback messages, which are sent on a send
+// module if possible (sender report), otherwise on receive module
+// (receiver report). For the latter case, we also keep track of the
+// receive modules.
 class PacketRouter : public PacedSender::PacketSender,
                      public TransportSequenceNumberAllocator {
  public:
   PacketRouter();
   virtual ~PacketRouter();
 
-  void AddRtpModule(RtpRtcp* rtp_module);
-  void RemoveRtpModule(RtpRtcp* rtp_module);
+  // TODO(nisse): Delete, as soon as downstream app is updated.
+  RTC_DEPRECATED void AddRtpModule(RtpRtcp* rtp_module) {
+    AddReceiveRtpModule(rtp_module);
+  }
+  RTC_DEPRECATED void RemoveRtpModule(RtpRtcp* rtp_module) {
+    RemoveReceiveRtpModule(rtp_module);
+  }
+  void AddSendRtpModule(RtpRtcp* rtp_module);
+  void RemoveSendRtpModule(RtpRtcp* rtp_module);
+
+  void AddReceiveRtpModule(RtpRtcp* rtp_module);
+  void RemoveReceiveRtpModule(RtpRtcp* rtp_module);
 
   // Implements PacedSender::Callback.
   bool TimeToSendPacket(uint32_t ssrc,
@@ -58,7 +72,8 @@ class PacketRouter : public PacedSender::PacketSender,
  private:
   rtc::ThreadChecker pacer_thread_checker_;
   rtc::CriticalSection modules_crit_;
-  std::list<RtpRtcp*> rtp_modules_ GUARDED_BY(modules_crit_);
+  std::list<RtpRtcp*> rtp_send_modules_ GUARDED_BY(modules_crit_);
+  std::vector<RtpRtcp*> rtp_receive_modules_ GUARDED_BY(modules_crit_);
 
   volatile int transport_seq_;
 

webrtc/modules/pacing/packet_router_unittest.cc

@@ -39,8 +39,8 @@ class PacketRouterTest : public ::testing::Test {
 TEST_F(PacketRouterTest, TimeToSendPacket) {
   MockRtpRtcp rtp_1;
   MockRtpRtcp rtp_2;
-  packet_router_->AddRtpModule(&rtp_1);
-  packet_router_->AddRtpModule(&rtp_2);
+  packet_router_->AddSendRtpModule(&rtp_1);
+  packet_router_->AddSendRtpModule(&rtp_2);
 
   const uint16_t kSsrc1 = 1234;
   uint16_t sequence_number = 17;
@@ -98,7 +98,7 @@ TEST_F(PacketRouterTest, TimeToSendPacket) {
       kSsrc1 + kSsrc2, sequence_number, timestamp, retransmission,
       PacedPacketInfo(1, kProbeMinProbes, kProbeMinBytes)));
 
-  packet_router_->RemoveRtpModule(&rtp_1);
+  packet_router_->RemoveSendRtpModule(&rtp_1);
 
   // rtp_1 has been removed, try sending a packet on that ssrc and make sure
   // it is dropped as expected by not expecting any calls to rtp_1.
@@ -110,7 +110,7 @@ TEST_F(PacketRouterTest, TimeToSendPacket) {
       PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes,
                       kProbeMinBytes)));
 
-  packet_router_->RemoveRtpModule(&rtp_2);
+  packet_router_->RemoveSendRtpModule(&rtp_2);
 }
 
 TEST_F(PacketRouterTest, TimeToSendPadding) {
@@ -124,8 +124,8 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
   // rtp_2 will be prioritized for padding.
   EXPECT_CALL(rtp_2, RtxSendStatus()).WillOnce(Return(kRtxRedundantPayloads));
   EXPECT_CALL(rtp_2, SSRC()).WillRepeatedly(Return(kSsrc2));
-  packet_router_->AddRtpModule(&rtp_1);
-  packet_router_->AddRtpModule(&rtp_2);
+  packet_router_->AddSendRtpModule(&rtp_1);
+  packet_router_->AddSendRtpModule(&rtp_2);
 
   // Default configuration, sending padding on all modules sending media,
   // ordered by priority (based on rtx mode).
@@ -191,7 +191,7 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
                 PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes,
                                 kProbeMinBytes)));
 
-  packet_router_->RemoveRtpModule(&rtp_1);
+  packet_router_->RemoveSendRtpModule(&rtp_1);
 
   // rtp_1 has been removed, try sending padding and make sure rtp_1 isn't asked
   // to send by not expecting any calls. Instead verify rtp_2 is called.
@@ -204,12 +204,12 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
                 PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes,
                                 kProbeMinBytes)));
 
-  packet_router_->RemoveRtpModule(&rtp_2);
+  packet_router_->RemoveSendRtpModule(&rtp_2);
 }
 
 TEST_F(PacketRouterTest, SenderOnlyFunctionsRespectSendingMedia) {
   MockRtpRtcp rtp;
-  packet_router_->AddRtpModule(&rtp);
+  packet_router_->AddSendRtpModule(&rtp);
   static const uint16_t kSsrc = 1234;
   EXPECT_CALL(rtp, SSRC()).WillRepeatedly(Return(kSsrc));
   EXPECT_CALL(rtp, SendingMedia()).WillRepeatedly(Return(false));
@@ -226,7 +226,7 @@ TEST_F(PacketRouterTest, SenderOnlyFunctionsRespectSendingMedia) {
                 200, PacedPacketInfo(PacedPacketInfo::kNotAProbe,
                                      kProbeMinBytes, kProbeMinBytes)));
 
-  packet_router_->RemoveRtpModule(&rtp);
+  packet_router_->RemoveSendRtpModule(&rtp);
 }
 
 TEST_F(PacketRouterTest, AllocateSequenceNumbers) {
@@ -245,15 +245,15 @@ TEST_F(PacketRouterTest, AllocateSequenceNumbers) {
 TEST_F(PacketRouterTest, SendFeedback) {
   MockRtpRtcp rtp_1;
   MockRtpRtcp rtp_2;
-  packet_router_->AddRtpModule(&rtp_1);
-  packet_router_->AddRtpModule(&rtp_2);
+  packet_router_->AddSendRtpModule(&rtp_1);
+  packet_router_->AddReceiveRtpModule(&rtp_2);
 
   rtcp::TransportFeedback feedback;
   EXPECT_CALL(rtp_1, SendFeedbackPacket(_)).Times(1);
   packet_router_->SendFeedback(&feedback);
-  packet_router_->RemoveRtpModule(&rtp_1);
+  packet_router_->RemoveSendRtpModule(&rtp_1);
   EXPECT_CALL(rtp_2, SendFeedbackPacket(_)).Times(1);
   packet_router_->SendFeedback(&feedback);
-  packet_router_->RemoveRtpModule(&rtp_2);
+  packet_router_->RemoveReceiveRtpModule(&rtp_2);
 }
 }  // namespace webrtc

webrtc/test/mock_voe_channel_proxy.h

@@ -35,7 +35,8 @@ class MockVoEChannelProxy : public voe::ChannelProxy {
                     RtcpBandwidthObserver* bandwidth_observer));
   MOCK_METHOD1(RegisterReceiverCongestionControlObjects,
                void(PacketRouter* packet_router));
-  MOCK_METHOD0(ResetCongestionControlObjects, void());
+  MOCK_METHOD0(ResetSenderCongestionControlObjects, void());
+  MOCK_METHOD0(ResetReceiverCongestionControlObjects, void());
   MOCK_CONST_METHOD0(GetRTCPStatistics, CallStatistics());
   MOCK_CONST_METHOD0(GetRemoteRTCPReportBlocks, std::vector<ReportBlock>());
   MOCK_CONST_METHOD0(GetNetworkStatistics, NetworkStatistics());

webrtc/video/rtp_stream_receiver.cc

@@ -115,7 +115,7 @@ RtpStreamReceiver::RtpStreamReceiver(
       complete_frame_callback_(complete_frame_callback),
       keyframe_request_sender_(keyframe_request_sender),
       timing_(timing) {
-  packet_router_->AddRtpModule(rtp_rtcp_.get());
+  packet_router_->AddReceiveRtpModule(rtp_rtcp_.get());
   rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy);
   rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
 
@@ -202,7 +202,7 @@ RtpStreamReceiver::~RtpStreamReceiver() {
 
   process_thread_->DeRegisterModule(rtp_rtcp_.get());
 
-  packet_router_->RemoveRtpModule(rtp_rtcp_.get());
+  packet_router_->RemoveReceiveRtpModule(rtp_rtcp_.get());
   rtp_rtcp_->SetREMBStatus(false);
   if (config_.rtp.remb) {
     remb_->RemoveReceiveChannel(rtp_rtcp_.get());

webrtc/video/video_send_stream.cc

@@ -816,7 +816,7 @@ VideoSendStreamImpl::VideoSendStreamImpl(
   // when sending padding, with the hope that the packet rate will be smaller,
   // and that it's more important to protect than the lower layers.
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
-    transport->packet_router()->AddRtpModule(rtp_rtcp);
+    transport->packet_router()->AddSendRtpModule(rtp_rtcp);
 
   for (size_t i = 0; i < config_->rtp.extensions.size(); ++i) {
     const std::string& extension = config_->rtp.extensions[i].uri;
@@ -895,7 +895,7 @@ VideoSendStreamImpl::~VideoSendStreamImpl() {
   remb_->RemoveRembSender(rtp_rtcp_modules_[0]);
 
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
-    transport_->packet_router()->RemoveRtpModule(rtp_rtcp);
+    transport_->packet_router()->RemoveSendRtpModule(rtp_rtcp);
     delete rtp_rtcp;
   }
 }

webrtc/voice_engine/channel.cc

@@ -2441,28 +2441,34 @@ void Channel::RegisterSenderCongestionControlObjects(
   seq_num_allocator_proxy_->SetSequenceNumberAllocator(packet_router);
   rtp_packet_sender_proxy_->SetPacketSender(rtp_packet_sender);
   _rtpRtcpModule->SetStorePacketsStatus(true, 600);
-  packet_router->AddRtpModule(_rtpRtcpModule.get());
+  packet_router->AddSendRtpModule(_rtpRtcpModule.get());
   packet_router_ = packet_router;
 }
 
 void Channel::RegisterReceiverCongestionControlObjects(
     PacketRouter* packet_router) {
   RTC_DCHECK(packet_router && !packet_router_);
-  packet_router->AddRtpModule(_rtpRtcpModule.get());
+  packet_router->AddReceiveRtpModule(_rtpRtcpModule.get());
   packet_router_ = packet_router;
 }
 
-void Channel::ResetCongestionControlObjects() {
+void Channel::ResetSenderCongestionControlObjects() {
   RTC_DCHECK(packet_router_);
   _rtpRtcpModule->SetStorePacketsStatus(false, 600);
   rtcp_observer_->SetBandwidthObserver(nullptr);
   feedback_observer_proxy_->SetTransportFeedbackObserver(nullptr);
   seq_num_allocator_proxy_->SetSequenceNumberAllocator(nullptr);
-  packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
+  packet_router_->RemoveSendRtpModule(_rtpRtcpModule.get());
   packet_router_ = nullptr;
   rtp_packet_sender_proxy_->SetPacketSender(nullptr);
 }
 
+void Channel::ResetReceiverCongestionControlObjects() {
+  RTC_DCHECK(packet_router_);
+  packet_router_->RemoveReceiveRtpModule(_rtpRtcpModule.get());
+  packet_router_ = nullptr;
+}
+
 void Channel::SetRTCPStatus(bool enable) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetRTCPStatus()");

webrtc/voice_engine/channel.h

@@ -282,8 +282,8 @@ class Channel
       RtpTransportControllerSendInterface* transport,
       RtcpBandwidthObserver* bandwidth_observer);
   void RegisterReceiverCongestionControlObjects(PacketRouter* packet_router);
-  void ResetCongestionControlObjects();
-
+  void ResetSenderCongestionControlObjects();
+  void ResetReceiverCongestionControlObjects();
   void SetRTCPStatus(bool enable);
   int GetRTCPStatus(bool& enabled);
   int SetRTCP_CNAME(const char cName[256]);

webrtc/voice_engine/channel_proxy.cc

@@ -90,9 +90,14 @@ void ChannelProxy::RegisterReceiverCongestionControlObjects(
   channel()->RegisterReceiverCongestionControlObjects(packet_router);
 }
 
-void ChannelProxy::ResetCongestionControlObjects() {
+void ChannelProxy::ResetSenderCongestionControlObjects() {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
-  channel()->ResetCongestionControlObjects();
+  channel()->ResetSenderCongestionControlObjects();
+}
+
+void ChannelProxy::ResetReceiverCongestionControlObjects() {
+  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+  channel()->ResetReceiverCongestionControlObjects();
 }
 
 CallStatistics ChannelProxy::GetRTCPStatistics() const {

webrtc/voice_engine/channel_proxy.h

@@ -67,7 +67,8 @@ class ChannelProxy {
       RtcpBandwidthObserver* bandwidth_observer);
   virtual void RegisterReceiverCongestionControlObjects(
       PacketRouter* packet_router);
-  virtual void ResetCongestionControlObjects();
+  virtual void ResetSenderCongestionControlObjects();
+  virtual void ResetReceiverCongestionControlObjects();
   virtual CallStatistics GetRTCPStatistics() const;
   virtual std::vector<ReportBlock> GetRemoteRTCPReportBlocks() const;
   virtual NetworkStatistics GetNetworkStatistics() const;