Move RtpPacketSender and merge it with RtpPacketPacer.

This interface is intended to only handle packet-sending parts of the
paced sender.

See https://webrtc-review.googlesource.com/c/src/+/145212 for context

Bug: webrtc:10809
Change-Id: I93f0b40e1865665c2d436db67021350a0ed0687b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145216
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28662}

audio/audio_send_stream.cc

@@ -332,7 +332,7 @@ void AudioSendStream::Start() {
   if (allocation_settings_.IncludeAudioInAllocationOnStart(
           config_.min_bitrate_bps, config_.max_bitrate_bps, config_.has_dscp,
           TransportSeqNumId(config_))) {
-    rtp_transport_->packet_sender()->SetAccountForAudioPackets(true);
+    rtp_transport_->AccountForAudioPacketsInPacedSender(true);
     rtp_rtcp_module_->SetAsPartOfAllocation(true);
     rtc::Event thread_sync_event;
     worker_queue_->PostTask([&] {
@@ -796,7 +796,7 @@ void AudioSendStream::ReconfigureBitrateObserver(
   if (stream->allocation_settings_.IncludeAudioInAllocationOnReconfigure(
           new_config.min_bitrate_bps, new_config.max_bitrate_bps,
           new_config.has_dscp, TransportSeqNumId(new_config))) {
-    stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(true);
+    stream->rtp_transport_->AccountForAudioPacketsInPacedSender(true);
     rtc::Event thread_sync_event;
     stream->worker_queue_->PostTask([&] {
       RTC_DCHECK_RUN_ON(stream->worker_queue_);
@@ -813,7 +813,7 @@ void AudioSendStream::ReconfigureBitrateObserver(
     thread_sync_event.Wait(rtc::Event::kForever);
     stream->rtp_rtcp_module_->SetAsPartOfAllocation(true);
   } else {
-    stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(false);
+    stream->rtp_transport_->AccountForAudioPacketsInPacedSender(false);
     stream->RemoveBitrateObserver();
     stream->rtp_rtcp_module_->SetAsPartOfAllocation(false);
   }

audio/channel_send.cc

@@ -365,11 +365,11 @@ class TransportSequenceNumberProxy : public TransportSequenceNumberAllocator {
   TransportSequenceNumberAllocator* seq_num_allocator_ RTC_GUARDED_BY(&crit_);
 };
 
-class RtpPacketSenderProxy : public RtpPacketPacer {
+class RtpPacketSenderProxy : public RtpPacketSender {
  public:
   RtpPacketSenderProxy() : rtp_packet_pacer_(nullptr) {}
 
-  void SetPacketPacer(RtpPacketPacer* rtp_packet_pacer) {
+  void SetPacketPacer(RtpPacketSender* rtp_packet_pacer) {
     RTC_DCHECK(thread_checker_.IsCurrent());
     rtc::CritScope lock(&crit_);
     rtp_packet_pacer_ = rtp_packet_pacer;
@@ -394,14 +394,10 @@ class RtpPacketSenderProxy : public RtpPacketPacer {
     }
   }
 
-  void SetAccountForAudioPackets(bool account_for_audio) override {
-    RTC_NOTREACHED();
-  }
-
  private:
   rtc::ThreadChecker thread_checker_;
   rtc::CriticalSection crit_;
-  RtpPacketPacer* rtp_packet_pacer_ RTC_GUARDED_BY(&crit_);
+  RtpPacketSender* rtp_packet_pacer_ RTC_GUARDED_BY(&crit_);
 };
 
 class VoERtcpObserver : public RtcpBandwidthObserver {
@@ -1005,7 +1001,7 @@ void ChannelSend::RegisterSenderCongestionControlObjects(
     RtpTransportControllerSendInterface* transport,
     RtcpBandwidthObserver* bandwidth_observer) {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);
-  RtpPacketPacer* rtp_packet_pacer = transport->packet_sender();
+  RtpPacketSender* rtp_packet_pacer = transport->packet_sender();
   TransportFeedbackObserver* transport_feedback_observer =
       transport->transport_feedback_observer();
   PacketRouter* packet_router = transport->packet_router();

call/call.cc

@@ -503,12 +503,13 @@ Call::~Call() {
     call_stats_->DeregisterStatsObserver(&receive_side_cc_);
   }
 
-  int64_t first_sent_packet_ms = transport_send_->GetFirstPacketTimeMs();
+  absl::optional<int64_t> first_sent_packet_ms =
+      transport_send_->GetFirstPacketTimeMs();
   // Only update histograms after process threads have been shut down, so that
   // they won't try to concurrently update stats.
-  {
+  if (first_sent_packet_ms) {
     rtc::CritScope lock(&bitrate_crit_);
-    UpdateSendHistograms(first_sent_packet_ms);
+    UpdateSendHistograms(*first_sent_packet_ms);
   }
   UpdateReceiveHistograms();
   UpdateHistograms();
@@ -619,8 +620,6 @@ void Call::UpdateHistograms() {
 }
 
 void Call::UpdateSendHistograms(int64_t first_sent_packet_ms) {
-  if (first_sent_packet_ms == -1)
-    return;
   int64_t elapsed_sec =
       (clock_->TimeInMilliseconds() - first_sent_packet_ms) / 1000;
   if (elapsed_sec < metrics::kMinRunTimeInSeconds)

call/rtp_transport_controller_send.cc

@@ -167,7 +167,7 @@ RtpTransportControllerSend::transport_feedback_observer() {
   return this;
 }
 
-RtpPacketPacer* RtpTransportControllerSend::packet_sender() {
+RtpPacketSender* RtpTransportControllerSend::packet_sender() {
   return &pacer_;
 }
 
@@ -398,6 +398,11 @@ void RtpTransportControllerSend::OnTransportOverheadChanged(
   }
 }
 
+void RtpTransportControllerSend::AccountForAudioPacketsInPacedSender(
+    bool account_for_audio) {
+  pacer_.SetAccountForAudioPackets(account_for_audio);
+}
+
 void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) {
   RemoteBitrateReport msg;
   msg.receive_time = Timestamp::ms(clock_->TimeInMilliseconds());

call/rtp_transport_controller_send.h

@@ -76,7 +76,7 @@ class RtpTransportControllerSend final
 
   NetworkStateEstimateObserver* network_state_estimate_observer() override;
   TransportFeedbackObserver* transport_feedback_observer() override;
-  RtpPacketPacer* packet_sender() override;
+  RtpPacketSender* packet_sender() override;
 
   void SetAllocatedSendBitrateLimits(int min_send_bitrate_bps,
                                      int max_padding_bitrate_bps,
@@ -106,6 +106,8 @@ class RtpTransportControllerSend final
   void OnTransportOverheadChanged(
       size_t transport_overhead_per_packet) override;
 
+  void AccountForAudioPacketsInPacedSender(bool account_for_audio) override;
+
   // Implements RtcpBandwidthObserver interface
   void OnReceivedEstimatedBitrate(uint32_t bitrate) override;
   void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks,

call/rtp_transport_controller_send_interface.h

@@ -27,7 +27,7 @@
 #include "logging/rtc_event_log/rtc_event_log.h"
 #include "modules/rtp_rtcp/include/report_block_data.h"
 #include "modules/rtp_rtcp/include/rtcp_statistics.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
 
@@ -119,7 +119,7 @@ class RtpTransportControllerSendInterface {
   virtual NetworkStateEstimateObserver* network_state_estimate_observer() = 0;
   virtual TransportFeedbackObserver* transport_feedback_observer() = 0;
 
-  virtual RtpPacketPacer* packet_sender() = 0;
+  virtual RtpPacketSender* packet_sender() = 0;
 
   // SetAllocatedSendBitrateLimits sets bitrates limits imposed by send codec
   // settings.
@@ -160,6 +160,8 @@ class RtpTransportControllerSendInterface {
 
   virtual void OnTransportOverheadChanged(
       size_t transport_overhead_per_packet) = 0;
+
+  virtual void AccountForAudioPacketsInPacedSender(bool account_for_audio) = 0;
 };
 
 }  // namespace webrtc

call/test/mock_rtp_transport_controller_send.h

@@ -48,7 +48,7 @@ class MockRtpTransportControllerSend
   MOCK_METHOD0(network_state_estimate_observer,
                NetworkStateEstimateObserver*());
   MOCK_METHOD0(transport_feedback_observer, TransportFeedbackObserver*());
-  MOCK_METHOD0(packet_sender, RtpPacketPacer*());
+  MOCK_METHOD0(packet_sender, RtpPacketSender*());
   MOCK_METHOD3(SetAllocatedSendBitrateLimits, void(int, int, int));
   MOCK_METHOD1(SetPacingFactor, void(float));
   MOCK_METHOD1(SetQueueTimeLimit, void(int));
@@ -67,6 +67,7 @@ class MockRtpTransportControllerSend
   MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&));
   MOCK_METHOD1(SetClientBitratePreferences, void(const BitrateSettings&));
   MOCK_METHOD1(OnTransportOverheadChanged, void(size_t));
+  MOCK_METHOD1(AccountForAudioPacketsInPacedSender, void(bool));
   MOCK_METHOD1(OnReceivedPacket, void(const ReceivedPacket&));
 };
 }  // namespace webrtc

modules/pacing/paced_sender.cc

@@ -212,10 +212,10 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority,
 
   RtpPacketToSend::Type type;
   switch (priority) {
-    case RtpPacketPacer::kHighPriority:
+    case RtpPacketSender::kHighPriority:
       type = RtpPacketToSend::Type::kAudio;
       break;
-    case RtpPacketPacer::kNormalPriority:
+    case RtpPacketSender::kNormalPriority:
       type = RtpPacketToSend::Type::kRetransmission;
       break;
     default:

modules/pacing/paced_sender.h

@@ -27,8 +27,7 @@
 #include "modules/pacing/interval_budget.h"
 #include "modules/pacing/packet_router.h"
 #include "modules/pacing/round_robin_packet_queue.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
 #include "modules/utility/include/process_thread.h"
 #include "rtc_base/critical_section.h"
@@ -39,7 +38,7 @@ namespace webrtc {
 class Clock;
 class RtcEventLog;
 
-class PacedSender : public Module, public RtpPacketPacer {
+class PacedSender : public Module, public RtpPacketSender {
  public:
   static constexpr int64_t kNoCongestionWindow = -1;
 
@@ -98,7 +97,7 @@ class PacedSender : public Module, public RtpPacketPacer {
   // With the introduction of audio BWE audio traffic will be accounted for
   // the pacer budget calculation. The audio traffic still will be injected
   // at high priority.
-  void SetAccountForAudioPackets(bool account_for_audio) override;
+  void SetAccountForAudioPackets(bool account_for_audio);
 
   // Returns the time since the oldest queued packet was enqueued.
   virtual int64_t QueueInMs() const;

modules/rtp_rtcp/BUILD.gn

@@ -15,7 +15,7 @@ rtc_source_set("rtp_rtcp_format") {
     "include/rtcp_statistics.h",
     "include/rtp_cvo.h",
     "include/rtp_header_extension_map.h",
-    "include/rtp_packet_pacer.h",
+    "include/rtp_packet_sender.h",
     "include/rtp_rtcp_defines.h",
     "source/byte_io.h",
     "source/rtcp_packet.h",

modules/rtp_rtcp/include/rtp_packet_pacer.h

@@ -1,39 +0,0 @@
-/*
- *  Copyright (c) 2019 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.
- */
-
-#ifndef MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_
-#define MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_
-
-#include <memory>
-
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
-namespace webrtc {
-
-// Interface for a paced sender, as implemented in the pacing module.
-// This intended to replace the RtpPacketSender interface defined in
-// rtp_rtcp_defines.h
-// TODO(bugs.webrtc.org/10633): Add things missing to this interface so that we
-// can use multiple different pacer implementations, and stop inheriting from
-// RtpPacketSender.
-class RtpPacketPacer : public RtpPacketSender {
- public:
-  RtpPacketPacer() = default;
-  ~RtpPacketPacer() override = default;
-
-  // Insert packet into queue, for eventual transmission. Based on the type of
-  // the packet, it will prioritized and scheduled relative to other packets and
-  // the current target send rate.
-  virtual void EnqueuePacket(std::unique_ptr<RtpPacketToSend> packet) = 0;
-};
-
-}  // namespace webrtc
-
-#endif  // MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_PACER_H_

modules/rtp_rtcp/include/rtp_packet_sender.h

@@ -0,0 +1,51 @@
+/*
+ *  Copyright (c) 2019 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.
+ */
+
+#ifndef MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_
+#define MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_
+
+#include <memory>
+
+#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
+
+namespace webrtc {
+
+// TODO(bugs.webrtc.org/10633): Remove Priority and InsertPacket when old pacer
+// code path is gone.
+class RtpPacketSender {
+ public:
+  virtual ~RtpPacketSender() = default;
+
+  // These are part of the legacy PacedSender interface and will be removed.
+  enum Priority {
+    kHighPriority = 0,    // Pass through; will be sent immediately.
+    kNormalPriority = 2,  // Put in back of the line.
+    kLowPriority = 3,     // Put in back of the low priority line.
+  };
+
+  // Adds the packet information to the queue and call TimeToSendPacket when
+  // it's time to send.
+  virtual void InsertPacket(Priority priority,
+                            uint32_t ssrc,
+                            uint16_t sequence_number,
+                            int64_t capture_time_ms,
+                            size_t bytes,
+                            bool retransmission) = 0;
+
+  // Insert packet into queue, for eventual transmission. Based on the type of
+  // the packet, it will be prioritized and scheduled relative to other packets
+  // and the current target send rate.
+  virtual void EnqueuePacket(std::unique_ptr<RtpPacketToSend> packet) = 0;
+};
+
+}  // namespace webrtc
+
+#endif  // MODULES_RTP_RTCP_INCLUDE_RTP_PACKET_SENDER_H_

modules/rtp_rtcp/include/rtp_rtcp.h

@@ -25,7 +25,7 @@
 #include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/receive_statistics.h"
 #include "modules/rtp_rtcp/include/report_block_data.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
 #include "rtc_base/constructor_magic.h"
@@ -89,7 +89,7 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
     RemoteBitrateEstimator* remote_bitrate_estimator = nullptr;
 
     // Spread any bursts of packets into smaller bursts to minimize packet loss.
-    RtpPacketPacer* paced_sender = nullptr;
+    RtpPacketSender* paced_sender = nullptr;
 
     // Generate FlexFEC packets.
     // TODO(brandtr): Remove when FlexfecSender is wired up to PacedSender.

modules/rtp_rtcp/include/rtp_rtcp_defines.h

@@ -340,37 +340,6 @@ class RtcpRttStats {
   virtual ~RtcpRttStats() {}
 };
 
-// This class will be deprecated and replaced with RtpPacketPacer.
-class RtpPacketSender {
- public:
-  RtpPacketSender() {}
-  virtual ~RtpPacketSender() {}
-
-  // These are part of the legacy PacedSender interface and will be removed.
-  enum Priority {
-    kHighPriority = 0,    // Pass through; will be sent immediately.
-    kNormalPriority = 2,  // Put in back of the line.
-    kLowPriority = 3,     // Put in back of the low priority line.
-  };
-
-  // Adds the packet information to the queue and call TimeToSendPacket when
-  // it's time to send.
-  virtual void InsertPacket(Priority priority,
-                            uint32_t ssrc,
-                            uint16_t sequence_number,
-                            int64_t capture_time_ms,
-                            size_t bytes,
-                            bool retransmission) = 0;
-
-  // Currently audio traffic is not accounted by pacer and passed through.
-  // With the introduction of audio BWE audio traffic will be accounted for
-  // the pacer budget calculation. The audio traffic still will be injected
-  // at high priority.
-  // TODO(alexnarest): Make it pure virtual after rtp_sender_unittest will be
-  // updated to support it.
-  virtual void SetAccountForAudioPackets(bool account_for_audio) {}
-};
-
 class TransportSequenceNumberAllocator {
  public:
   TransportSequenceNumberAllocator() {}

modules/rtp_rtcp/source/rtp_sender.cc

@@ -213,7 +213,7 @@ RTPSender::RTPSender(
     bool audio,
     Clock* clock,
     Transport* transport,
-    RtpPacketPacer* paced_sender,
+    RtpPacketSender* paced_sender,
     absl::optional<uint32_t> flexfec_ssrc,
     TransportSequenceNumberAllocator* sequence_number_allocator,
     TransportFeedbackObserver* transport_feedback_observer,

modules/rtp_rtcp/source/rtp_sender.h

@@ -24,7 +24,7 @@
 #include "api/transport/webrtc_key_value_config.h"
 #include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_packet_history.h"
@@ -52,7 +52,7 @@ class RTPSender {
   RTPSender(bool audio,
             Clock* clock,
             Transport* transport,
-            RtpPacketPacer* paced_sender,
+            RtpPacketSender* paced_sender,
             absl::optional<uint32_t> flexfec_ssrc,
             TransportSequenceNumberAllocator* sequence_number_allocator,
             TransportFeedbackObserver* transport_feedback_callback,
@@ -249,7 +249,7 @@ class RTPSender {
 
   const absl::optional<uint32_t> flexfec_ssrc_;
 
-  RtpPacketPacer* const paced_sender_;
+  RtpPacketSender* const paced_sender_;
   TransportSequenceNumberAllocator* const transport_sequence_number_allocator_;
   TransportFeedbackObserver* const transport_feedback_observer_;
   rtc::CriticalSection send_critsect_;

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -22,7 +22,7 @@
 #include "modules/rtp_rtcp/include/rtp_cvo.h"
 #include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
 #include "modules/rtp_rtcp/include/rtp_header_parser.h"
-#include "modules/rtp_rtcp/include/rtp_packet_pacer.h"
+#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "modules/rtp_rtcp/source/rtp_format_video_generic.h"
@@ -166,7 +166,7 @@ std::string ToFieldTrialString(TestConfig config) {
 
 }  // namespace
 
-class MockRtpPacketPacer : public RtpPacketPacer {
+class MockRtpPacketPacer : public RtpPacketSender {
  public:
   MockRtpPacketPacer() {}
   virtual ~MockRtpPacketPacer() {}
@@ -180,6 +180,15 @@ class MockRtpPacketPacer : public RtpPacketPacer {
                     int64_t capture_time_ms,
                     size_t bytes,
                     bool retransmission));
+
+  MOCK_METHOD2(CreateProbeCluster, void(int bitrate_bps, int cluster_id));
+
+  MOCK_METHOD0(Pause, void());
+  MOCK_METHOD0(Resume, void());
+  MOCK_METHOD1(SetCongestionWindow,
+               void(absl::optional<int64_t> congestion_window_bytes));
+  MOCK_METHOD1(UpdateOutstandingData, void(int64_t outstanding_bytes));
+  MOCK_METHOD1(SetAccountForAudioPackets, void(bool account_for_audio));
 };
 
 class MockTransportSequenceNumberAllocator