From 8088aad5ac0154d8fdc252de9e8ab4e0172d7da2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= <sprang@webrtc.org>
Date: Thu, 21 Apr 2022 11:13:38 +0200
Subject: [PATCH] Send first probe packet directly instead of enqueuing it.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This avoids potentially creating needless containers in the packet
queue and removes usage of the packet prio, allowing it to be moved in
an upcoming CL.

Bug: webrtc:11340
Change-Id: Iddd9e7e4e73c97ab25a85e42bcc0094d61fd60d3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259524
Reviewed-by: Emil Lundmark <lndmrk@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36602}
---
 modules/pacing/pacing_controller.cc | 63 ++++++++++++++++-------------
 modules/pacing/pacing_controller.h  |  3 ++
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 5ef10db04f..3593b34169 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -456,6 +456,7 @@ void PacingController::ProcessPackets() {
 
   PacedPacketInfo pacing_info;
   DataSize recommended_probe_size = DataSize::Zero();
+  DataSize data_sent = DataSize::Zero();
   bool is_probing = prober_.is_probing();
   if (is_probing) {
     // Probe timing is sensitive, and handled explicitly by BitrateProber, so
@@ -472,8 +473,8 @@ void PacingController::ProcessPackets() {
         // If no RTP modules sending media are registered, we may not get a
         // padding packet back.
         if (!padding.empty()) {
-          // Insert with high priority so larger media packets don't preempt it.
-          EnqueuePacketInternal(std::move(padding[0]), kFirstPriority);
+          // Send packet immediately to avoid priority inversions.
+          data_sent += SendPacket(std::move(padding[0]), pacing_info, now);
           // We should never get more than one padding packets with a requested
           // size of 1 byte.
           RTC_DCHECK_EQ(padding.size(), 1u);
@@ -485,13 +486,18 @@ void PacingController::ProcessPackets() {
     }
   }
 
-  DataSize data_sent = DataSize::Zero();
   // Circuit breaker, making sure main loop isn't forever.
   static constexpr int kMaxIterations = 1 << 16;
   int iteration = 0;
   int packets_sent = 0;
   int padding_packets_generated = 0;
   for (; iteration < kMaxIterations; ++iteration) {
+    // If we are currently probing, we need to stop the send loop when we have
+    // reached the send target.
+    if (is_probing && data_sent >= recommended_probe_size) {
+      break;
+    }
+
     // Fetch packet, so long as queue is not empty or budget is not
     // exhausted.
     std::unique_ptr<RtpPacketToSend> rtp_packet =
@@ -518,33 +524,9 @@ void PacingController::ProcessPackets() {
       // Can't fetch new packet and no padding to send, exit send loop.
       break;
     } else {
-      RTC_DCHECK(rtp_packet);
-      RTC_DCHECK(rtp_packet->packet_type().has_value());
-      const RtpPacketMediaType packet_type = *rtp_packet->packet_type();
-      DataSize packet_size = DataSize::Bytes(rtp_packet->payload_size() +
-                                             rtp_packet->padding_size());
-
-      if (include_overhead_) {
-        packet_size += DataSize::Bytes(rtp_packet->headers_size()) +
-                       transport_overhead_per_packet_;
-      }
-
-      packet_sender_->SendPacket(std::move(rtp_packet), pacing_info);
-      for (auto& packet : packet_sender_->FetchFec()) {
-        EnqueuePacket(std::move(packet));
-      }
-      data_sent += packet_size;
+      data_sent += SendPacket(std::move(rtp_packet), pacing_info, now);
       ++packets_sent;
 
-      // Send done, update send time.
-      OnPacketSent(packet_type, packet_size, now);
-
-      // If we are currently probing, we need to stop the send loop when we
-      // have reached the send target.
-      if (is_probing && data_sent >= recommended_probe_size) {
-        break;
-      }
-
       // Update target send time in case that are more packets that we are late
       // in processing.
       if (mode_ == ProcessMode::kDynamic) {
@@ -658,6 +640,31 @@ std::unique_ptr<RtpPacketToSend> PacingController::GetPendingPacket(
   return packet_queue_.Pop();
 }
 
+DataSize PacingController::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                                      const PacedPacketInfo& pacing_info,
+                                      Timestamp now) {
+  RTC_DCHECK(packet);
+  RTC_DCHECK(packet->packet_type().has_value());
+  const RtpPacketMediaType packet_type = *packet->packet_type();
+  DataSize packet_size =
+      DataSize::Bytes(packet->payload_size() + packet->padding_size());
+
+  if (include_overhead_) {
+    packet_size += DataSize::Bytes(packet->headers_size()) +
+                   transport_overhead_per_packet_;
+  }
+
+  packet_sender_->SendPacket(std::move(packet), pacing_info);
+  for (std::unique_ptr<RtpPacketToSend>& packet : packet_sender_->FetchFec()) {
+    EnqueuePacket(std::move(packet));
+  }
+
+  // Sending complete, update send time.
+  OnPacketSent(packet_type, packet_size, now);
+
+  return packet_size;
+}
+
 void PacingController::OnPacketSent(RtpPacketMediaType packet_type,
                                     DataSize packet_size,
                                     Timestamp send_time) {
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index c3e1dde2fb..7c5b4a6d28 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -167,6 +167,9 @@ class PacingController {
       const PacedPacketInfo& pacing_info,
       Timestamp target_send_time,
       Timestamp now);
+  DataSize SendPacket(std::unique_ptr<RtpPacketToSend> packet,
+                      const PacedPacketInfo& pacing_info,
+                      Timestamp now);
   void OnPacketSent(RtpPacketMediaType packet_type,
                     DataSize packet_size,
                     Timestamp send_time);