Lets PacingController call PacketRouter directly.

Since locking model has been cleaned up, PacingController can now call
PacketRouter directly - without having to go via PacedSender or
TaskQueuePacedSender.

Bug: webrtc:10809
Change-Id: I181f04167d677c35395286f8b246aefb4c3e7ec7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175909
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31342}
diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc
index 1d02fe9..88effe4 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -28,7 +28,8 @@
 const int64_t PacedSender::kMaxQueueLengthMs = 2000;
 const float PacedSender::kDefaultPaceMultiplier = 2.5f;
 
-PacedSender::PacedSender(Clock* clock, PacketRouter* packet_router,
+PacedSender::PacedSender(Clock* clock,
+                         PacketRouter* packet_router,
                          RtcEventLog* event_log,
                          const WebRtcKeyValueConfig* field_trials,
                          ProcessThread* process_thread)
@@ -39,10 +40,11 @@
               ? PacingController::ProcessMode::kDynamic
               : PacingController::ProcessMode::kPeriodic),
       pacing_controller_(clock,
-                         static_cast<PacingController::PacketSender*>(this),
-                         event_log, field_trials, process_mode_),
+                         packet_router,
+                         event_log,
+                         field_trials,
+                         process_mode_),
       clock_(clock),
-      packet_router_(packet_router),
       process_thread_(process_thread) {
   if (process_thread_)
     process_thread_->RegisterModule(&module_proxy_, RTC_FROM_HERE);
@@ -194,13 +196,4 @@
   MaybeWakupProcessThread();
 }
 
-void PacedSender::SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                                const PacedPacketInfo& cluster_info) {
-  packet_router_->SendPacket(std::move(packet), cluster_info);
-}
-
-std::vector<std::unique_ptr<RtpPacketToSend>> PacedSender::GeneratePadding(
-    DataSize size) {
-  return packet_router_->GeneratePadding(size.bytes());
-}
 }  // namespace webrtc
diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h
index 16137df..cc83b40 100644
--- a/modules/pacing/paced_sender.h
+++ b/modules/pacing/paced_sender.h
@@ -43,8 +43,7 @@
 // updating dependencies.
 class PacedSender : public Module,
                     public RtpPacketPacer,
-                    public RtpPacketSender,
-                    private PacingController::PacketSender {
+                    public RtpPacketSender {
  public:
   // Expected max pacer delay in ms. If ExpectedQueueTime() is higher than
   // this value, the packet producers should wait (eg drop frames rather than
@@ -140,14 +139,6 @@
   // In dynamic process mode, refreshes the next process time.
   void MaybeWakupProcessThread();
 
-  // Methods implementing PacedSenderController:PacketSender.
-  void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                     const PacedPacketInfo& cluster_info) override
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
-
-  std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
-      DataSize size) override RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
-
   // Private implementation of Module to not expose those implementation details
   // publicly and control when the class is registered/deregistered.
   class ModuleProxy : public Module {
@@ -171,7 +162,6 @@
   PacingController pacing_controller_ RTC_GUARDED_BY(critsect_);
 
   Clock* const clock_;
-  PacketRouter* const packet_router_;
   ProcessThread* const process_thread_;
 };
 }  // namespace webrtc
diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc
index 26d2eac..eaee276 100644
--- a/modules/pacing/paced_sender_unittest.cc
+++ b/modules/pacing/paced_sender_unittest.cc
@@ -44,7 +44,7 @@
                     const PacedPacketInfo& cluster_info));
   MOCK_METHOD1(
       GeneratePadding,
-      std::vector<std::unique_ptr<RtpPacketToSend>>(size_t target_size_bytes));
+      std::vector<std::unique_ptr<RtpPacketToSend>>(DataSize target_size));
 };
 
 class ProcessModeTrials : public WebRtcKeyValueConfig {
diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index b1f6e89..77f21be 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -438,7 +438,7 @@
       for (auto& packet : keepalive_packets) {
         keepalive_data_sent +=
             DataSize::Bytes(packet->payload_size() + packet->padding_size());
-        packet_sender_->SendRtpPacket(std::move(packet), PacedPacketInfo());
+        packet_sender_->SendPacket(std::move(packet), PacedPacketInfo());
       }
       OnPaddingSent(keepalive_data_sent);
     }
@@ -557,7 +557,7 @@
       packet_size += DataSize::Bytes(rtp_packet->headers_size()) +
                      transport_overhead_per_packet_;
     }
-    packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info);
+    packet_sender_->SendPacket(std::move(rtp_packet), pacing_info);
 
     data_sent += packet_size;
 
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index 20d2539..6e361ae 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -55,8 +55,8 @@
   class PacketSender {
    public:
     virtual ~PacketSender() = default;
-    virtual void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                               const PacedPacketInfo& cluster_info) = 0;
+    virtual void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                            const PacedPacketInfo& cluster_info) = 0;
     virtual std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
         DataSize size) = 0;
   };
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index fa23da7..6d6e6e2 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -69,8 +69,8 @@
 // methods that focus on core aspects.
 class MockPacingControllerCallback : public PacingController::PacketSender {
  public:
-  void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                     const PacedPacketInfo& cluster_info) override {
+  void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                  const PacedPacketInfo& cluster_info) override {
     SendPacket(packet->Ssrc(), packet->SequenceNumber(),
                packet->capture_time_ms(),
                packet->packet_type() == RtpPacketMediaType::kRetransmission,
@@ -102,7 +102,7 @@
 // Mock callback implementing the raw api.
 class MockPacketSender : public PacingController::PacketSender {
  public:
-  MOCK_METHOD2(SendRtpPacket,
+  MOCK_METHOD2(SendPacket,
                void(std::unique_ptr<RtpPacketToSend> packet,
                     const PacedPacketInfo& cluster_info));
   MOCK_METHOD1(
@@ -116,8 +116,8 @@
 
   PacingControllerPadding() : padding_sent_(0), total_bytes_sent_(0) {}
 
-  void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                     const PacedPacketInfo& pacing_info) override {
+  void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                  const PacedPacketInfo& pacing_info) override {
     total_bytes_sent_ += packet->payload_size();
   }
 
@@ -147,8 +147,8 @@
  public:
   PacingControllerProbing() : packets_sent_(0), padding_sent_(0) {}
 
-  void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                     const PacedPacketInfo& pacing_info) override {
+  void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                  const PacedPacketInfo& pacing_info) override {
     if (packet->packet_type() != RtpPacketMediaType::kPadding) {
       ++packets_sent_;
     }
@@ -1571,7 +1571,7 @@
 
   // First probing cluster.
   EXPECT_CALL(callback,
-              SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 0)))
+              SendPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 0)))
       .Times(5);
 
   for (int i = 0; i < 5; ++i) {
@@ -1580,7 +1580,7 @@
 
   // Second probing cluster.
   EXPECT_CALL(callback,
-              SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 1)))
+              SendPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 1)))
       .Times(5);
 
   for (int i = 0; i < 5; ++i) {
@@ -1598,7 +1598,7 @@
     return padding_packets;
   });
   bool non_probe_packet_seen = false;
-  EXPECT_CALL(callback, SendRtpPacket)
+  EXPECT_CALL(callback, SendPacket)
       .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
                     const PacedPacketInfo& cluster_info) {
         EXPECT_EQ(cluster_info.probe_cluster_id, kNotAProbe);
@@ -1628,23 +1628,23 @@
   ::testing::InSequence seq;
   EXPECT_CALL(
       callback,
-      SendRtpPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kAudioSsrc)), _));
-  EXPECT_CALL(callback,
-              SendRtpPacket(
-                  Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));
+      SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kAudioSsrc)), _));
+  EXPECT_CALL(
+      callback,
+      SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));
 
   // FEC and video actually have the same priority, so will come out in
   // insertion order.
-  EXPECT_CALL(callback,
-              SendRtpPacket(
-                  Pointee(Property(&RtpPacketToSend::Ssrc, kFlexFecSsrc)), _));
   EXPECT_CALL(
       callback,
-      SendRtpPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoSsrc)), _));
+      SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kFlexFecSsrc)), _));
+  EXPECT_CALL(
+      callback,
+      SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoSsrc)), _));
 
-  EXPECT_CALL(callback,
-              SendRtpPacket(
-                  Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));
+  EXPECT_CALL(
+      callback,
+      SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));
 
   while (pacer_->QueueSizePackets() > 0) {
     if (PeriodicProcess()) {
@@ -1679,7 +1679,7 @@
 
   size_t packets_sent = 0;
   bool media_seen = false;
-  EXPECT_CALL(callback, SendRtpPacket)
+  EXPECT_CALL(callback, SendPacket)
       .Times(::testing::AnyNumber())
       .WillRepeatedly([&](std::unique_ptr<RtpPacketToSend> packet,
                           const PacedPacketInfo& cluster_info) {
@@ -1817,7 +1817,7 @@
   for (bool account_for_audio : {false, true}) {
     uint16_t sequence_number = 1234;
     MockPacketSender callback;
-    EXPECT_CALL(callback, SendRtpPacket).Times(::testing::AnyNumber());
+    EXPECT_CALL(callback, SendPacket).Times(::testing::AnyNumber());
     pacer_ = std::make_unique<PacingController>(&clock_, &callback, nullptr,
                                                 nullptr, GetParam());
     pacer_->SetAccountForAudioPackets(account_for_audio);
diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc
index fa64331..e9e8d4b 100644
--- a/modules/pacing/packet_router.cc
+++ b/modules/pacing/packet_router.cc
@@ -167,7 +167,7 @@
 }
 
 std::vector<std::unique_ptr<RtpPacketToSend>> PacketRouter::GeneratePadding(
-    size_t target_size_bytes) {
+    DataSize size) {
   rtc::CritScope cs(&modules_crit_);
   // First try on the last rtp module to have sent media. This increases the
   // the chance that any payload based padding will be useful as it will be
@@ -178,7 +178,7 @@
   std::vector<std::unique_ptr<RtpPacketToSend>> padding_packets;
   if (last_send_module_ != nullptr &&
       last_send_module_->SupportsRtxPayloadPadding()) {
-    padding_packets = last_send_module_->GeneratePadding(target_size_bytes);
+    padding_packets = last_send_module_->GeneratePadding(size.bytes());
     if (!padding_packets.empty()) {
       return padding_packets;
     }
@@ -189,7 +189,7 @@
   // be taken into account by the bandwidth estimator, e.g. in FF.
   for (RtpRtcp* rtp_module : send_modules_list_) {
     if (rtp_module->SupportsPadding()) {
-      padding_packets = rtp_module->GeneratePadding(target_size_bytes);
+      padding_packets = rtp_module->GeneratePadding(size.bytes());
       if (!padding_packets.empty()) {
         last_send_module_ = rtp_module;
         break;
diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h
index 40b3ad1..2c03e96 100644
--- a/modules/pacing/packet_router.h
+++ b/modules/pacing/packet_router.h
@@ -21,6 +21,7 @@
 #include <vector>
 
 #include "api/transport/network_types.h"
+#include "modules/pacing/pacing_controller.h"
 #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtcp_packet.h"
@@ -39,7 +40,8 @@
 // (receiver report). For the latter case, we also keep track of the
 // receive modules.
 class PacketRouter : public RemoteBitrateObserver,
-                     public TransportFeedbackSenderInterface {
+                     public TransportFeedbackSenderInterface,
+                     public PacingController::PacketSender {
  public:
   PacketRouter();
   explicit PacketRouter(uint16_t start_transport_seq);
@@ -52,11 +54,10 @@
                            bool remb_candidate);
   void RemoveReceiveRtpModule(RtcpFeedbackSenderInterface* rtcp_sender);
 
-  virtual void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
-                          const PacedPacketInfo& cluster_info);
-
-  virtual std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
-      size_t target_size_bytes);
+  void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                  const PacedPacketInfo& cluster_info) override;
+  std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
+      DataSize size) override;
 
   uint16_t CurrentTransportSequenceNumber() const;
 
diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc
index b8f16cb..75729cb 100644
--- a/modules/pacing/packet_router_unittest.cc
+++ b/modules/pacing/packet_router_unittest.cc
@@ -68,7 +68,7 @@
 };
 
 TEST_F(PacketRouterTest, Sanity_NoModuleRegistered_GeneratePadding) {
-  constexpr size_t bytes = 300;
+  constexpr DataSize bytes = DataSize::Bytes(300);
   const PacedPacketInfo paced_info(1, kProbeMinProbes, kProbeMinBytes);
 
   EXPECT_TRUE(packet_router_.GeneratePadding(bytes).empty());
@@ -122,7 +122,8 @@
         return std::vector<std::unique_ptr<RtpPacketToSend>>(
             kExpectedPaddingPackets);
       });
-  auto generated_padding = packet_router_.GeneratePadding(kPaddingSize);
+  auto generated_padding =
+      packet_router_.GeneratePadding(DataSize::Bytes(kPaddingSize));
   EXPECT_EQ(generated_padding.size(), kExpectedPaddingPackets);
 
   packet_router_.RemoveSendRtpModule(&rtp_1);
@@ -159,7 +160,7 @@
   packet_router_.AddSendRtpModule(&audio_module, false);
   EXPECT_CALL(audio_module, GeneratePadding(kPaddingSize))
       .WillOnce(generate_padding);
-  packet_router_.GeneratePadding(kPaddingSize);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingSize));
 
   // Add the video module, this should now be prioritized since we cannot
   // guarantee that audio packets will be included in the BWE.
@@ -167,7 +168,7 @@
   EXPECT_CALL(audio_module, GeneratePadding).Times(0);
   EXPECT_CALL(video_module, GeneratePadding(kPaddingSize))
       .WillOnce(generate_padding);
-  packet_router_.GeneratePadding(kPaddingSize);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingSize));
 
   // Remove and the add audio module again. Module order shouldn't matter;
   // video should still be prioritized.
@@ -176,14 +177,14 @@
   EXPECT_CALL(audio_module, GeneratePadding).Times(0);
   EXPECT_CALL(video_module, GeneratePadding(kPaddingSize))
       .WillOnce(generate_padding);
-  packet_router_.GeneratePadding(kPaddingSize);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingSize));
 
   // Remove and the video module, we should fall back to padding on the
   // audio module again.
   packet_router_.RemoveSendRtpModule(&video_module);
   EXPECT_CALL(audio_module, GeneratePadding(kPaddingSize))
       .WillOnce(generate_padding);
-  packet_router_.GeneratePadding(kPaddingSize);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingSize));
 
   packet_router_.RemoveSendRtpModule(&audio_module);
 }
@@ -243,7 +244,7 @@
         packets.push_back(BuildRtpPacket(kSsrc2));
         return packets;
       });
-  packet_router_.GeneratePadding(kPaddingBytes);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingBytes));
 
   // Send media on first module. Padding should be sent on that module.
   packet_router_.SendPacket(BuildRtpPacket(kSsrc1), PacedPacketInfo());
@@ -255,7 +256,7 @@
         packets.push_back(BuildRtpPacket(kSsrc1));
         return packets;
       });
-  packet_router_.GeneratePadding(kPaddingBytes);
+  packet_router_.GeneratePadding(DataSize::Bytes(kPaddingBytes));
 
   // Send media on second module. Padding should be sent there.
   packet_router_.SendPacket(BuildRtpPacket(kSsrc2), PacedPacketInfo());
@@ -285,7 +286,7 @@
 
   for (int i = 0; i < 2; ++i) {
     last_send_module = nullptr;
-    packet_router_.GeneratePadding(kPaddingBytes);
+    packet_router_.GeneratePadding(DataSize::Bytes(kPaddingBytes));
     EXPECT_NE(last_send_module, nullptr);
     packet_router_.RemoveSendRtpModule(last_send_module);
   }
diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc
index d460d60..41eebea 100644
--- a/modules/pacing/task_queue_paced_sender.cc
+++ b/modules/pacing/task_queue_paced_sender.cc
@@ -38,9 +38,8 @@
     TimeDelta hold_back_window)
     : clock_(clock),
       hold_back_window_(hold_back_window),
-      packet_router_(packet_router),
       pacing_controller_(clock,
-                         static_cast<PacingController::PacketSender*>(this),
+                         packet_router,
                          event_log,
                          field_trials,
                          PacingController::ProcessMode::kDynamic),
@@ -221,17 +220,6 @@
   MaybeUpdateStats(false);
 }
 
-std::vector<std::unique_ptr<RtpPacketToSend>>
-TaskQueuePacedSender::GeneratePadding(DataSize size) {
-  return packet_router_->GeneratePadding(size.bytes());
-}
-
-void TaskQueuePacedSender::SendRtpPacket(
-    std::unique_ptr<RtpPacketToSend> packet,
-    const PacedPacketInfo& cluster_info) {
-  packet_router_->SendPacket(std::move(packet), cluster_info);
-}
-
 void TaskQueuePacedSender::MaybeUpdateStats(bool is_scheduled_call) {
   if (is_shutdown_) {
     return;
diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h
index 3241d3f..5e6a177 100644
--- a/modules/pacing/task_queue_paced_sender.h
+++ b/modules/pacing/task_queue_paced_sender.h
@@ -38,9 +38,7 @@
 class Clock;
 class RtcEventLog;
 
-class TaskQueuePacedSender : public RtpPacketPacer,
-                             public RtpPacketSender,
-                             private PacingController::PacketSender {
+class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender {
  public:
   // The |hold_back_window| parameter sets a lower bound on time to sleep if
   // there is currently a pacer queue and packets can't immediately be
@@ -125,21 +123,11 @@
   // method again with desired (finite) scheduled process time.
   void MaybeProcessPackets(Timestamp scheduled_process_time);
 
-  // Methods implementing PacedSenderController:PacketSender.
-
-  void SendRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
-                     const PacedPacketInfo& cluster_info) override
-      RTC_RUN_ON(task_queue_);
-
-  std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
-      DataSize size) override RTC_RUN_ON(task_queue_);
-
   void MaybeUpdateStats(bool is_scheduled_call) RTC_RUN_ON(task_queue_);
   Stats GetStats() const;
 
   Clock* const clock_;
   const TimeDelta hold_back_window_;
-  PacketRouter* const packet_router_ RTC_GUARDED_BY(task_queue_);
   PacingController pacing_controller_ RTC_GUARDED_BY(task_queue_);
 
   // We want only one (valid) delayed process task in flight at a time.
diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc
index e93f776..891936d 100644
--- a/modules/pacing/task_queue_paced_sender_unittest.cc
+++ b/modules/pacing/task_queue_paced_sender_unittest.cc
@@ -43,7 +43,7 @@
                     const PacedPacketInfo& cluster_info));
   MOCK_METHOD1(
       GeneratePadding,
-      std::vector<std::unique_ptr<RtpPacketToSend>>(size_t target_size_bytes));
+      std::vector<std::unique_ptr<RtpPacketToSend>>(DataSize target_size));
 };
 }  // namespace