diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc index ec9182fca2..927e1c2e93 100644 --- a/modules/pacing/bitrate_prober.cc +++ b/modules/pacing/bitrate_prober.cc @@ -34,7 +34,7 @@ constexpr TimeDelta kProbeClusterTimeout = TimeDelta::Seconds(5); BitrateProberConfig::BitrateProberConfig( const FieldTrialsView* key_value_config) - : min_probe_delta("min_probe_delta", TimeDelta::Millis(1)), + : min_probe_delta("min_probe_delta", TimeDelta::Millis(2)), max_probe_delay("max_probe_delay", TimeDelta::Millis(10)) { ParseFieldTrial({&min_probe_delta, &max_probe_delay}, key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior")); @@ -107,6 +107,7 @@ void BitrateProber::CreateProbeCluster( << 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) @@ -145,16 +146,13 @@ absl::optional BitrateProber::CurrentCluster(Timestamp now) { return info; } -// Probe size is recommended based on the probe bitrate required. We choose -// a minimum of twice `kMinProbeDeltaMs` interval to allow scheduling to be -// feasible. DataSize BitrateProber::RecommendedMinProbeSize() const { if (clusters_.empty()) { return DataSize::Zero(); } DataRate send_rate = DataRate::BitsPerSec(clusters_.front().pace_info.send_bitrate_bps); - return 2 * send_rate * config_.min_probe_delta; + return send_rate * config_.min_probe_delta; } void BitrateProber::ProbeSent(Timestamp now, DataSize size) { @@ -197,6 +195,7 @@ Timestamp BitrateProber::CalculateNextProbeTime( DataSize sent_bytes = DataSize::Bytes(cluster.sent_bytes); DataRate send_bitrate = DataRate::BitsPerSec(cluster.pace_info.send_bitrate_bps); + TimeDelta delta = sent_bytes / send_bitrate; return cluster.started_at + delta; } diff --git a/modules/pacing/bitrate_prober.h b/modules/pacing/bitrate_prober.h index d38da7f841..222a28f58e 100644 --- a/modules/pacing/bitrate_prober.h +++ b/modules/pacing/bitrate_prober.h @@ -66,7 +66,8 @@ class BitrateProber { absl::optional CurrentCluster(Timestamp now); // Returns the minimum number of bytes that the prober recommends for - // the next probe, or zero if not probing. + // the next probe, or zero if not probing. A probe can consist of multiple + // packets that are sent back to back. DataSize RecommendedMinProbeSize() const; // Called to report to the prober that a probe has been sent. In case of diff --git a/modules/pacing/bitrate_prober_unittest.cc b/modules/pacing/bitrate_prober_unittest.cc index dd9bfe482b..1ebd337066 100644 --- a/modules/pacing/bitrate_prober_unittest.cc +++ b/modules/pacing/bitrate_prober_unittest.cc @@ -170,6 +170,30 @@ TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) { kHighBitrate * TimeDelta::Millis(1)); } +TEST(BitrateProberTest, ProbeSizeCanBeSetWithFieldTrial) { + const test::ExplicitKeyValueConfig trials( + "WebRTC-Bwe-ProbingBehavior/min_probe_delta:20ms/"); + BitrateProber prober(trials); + prober.SetEnabled(true); + + const DataRate kHighBitrate = DataRate::KilobitsPerSec(10000); // 10 Mbps + + prober.CreateProbeCluster({.at_time = Timestamp::Zero(), + .target_data_rate = kHighBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); + EXPECT_EQ(prober.RecommendedMinProbeSize(), + kHighBitrate * TimeDelta::Millis(20)); + + prober.OnIncomingPacket(DataSize::Bytes(1000)); + // Next time to send probe should be "min_probe_delta" if the recommended + // number of bytes has been sent. + prober.ProbeSent(Timestamp::Zero(), prober.RecommendedMinProbeSize()); + EXPECT_EQ(prober.NextProbeTime(Timestamp::Zero()), + Timestamp::Zero() + TimeDelta::Millis(20)); +} + TEST(BitrateProberTest, MinumumNumberOfProbingPackets) { const FieldTrialBasedConfig config; BitrateProber prober(config); diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc index e6dfe26834..84d6117d4b 100644 --- a/modules/pacing/task_queue_paced_sender_unittest.cc +++ b/modules/pacing/task_queue_paced_sender_unittest.cc @@ -459,9 +459,9 @@ TEST(TaskQueuePacedSenderTest, SchedulesProbeAtSentTime) { TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) { // Set min_probe_delta to be less than kMinSleepTime (1ms). - const TimeDelta kMinProbeDelta = TimeDelta::Micros(100); + const TimeDelta kMinProbeDelta = TimeDelta::Micros(200); ScopedKeyValueConfig trials( - "WebRTC-Bwe-ProbingBehavior/min_probe_delta:100us/"); + "WebRTC-Bwe-ProbingBehavior/min_probe_delta:200us/"); GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234)); MockPacketRouter packet_router; TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials, @@ -495,7 +495,7 @@ TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) { // Advance time less than PacingController::kMinSleepTime, probing packets // for the first millisecond should be sent immediately. Min delta between - // probes is 2x 100us, meaning 4 times per ms we will get least one call to + // probes is 200us, meaning 4 times per ms we will get least one call to // SendPacket(). DataSize data_sent = DataSize::Zero(); EXPECT_CALL(packet_router, @@ -515,7 +515,7 @@ TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) { // Verify the amount of probing data sent. // Probe always starts with a small (1 byte) padding packet that's not // counted into the probe rate here. - const DataSize kMinProbeSize = 2 * kMinProbeDelta * kProbingRate; + const DataSize kMinProbeSize = kMinProbeDelta * kProbingRate; EXPECT_EQ(data_sent, DataSize::Bytes(1) + kPacketSize + 4 * kMinProbeSize); }