Add possibility to start probe without waiting for media packet

Bug: webrtc:14928
Change-Id: If3c7fe39979e7a4d45b01dd59debd8ac69de3d01
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294521
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39496}

modules/pacing/bitrate_prober.cc

@@ -15,6 +15,7 @@
 #include "absl/memory/memory.h"
 #include "api/rtc_event_log/rtc_event.h"
 #include "api/rtc_event_log/rtc_event_log.h"
+#include "api/units/data_size.h"
 #include "logging/rtc_event_log/events/rtc_event_probe_cluster_created.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
@@ -92,15 +93,30 @@ void BitrateProber::CreateProbeCluster(
   cluster.pace_info.probe_cluster_id = cluster_config.id;
   clusters_.push(cluster);
 
-  RTC_LOG(LS_INFO) << "Probe cluster (bitrate:min bytes:min packets): ("
+  if (config_.min_packet_size.Get() > DataSize::Zero()) {
+    // If we are already probing, continue to do so. Otherwise set it to
+    // kInactive and wait for OnIncomingPacket with a large enough packet
+    // to start the probing.
+    if (probing_state_ != ProbingState::kActive) {
+      probing_state_ = ProbingState::kInactive;
+    }
+  } else {
+    // Probing is allowed to start without first sending a "large enough"
+    // packet.
+    if (probing_state_ == ProbingState::kInactive) {
+      // Send next probe right away.
+      next_probe_time_ = Timestamp::MinusInfinity();
+      probing_state_ = ProbingState::kActive;
+    }
+  }
+
+  RTC_LOG(LS_INFO) << "Probe cluster (bitrate_bps:min bytes:min packets): ("
                    << cluster.pace_info.send_bitrate_bps << ":"
                    << cluster.pace_info.probe_cluster_min_bytes << ":"
-                   << cluster.pace_info.probe_cluster_min_probes << ")";
-
-  // If we are already probing, continue to do so. Otherwise set it to
-  // kInactive and wait for OnIncomingPacket to start the probing.
-  if (probing_state_ != ProbingState::kActive)
-    probing_state_ = ProbingState::kInactive;
+                   << cluster.pace_info.probe_cluster_min_probes << ", "
+                   << (probing_state_ == ProbingState::kInactive ? "Inactive"
+                                                                 : "Active")
+                   << ")";
 }
 
 Timestamp BitrateProber::NextProbeTime(Timestamp now) const {

modules/pacing/pacing_controller.cc

@@ -553,12 +553,6 @@ DataSize PacingController::PaddingToAdd(DataSize recommended_probe_size,
     return DataSize::Zero();
   }
 
-  if (!seen_first_packet_) {
-    // We can not send padding unless a normal packet has first been sent. If
-    // we do, timestamps get messed up.
-    return DataSize::Zero();
-  }
-
   if (!recommended_probe_size.IsZero()) {
     if (recommended_probe_size > data_sent) {
       return recommended_probe_size - data_sent;

modules/pacing/pacing_controller_unittest.cc

@@ -20,6 +20,7 @@
 #include "api/transport/network_types.h"
 #include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
 #include "modules/pacing/packet_router.h"
 #include "system_wrappers/include/clock.h"
 #include "test/explicit_key_value_config.h"
@@ -211,12 +212,20 @@ class PacingControllerPadding : public PacingController::PacketSender {
 
 class PacingControllerProbing : public PacingController::PacketSender {
  public:
-  PacingControllerProbing() : packets_sent_(0), padding_sent_(0) {}
+  PacingControllerProbing() = default;
+  // Controls if padding can be generated or not.
+  // In real implementation, padding can only be generated after a sent
+  // media packet, or if the sender support RTX.
+  void SetCanGeneratePadding(bool can_generate) {
+    can_generate_padding_ = can_generate;
+  }
 
   void SendPacket(std::unique_ptr<RtpPacketToSend> packet,
                   const PacedPacketInfo& pacing_info) override {
     if (packet->packet_type() != RtpPacketMediaType::kPadding) {
       ++packets_sent_;
+    } else {
+      ++padding_packets_sent_;
     }
     last_pacing_info_ = pacing_info;
   }
@@ -227,6 +236,9 @@ class PacingControllerProbing : public PacingController::PacketSender {
 
   std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
       DataSize target_size) override {
+    if (!can_generate_padding_) {
+      return {};
+    }
     // From RTPSender:
     // Max in the RFC 3550 is 255 bytes, we limit it to be modulus 32 for SRTP.
     const DataSize kMaxPadding = DataSize::Bytes(224);
@@ -250,13 +262,16 @@ class PacingControllerProbing : public PacingController::PacketSender {
   }
 
   int packets_sent() const { return packets_sent_; }
+  int padding_packets_sent() const { return padding_packets_sent_; }
   int padding_sent() const { return padding_sent_; }
   int total_packets_sent() const { return packets_sent_ + padding_sent_; }
   PacedPacketInfo last_pacing_info() const { return last_pacing_info_; }
 
  private:
-  int packets_sent_;
-  int padding_sent_;
+  bool can_generate_padding_ = true;
+  int packets_sent_ = 0;
+  int padding_packets_sent_ = 0;
+  int padding_sent_ = 0;
   PacedPacketInfo last_pacing_info_;
 };
 
@@ -1337,6 +1352,75 @@ TEST_F(PacingControllerTest, ProbingWithPaddingSupport) {
               kFirstClusterRate.bps(), kProbingErrorMargin.bps());
 }
 
+TEST_F(PacingControllerTest, CanProbeWithPaddingBeforeFirstMediaPacket) {
+  // const size_t kPacketSize = 1200;
+  const int kInitialBitrateBps = 300000;
+
+  PacingControllerProbing packet_sender;
+  const test::ExplicitKeyValueConfig trials(
+      "WebRTC-Bwe-ProbingBehavior/min_packet_size:0/");
+  auto pacer =
+      std::make_unique<PacingController>(&clock_, &packet_sender, trials);
+  std::vector<ProbeClusterConfig> probe_clusters = {
+      {.at_time = clock_.CurrentTime(),
+       .target_data_rate = kFirstClusterRate,
+       .target_duration = TimeDelta::Millis(15),
+       .target_probe_count = 5,
+       .id = 0}};
+  pacer->CreateProbeClusters(probe_clusters);
+
+  pacer->SetPacingRates(
+      DataRate::BitsPerSec(kInitialBitrateBps * kPaceMultiplier),
+      DataRate::Zero());
+
+  Timestamp start = clock_.CurrentTime();
+  Timestamp next_process = pacer->NextSendTime();
+  while (clock_.CurrentTime() < start + TimeDelta::Millis(100) &&
+         next_process.IsFinite()) {
+    AdvanceTimeUntil(next_process);
+    pacer->ProcessPackets();
+    next_process = pacer->NextSendTime();
+  }
+  EXPECT_GT(packet_sender.padding_packets_sent(), 5);
+}
+
+TEST_F(PacingControllerTest, CanNotProbeWithPaddingIfGeneratePaddingFails) {
+  // const size_t kPacketSize = 1200;
+  const int kInitialBitrateBps = 300000;
+
+  PacingControllerProbing packet_sender;
+  packet_sender.SetCanGeneratePadding(false);
+  const test::ExplicitKeyValueConfig trials(
+      "WebRTC-Bwe-ProbingBehavior/min_packet_size:0/");
+  auto pacer =
+      std::make_unique<PacingController>(&clock_, &packet_sender, trials);
+  std::vector<ProbeClusterConfig> probe_clusters = {
+      {.at_time = clock_.CurrentTime(),
+       .target_data_rate = kFirstClusterRate,
+       .target_duration = TimeDelta::Millis(15),
+       .target_probe_count = 5,
+       .id = 0}};
+  pacer->CreateProbeClusters(probe_clusters);
+
+  pacer->SetPacingRates(
+      DataRate::BitsPerSec(kInitialBitrateBps * kPaceMultiplier),
+      DataRate::Zero());
+
+  Timestamp start = clock_.CurrentTime();
+  int process_count = 0;
+  Timestamp next_process = pacer->NextSendTime();
+  while (clock_.CurrentTime() < start + TimeDelta::Millis(100) &&
+         next_process.IsFinite()) {
+    AdvanceTimeUntil(next_process);
+    pacer->ProcessPackets();
+    ++process_count;
+    next_process = pacer->NextSendTime();
+  }
+
+  EXPECT_LT(process_count, 10);
+  EXPECT_EQ(packet_sender.padding_packets_sent(), 0);
+}
+
 TEST_F(PacingControllerTest, PaddingOveruse) {
   uint32_t ssrc = 12346;
   uint16_t sequence_number = 1234;