From 807736ef02a9b0693dd3eb35ff16cd62b0394656 Mon Sep 17 00:00:00 2001 From: tschumim Date: Thu, 8 Jun 2017 00:10:31 -0700 Subject: [PATCH] Revert of Refactored incoming bitrate estimator. (patchset #8 id:140001 of https://codereview.webrtc.org/2917873002/ ) Reason for revert: Breaks Vice tests Original issue's description: > Refactored incoming bitrate estimator. > > BUG=webrtc:7746 > > Review-Url: https://codereview.webrtc.org/2917873002 > Cr-Commit-Position: refs/heads/master@{#18478} > Committed: https://chromium.googlesource.com/external/webrtc/+/5fc8bf8b87495c225f5060760fa9d967b8dc9f9e TBR=philipel@webrtc.org,terelius@webrtc.org,stefan@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:7746 Review-Url: https://codereview.webrtc.org/2924243002 Cr-Commit-Position: refs/heads/master@{#18486} --- webrtc/modules/congestion_controller/BUILD.gn | 2 - .../acknowledge_bitrate_estimator.cc | 105 --------------- .../acknowledge_bitrate_estimator.h | 48 ------- .../congestion_controller/delay_based_bwe.cc | 125 ++++++++++++++++-- .../congestion_controller/delay_based_bwe.h | 27 +++- .../delay_based_bwe_unittest.cc | 6 +- .../delay_based_bwe_unittest_helper.cc | 12 +- .../delay_based_bwe_unittest_helper.h | 2 - .../include/send_side_congestion_controller.h | 2 - .../send_side_congestion_controller.cc | 34 +---- .../test/estimators/send_side.cc | 9 +- .../test/estimators/send_side.h | 2 - .../rtp_rtcp/include/rtp_rtcp_defines.h | 11 -- webrtc/tools/event_log_visualizer/analyzer.cc | 12 ++ 14 files changed, 157 insertions(+), 240 deletions(-) delete mode 100644 webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc delete mode 100644 webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index ef45297031..f32b7b62f5 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -10,8 +10,6 @@ import("../../webrtc.gni") rtc_static_library("congestion_controller") { sources = [ - "acknowledge_bitrate_estimator.cc", - "acknowledge_bitrate_estimator.h", "congestion_controller.cc", "delay_based_bwe.cc", "delay_based_bwe.h", diff --git a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc deleted file mode 100644 index 4a4e1861ef..0000000000 --- a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.cc +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" - -#include - -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" - -namespace webrtc { - -namespace { -constexpr int kInitialRateWindowMs = 500; -constexpr int kRateWindowMs = 150; -} // namespace - -AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator() - : sum_(0), - current_win_ms_(0), - prev_time_ms_(-1), - bitrate_estimate_(-1.0f), - bitrate_estimate_var_(50.0f) {} - -void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector) { - RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), - packet_feedback_vector.end(), - PacketFeedbackComparator())); - for (const auto& packet : packet_feedback_vector) - Update(packet.arrival_time_ms, packet.payload_size); -} - -void AcknowledgedBitrateEstimator::Update(int64_t now_ms, int bytes) { - int rate_window_ms = kRateWindowMs; - // We use a larger window at the beginning to get a more stable sample that - // we can use to initialize the estimate. - if (bitrate_estimate_ < 0.f) - rate_window_ms = kInitialRateWindowMs; - float bitrate_sample = UpdateWindow(now_ms, bytes, rate_window_ms); - if (bitrate_sample < 0.0f) - return; - if (bitrate_estimate_ < 0.0f) { - // This is the very first sample we get. Use it to initialize the estimate. - bitrate_estimate_ = bitrate_sample; - return; - } - // Define the sample uncertainty as a function of how far away it is from the - // current estimate. - float sample_uncertainty = - 10.0f * std::abs(bitrate_estimate_ - bitrate_sample) / bitrate_estimate_; - float sample_var = sample_uncertainty * sample_uncertainty; - // Update a bayesian estimate of the rate, weighting it lower if the sample - // uncertainty is large. - // The bitrate estimate uncertainty is increased with each update to model - // that the bitrate changes over time. - float pred_bitrate_estimate_var = bitrate_estimate_var_ + 5.f; - bitrate_estimate_ = (sample_var * bitrate_estimate_ + - pred_bitrate_estimate_var * bitrate_sample) / - (sample_var + pred_bitrate_estimate_var); - bitrate_estimate_var_ = sample_var * pred_bitrate_estimate_var / - (sample_var + pred_bitrate_estimate_var); -} - -float AcknowledgedBitrateEstimator::UpdateWindow(int64_t now_ms, - int bytes, - int rate_window_ms) { - // Reset if time moves backwards. - if (now_ms < prev_time_ms_) { - prev_time_ms_ = -1; - sum_ = 0; - current_win_ms_ = 0; - } - if (prev_time_ms_ >= 0) { - current_win_ms_ += now_ms - prev_time_ms_; - // Reset if nothing has been received for more than a full window. - if (now_ms - prev_time_ms_ > rate_window_ms) { - sum_ = 0; - current_win_ms_ %= rate_window_ms; - } - } - prev_time_ms_ = now_ms; - float bitrate_sample = -1.0f; - if (current_win_ms_ >= rate_window_ms) { - bitrate_sample = 8.0f * sum_ / static_cast(rate_window_ms); - current_win_ms_ -= rate_window_ms; - sum_ = 0; - } - sum_ += bytes; - return bitrate_sample; -} - -rtc::Optional AcknowledgedBitrateEstimator::bitrate_bps() const { - if (bitrate_estimate_ < 0.f) - return rtc::Optional(); - return rtc::Optional(bitrate_estimate_ * 1000); -} - -} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h b/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h deleted file mode 100644 index 7a9d669a1c..0000000000 --- a/webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ -#define WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ - -#include - -#include "webrtc/base/optional.h" - -namespace webrtc { - -struct PacketFeedback; - -// Computes a bayesian estimate of the throughput given acks containing -// the arrival time and payload size. Samples which are far from the current -// estimate or are based on few packets are given a smaller weight, as they -// are considered to be more likely to have been caused by, e.g., delay spikes -// unrelated to congestion. -class AcknowledgedBitrateEstimator { - public: - AcknowledgedBitrateEstimator(); - - void IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector); - rtc::Optional bitrate_bps() const; - - private: - void Update(int64_t now_ms, int bytes); - float UpdateWindow(int64_t now_ms, int bytes, int rate_window_ms); - - int sum_; - int64_t current_win_ms_; - int64_t prev_time_ms_; - float bitrate_estimate_; - float bitrate_estimate_var_; -}; - -} // namespace webrtc - -#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_ACKNOWLEDGE_BITRATE_ESTIMATOR_H_ diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 147f2432b2..fca5143bb1 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -38,6 +38,8 @@ constexpr double kTimestampToMs = // This ssrc is used to fulfill the current API but will be removed // after the API has been changed. constexpr uint32_t kFixedSsrc = 0; +constexpr int kInitialRateWindowMs = 500; +constexpr int kRateWindowMs = 150; // Parameters for linear least squares fit of regression line to noisy data. constexpr size_t kDefaultTrendlineWindowSize = 20; @@ -53,16 +55,111 @@ bool BweSparseUpdateExperimentIsEnabled() { webrtc::field_trial::FindFullName(kBweSparseUpdateExperiment); return experiment_string == "Enabled"; } + +class PacketFeedbackComparator { + public: + inline bool operator()(const webrtc::PacketFeedback& lhs, + const webrtc::PacketFeedback& rhs) { + if (lhs.arrival_time_ms != rhs.arrival_time_ms) + return lhs.arrival_time_ms < rhs.arrival_time_ms; + if (lhs.send_time_ms != rhs.send_time_ms) + return lhs.send_time_ms < rhs.send_time_ms; + return lhs.sequence_number < rhs.sequence_number; + } +}; + +void SortPacketFeedbackVector(const std::vector& input, + std::vector* output) { + auto pred = [](const webrtc::PacketFeedback& packet_feedback) { + return packet_feedback.arrival_time_ms != + webrtc::PacketFeedback::kNotReceived; + }; + std::copy_if(input.begin(), input.end(), std::back_inserter(*output), pred); + std::sort(output->begin(), output->end(), PacketFeedbackComparator()); +} } // namespace namespace webrtc { +DelayBasedBwe::BitrateEstimator::BitrateEstimator() + : sum_(0), + current_win_ms_(0), + prev_time_ms_(-1), + bitrate_estimate_(-1.0f), + bitrate_estimate_var_(50.0f) {} + +void DelayBasedBwe::BitrateEstimator::Update(int64_t now_ms, int bytes) { + int rate_window_ms = kRateWindowMs; + // We use a larger window at the beginning to get a more stable sample that + // we can use to initialize the estimate. + if (bitrate_estimate_ < 0.f) + rate_window_ms = kInitialRateWindowMs; + float bitrate_sample = UpdateWindow(now_ms, bytes, rate_window_ms); + if (bitrate_sample < 0.0f) + return; + if (bitrate_estimate_ < 0.0f) { + // This is the very first sample we get. Use it to initialize the estimate. + bitrate_estimate_ = bitrate_sample; + return; + } + // Define the sample uncertainty as a function of how far away it is from the + // current estimate. + float sample_uncertainty = + 10.0f * std::abs(bitrate_estimate_ - bitrate_sample) / bitrate_estimate_; + float sample_var = sample_uncertainty * sample_uncertainty; + // Update a bayesian estimate of the rate, weighting it lower if the sample + // uncertainty is large. + // The bitrate estimate uncertainty is increased with each update to model + // that the bitrate changes over time. + float pred_bitrate_estimate_var = bitrate_estimate_var_ + 5.f; + bitrate_estimate_ = (sample_var * bitrate_estimate_ + + pred_bitrate_estimate_var * bitrate_sample) / + (sample_var + pred_bitrate_estimate_var); + bitrate_estimate_var_ = sample_var * pred_bitrate_estimate_var / + (sample_var + pred_bitrate_estimate_var); +} + +float DelayBasedBwe::BitrateEstimator::UpdateWindow(int64_t now_ms, + int bytes, + int rate_window_ms) { + // Reset if time moves backwards. + if (now_ms < prev_time_ms_) { + prev_time_ms_ = -1; + sum_ = 0; + current_win_ms_ = 0; + } + if (prev_time_ms_ >= 0) { + current_win_ms_ += now_ms - prev_time_ms_; + // Reset if nothing has been received for more than a full window. + if (now_ms - prev_time_ms_ > rate_window_ms) { + sum_ = 0; + current_win_ms_ %= rate_window_ms; + } + } + prev_time_ms_ = now_ms; + float bitrate_sample = -1.0f; + if (current_win_ms_ >= rate_window_ms) { + bitrate_sample = 8.0f * sum_ / static_cast(rate_window_ms); + current_win_ms_ -= rate_window_ms; + sum_ = 0; + } + sum_ += bytes; + return bitrate_sample; +} + +rtc::Optional DelayBasedBwe::BitrateEstimator::bitrate_bps() const { + if (bitrate_estimate_ < 0.f) + return rtc::Optional(); + return rtc::Optional(bitrate_estimate_ * 1000); +} + DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock) : event_log_(event_log), clock_(clock), inter_arrival_(), trendline_estimator_(), detector_(), + receiver_incoming_bitrate_(), last_seen_packet_ms_(-1), uma_recorded_(false), probe_bitrate_estimator_(event_log), @@ -80,17 +177,16 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock) DelayBasedBwe::~DelayBasedBwe() {} DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector, - rtc::Optional acked_bitrate_bps) { - RTC_DCHECK(std::is_sorted(packet_feedback_vector.begin(), - packet_feedback_vector.end(), - PacketFeedbackComparator())); + const std::vector& packet_feedback_vector) { RTC_DCHECK(network_thread_.CalledOnValidThread()); + 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 (packet_feedback_vector.empty()) { + if (sorted_packet_feedback_vector.empty()) { LOG(LS_WARNING) << "Very late feedback received."; return DelayBasedBwe::Result(); } @@ -103,7 +199,7 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( } bool overusing = false; bool delayed_feedback = true; - for (const auto& packet_feedback : packet_feedback_vector) { + for (const auto& packet_feedback : sorted_packet_feedback_vector) { if (packet_feedback.send_time_ms < 0) continue; delayed_feedback = false; @@ -117,11 +213,12 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( ++consecutive_delayed_feedbacks_; if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) { consecutive_delayed_feedbacks_ = 0; - return OnLongFeedbackDelay(packet_feedback_vector.back().arrival_time_ms); + return OnLongFeedbackDelay( + sorted_packet_feedback_vector.back().arrival_time_ms); } } else { consecutive_delayed_feedbacks_ = 0; - return MaybeUpdateEstimate(overusing, acked_bitrate_bps); + return MaybeUpdateEstimate(overusing); } return Result(); } @@ -146,6 +243,10 @@ DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay( void DelayBasedBwe::IncomingPacketFeedback( const PacketFeedback& packet_feedback) { int64_t now_ms = clock_->TimeInMilliseconds(); + + receiver_incoming_bitrate_.Update(packet_feedback.arrival_time_ms, + packet_feedback.payload_size); + Result result; // Reset if the stream has timed out. if (last_seen_packet_ms_ == -1 || now_ms - last_seen_packet_ms_ > kStreamTimeOutMs) { @@ -188,12 +289,12 @@ void DelayBasedBwe::IncomingPacketFeedback( } } -DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( - bool overusing, - rtc::Optional acked_bitrate_bps) { +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. diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index c44ea12e1c..eccd73ac4d 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -48,8 +48,7 @@ class DelayBasedBwe { virtual ~DelayBasedBwe(); Result IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector, - rtc::Optional acked_bitrate_bps); + const std::vector& packet_feedback_vector); void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms); bool LatestEstimate(std::vector* ssrcs, uint32_t* bitrate_bps) const; @@ -58,11 +57,30 @@ class DelayBasedBwe { int64_t GetExpectedBwePeriodMs() const; private: + // Computes a bayesian estimate of the throughput given acks containing + // the arrival time and payload size. Samples which are far from the current + // estimate or are based on few packets are given a smaller weight, as they + // are considered to be more likely to have been caused by, e.g., delay spikes + // unrelated to congestion. + class BitrateEstimator { + public: + BitrateEstimator(); + void Update(int64_t now_ms, int bytes); + rtc::Optional bitrate_bps() const; + + private: + float UpdateWindow(int64_t now_ms, int bytes, int rate_window_ms); + int sum_; + int64_t current_win_ms_; + int64_t prev_time_ms_; + float bitrate_estimate_; + float bitrate_estimate_var_; + }; + void IncomingPacketFeedback(const PacketFeedback& packet_feedback); Result OnLongFeedbackDelay(int64_t arrival_time_ms); - Result MaybeUpdateEstimate(bool overusing, - rtc::Optional acked_bitrate_bps); + Result MaybeUpdateEstimate(bool overusing); // Updates the current remote rate estimate and returns true if a valid // estimate exists. bool UpdateEstimate(int64_t now_ms, @@ -76,6 +94,7 @@ class DelayBasedBwe { std::unique_ptr inter_arrival_; std::unique_ptr trendline_estimator_; OveruseDetector detector_; + BitrateEstimator receiver_incoming_bitrate_; 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 a368658b7a..dcec0f8000 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -27,8 +27,7 @@ const PacedPacketInfo kPacingInfo1(1, kNumProbesCluster1, 4000); TEST_F(DelayBasedBweTest, NoCrashEmptyFeedback) { std::vector packet_feedback_vector; - bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector, - rtc::Optional()); + bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector); } TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { @@ -37,8 +36,7 @@ TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { PacketFeedback(-1, -1, 0, 1500, PacedPacketInfo())); packet_feedback_vector.push_back( PacketFeedback(-1, -1, 1, 1500, PacedPacketInfo())); - bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector, - rtc::Optional()); + bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector); } TEST_F(DelayBasedBweTest, ProbeDetection) { 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 62c9f881bc..479ecb457b 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -14,7 +14,6 @@ #include #include "webrtc/base/checks.h" -#include "webrtc/base/ptr_util.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" namespace webrtc { @@ -150,8 +149,6 @@ int64_t StreamGenerator::GenerateFrame(std::vector* packets, DelayBasedBweTest::DelayBasedBweTest() : clock_(100000000), - acknowledged_bitrate_estimator_( - rtc::MakeUnique()), bitrate_estimator_(new DelayBasedBwe(nullptr, &clock_)), stream_generator_(new test::StreamGenerator(1e6, // Capacity. clock_.TimeInMicroseconds())), @@ -184,10 +181,8 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, sequence_number, payload_size, pacing_info); std::vector packets; packets.push_back(packet); - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = - bitrate_estimator_->IncomingPacketFeedbackVector( - packets, acknowledged_bitrate_estimator_->bitrate_bps()); + bitrate_estimator_->IncomingPacketFeedbackVector(packets); const uint32_t kDummySsrc = 0; if (result.updated) { bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, @@ -218,11 +213,8 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, RTC_CHECK_GE(packet.arrival_time_ms + arrival_time_offset_ms_, 0); packet.arrival_time_ms += arrival_time_offset_ms_; } - - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(packets); DelayBasedBwe::Result result = - bitrate_estimator_->IncomingPacketFeedbackVector( - packets, acknowledged_bitrate_estimator_->bitrate_bps()); + bitrate_estimator_->IncomingPacketFeedbackVector(packets); const uint32_t kDummySsrc = 0; if (result.updated) { bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h index a12bee52b9..089391f532 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h @@ -18,7 +18,6 @@ #include #include "webrtc/base/constructormagic.h" -#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/system_wrappers/include/clock.h" @@ -165,7 +164,6 @@ class DelayBasedBweTest : public ::testing::Test { SimulatedClock clock_; // Time at the receiver. test::TestBitrateObserver bitrate_observer_; - std::unique_ptr acknowledged_bitrate_estimator_; std::unique_ptr bitrate_estimator_; std::unique_ptr stream_generator_; int64_t arrival_time_offset_ms_; diff --git a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h index 4533d46e8e..fd76ba0cde 100644 --- a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h @@ -34,7 +34,6 @@ namespace webrtc { class BitrateController; class Clock; -class AcknowledgedBitrateEstimator; class ProbeController; class RateLimiter; class RtcEventLog; @@ -142,7 +141,6 @@ class SendSideCongestionController : public CallStatsObserver, RtcEventLog* const event_log_; const std::unique_ptr pacer_; const std::unique_ptr bitrate_controller_; - std::unique_ptr acknowledged_bitrate_estimator_; const std::unique_ptr probe_controller_; const std::unique_ptr retransmission_rate_limiter_; TransportFeedbackAdapter transport_feedback_adapter_; diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index 57e7685f88..354ccaaaa5 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -16,11 +16,9 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" -#include "webrtc/base/ptr_util.h" #include "webrtc/base/rate_limiter.h" #include "webrtc/base/socket.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" -#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/congestion_controller/probe_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" @@ -44,25 +42,6 @@ static void ClampBitrates(int* bitrate_bps, *bitrate_bps = std::max(*min_bitrate_bps, *bitrate_bps); } -std::vector ReceivedPacketFeedbackVector( - const std::vector& input) { - std::vector received_packet_feedback_vector; - auto is_received = [](const webrtc::PacketFeedback& packet_feedback) { - return packet_feedback.arrival_time_ms != - webrtc::PacketFeedback::kNotReceived; - }; - std::copy_if(input.begin(), input.end(), - std::back_inserter(received_packet_feedback_vector), - is_received); - return received_packet_feedback_vector; -} - -void SortPacketFeedbackVector( - std::vector* const input) { - RTC_DCHECK(input); - std::sort(input->begin(), input->end(), PacketFeedbackComparator()); -} - } // namespace SendSideCongestionController::SendSideCongestionController( @@ -88,8 +67,6 @@ SendSideCongestionController::SendSideCongestionController( pacer_(std::move(pacer)), bitrate_controller_( BitrateController::CreateBitrateController(clock_, event_log)), - acknowledged_bitrate_estimator_( - rtc::MakeUnique()), probe_controller_(new ProbeController(pacer_.get(), clock_)), retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), @@ -168,7 +145,6 @@ void SendSideCongestionController::OnNetworkRouteChanged( rtc::CritScope cs(&bwe_lock_); min_bitrate_bps_ = min_bitrate_bps; delay_based_bwe_.reset(new DelayBasedBwe(event_log_, clock_)); - acknowledged_bitrate_estimator_.reset(new AcknowledgedBitrateEstimator()); delay_based_bwe_->SetStartBitrate(bitrate_bps); delay_based_bwe_->SetMinBitrate(min_bitrate_bps); } @@ -275,16 +251,12 @@ void SendSideCongestionController::OnTransportFeedback( const rtcp::TransportFeedback& feedback) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); transport_feedback_adapter_.OnTransportFeedback(feedback); - std::vector feedback_vector = ReceivedPacketFeedbackVector( - transport_feedback_adapter_.GetTransportFeedbackVector()); - SortPacketFeedbackVector(&feedback_vector); - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - feedback_vector); + std::vector feedback_vector = + transport_feedback_adapter_.GetTransportFeedbackVector(); DelayBasedBwe::Result result; { rtc::CritScope cs(&bwe_lock_); - result = delay_based_bwe_->IncomingPacketFeedbackVector( - feedback_vector, acknowledged_bitrate_estimator_->bitrate_bps()); + result = delay_based_bwe_->IncomingPacketFeedbackVector(feedback_vector); } if (result.updated) bitrate_controller_->OnDelayBasedBweResult(result); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 290f0cb125..338c4d8e4d 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -13,7 +13,6 @@ #include #include "webrtc/base/logging.h" -#include "webrtc/base/ptr_util.h" #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h" @@ -30,8 +29,6 @@ SendSideBweSender::SendSideBweSender(int kbps, BitrateController::CreateBitrateController(clock, observer, &event_log_)), - acknowledged_bitrate_estimator_( - rtc::MakeUnique()), bwe_(new DelayBasedBwe(nullptr, clock)), feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()), clock_(clock), @@ -75,10 +72,8 @@ void SendSideBweSender::GiveFeedback(const FeedbackPacket& feedback) { bwe_->OnRttUpdate(rtt_ms, rtt_ms); BWE_TEST_LOGGING_PLOT(1, "RTT", clock_->TimeInMilliseconds(), rtt_ms); - acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector( - packet_feedback_vector); - DelayBasedBwe::Result result = bwe_->IncomingPacketFeedbackVector( - packet_feedback_vector, acknowledged_bitrate_estimator_->bitrate_bps()); + DelayBasedBwe::Result result = + bwe_->IncomingPacketFeedbackVector(packet_feedback_vector); if (result.updated) bitrate_controller_->OnDelayBasedBweResult(result); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h index d2aa8de930..73474eeb19 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -15,7 +15,6 @@ #include #include "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h" -#include "webrtc/modules/congestion_controller/acknowledge_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" @@ -38,7 +37,6 @@ class SendSideBweSender : public BweSender, public RemoteBitrateObserver { protected: std::unique_ptr bitrate_controller_; - std::unique_ptr acknowledged_bitrate_estimator_; std::unique_ptr bwe_; std::unique_ptr feedback_observer_; diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 90537c3e30..183755eff3 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -347,17 +347,6 @@ struct PacketFeedback { PacedPacketInfo pacing_info; }; -class PacketFeedbackComparator { - public: - inline bool operator()(const PacketFeedback& lhs, const PacketFeedback& rhs) { - if (lhs.arrival_time_ms != rhs.arrival_time_ms) - return lhs.arrival_time_ms < rhs.arrival_time_ms; - if (lhs.send_time_ms != rhs.send_time_ms) - return lhs.send_time_ms < rhs.send_time_ms; - return lhs.sequence_number < rhs.sequence_number; - } -}; - class TransportFeedbackObserver { public: TransportFeedbackObserver() {} diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index dcff92c9d2..b5b05dd218 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -44,6 +44,18 @@ namespace plotting { namespace { +class PacketFeedbackComparator { + public: + inline bool operator()(const webrtc::PacketFeedback& lhs, + const webrtc::PacketFeedback& rhs) { + if (lhs.arrival_time_ms != rhs.arrival_time_ms) + return lhs.arrival_time_ms < rhs.arrival_time_ms; + if (lhs.send_time_ms != rhs.send_time_ms) + return lhs.send_time_ms < rhs.send_time_ms; + return lhs.sequence_number < rhs.sequence_number; + } +}; + void SortPacketFeedbackVector(std::vector* vec) { auto pred = [](const PacketFeedback& packet_feedback) { return packet_feedback.arrival_time_ms == PacketFeedback::kNotReceived;