diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index c091cb9a81..73120381cc 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -153,7 +153,6 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock) trendline_estimator_(), detector_(), receiver_incoming_bitrate_(), - last_update_ms_(-1), last_seen_packet_ms_(-1), uma_recorded_(false), probe_bitrate_estimator_(event_log), @@ -189,27 +188,27 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( BweNames::kBweNamesMax); uma_recorded_ = true; } - Result aggregated_result; + bool overusing = false; bool delayed_feedback = true; for (const auto& packet_feedback : sorted_packet_feedback_vector) { if (packet_feedback.send_time_ms < 0) continue; delayed_feedback = false; - Result result = IncomingPacketFeedback(packet_feedback); - if (result.updated) - aggregated_result = result; + IncomingPacketFeedback(packet_feedback); + overusing |= detector_.State() == BandwidthUsage::kBwOverusing; } if (delayed_feedback) { ++consecutive_delayed_feedbacks_; + if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) { + consecutive_delayed_feedbacks_ = 0; + return OnLongFeedbackDelay( + sorted_packet_feedback_vector.back().arrival_time_ms); + } } else { consecutive_delayed_feedbacks_ = 0; + return MaybeUpdateEstimate(overusing); } - if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) { - aggregated_result = OnLongFeedbackDelay( - sorted_packet_feedback_vector.back().arrival_time_ms); - consecutive_delayed_feedbacks_ = 0; - } - return aggregated_result; + return Result(); } DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay( @@ -229,7 +228,7 @@ DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay( return result; } -DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedback( +void DelayBasedBwe::IncomingPacketFeedback( const PacketFeedback& packet_feedback) { int64_t now_ms = clock_->TimeInMilliseconds(); @@ -272,40 +271,36 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedback( trendline_estimator_->num_of_deltas(), packet_feedback.arrival_time_ms); } - - int probing_bps = 0; if (packet_feedback.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) { - probing_bps = - probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback); + probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback); } +} + +DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate(bool overusing) { + Result result; + int64_t now_ms = clock_->TimeInMilliseconds(); + rtc::Optional acked_bitrate_bps = receiver_incoming_bitrate_.bitrate_bps(); + rtc::Optional probe_bitrate_bps = + probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps(); // Currently overusing the bandwidth. - if (detector_.State() == BandwidthUsage::kBwOverusing) { + if (overusing) { if (acked_bitrate_bps && rate_control_.TimeToReduceFurther(now_ms, *acked_bitrate_bps)) { - result.updated = - UpdateEstimate(packet_feedback.arrival_time_ms, now_ms, - acked_bitrate_bps, &result.target_bitrate_bps); + result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing, + &result.target_bitrate_bps); } - } else if (probing_bps > 0) { - // No overuse, but probing measured a bitrate. - rate_control_.SetEstimate(probing_bps, packet_feedback.arrival_time_ms); - result.probe = true; - result.updated = - UpdateEstimate(packet_feedback.arrival_time_ms, now_ms, - acked_bitrate_bps, &result.target_bitrate_bps); - } - if (!result.updated && - (last_update_ms_ == -1 || - now_ms - last_update_ms_ > rate_control_.GetFeedbackInterval())) { - result.updated = - UpdateEstimate(packet_feedback.arrival_time_ms, now_ms, - acked_bitrate_bps, &result.target_bitrate_bps); + } else { + if (probe_bitrate_bps) { + rate_control_.SetEstimate(*probe_bitrate_bps, now_ms); + result.probe = true; + } + result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing, + &result.target_bitrate_bps); } if (result.updated) { - last_update_ms_ = now_ms; BWE_TEST_LOGGING_PLOT(1, "target_bitrate_bps", now_ms, result.target_bitrate_bps); if (event_log_ && (result.target_bitrate_bps != last_logged_bitrate_ || @@ -316,17 +311,18 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedback( last_logged_state_ = detector_.State(); } } - return result; } -bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, - int64_t now_ms, +bool DelayBasedBwe::UpdateEstimate(int64_t now_ms, rtc::Optional acked_bitrate_bps, + bool overusing, uint32_t* target_bitrate_bps) { // TODO(terelius): RateControlInput::noise_var is deprecated and will be // removed. In the meantime, we set it to zero. - const RateControlInput input(detector_.State(), acked_bitrate_bps, 0); + const RateControlInput input( + overusing ? BandwidthUsage::kBwOverusing : detector_.State(), + acked_bitrate_bps, 0); *target_bitrate_bps = rate_control_.Update(&input, now_ms); return rate_control_.ValidEstimate(); } diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index c1c68c28af..49a767f1e9 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -77,13 +77,15 @@ class DelayBasedBwe { float bitrate_estimate_var_; }; - Result IncomingPacketFeedback(const PacketFeedback& packet_feedback); + void IncomingPacketFeedback(const PacketFeedback& packet_feedback); Result OnLongFeedbackDelay(int64_t arrival_time_ms); + + Result MaybeUpdateEstimate(bool overusing); // Updates the current remote rate estimate and returns true if a valid // estimate exists. - bool UpdateEstimate(int64_t packet_arrival_time_ms, - int64_t now_ms, + bool UpdateEstimate(int64_t now_ms, rtc::Optional acked_bitrate_bps, + bool overusing, uint32_t* target_bitrate_bps); rtc::ThreadChecker network_thread_; @@ -93,7 +95,6 @@ class DelayBasedBwe { std::unique_ptr trendline_estimator_; OveruseDetector detector_; BitrateEstimator receiver_incoming_bitrate_; - int64_t last_update_ms_; int64_t last_seen_packet_ms_; bool uma_recorded_; AimdRateControl rate_control_; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index fae4656988..dcec0f8000 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -140,18 +140,18 @@ TEST_F(DelayBasedBweTest, GetExpectedBwePeriodMs) { } TEST_F(DelayBasedBweTest, InitialBehavior) { - InitialBehaviorTestHelper(674840); + InitialBehaviorTestHelper(730000); } TEST_F(DelayBasedBweTest, RateIncreaseReordering) { - RateIncreaseReorderingTestHelper(674840); + RateIncreaseReorderingTestHelper(730000); } TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(1288); + RateIncreaseRtpTimestampsTestHelper(627); } TEST_F(DelayBasedBweTest, CapacityDropOneStream) { - CapacityDropTestHelper(1, false, 333, 0); + CapacityDropTestHelper(1, false, 300, 0); } TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) { @@ -159,12 +159,13 @@ TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) { } TEST_F(DelayBasedBweTest, CapacityDropNegOffsetChange) { - CapacityDropTestHelper(1, false, 867, -30000); + CapacityDropTestHelper(1, false, 933, -30000); } TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) { CapacityDropTestHelper(1, true, 333, 0); } + TEST_F(DelayBasedBweTest, TestTimestampGrouping) { TestTimestampGroupingTestHelper(); } diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index e7a98c350c..479ecb457b 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -492,7 +492,7 @@ void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { clock_.AdvanceTimeMilliseconds(silence_time_s * 1000); send_time_ms += silence_time_s * 1000; - for (size_t i = 0; i < 23; ++i) { + for (size_t i = 0; i < 24; ++i) { IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc index 8406387ece..2d66198ef8 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc @@ -133,10 +133,19 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( << " [receive: " << receive_size << " bytes / " << receive_interval_ms << " ms = " << receive_bps / 1000 << " kb/s]"; + float res = std::min(send_bps, receive_bps); if (event_log_) event_log_->LogProbeResultSuccess(cluster_id, res); - return res; + estimated_bitrate_bps_ = rtc::Optional(std::min(send_bps, res)); + return *estimated_bitrate_bps_; +} + +rtc::Optional +ProbeBitrateEstimator::FetchAndResetLastEstimatedBitrateBps() { + rtc::Optional estimated_bitrate_bps = estimated_bitrate_bps_; + estimated_bitrate_bps_.reset(); + return estimated_bitrate_bps; } void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) { diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h index 247e905fd0..7286f72af2 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h @@ -27,6 +27,8 @@ class ProbeBitrateEstimator { // Returns the estimated bitrate if the probe completes a valid cluster. int HandleProbeAndEstimateBitrate(const PacketFeedback& packet_feedback); + rtc::Optional FetchAndResetLastEstimatedBitrateBps(); + private: struct AggregatedCluster { int num_probes = 0; @@ -44,6 +46,7 @@ class ProbeBitrateEstimator { std::map clusters_; RtcEventLog* const event_log_; + rtc::Optional estimated_bitrate_bps_; }; } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc index f86c93fae3..5686f57724 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc @@ -193,4 +193,21 @@ TEST_F(TestProbeBitrateEstimator, IgnoreSizeFirstReceivePacket) { EXPECT_NEAR(measured_bps_, 800000, 10); } +TEST_F(TestProbeBitrateEstimator, NoLastEstimatedBitrateBps) { + EXPECT_FALSE(probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps()); +} + +TEST_F(TestProbeBitrateEstimator, FetchLastEstimatedBitrateBps) { + AddPacketFeedback(0, 1000, 0, 10); + AddPacketFeedback(0, 1000, 10, 20); + AddPacketFeedback(0, 1000, 20, 30); + AddPacketFeedback(0, 1000, 30, 40); + + auto estimated_bitrate_bps = + probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps(); + EXPECT_TRUE(estimated_bitrate_bps); + EXPECT_NEAR(*estimated_bitrate_bps, 800000, 10); + EXPECT_FALSE(probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps()); +} + } // namespace webrtc