From 952dc0ce846bba6f1620231eba928c0deb6cca7d Mon Sep 17 00:00:00 2001 From: Diep Bui Date: Tue, 29 Mar 2022 15:26:33 +0000 Subject: [PATCH] Fix division by 0 in loss_based_bwe_v2. Bug: webrtc:12707 Change-Id: I16a643235aa2e51f0a062734482000562c8f6b4d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257080 Reviewed-by: Per Kjellander Commit-Queue: Diep Bui Cr-Commit-Position: refs/heads/main@{#36374} --- .../goog_cc/loss_based_bwe_v2.cc | 7 ++-- .../goog_cc/loss_based_bwe_v2_test.cc | 34 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc index 616fe710c8..d13e23d1b4 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -838,9 +838,10 @@ bool LossBasedBweV2::PushBackObservation( last_send_time - last_send_time_most_recent_observation_; // Too small to be meaningful. - if (observation_duration < config_->observation_duration_lower_bound && - (delay_detector_state == BandwidthUsage::kBwNormal || - !config_->trendline_integration_enabled)) { + if (observation_duration <= TimeDelta::Zero() || + (observation_duration < config_->observation_duration_lower_bound && + (delay_detector_state == BandwidthUsage::kBwNormal || + !config_->trendline_integration_enabled))) { return false; } diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc index 55de52d9c9..dbe745745e 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc @@ -581,5 +581,39 @@ TEST(LossBasedBweV2Test, NotUseAckedBitrateInNormalState) { acked_bitrate); } +TEST(LossBasedBweV2Test, NoUpdateIfObservationDurationUnchanged) { + PacketResult enough_feedback_1[2]; + enough_feedback_1[0].sent_packet.size = DataSize::Bytes(15'000); + enough_feedback_1[1].sent_packet.size = DataSize::Bytes(15'000); + enough_feedback_1[0].sent_packet.send_time = Timestamp::Zero(); + enough_feedback_1[1].sent_packet.send_time = + Timestamp::Zero() + kObservationDurationLowerBound; + enough_feedback_1[0].receive_time = + Timestamp::Zero() + kObservationDurationLowerBound; + enough_feedback_1[1].receive_time = + Timestamp::Zero() + 2 * kObservationDurationLowerBound; + + ExplicitKeyValueConfig key_value_config( + Config(/*enabled=*/true, /*valid=*/true)); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(600)); + DataRate acked_bitrate = DataRate::KilobitsPerSec(300); + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acked_bitrate); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_1, DataRate::PlusInfinity(), BandwidthUsage::kBwNormal); + DataRate current_estimate = + loss_based_bandwidth_estimator.GetBandwidthEstimate( + /*delay_based_limit=*/DataRate::PlusInfinity()); + + // Use the same feedback and check if the estimate is unchanged. + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_1, DataRate::PlusInfinity(), BandwidthUsage::kBwNormal); + DataRate new_estimate = loss_based_bandwidth_estimator.GetBandwidthEstimate( + /*delay_based_limit=*/DataRate::PlusInfinity()); + EXPECT_EQ(current_estimate, new_estimate); +} + } // namespace } // namespace webrtc