From 80fba25fc394f6a603ef452f6fb0768326079ce8 Mon Sep 17 00:00:00 2001 From: stefan Date: Tue, 18 Apr 2017 06:45:12 -0700 Subject: [PATCH] Fix crash when all acks in a feedback message arrive too late. BUG=b/37279144 Review-Url: https://codereview.webrtc.org/2820353002 Cr-Commit-Position: refs/heads/master@{#17739} --- .../congestion_controller/delay_based_bwe.cc | 8 +++++++- .../delay_based_bwe_unittest.cc | 14 ++++++++++++++ .../send_side_congestion_controller.cc | 2 -- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 453b03e347..7b19685d99 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -172,11 +172,17 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock) DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector) { RTC_DCHECK(network_thread_.CalledOnValidThread()); - RTC_DCHECK(!packet_feedback_vector.empty()); std::vector sorted_packet_feedback_vector; SortPacketFeedbackVector(packet_feedback_vector, &sorted_packet_feedback_vector); + // TOOD(holmer): An empty feedback vector here likely means that + // all acks were too late and that the send time history had + // timed out. We should reduce the rate when this occurs. + if (sorted_packet_feedback_vector.empty()) { + LOG(LS_WARNING) << "Very late feedback received."; + return DelayBasedBwe::Result(); + } if (!uma_recorded_) { RTC_HISTOGRAM_ENUMERATION(kBweTypeHistogram, diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index 44407b37e9..b72d00604f 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -25,6 +25,20 @@ const PacedPacketInfo kPacingInfo0(0, kNumProbesCluster0, 2000); const PacedPacketInfo kPacingInfo1(1, kNumProbesCluster1, 4000); } // namespace +TEST_F(DelayBasedBweTest, NoCrashEmptyFeedback) { + std::vector packet_feedback_vector; + bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector); +} + +TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { + std::vector packet_feedback_vector; + packet_feedback_vector.push_back( + PacketFeedback(-1, -1, 0, 1500, PacedPacketInfo())); + packet_feedback_vector.push_back( + PacketFeedback(-1, -1, 1, 1500, PacedPacketInfo())); + bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector); +} + TEST_F(DelayBasedBweTest, ProbeDetection) { int64_t now_ms = clock_.TimeInMilliseconds(); uint16_t seq_num = 0; diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index e3f84d0e53..78b7f32ad8 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -240,8 +240,6 @@ void SendSideCongestionController::OnTransportFeedback( transport_feedback_adapter_.OnTransportFeedback(feedback); std::vector feedback_vector = transport_feedback_adapter_.GetTransportFeedbackVector(); - if (feedback_vector.empty()) - return; DelayBasedBwe::Result result; { rtc::CritScope cs(&bwe_lock_);