From a36165c77b81e48201fb907df0eb49fa81f13295 Mon Sep 17 00:00:00 2001 From: gnish Date: Sun, 20 Aug 2017 09:19:58 -0700 Subject: [PATCH] Final version of BBR, with tweaks made for WebRTC, major changes: 1) Entering PROBE_RTT when necessary. 2) Congestion window gain of 0.65 instead of constant 4 packets. 3) {1.1, 0.9} pair instead of {1.25, 0.75} 4) Recovery mode. 5) No reaction to losses due to Recovery mode's implementation. 6) Supports encoder. 7) A new test compiling most of the simulation tests. 8) Bucket for high gain phase, disabled by default. 9) Pacer specific to BBR. BUG=webrtc:7713 Review-Url: https://codereview.webrtc.org/2999073002 Cr-Commit-Position: refs/heads/master@{#19418} --- .../include/bitrate_controller.h | 14 +- webrtc/modules/pacing/BUILD.gn | 1 + webrtc/modules/pacing/paced_sender.cc | 4 +- webrtc/modules/pacing/paced_sender.h | 9 +- webrtc/modules/pacing/pacer.h | 37 +++++ .../modules/remote_bitrate_estimator/BUILD.gn | 2 + .../bwe_simulations.cc | 60 ++++++++ .../test/bbr_paced_sender.cc | 140 ++++++++++++++++++ .../test/bbr_paced_sender.h | 92 ++++++++++++ .../remote_bitrate_estimator/test/bwe.cc | 2 +- .../test/bwe_test_framework.cc | 6 +- .../test/estimators/bbr.cc | 137 ++++++++++++----- .../test/estimators/bbr.h | 11 +- .../test/estimators/congestion_window.cc | 10 +- .../test/estimators/congestion_window.h | 4 - .../estimators/congestion_window_unittest.cc | 31 +--- .../test/estimators/min_rtt_filter.h | 35 +++-- .../estimators/min_rtt_filter_unittest.cc | 16 +- .../remote_bitrate_estimator/test/packet.h | 6 +- .../test/packet_sender.cc | 32 +++- .../test/packet_sender.h | 14 +- .../test/plot_dynamics.py | 13 +- 22 files changed, 545 insertions(+), 131 deletions(-) create mode 100644 webrtc/modules/pacing/pacer.h create mode 100644 webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.cc create mode 100644 webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.h diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index c6695e9311..f479e52d76 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -15,6 +15,8 @@ #ifndef WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_ #define WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_ +#include + #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/pacing/paced_sender.h" @@ -36,12 +38,18 @@ class BitrateObserver { virtual void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, // 0 - 255. int64_t rtt_ms) = 0; - + // TODO(gnish): Merge these two into one function. + virtual void OnNetworkChanged(uint32_t bitrate_for_encoder_bps, + uint32_t bitrate_for_pacer_bps, + bool in_probe_rtt, + int64_t target_set_time, + uint64_t congestion_window) {} + virtual void OnBytesAcked(size_t bytes) {} + virtual size_t pacer_queue_size_in_bytes() { return 0; } virtual ~BitrateObserver() {} }; -class BitrateController : public Module, - public RtcpBandwidthObserver { +class BitrateController : public Module, public RtcpBandwidthObserver { // This class collects feedback from all streams sent to a peer (via // RTCPBandwidthObservers). It does one aggregated send side bandwidth // estimation and divide the available bitrate between all its registered diff --git a/webrtc/modules/pacing/BUILD.gn b/webrtc/modules/pacing/BUILD.gn index b22c1783ed..bc9d14423c 100644 --- a/webrtc/modules/pacing/BUILD.gn +++ b/webrtc/modules/pacing/BUILD.gn @@ -18,6 +18,7 @@ rtc_static_library("pacing") { "interval_budget.h", "paced_sender.cc", "paced_sender.h", + "pacer.h", "packet_router.cc", "packet_router.h", ] diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 74a885a66f..d0a8864f14 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -37,8 +37,8 @@ const int64_t kMaxIntervalTimeMs = 30; } // namespace -// TODO(sprang): Move at least PacketQueue out to separate -// files, so that we can more easily test them. +// TODO(sprang): Move at least PacketQueue out to separate files, so that we can +// more easily test them. namespace webrtc { namespace paced_sender { diff --git a/webrtc/modules/pacing/paced_sender.h b/webrtc/modules/pacing/paced_sender.h index a1f7ebefec..74b87804de 100644 --- a/webrtc/modules/pacing/paced_sender.h +++ b/webrtc/modules/pacing/paced_sender.h @@ -15,8 +15,7 @@ #include #include -#include "webrtc/modules/include/module.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "webrtc/modules/pacing/pacer.h" #include "webrtc/rtc_base/criticalsection.h" #include "webrtc/rtc_base/optional.h" #include "webrtc/rtc_base/thread_annotations.h" @@ -31,11 +30,12 @@ class RtcEventLog; class IntervalBudget; namespace paced_sender { +class IntervalBudget; struct Packet; class PacketQueue; } // namespace paced_sender -class PacedSender : public Module, public RtpPacketSender { +class PacedSender : public Pacer { public: class PacketSender { public: @@ -93,7 +93,7 @@ class PacedSender : public Module, public RtpPacketSender { // |bitrate_bps| is our estimate of what we are allowed to send on average. // We will pace out bursts of packets at a bitrate of // |bitrate_bps| * kDefaultPaceMultiplier. - virtual void SetEstimatedBitrate(uint32_t bitrate_bps); + void SetEstimatedBitrate(uint32_t bitrate_bps) override; // Sets the minimum send bitrate and maximum padding bitrate requested by send // streams. @@ -149,7 +149,6 @@ class PacedSender : public Module, public RtpPacketSender { // Called when the prober is associated with a process thread. void ProcessThreadAttached(ProcessThread* process_thread) override; - void SetPacingFactor(float pacing_factor); void SetQueueTimeLimit(int limit_ms); diff --git a/webrtc/modules/pacing/pacer.h b/webrtc/modules/pacing/pacer.h new file mode 100644 index 0000000000..b5ac2ec597 --- /dev/null +++ b/webrtc/modules/pacing/pacer.h @@ -0,0 +1,37 @@ +/* + * 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_PACING_PACER_H_ +#define WEBRTC_MODULES_PACING_PACER_H_ + +#include "webrtc/modules/include/module.h" +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" + +namespace webrtc { +class Pacer : public Module, public RtpPacketSender { + public: + virtual void SetEstimatedBitrate(uint32_t bitrate_bps) {} + virtual void SetEstimatedBitrateAndCongestionWindow( + uint32_t bitrate_bps, + bool in_probe_rtt, + uint64_t congestion_window) {} + virtual void OnBytesAcked(size_t bytes) {} + void InsertPacket(RtpPacketSender::Priority priority, + uint32_t ssrc, + uint16_t sequence_number, + int64_t capture_time_ms, + size_t bytes, + bool retransmission) override = 0; + int64_t TimeUntilNextProcess() override = 0; + void Process() override = 0; + ~Pacer() override {} +}; +} // namespace webrtc +#endif // WEBRTC_MODULES_PACING_PACER_H_ diff --git a/webrtc/modules/remote_bitrate_estimator/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn index 8a48077a78..71487a5162 100644 --- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn +++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn @@ -61,6 +61,8 @@ if (rtc_include_tests) { rtc_static_library("bwe_simulator_lib") { testonly = true sources = [ + "test/bbr_paced_sender.cc", + "test/bbr_paced_sender.h", "test/bwe.cc", "test/bwe.h", "test/bwe_test.cc", diff --git a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc index b3cb98d4e8..92d5a55b4d 100644 --- a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc +++ b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc @@ -122,9 +122,64 @@ TEST_P(BweSimulation, Choke1000kbps500kbps1000kbps) { RunFor(60 * 1000); } +TEST_P(BweSimulation, SimulationsCompiled) { + AdaptiveVideoSource source(0, 30, 300, 0, 0); + PacedVideoSender sender(&uplink_, &source, GetParam()); + int zero = 0; + // CreateFlowIds() doesn't support passing int as a flow id, so we pass + // pointer instead. + DelayFilter delay(&uplink_, CreateFlowIds(&zero, 1)); + delay.SetOneWayDelayMs(100); + ChokeFilter filter(&uplink_, 0); + RateCounterFilter counter(&uplink_, 0, "Receiver", bwe_names[GetParam()]); + PacketReceiver receiver(&uplink_, 0, GetParam(), true, true); + filter.set_max_delay_ms(500); + filter.set_capacity_kbps(1000); + RunFor(60 * 1000); + filter.set_capacity_kbps(500); + RunFor(50 * 1000); + filter.set_capacity_kbps(1000); + RunFor(60 * 1000); + filter.set_capacity_kbps(200); + RunFor(60 * 1000); + filter.set_capacity_kbps(50); + RunFor(60 * 1000); + filter.set_capacity_kbps(200); + RunFor(60 * 1000); + filter.set_capacity_kbps(500); + RunFor(60 * 1000); + filter.set_capacity_kbps(300); + RunFor(60 * 1000); + filter.set_capacity_kbps(1000); + RunFor(60 * 1000); + const int kStartingCapacityKbps = 150; + const int kEndingCapacityKbps = 1500; + const int kStepKbps = 5; + const int kStepTimeMs = 1000; + for (int i = kStartingCapacityKbps; i <= kEndingCapacityKbps; + i += kStepKbps) { + filter.set_capacity_kbps(i); + RunFor(kStepTimeMs); + } + for (int i = kEndingCapacityKbps; i >= kStartingCapacityKbps; + i -= kStepKbps) { + filter.set_capacity_kbps(i); + RunFor(kStepTimeMs); + } + filter.set_capacity_kbps(150); + RunFor(120 * 1000); + filter.set_capacity_kbps(500); + RunFor(60 * 1000); +} + TEST_P(BweSimulation, PacerChoke1000kbps500kbps1000kbps) { AdaptiveVideoSource source(0, 30, 300, 0, 0); PacedVideoSender sender(&uplink_, &source, GetParam()); + const int kFlowId = 0; + // CreateFlowIds() doesn't support passing int as a flow id, so we pass + // pointer instead. + DelayFilter delay(&uplink_, CreateFlowIds(&kFlowId, 1)); + delay.SetOneWayDelayMs(100); ChokeFilter filter(&uplink_, 0); RateCounterFilter counter(&uplink_, 0, "Receiver", bwe_names[GetParam()]); PacketReceiver receiver(&uplink_, 0, GetParam(), true, true); @@ -262,6 +317,11 @@ TEST_P(BweSimulation, LinearDecreasingCapacity) { TEST_P(BweSimulation, PacerGoogleWifiTrace3Mbps) { PeriodicKeyFrameSource source(0, 30, 300, 0, 0, 1000); PacedVideoSender sender(&uplink_, &source, GetParam()); + int kFlowId = 0; + // CreateFlowIds() doesn't support passing int as a flow id, so we pass + // pointer instead. + DelayFilter delay(&uplink_, CreateFlowIds(&kFlowId, 1)); + delay.SetOneWayDelayMs(100); RateCounterFilter counter1(&uplink_, 0, "sender_output", bwe_names[GetParam()]); TraceBasedDeliveryFilter filter(&uplink_, 0, "link_capacity"); diff --git a/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.cc new file mode 100644 index 0000000000..3df97aad18 --- /dev/null +++ b/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.cc @@ -0,0 +1,140 @@ +/* + * 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/remote_bitrate_estimator/test/bbr_paced_sender.h" + +#include +#include +#include +#include + +#include "webrtc/modules/pacing/paced_sender.h" +#include "webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.h" +#include "webrtc/system_wrappers/include/clock.h" + +namespace webrtc { + +BbrPacedSender::BbrPacedSender(const Clock* clock, + PacedSender::PacketSender* packet_sender, + RtcEventLog* event_log) + : clock_(clock), + packet_sender_(packet_sender), + estimated_bitrate_bps_(100000), + min_send_bitrate_kbps_(0), + pacing_bitrate_kbps_(0), + time_last_update_us_(clock->TimeInMicroseconds()), + time_last_update_ms_(clock->TimeInMilliseconds()), + next_packet_send_time_(clock_->TimeInMilliseconds()), + rounding_error_time_ms_(0.0f), + packets_(), + max_data_inflight_bytes_(10000), + congestion_window_(new testing::bwe::CongestionWindow()) {} +BbrPacedSender::~BbrPacedSender() {} + +void BbrPacedSender::SetEstimatedBitrateAndCongestionWindow( + uint32_t bitrate_bps, + bool in_probe_rtt, + uint64_t congestion_window) { + estimated_bitrate_bps_ = bitrate_bps; + max_data_inflight_bytes_ = congestion_window; +} + +void BbrPacedSender::SetMinBitrate(int min_send_bitrate_bps) { + min_send_bitrate_kbps_ = min_send_bitrate_bps / 1000; + pacing_bitrate_kbps_ = + std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000); +} + +void BbrPacedSender::InsertPacket(RtpPacketSender::Priority priority, + uint32_t ssrc, + uint16_t sequence_number, + int64_t capture_time_ms, + size_t bytes, + bool retransmission) { + int64_t now_ms = clock_->TimeInMilliseconds(); + if (capture_time_ms < 0) + capture_time_ms = now_ms; + packets_.push_back(new Packet(priority, ssrc, sequence_number, + capture_time_ms, now_ms, bytes, + retransmission)); +} + +int64_t BbrPacedSender::TimeUntilNextProcess() { + // Once errors absolute value hits 1 millisecond, add compensating term to + // the |next_packet_send_time_|, so that we can send packet earlier or later, + // depending on the error. + rounding_error_time_ms_ = std::min(rounding_error_time_ms_, 1.0f); + if (rounding_error_time_ms_ < -0.9f) + rounding_error_time_ms_ = -1.0f; + int64_t result = + std::max(next_packet_send_time_ + time_last_update_ms_ - + clock_->TimeInMilliseconds(), + 0); + if (rounding_error_time_ms_ == 1.0f || rounding_error_time_ms_ == -1.0f) { + next_packet_send_time_ -= rounding_error_time_ms_; + result = std::max(next_packet_send_time_ + time_last_update_ms_ - + clock_->TimeInMilliseconds(), + 0); + rounding_error_time_ms_ = 0; + } + return result; +} + +void BbrPacedSender::OnBytesAcked(size_t bytes) { + congestion_window_->AckReceived(bytes); +} + +void BbrPacedSender::Process() { + pacing_bitrate_kbps_ = + std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000); + // If we have nothing to send, try sending again in 1 millisecond. + if (packets_.empty()) { + next_packet_send_time_ = 1; + return; + } + // If congestion window doesn't allow sending, try again in 1 millisecond. + if (packets_.front()->size_in_bytes + congestion_window_->data_inflight() > + max_data_inflight_bytes_) { + next_packet_send_time_ = 1; + return; + } + bool sent = TryToSendPacket(packets_.front()); + if (sent) { + congestion_window_->PacketSent(packets_.front()->size_in_bytes); + delete packets_.front(); + packets_.pop_front(); + time_last_update_ms_ = clock_->TimeInMilliseconds(); + if (!packets_.empty()) { + // Calculate in what time we should send current packet. + next_packet_send_time_ = (packets_.front()->size_in_bytes * 8000 + + estimated_bitrate_bps_ / 2) / + estimated_bitrate_bps_; + // As rounding errors may happen, |rounding_error_time_ms_| could be + // positive or negative depending on packet was sent earlier or later, + // after it hits certain threshold we will send a packet earlier or later + // depending on error we had so far. + rounding_error_time_ms_ += + (next_packet_send_time_ - packets_.front()->size_in_bytes * 8000.0f / + estimated_bitrate_bps_ * 1.0f); + } else { + // If sending was unsuccessful try again in 1 millisecond. + next_packet_send_time_ = 1; + } + } +} + +bool BbrPacedSender::TryToSendPacket(Packet* packet) { + PacedPacketInfo pacing_info; + return packet_sender_->TimeToSendPacket(packet->ssrc, packet->sequence_number, + packet->capture_time_ms, + packet->retransmission, pacing_info); +} + +} // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.h b/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.h new file mode 100644 index 0000000000..f5ddaeff68 --- /dev/null +++ b/webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.h @@ -0,0 +1,92 @@ +/* + * 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_REMOTE_BITRATE_ESTIMATOR_TEST_BBR_PACED_SENDER_H_ +#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TEST_BBR_PACED_SENDER_H_ + +#include +#include + +#include "webrtc/modules/pacing/paced_sender.h" +#include "webrtc/modules/pacing/pacer.h" + +namespace webrtc { +namespace testing { +namespace bwe { +class CongestionWindow; +} +} // namespace testing + +struct Packet { + Packet(RtpPacketSender::Priority priority, + uint32_t ssrc, + uint16_t seq_number, + int64_t capture_time_ms, + int64_t enqueue_time_ms, + size_t size_in_bytes, + bool retransmission) + : priority(priority), + ssrc(ssrc), + sequence_number(seq_number), + capture_time_ms(capture_time_ms), + enqueue_time_ms(enqueue_time_ms), + size_in_bytes(size_in_bytes), + retransmission(retransmission) {} + RtpPacketSender::Priority priority; + uint32_t ssrc; + uint16_t sequence_number; + int64_t capture_time_ms; + int64_t enqueue_time_ms; + size_t size_in_bytes; + bool retransmission; +}; + +class Clock; +class RtcEventLog; +struct Packet; +class BbrPacedSender : public Pacer { + public: + BbrPacedSender(const Clock* clock, + PacedSender::PacketSender* packet_sender, + RtcEventLog* event_log); + ~BbrPacedSender() override; + void SetEstimatedBitrateAndCongestionWindow( + uint32_t bitrate_bps, + bool in_probe_rtt, + uint64_t congestion_window) override; + void SetMinBitrate(int min_send_bitrate_bps); + void InsertPacket(RtpPacketSender::Priority priority, + uint32_t ssrc, + uint16_t sequence_number, + int64_t capture_time_ms, + size_t bytes, + bool retransmission) override; + int64_t TimeUntilNextProcess() override; + void OnBytesAcked(size_t bytes) override; + void Process() override; + bool TryToSendPacket(Packet* packet); + + private: + const Clock* const clock_; + PacedSender::PacketSender* const packet_sender_; + uint32_t estimated_bitrate_bps_; + uint32_t min_send_bitrate_kbps_; + uint32_t pacing_bitrate_kbps_; + int64_t time_last_update_us_; + int64_t time_last_update_ms_; + int64_t next_packet_send_time_; + float rounding_error_time_ms_; + std::list packets_; + // TODO(gnish): integrate |max_data_inflight| into congestion window class. + size_t max_data_inflight_bytes_; + std::unique_ptr congestion_window_; +}; +} // namespace webrtc +#endif // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TEST_BBR_PACED_SENDER_H_ diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe.cc index 755e59d04b..abc301cfca 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe.cc @@ -95,7 +95,7 @@ BweSender* CreateBweSender(BandwidthEstimatorType estimator, case kNadaEstimator: return new NadaBweSender(kbps, observer, clock); case kBbrEstimator: - return new BbrBweSender(clock); + return new BbrBweSender(observer, clock); case kTcpEstimator: FALLTHROUGH(); case kNullEstimator: diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc index edaeffc5e8..51c1f55afe 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc @@ -157,7 +157,7 @@ BbrBweFeedback::BbrBweFeedback( int flow_id, int64_t send_time_us, int64_t latest_send_time_ms, - const std::vector& packet_feedback_vector) + const std::vector& packet_feedback_vector) : FeedbackPacket(flow_id, send_time_us, latest_send_time_ms), packet_feedback_vector_(packet_feedback_vector) {} @@ -518,12 +518,12 @@ void ChokeFilter::RunFor(int64_t /*time_ms*/, Packets* in_out) { for (PacketsIt it = in_out->begin(); it != in_out->end(); ) { int64_t earliest_send_time_us = std::max(last_send_time_us_, (*it)->send_time_us()); - int64_t new_send_time_us = earliest_send_time_us + ((*it)->payload_size() * 8 * 1000 + capacity_kbps_ / 2) / capacity_kbps_; - + BWE_TEST_LOGGING_PLOT(0, "MaxThroughput_", new_send_time_us / 1000, + capacity_kbps_); if (delay_cap_helper_->ShouldSendPacket(new_send_time_us, (*it)->send_time_us())) { (*it)->set_send_time_us(new_send_time_us); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.cc index a45700b19e..64e6bf253c 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.cc @@ -34,7 +34,7 @@ const float kDrainGain = 1 / kHighGain; const float kStartupGrowthTarget = 1.25f; const int kMaxRoundsWithoutGrowth = 3; // Pacing gain values for Probe Bandwidth mode. -const float kPacingGain[] = {1.25, 0.75, 1, 1, 1, 1, 1, 1}; +const float kPacingGain[] = {1.1, 0.9, 1, 1, 1, 1, 1, 1}; const size_t kGainCycleLength = sizeof(kPacingGain) / sizeof(kPacingGain[0]); // Least number of rounds PROBE_RTT should last. const int kProbeRttDurationRounds = 1; @@ -46,7 +46,7 @@ const float kTargetCongestionWindowGain = 1; // equal to 1, but in practice because of delayed acks and the way networks // work, it is nice to have some extra room in congestion window for full link // utilization. Value chosen by observations on different tests. -const float kCruisingCongestionWindowGain = 1.5f; +const float kCruisingCongestionWindowGain = 2; // Pacing gain specific for Recovery mode. Chosen by experiments in simulation // tool. const float kRecoveryPacingGain = 0.5f; @@ -61,10 +61,29 @@ const float kRttIncreaseThreshold = 3; // Threshold to assume average RTT has decreased for a round. Chosen by // experiments in simulation tool. const float kRttDecreaseThreshold = 1.5f; +// If |kCongestionWindowThreshold| of the congestion window is filled up, tell +// encoder to stop, to avoid building sender side queues. +const float kCongestionWindowThreshold = 0.69f; +// Duration we send at |kDefaultRatebps| in order to ensure BBR has data to work +// with. +const int64_t kDefaultDurationMs = 200; +const int64_t kDefaultRatebps = 300000; +// Congestion window gain for PROBE_RTT mode. +const float kProbeRttCongestionWindowGain = 0.65f; +// We need to be sure that data inflight has increased by at least +// |kTargetCongestionWindowGainForHighGain| compared to the congestion window in +// PROBE_BW's high gain phase, to make ramp-up quicker. As high gain value has +// been decreased from 1.25 to 1.1 we need to make +// |kTargetCongestionWindowGainForHighGain| slightly higher than the actual high +// gain value. +const float kTargetCongestionWindowGainForHighGain = 1.15f; +// Encoder rate gain value for PROBE_RTT mode. +const float kEncoderRateGainForProbeRtt = 0.1f; } // namespace -BbrBweSender::BbrBweSender(Clock* clock) +BbrBweSender::BbrBweSender(BitrateObserver* observer, Clock* clock) : BweSender(0), + observer_(observer), clock_(clock), mode_(STARTUP), max_bandwidth_filter_(new MaxBandwidthFilter()), @@ -113,14 +132,13 @@ void BbrBweSender::CalculatePacingRate() { max_bandwidth_filter_->max_bandwidth_estimate_bps() * pacing_gain_; } +// Declare lost packets as acked. void BbrBweSender::HandleLoss(uint64_t last_acked_packet, uint64_t recently_acked_packet) { - // Logic specific to wrapping sequence numbers. - if (!last_acked_packet) - return; for (uint16_t i = last_acked_packet + 1; AheadOrAt(recently_acked_packet - 1, i); i++) { congestion_window_->AckReceived(packet_stats_[i].payload_size_bytes); + observer_->OnBytesAcked(packet_stats_[i].payload_size_bytes); } } @@ -148,7 +166,7 @@ void BbrBweSender::GiveFeedback(const FeedbackPacket& feedback) { last_packet_ack_time_ = now_ms; const BbrBweFeedback& fb = static_cast(feedback); // feedback_vector holds values of acknowledged packets' sequence numbers. - const std::vector& feedback_vector = fb.packet_feedback_vector(); + const std::vector& feedback_vector = fb.packet_feedback_vector(); // Go through all the packets acked, update variables/containers accordingly. for (uint16_t sequence_number : feedback_vector) { // Completing packet information with a recently received ack. @@ -175,9 +193,7 @@ void BbrBweSender::GiveFeedback(const FeedbackPacket& feedback) { bytes_acked_ - packet->payload_size_bytes / 2; high_gain_over_ = true; } - // Notify pacer that an ack was received, to adjust data inflight. - // TODO(gnish): Add implementation for BitrateObserver class, to notify - // pacer about incoming acks. + observer_->OnBytesAcked(packet->payload_size_bytes); congestion_window_->AckReceived(packet->payload_size_bytes); HandleLoss(last_packet_acked_sequence_number_, packet->sequence_number); last_packet_acked_sequence_number_ = packet->sequence_number; @@ -196,9 +212,8 @@ void BbrBweSender::GiveFeedback(const FeedbackPacket& feedback) { round_trip_end_ = last_packet_sent_sequence_number_; } } - bool min_rtt_expired = false; - min_rtt_expired = - UpdateBandwidthAndMinRtt(now_ms, feedback_vector, bytes_acked_); + TryEnteringProbeRtt(now_ms); + UpdateBandwidthAndMinRtt(now_ms, feedback_vector, bytes_acked_); if (new_round_started && !full_bandwidth_reached_) { full_bandwidth_reached_ = max_bandwidth_filter_->FullBandwidthReached( kStartupGrowthTarget, kMaxRoundsWithoutGrowth); @@ -221,19 +236,37 @@ void BbrBweSender::GiveFeedback(const FeedbackPacket& feedback) { TryExitingRecovery(new_round_started); break; } - TryEnteringProbeRtt(now_ms); TryEnteringRecovery(new_round_started); // Comment this line to disable // entering Recovery mode. - for (uint64_t f : feedback_vector) + for (uint16_t f : feedback_vector) AddToPastRtts(packet_stats_[f].ack_time_ms - packet_stats_[f].send_time_ms); CalculatePacingRate(); + size_t cwnd = congestion_window_->GetCongestionWindow( + mode_, max_bandwidth_filter_->max_bandwidth_estimate_bps(), + min_rtt_filter_->min_rtt_ms(), congestion_window_gain_); // Make sure we don't get stuck when pacing_rate is 0, because of simulation // tool specifics. if (!pacing_rate_bps_) pacing_rate_bps_ = 100; BWE_TEST_LOGGING_PLOT(1, "SendRate", now_ms, pacing_rate_bps_ / 1000); - // TODO(gnish): Add implementation for BitrateObserver class to update pacing - // rate for the pacer and the encoder. + int64_t rate_for_pacer_bps = pacing_rate_bps_; + int64_t rate_for_encoder_bps = pacing_rate_bps_; + if (congestion_window_->data_inflight() >= cwnd * kCongestionWindowThreshold) + rate_for_encoder_bps = 0; + // We dont completely stop sending during PROBE_RTT, so we need encoder to + // produce something, another way of doing this would be telling encoder to + // stop and send padding instead of actual data. + if (mode_ == PROBE_RTT) + rate_for_encoder_bps = rate_for_pacer_bps * kEncoderRateGainForProbeRtt; + // Send for 300 kbps for first 200 ms, so that BBR has data to work with. + if (now_ms <= kDefaultDurationMs) + observer_->OnNetworkChanged( + kDefaultRatebps, kDefaultRatebps, false, + clock_->TimeInMicroseconds() + kFeedbackIntervalsMs * 1000, cwnd); + else + observer_->OnNetworkChanged( + rate_for_encoder_bps, rate_for_pacer_bps, mode_ == PROBE_RTT, + clock_->TimeInMicroseconds() + kFeedbackIntervalsMs * 1000, cwnd); } size_t BbrBweSender::TargetCongestionWindow(float gain) { @@ -251,16 +284,16 @@ rtc::Optional BbrBweSender::CalculateBandwidthSample( int64_t ack_time_delta_ms) { rtc::Optional bandwidth_sample; if (send_time_delta_ms > 0) - *bandwidth_sample = data_sent_bytes * 8000 / send_time_delta_ms; + bandwidth_sample.emplace(data_sent_bytes * 8000 / send_time_delta_ms); rtc::Optional ack_rate; if (ack_time_delta_ms > 0) - *ack_rate = data_acked_bytes * 8000 / ack_time_delta_ms; + ack_rate.emplace(data_acked_bytes * 8000 / ack_time_delta_ms); // If send rate couldn't be calculated automaticaly set |bandwidth_sample| to // ack_rate. if (!bandwidth_sample) bandwidth_sample = ack_rate; if (bandwidth_sample && ack_rate) - *bandwidth_sample = std::min(*bandwidth_sample, *ack_rate); + bandwidth_sample.emplace(std::min(*bandwidth_sample, *ack_rate)); return bandwidth_sample; } @@ -285,12 +318,12 @@ void BbrBweSender::AddSampleForHighGain() { first_packet_send_time_during_high_gain_ms_.reset(); } -bool BbrBweSender::UpdateBandwidthAndMinRtt( +void BbrBweSender::UpdateBandwidthAndMinRtt( int64_t now_ms, - const std::vector& feedback_vector, + const std::vector& feedback_vector, int64_t bytes_acked) { rtc::Optional min_rtt_sample_ms; - for (uint64_t f : feedback_vector) { + for (uint16_t f : feedback_vector) { PacketStats packet = packet_stats_[f]; size_t data_sent_bytes = packet.data_sent_bytes - packet.data_sent_before_last_sent_packet_bytes; @@ -306,20 +339,22 @@ bool BbrBweSender::UpdateBandwidthAndMinRtt( if (bandwidth_sample) max_bandwidth_filter_->AddBandwidthSample(*bandwidth_sample, round_count_); - AddSampleForHighGain(); // Comment to disable bucket for high gain. + // AddSampleForHighGain(); // Comment to disable bucket for high gain. if (!min_rtt_sample_ms) - *min_rtt_sample_ms = packet.ack_time_ms - packet.send_time_ms; + min_rtt_sample_ms.emplace(packet.ack_time_ms - packet.send_time_ms); else *min_rtt_sample_ms = std::min(*min_rtt_sample_ms, packet.ack_time_ms - packet.send_time_ms); BWE_TEST_LOGGING_PLOT(1, "MinRtt", now_ms, packet.ack_time_ms - packet.send_time_ms); } - if (!min_rtt_sample_ms) - return false; - min_rtt_filter_->AddRttSample(*min_rtt_sample_ms, now_ms); - bool min_rtt_expired = min_rtt_filter_->MinRttExpired(now_ms); - return min_rtt_expired; + // We only feed RTT samples into the min_rtt filter which were not produced + // during 1.1 gain phase, to ensure they contain no queueing delay. But if the + // rtt sample from 1.1 gain phase improves the current estimate then we should + // make it as a new best estimate. + if (pacing_gain_ <= 1.0f || !min_rtt_filter_->min_rtt_ms() || + *min_rtt_filter_->min_rtt_ms() >= *min_rtt_sample_ms) + min_rtt_filter_->AddRttSample(*min_rtt_sample_ms, now_ms); } void BbrBweSender::EnterStartup() { @@ -365,8 +400,9 @@ void BbrBweSender::TryUpdatingCyclePhase(int64_t now_ms) { // If BBR was probing and it couldn't increase data inflight sufficiently in // one min_rtt time, continue probing. BBR design doc isn't clear about this, // but condition helps in quicker ramp-up and performs better. - if (pacing_gain_ > 1.0 && congestion_window_->data_inflight() < - TargetCongestionWindow(pacing_gain_)) + if (pacing_gain_ > 1.0 && + congestion_window_->data_inflight() < + TargetCongestionWindow(kTargetCongestionWindowGainForHighGain)) advance_cycle_phase = false; // If BBR has already drained queues there is no point in continuing draining // phase. @@ -385,6 +421,7 @@ void BbrBweSender::TryEnteringProbeRtt(int64_t now_ms) { if (min_rtt_filter_->MinRttExpired(now_ms) && mode_ != PROBE_RTT) { mode_ = PROBE_RTT; pacing_gain_ = 1; + congestion_window_gain_ = kProbeRttCongestionWindowGain; probe_rtt_start_time_ms_ = now_ms; minimum_congestion_window_start_time_ms_.reset(); } @@ -397,22 +434,23 @@ void BbrBweSender::TryEnteringProbeRtt(int64_t now_ms) { void BbrBweSender::TryExitingProbeRtt(int64_t now_ms, int64_t round) { if (!minimum_congestion_window_start_time_ms_) { if (congestion_window_->data_inflight() <= - CongestionWindow::kMinimumCongestionWindowBytes) { - *minimum_congestion_window_start_time_ms_ = now_ms; + TargetCongestionWindow(kProbeRttCongestionWindowGain)) { + minimum_congestion_window_start_time_ms_.emplace(now_ms); minimum_congestion_window_start_round_ = round; } } else { if (now_ms - *minimum_congestion_window_start_time_ms_ >= kProbeRttDurationMs && round - minimum_congestion_window_start_round_ >= - kProbeRttDurationRounds) + kProbeRttDurationRounds) { EnterProbeBw(now_ms); + } } } void BbrBweSender::TryEnteringRecovery(bool new_round_started) { - // If we are already in Recovery don't try to enter. - if (mode_ == RECOVERY || !new_round_started || !full_bandwidth_reached_) + if (mode_ == RECOVERY || !new_round_started || !full_bandwidth_reached_ || + !min_rtt_filter_->min_rtt_ms()) return; uint64_t increased_rtt_round_counter = 0; // If average RTT for past |kPastRttsFilterSize| rounds has been more than @@ -460,7 +498,8 @@ void BbrBweSender::OnPacketsSent(const Packets& packets) { last_packet_sent_sequence_number_ = media_packet->sequence_number(); // If this is the first packet sent for high gain phase, save data for it. if (!first_packet_send_time_during_high_gain_ms_ && pacing_gain_ > 1) { - *first_packet_send_time_during_high_gain_ms_ = last_packet_send_time_; + first_packet_send_time_during_high_gain_ms_.emplace( + last_packet_send_time_); data_sent_before_high_gain_started_bytes_ = bytes_sent_ - media_packet->payload_size() / 2; first_packet_seq_num_during_high_gain_ = @@ -483,15 +522,31 @@ void BbrBweSender::OnPacketsSent(const Packets& packets) { void BbrBweSender::Process() {} BbrBweReceiver::BbrBweReceiver(int flow_id) - : BweReceiver(flow_id, kReceivingRateTimeWindowMs), clock_(0) {} + : BweReceiver(flow_id, kReceivingRateTimeWindowMs), + clock_(0), + packet_feedbacks_(), + last_feedback_ms_(0) {} BbrBweReceiver::~BbrBweReceiver() {} void BbrBweReceiver::ReceivePacket(int64_t arrival_time_ms, - const MediaPacket& media_packet) {} + const MediaPacket& media_packet) { + packet_feedbacks_.push_back(media_packet.sequence_number()); + BweReceiver::ReceivePacket(arrival_time_ms, media_packet); +} FeedbackPacket* BbrBweReceiver::GetFeedback(int64_t now_ms) { - return nullptr; + last_feedback_ms_ = now_ms; + int64_t corrected_send_time_ms = 0L; + if (!received_packets_.empty()) { + PacketIdentifierNode* latest = *(received_packets_.begin()); + corrected_send_time_ms = + latest->send_time_ms + now_ms - latest->arrival_time_ms; + } + FeedbackPacket* fb = new BbrBweFeedback( + flow_id_, now_ms * 1000, corrected_send_time_ms, packet_feedbacks_); + packet_feedbacks_.clear(); + return fb; } } // namespace bwe } // namespace testing diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.h index 63dc78b123..6f4d831bc2 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.h @@ -30,7 +30,7 @@ class MinRttFilter; class CongestionWindow; class BbrBweSender : public BweSender { public: - explicit BbrBweSender(Clock* clock); + explicit BbrBweSender(BitrateObserver* observer, Clock* clock); virtual ~BbrBweSender(); enum Mode { // Startup phase. @@ -113,8 +113,8 @@ class BbrBweSender : public BweSender { private: void EnterStartup(); - bool UpdateBandwidthAndMinRtt(int64_t now_ms, - const std::vector& feedback_vector, + void UpdateBandwidthAndMinRtt(int64_t now_ms, + const std::vector& feedback_vector, int64_t bytes_acked); void TryExitingStartup(); void TryExitingDrain(int64_t now_ms); @@ -145,6 +145,7 @@ class BbrBweSender : public BweSender { // declare those packets as lost immediately. void HandleLoss(uint64_t last_acked_packet, uint64_t recently_acked_packet); void AddToPastRtts(int64_t rtt_sample_ms); + BitrateObserver* observer_; Clock* const clock_; Mode mode_; std::unique_ptr max_bandwidth_filter_; @@ -229,10 +230,10 @@ class BbrBweReceiver : public BweReceiver { void ReceivePacket(int64_t arrival_time_ms, const MediaPacket& media_packet) override; FeedbackPacket* GetFeedback(int64_t now_ms) override; - private: SimulatedClock clock_; - std::vector packet_feedbacks_; + std::vector packet_feedbacks_; + int64_t last_feedback_ms_; }; } // namespace bwe } // namespace testing diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.cc index 3d357704b5..5c01e89aa1 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.cc @@ -27,8 +27,6 @@ namespace { const int kStartingCongestionWindowBytes = 6000; } // namespace -const int CongestionWindow::kMinimumCongestionWindowBytes; - CongestionWindow::CongestionWindow() : data_inflight_bytes_(0) {} CongestionWindow::~CongestionWindow() {} @@ -37,8 +35,6 @@ int CongestionWindow::GetCongestionWindow(BbrBweSender::Mode mode, int64_t bandwidth_estimate_bps, rtc::Optional min_rtt_ms, float gain) { - if (mode == BbrBweSender::PROBE_RTT) - return CongestionWindow::kMinimumCongestionWindowBytes; return GetTargetCongestionWindow(bandwidth_estimate_bps, min_rtt_ms, gain); } @@ -47,6 +43,7 @@ void CongestionWindow::PacketSent(size_t sent_packet_size_bytes) { } void CongestionWindow::AckReceived(size_t received_packet_size_bytes) { + RTC_DCHECK_GE(data_inflight_bytes_ >= received_packet_size_bytes, true); data_inflight_bytes_ -= received_packet_size_bytes; } @@ -57,14 +54,13 @@ int CongestionWindow::GetTargetCongestionWindow( // If we have no rtt sample yet, return the starting congestion window size. if (!min_rtt_ms) return gain * kStartingCongestionWindowBytes; - int bdp = *min_rtt_ms * bandwidth_estimate_bps; + int bdp = *min_rtt_ms * bandwidth_estimate_bps / 8000; int congestion_window = bdp * gain; // Congestion window could be zero in rare cases, when either no bandwidth // estimate is available, or path's min_rtt value is zero. if (!congestion_window) congestion_window = gain * kStartingCongestionWindowBytes; - return std::max(congestion_window, - CongestionWindow::kMinimumCongestionWindowBytes); + return congestion_window; } } // namespace bwe } // namespace testing diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.h index b9b2300d19..105a748d63 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window.h @@ -21,10 +21,6 @@ namespace testing { namespace bwe { class CongestionWindow { public: - // Size of congestion window while in PROBE_RTT mode, suggested by BBR's - // source code of QUIC's implementation. - static const int kMinimumCongestionWindowBytes = 4000; - CongestionWindow(); ~CongestionWindow(); int GetCongestionWindow(BbrBweSender::Mode mode, diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window_unittest.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window_unittest.cc index 0c6d59cbc6..2415c137ac 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/congestion_window_unittest.cc @@ -17,9 +17,8 @@ namespace webrtc { namespace testing { namespace bwe { namespace { -// These are the same values used in CongestionWindow class. +// Same value used in CongestionWindow class. const int64_t kStartingCongestionWindow = 6000; -const int64_t kMinimumCongestionWindow = 4000; } // namespace TEST(CongestionWindowTest, InitializationCheck) { @@ -51,33 +50,13 @@ TEST(CongestionWindowTest, ZeroBandwidthDelayProduct) { EXPECT_EQ(target_congestion_window, 2.885f * kStartingCongestionWindow); } -TEST(CongestionWindowTest, BelowMinimumTargetCongestionWindow) { - CongestionWindow congestion_window; - int64_t target_congestion_window = - congestion_window.GetTargetCongestionWindow( - 100, rtc::Optional(2), 2.885f); - EXPECT_EQ(target_congestion_window, kMinimumCongestionWindow); -} - -TEST(CongestionWindowTest, AboveMinimumTargetCongestionWindow) { - CongestionWindow congestion_window; - int64_t target_congestion_window = - congestion_window.GetTargetCongestionWindow( - 100000, rtc::Optional(2), 2.885f); - EXPECT_EQ(target_congestion_window, 577000); -} - -TEST(CongestionWindowTest, MinimumCongestionWindow) { - CongestionWindow congestion_window; - int64_t cwnd = congestion_window.GetCongestionWindow( - BbrBweSender::PROBE_RTT, 100, rtc::Optional(100), 2.885f); - EXPECT_EQ(cwnd, kMinimumCongestionWindow); -} - TEST(CongestionWindowTest, CalculateCongestionWindow) { CongestionWindow congestion_window; int64_t cwnd = congestion_window.GetCongestionWindow( - BbrBweSender::STARTUP, 100, rtc::Optional(100l), 2.885f); + BbrBweSender::STARTUP, 800000, rtc::Optional(100l), 2.885f); + EXPECT_EQ(cwnd, 28850); + cwnd = congestion_window.GetCongestionWindow( + BbrBweSender::STARTUP, 400000, rtc::Optional(200l), 2.885f); EXPECT_EQ(cwnd, 28850); } } // namespace bwe diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter.h index b4932e51ca..f45437eee5 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter.h @@ -14,6 +14,7 @@ #include #include +#include #include "webrtc/rtc_base/optional.h" @@ -21,12 +22,17 @@ namespace webrtc { namespace testing { namespace bwe { -// Expiration time for min_rtt sample, which is set to 10 seconds according to -// BBR design doc. -const int64_t kMinRttFilterSizeMs = 10000; +// Average rtt for past |kRttFilterSize| packets should grow by +// |kRttIncreaseThresholdForExpiry| in order to enter PROBE_RTT mode and expire. +// old min_rtt estimate. +const float kRttIncreaseThresholdForExpiry = 2.3f; +const size_t kRttFilterSize = 25; class MinRttFilter { public: + // This class implements a simple filter to ensure that PROBE_RTT is only + // entered when RTTs start to increase, instead of fixed 10 second window as + // in orginal BBR design doc, to avoid unnecessary freezes in stream. MinRttFilter() {} ~MinRttFilter() {} @@ -34,20 +40,31 @@ class MinRttFilter { void AddRttSample(int64_t rtt_ms, int64_t now_ms) { if (!min_rtt_ms_ || rtt_ms <= *min_rtt_ms_ || MinRttExpired(now_ms)) { min_rtt_ms_.emplace(rtt_ms); - discovery_time_ms_ = now_ms; } + rtt_samples_.push_back(rtt_ms); + if (rtt_samples_.size() > kRttFilterSize) + rtt_samples_.pop_front(); } - int64_t discovery_time() { return discovery_time_ms_; } - // Checks whether or not last discovered min_rtt value is older than x - // milliseconds. + // Checks whether or not last RTT values for past |kRttFilterSize| packets + // started to increase, meaning we have to update min_rtt estimate. bool MinRttExpired(int64_t now_ms) { - return now_ms - discovery_time_ms_ >= kMinRttFilterSizeMs; + if (rtt_samples_.size() < kRttFilterSize || !min_rtt_ms_) + return false; + int64_t sum_of_rtts_ms = 0; + for (int64_t i : rtt_samples_) + sum_of_rtts_ms += i; + if (sum_of_rtts_ms >= + *min_rtt_ms_ * kRttIncreaseThresholdForExpiry * kRttFilterSize) { + rtt_samples_.clear(); + return true; + } + return false; } private: rtc::Optional min_rtt_ms_; - int64_t discovery_time_ms_ = 0; + std::list rtt_samples_; }; } // namespace bwe } // namespace testing diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter_unittest.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter_unittest.cc index a8e76e127e..5cf08d4111 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/min_rtt_filter_unittest.cc @@ -18,28 +18,24 @@ namespace bwe { TEST(MinRttFilterTest, InitializationCheck) { MinRttFilter min_rtt_filter; EXPECT_FALSE(min_rtt_filter.min_rtt_ms()); - EXPECT_EQ(min_rtt_filter.discovery_time(), 0); } TEST(MinRttFilterTest, AddRttSample) { MinRttFilter min_rtt_filter; min_rtt_filter.AddRttSample(120, 5); - EXPECT_EQ(min_rtt_filter.min_rtt_ms(), 120); - EXPECT_EQ(min_rtt_filter.discovery_time(), 5); + EXPECT_EQ(*min_rtt_filter.min_rtt_ms(), 120); min_rtt_filter.AddRttSample(121, 6); - EXPECT_EQ(min_rtt_filter.discovery_time(), 5); min_rtt_filter.AddRttSample(119, 7); - EXPECT_EQ(min_rtt_filter.discovery_time(), 7); min_rtt_filter.AddRttSample(140, 10007); - EXPECT_EQ(min_rtt_filter.discovery_time(), 10007); - EXPECT_EQ(min_rtt_filter.min_rtt_ms(), 140); + EXPECT_EQ(*min_rtt_filter.min_rtt_ms(), 119); } TEST(MinRttFilterTest, MinRttExpired) { MinRttFilter min_rtt_filter; - min_rtt_filter.AddRttSample(120, 5); - EXPECT_EQ(min_rtt_filter.MinRttExpired(10006), true); - EXPECT_EQ(min_rtt_filter.MinRttExpired(10), false); + for (int i = 1; i <= 25; i++) + min_rtt_filter.AddRttSample(i, i); + EXPECT_EQ(min_rtt_filter.MinRttExpired(25), true); + EXPECT_EQ(min_rtt_filter.MinRttExpired(24), false); } } // namespace bwe } // namespace testing diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet.h b/webrtc/modules/remote_bitrate_estimator/test/packet.h index 525397c50d..05e1267436 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet.h @@ -114,15 +114,15 @@ class BbrBweFeedback : public FeedbackPacket { BbrBweFeedback(int flow_id, int64_t send_time_us, int64_t latest_send_time_ms, - const std::vector& packet_feedback_vector); + const std::vector& packet_feedback_vector); virtual ~BbrBweFeedback() {} - const std::vector& packet_feedback_vector() const { + const std::vector& packet_feedback_vector() const { return packet_feedback_vector_; } private: - const std::vector packet_feedback_vector_; + const std::vector packet_feedback_vector_; }; class RembFeedback : public FeedbackPacket { diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index de4d93def3..33209cab26 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -15,6 +15,8 @@ #include #include "webrtc/modules/include/module_common_types.h" +#include "webrtc/modules/pacing/pacer.h" +#include "webrtc/modules/remote_bitrate_estimator/test/bbr_paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" #include "webrtc/modules/remote_bitrate_estimator/test/metric_recorder.h" #include "webrtc/rtc_base/checks.h" @@ -156,9 +158,12 @@ uint32_t VideoSender::TargetBitrateKbps() { PacedVideoSender::PacedVideoSender(PacketProcessorListener* listener, VideoSource* source, BandwidthEstimatorType estimator) - : VideoSender(listener, source, estimator), pacer_(&clock_, this, nullptr) { - modules_.push_back(&pacer_); - pacer_.SetEstimatedBitrate(source->bits_per_second()); + : VideoSender(listener, source, estimator), + // Ugly hack to use BBR's pacer. + // TODO(gnish): Make pacer choice dependant on the algorithm being used. + pacer_(new BbrPacedSender(&clock_, this, nullptr)) { + modules_.push_back(pacer_.get()); + pacer_->SetEstimatedBitrate(source->bits_per_second()); } PacedVideoSender::~PacedVideoSender() { @@ -204,10 +209,11 @@ void PacedVideoSender::RunFor(int64_t time_ms, Packets* in_out) { if (!generated_packets.empty()) { for (Packet* packet : generated_packets) { MediaPacket* media_packet = static_cast(packet); - pacer_.InsertPacket( + pacer_->InsertPacket( PacedSender::kNormalPriority, media_packet->header().ssrc, media_packet->header().sequenceNumber, media_packet->send_time_ms(), media_packet->payload_size(), false); + pacer_queue_size_in_bytes_ += media_packet->payload_size(); pacer_queue_.push_back(packet); assert(pacer_queue_.size() < 10000); } @@ -284,11 +290,11 @@ bool PacedVideoSender::TimeToSendPacket(uint32_t ssrc, // Make sure a packet is never paced out earlier than when it was put into // the pacer. assert(pace_out_time_ms >= media_packet->send_time_ms()); - media_packet->SetAbsSendTimeMs(pace_out_time_ms); media_packet->set_send_time_us(1000 * pace_out_time_ms); media_packet->set_sender_timestamp_us(1000 * pace_out_time_ms); queue_.push_back(media_packet); + pacer_queue_size_in_bytes_ -= media_packet->payload_size(); pacer_queue_.erase(it); return true; } @@ -305,7 +311,21 @@ void PacedVideoSender::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_lost, int64_t rtt) { VideoSender::OnNetworkChanged(target_bitrate_bps, fraction_lost, rtt); - pacer_.SetEstimatedBitrate(target_bitrate_bps); + pacer_->SetEstimatedBitrate(target_bitrate_bps); +} + +void PacedVideoSender::OnNetworkChanged(uint32_t bitrate_for_encoder_bps, + uint32_t bitrate_for_pacer_bps, + bool in_probe_rtt, + int64_t target_set_time, + uint64_t congestion_window) { + VideoSender::OnNetworkChanged(bitrate_for_encoder_bps, 0u, 0u); + pacer_->SetEstimatedBitrateAndCongestionWindow( + bitrate_for_pacer_bps, in_probe_rtt, congestion_window); +} + +void PacedVideoSender::OnBytesAcked(size_t bytes) { + pacer_->OnBytesAcked(bytes); } const int kNoLimit = std::numeric_limits::max(); diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h index 0df61b039a..86ad0e812f 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h @@ -81,7 +81,6 @@ class VideoSender : public PacketSender, public BitrateObserver { void OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_lost, int64_t rtt) override; - void Pause() override; void Resume(int64_t paused_time_ms) override; @@ -123,12 +122,23 @@ class PacedVideoSender : public VideoSender, public PacedSender::PacketSender { uint8_t fraction_lost, int64_t rtt) override; + void OnNetworkChanged(uint32_t bitrate_for_encoder_bps, + uint32_t bitrate_for_pacer_bps, + bool in_probe_rtt, + int64_t rtt, + uint64_t congestion_window) override; + size_t pacer_queue_size_in_bytes() override { + return pacer_queue_size_in_bytes_; + } + void OnBytesAcked(size_t bytes) override; + private: int64_t TimeUntilNextProcess(const std::list& modules); void CallProcess(const std::list& modules); void QueuePackets(Packets* batch, int64_t end_of_batch_time_us); - PacedSender pacer_; + size_t pacer_queue_size_in_bytes_ = 0; + std::unique_ptr pacer_; Packets queue_; Packets pacer_queue_; diff --git a/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py b/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py index 51aca1cf4c..02b20523e8 100755 --- a/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py +++ b/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py @@ -78,7 +78,6 @@ class Figure(object): axis = fig.add_subplot(n, 1, i+1) self.subplots[i].PlotSubplot(axis) - class Subplot(object): def __init__(self, var_names, xlabel, ylabel): self.xlabel = xlabel @@ -111,10 +110,12 @@ class Subplot(object): y = [sample[1] for sample in self.samples[alg_name][ssrc][var_name]] x = numpy.array(x) y = numpy.array(y) - ssrc_count = len(self.samples[alg_name].keys()) l = GenerateLabel(var_name, ssrc, ssrc_count, alg_name) - plt.plot(x, y, label=l, linewidth=2.0) + if l == 'MaxThroughput_': + plt.plot(x, y, label=l, linewidth=4.0) + else: + plt.plot(x, y, label=l, linewidth=2.0) count += 1 plt.grid(True) @@ -148,8 +149,12 @@ def main(): target_bitrate.AddSubplot(['target_bitrate_bps', 'acknowledged_bitrate'], "Time (s)", "Bitrate (bps)") + min_rtt_state = Figure("MinRttState") + min_rtt_state.AddSubplot(['MinRtt'], "Time (s)", "Time (ms)") + # Select which figures to plot here. - figures = [receiver, detector_state, trendline_state, target_bitrate] + figures = [receiver, detector_state, trendline_state, target_bitrate, + min_rtt_state] # Add samples to the figures. for line in sys.stdin: