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

webrtc/modules/pacing/packet_router.cc

 /*
  *  Copyright (c) 2015 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.
  */
 
 #include "webrtc/modules/pacing/packet_router.h"
 
 #include "webrtc/base/atomicops.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 
 namespace webrtc {
 
+namespace {
+void AddModule(RtpRtcp* rtp_module, std::list<RtpRtcp*>* rtp_modules) {
+  RTC_DCHECK(std::find(rtp_modules->begin(), rtp_modules->end(), rtp_module) ==
+             rtp_modules->end());
+  rtp_modules->push_back(rtp_module);
+}
+
+void RemoveModule(RtpRtcp* rtp_module, std::list<RtpRtcp*>* rtp_modules) {
+  RTC_DCHECK(std::find(rtp_modules->begin(), rtp_modules->end(), rtp_module) !=
+             rtp_modules->end());
+  rtp_modules->remove(rtp_module);
+}
+
+bool SendFeedback(rtcp::TransportFeedback* packet,
+                  std::list<RtpRtcp*>* rtp_modules) {
+  for (auto* rtp_module : *rtp_modules) {
+    packet->WithPacketSenderSsrc(rtp_module->SSRC());
+    if (rtp_module->SendFeedbackPacket(*packet))
+      return true;
+  }
+  return false;
+}
+}
+
 PacketRouter::PacketRouter() : transport_seq_(0) {
 }
 
 PacketRouter::~PacketRouter() {
-  RTC_DCHECK(rtp_modules_.empty());
+  RTC_DCHECK(send_rtp_modules_.empty());
+  RTC_DCHECK(recv_rtp_modules_.empty());
 }
 
-void PacketRouter::AddRtpModule(RtpRtcp* rtp_module) {
-  rtc::CritScope cs(&modules_lock_);
-  RTC_DCHECK(std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module) ==
-             rtp_modules_.end());
-  rtp_modules_.push_back(rtp_module);
+void PacketRouter::AddRtpModule(RtpRtcp* rtp_module, bool sender) {
+  if (sender) {
+    rtc::CritScope cs(&send_modules_crit_);
+    AddModule(rtp_module, &send_rtp_modules_);
+  } else {
+    rtc::CritScope cs(&recv_modules_crit_);
+    AddModule(rtp_module, &recv_rtp_modules_);
+  }
 }
 
-void PacketRouter::RemoveRtpModule(RtpRtcp* rtp_module) {
-  rtc::CritScope cs(&modules_lock_);
-  auto it = std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module);
-  RTC_DCHECK(it != rtp_modules_.end());
-  rtp_modules_.erase(it);
+void PacketRouter::RemoveRtpModule(RtpRtcp* rtp_module, bool sender) {
+  if (sender) {
+    rtc::CritScope cs(&send_modules_crit_);
+    RemoveModule(rtp_module, &send_rtp_modules_);
+  } else {
+    rtc::CritScope cs(&recv_modules_crit_);
+    RemoveModule(rtp_module, &recv_rtp_modules_);
+  }
 }
 
 bool PacketRouter::TimeToSendPacket(uint32_t ssrc,
                                     uint16_t sequence_number,
                                     int64_t capture_timestamp,
                                     bool retransmission) {
-  rtc::CritScope cs(&modules_lock_);
-  for (auto* rtp_module : rtp_modules_) {
+  rtc::CritScope cs(&send_modules_crit_);
+  for (auto* rtp_module : send_rtp_modules_) {
     if (rtp_module->SendingMedia() && ssrc == rtp_module->SSRC()) {
       return rtp_module->TimeToSendPacket(ssrc, sequence_number,
                                           capture_timestamp, retransmission);
     }
   }
   return true;
 }
 
 size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send) {
   size_t total_bytes_sent = 0;
-  rtc::CritScope cs(&modules_lock_);
-  for (RtpRtcp* module : rtp_modules_) {
+  rtc::CritScope cs(&send_modules_crit_);
+  for (RtpRtcp* module : send_rtp_modules_) {
     if (module->SendingMedia()) {
       size_t bytes_sent =
           module->TimeToSendPadding(bytes_to_send - total_bytes_sent);
       total_bytes_sent += bytes_sent;
       if (total_bytes_sent >= bytes_to_send)
         break;
     }
   }
   return total_bytes_sent;
 }
@@ skipping 14 matching lines @@
     // operation was successful - otherwise we need to retry. Saving the
     // return value saves us a load on retry.
     prev_seq = rtc::AtomicOps::CompareAndSwap(&transport_seq_, desired_prev_seq,
                                               new_seq);
   } while (prev_seq != desired_prev_seq);
 
   return new_seq;
 }
 
 bool PacketRouter::SendFeedback(rtcp::TransportFeedback* packet) {
-  rtc::CritScope cs(&modules_lock_);
-  for (auto* rtp_module : rtp_modules_) {
-    packet->WithPacketSenderSsrc(rtp_module->SSRC());
-    if (rtp_module->SendFeedbackPacket(*packet))
+  {
+    rtc::CritScope cs(&recv_modules_crit_);
+    if (::webrtc::SendFeedback(packet, &recv_rtp_modules_))

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

I don't really follow this. Doesn't it make any difference which transport
feedback packets go out on?

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

No, since the feedback message is covering the whole call. You could say that we
shouldn't send feedback on an SSRC which hasn't been configured with transport
cc feedback, but we leave that decision to the rtp module.

In the future we will may add support for having separate BWE (and therefore
also feedback messages) for different transports (e.g., the non-BUNDLE case),
but that is not something we'll support in this first step.

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

So when would sending an RTCP packet fail? Won't we always use the first of the
available modules?

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

Most of the time, yes. But it could be that in the future the first module
doesn't support cc feedback, or that the first module isn't currently enabled
for sending packets.

+      return true;
+  }
+  {
+    rtc::CritScope cs(&send_modules_crit_);
+    if (::webrtc::SendFeedback(packet, &send_rtp_modules_))
       return true;
   }
   return false;
 }
 
 }  // namespace webrtc