From 492ee28b731e33913acbd4bd62dd45432df58af2 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Thu, 27 Oct 2016 17:19:20 +0200 Subject: [PATCH] Use bayesian estimate of acked bitrate. This helps a lot to avoid reducing the bitrate too quickly when there's a short period of very few packets delivered, followed by the rate resuming at the regular rate. It specifically avoids the BWE going down to super low values as a response delay spikes. BUG=webrtc:6566 R=terelius@webrtc.org Review URL: https://codereview.webrtc.org/2422063002 . Cr-Commit-Position: refs/heads/master@{#14802} --- .../bitrate_controller_impl.cc | 3 - .../congestion_controller/delay_based_bwe.cc | 129 +++++++++++++++--- .../congestion_controller/delay_based_bwe.h | 28 +++- .../delay_based_bwe_unittest.cc | 54 +++++--- .../delay_based_bwe_unittest_helper.cc | 27 ++-- .../delay_based_bwe_unittest_helper.h | 3 +- webrtc/tools/event_log_visualizer/analyzer.cc | 7 +- 7 files changed, 189 insertions(+), 62 deletions(-) diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index a1f1fd3a70..a33d97e0b6 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -38,9 +38,6 @@ class BitrateControllerImpl::RtcpBandwidthObserverImpl void OnReceivedRtcpReceiverReport(const ReportBlockList& report_blocks, int64_t rtt, int64_t now_ms) override { - if (report_blocks.empty()) - return; - int fraction_lost_aggregate = 0; int total_number_of_packets = 0; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index fb6ffd157a..af27505268 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -10,9 +10,8 @@ #include "webrtc/modules/congestion_controller/delay_based_bwe.h" -#include - #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" @@ -21,6 +20,7 @@ #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "webrtc/system_wrappers/include/field_trial.h" #include "webrtc/system_wrappers/include/metrics.h" #include "webrtc/typedefs.h" @@ -35,16 +35,106 @@ 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; + +const char kBitrateEstimateExperiment[] = "WebRTC-ImprovedBitrateEstimate"; + +bool BitrateEstimateExperimentIsEnabled() { + return webrtc::field_trial::FindFullName(kBitrateEstimateExperiment) == + "Enabled"; +} } // 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), + old_estimator_(kBitrateWindowMs, 8000), + in_experiment_(BitrateEstimateExperimentIsEnabled()) {} + +void DelayBasedBwe::BitrateEstimator::Update(int64_t now_ms, int bytes) { + if (!in_experiment_) { + old_estimator_.Update(bytes, now_ms); + rtc::Optional rate = old_estimator_.Rate(now_ms); + bitrate_estimate_ = -1.0f; + if (rate) + bitrate_estimate_ = *rate / 1000.0f; + return; + } + 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(Clock* clock) : clock_(clock), inter_arrival_(), estimator_(), detector_(OverUseDetectorOptions()), - receiver_incoming_bitrate_(kBitrateWindowMs, 8000), + receiver_incoming_bitrate_(), last_update_ms_(-1), last_seen_packet_ms_(-1), uma_recorded_(false) { @@ -73,7 +163,7 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( const PacketInfo& info) { int64_t now_ms = clock_->TimeInMilliseconds(); - receiver_incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms); + receiver_incoming_bitrate_.Update(info.arrival_time_ms, info.payload_size); Result result; // Reset if the stream has timed out. if (last_seen_packet_ms_ == -1 || @@ -112,28 +202,30 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( if (info.probe_cluster_id != PacketInfo::kNotAProbe) { probing_bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); } - + rtc::Optional acked_bitrate_bps = + receiver_incoming_bitrate_.bitrate_bps(); // Currently overusing the bandwidth. if (detector_.State() == kBwOverusing) { - rtc::Optional incoming_rate = - receiver_incoming_bitrate_.Rate(info.arrival_time_ms); - if (incoming_rate && - rate_control_.TimeToReduceFurther(now_ms, *incoming_rate)) { - result.updated = UpdateEstimate(info.arrival_time_ms, now_ms, - &result.target_bitrate_bps); + if (acked_bitrate_bps && + rate_control_.TimeToReduceFurther(now_ms, *acked_bitrate_bps)) { + result.updated = + UpdateEstimate(info.arrival_time_ms, now_ms, acked_bitrate_bps, + &result.target_bitrate_bps); } } else if (probing_bps > 0) { // No overuse, but probing measured a bitrate. rate_control_.SetEstimate(probing_bps, info.arrival_time_ms); result.probe = true; - result.updated = UpdateEstimate(info.arrival_time_ms, now_ms, - &result.target_bitrate_bps); + result.updated = + UpdateEstimate(info.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(info.arrival_time_ms, now_ms, - &result.target_bitrate_bps); + result.updated = + UpdateEstimate(info.arrival_time_ms, now_ms, acked_bitrate_bps, + &result.target_bitrate_bps); } if (result.updated) last_update_ms_ = now_ms; @@ -143,12 +235,9 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, int64_t now_ms, + rtc::Optional acked_bitrate_bps, uint32_t* target_bitrate_bps) { - // The first overuse should immediately trigger a new estimate. - // We also have to update the estimate immediately if we are overusing - // and the target bitrate is too high compared to what we are receiving. - const RateControlInput input(detector_.State(), - receiver_incoming_bitrate_.Rate(arrival_time_ms), + const RateControlInput input(detector_.State(), acked_bitrate_bps, estimator_->var_noise()); rate_control_.Update(&input, now_ms); *target_bitrate_bps = rate_control_.UpdateBandwidthEstimate(now_ms); diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index c5be765232..6aab54918c 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -11,9 +11,8 @@ #ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_DELAY_BASED_BWE_H_ #define WEBRTC_MODULES_CONGESTION_CONTROLLER_DELAY_BASED_BWE_H_ -#include -#include #include +#include #include #include "webrtc/base/checks.h" @@ -53,11 +52,34 @@ class DelayBasedBwe { void SetMinBitrate(int min_bitrate_bps); 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_; + RateStatistics old_estimator_; + const bool in_experiment_; + }; + Result IncomingPacketInfo(const PacketInfo& info); // 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, + rtc::Optional acked_bitrate_bps, uint32_t* target_bitrate_bps); rtc::ThreadChecker network_thread_; @@ -65,7 +87,7 @@ class DelayBasedBwe { std::unique_ptr inter_arrival_; std::unique_ptr estimator_; OveruseDetector detector_; - RateStatistics receiver_incoming_bitrate_; + BitrateEstimator receiver_incoming_bitrate_; int64_t last_update_ms_; int64_t last_seen_packet_ms_; bool uma_recorded_; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index e751013a5d..287746980a 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -14,6 +14,7 @@ #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h" #include "webrtc/system_wrappers/include/clock.h" +#include "webrtc/test/field_trial.h" namespace webrtc { @@ -143,26 +144,6 @@ TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) { CapacityDropTestHelper(1, true, 633, 0); } -TEST_F(DelayBasedBweTest, CapacityDropTwoStreamsWrap) { - CapacityDropTestHelper(2, true, 767, 0); -} - -TEST_F(DelayBasedBweTest, CapacityDropThreeStreamsWrap) { - CapacityDropTestHelper(3, true, 633, 0); -} - -TEST_F(DelayBasedBweTest, CapacityDropThirteenStreamsWrap) { - CapacityDropTestHelper(13, true, 733, 0); -} - -TEST_F(DelayBasedBweTest, CapacityDropNineteenStreamsWrap) { - CapacityDropTestHelper(19, true, 667, 0); -} - -TEST_F(DelayBasedBweTest, CapacityDropThirtyStreamsWrap) { - CapacityDropTestHelper(30, true, 667, 0); -} - TEST_F(DelayBasedBweTest, TestTimestampGrouping) { TestTimestampGroupingTestHelper(); } @@ -181,4 +162,37 @@ TEST_F(DelayBasedBweTest, TestLongTimeoutAndWrap) { // properly timed out. TestWrappingHelper(10 * 64); } + +class DelayBasedBweExperimentTest : public DelayBasedBweTest { + public: + DelayBasedBweExperimentTest() + : override_field_trials_("WebRTC-ImprovedBitrateEstimate/Enabled/") {} + + protected: + void SetUp() override { + bitrate_estimator_.reset(new DelayBasedBwe(&clock_)); + } + + test::ScopedFieldTrials override_field_trials_; +}; + +TEST_F(DelayBasedBweExperimentTest, RateIncreaseRtpTimestamps) { + RateIncreaseRtpTimestampsTestHelper(1288); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropOneStream) { + CapacityDropTestHelper(1, false, 333, 0); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropPosOffsetChange) { + CapacityDropTestHelper(1, false, 300, 30000); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropNegOffsetChange) { + CapacityDropTestHelper(1, false, 300, -30000); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropOneStreamWrap) { + CapacityDropTestHelper(1, true, 333, 0); +} } // namespace webrtc 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 a3a1893cbe..9aafc7b516 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -150,10 +150,11 @@ int64_t StreamGenerator::GenerateFrame(std::vector* packets, DelayBasedBweTest::DelayBasedBweTest() : clock_(100000000), - bitrate_estimator_(&clock_), + bitrate_estimator_(new DelayBasedBwe(&clock_)), stream_generator_(new test::StreamGenerator(1e6, // Capacity. clock_.TimeInMicroseconds())), - arrival_time_offset_ms_(0) {} + arrival_time_offset_ms_(0), + first_update_(true) {} DelayBasedBweTest::~DelayBasedBweTest() {} @@ -182,7 +183,7 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, std::vector packets; packets.push_back(packet); DelayBasedBwe::Result result = - bitrate_estimator_.IncomingPacketFeedbackVector(packets); + bitrate_estimator_->IncomingPacketFeedbackVector(packets); const uint32_t kDummySsrc = 0; if (result.updated) { bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, @@ -214,13 +215,14 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, packet.arrival_time_ms += arrival_time_offset_ms_; } DelayBasedBwe::Result result = - bitrate_estimator_.IncomingPacketFeedbackVector(packets); + bitrate_estimator_->IncomingPacketFeedbackVector(packets); const uint32_t kDummySsrc = 0; if (result.updated) { bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, result.target_bitrate_bps); - if (result.target_bitrate_bps < bitrate_bps) + if (!first_update_ && result.target_bitrate_bps < bitrate_bps) overuse = true; + first_update_ = false; } clock_.AdvanceTimeMicroseconds(next_time_us - clock_.TimeInMicroseconds()); @@ -267,10 +269,10 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( int64_t send_time_ms = 0; uint16_t sequence_number = 0; std::vector ssrcs; - EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); clock_.AdvanceTimeMilliseconds(1000); - EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_FALSE(bitrate_observer_.updated()); bitrate_observer_.Reset(); clock_.AdvanceTimeMilliseconds(1000); @@ -281,7 +283,7 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; if (i == kNumInitialPackets) { - EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); EXPECT_FALSE(bitrate_observer_.updated()); bitrate_observer_.Reset(); @@ -291,7 +293,7 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( clock_.AdvanceTimeMilliseconds(1000 / kFramerate); send_time_ms += kFrameIntervalMs; } - EXPECT_TRUE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); ASSERT_EQ(1u, ssrcs.size()); EXPECT_EQ(kDefaultSsrc, ssrcs.front()); EXPECT_NEAR(expected_converge_bitrate, bitrate_bps, kAcceptedBitrateErrorBps); @@ -364,7 +366,6 @@ void DelayBasedBweTest::RateIncreaseRtpTimestampsTestHelper( bitrate_observer_.Reset(); } ++iterations; - // ASSERT_LE(iterations, expected_iterations); } ASSERT_EQ(expected_iterations, iterations); } @@ -483,19 +484,19 @@ void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { } uint32_t bitrate_before = 0; std::vector ssrcs; - bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_before); + bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_before); clock_.AdvanceTimeMilliseconds(silence_time_s * 1000); send_time_ms += silence_time_s * 1000; - for (size_t i = 0; i < 21; ++i) { + for (size_t i = 0; i < 22; ++i) { IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } uint32_t bitrate_after = 0; - bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_after); + bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_after); EXPECT_LT(bitrate_after, bitrate_before); } } // namespace webrtc 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 55aa65063d..3765e4c944 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h @@ -162,9 +162,10 @@ class DelayBasedBweTest : public ::testing::Test { SimulatedClock clock_; // Time at the receiver. test::TestBitrateObserver bitrate_observer_; - DelayBasedBwe bitrate_estimator_; + std::unique_ptr bitrate_estimator_; std::unique_ptr stream_generator_; int64_t arrival_time_offset_ms_; + bool first_update_; RTC_DISALLOW_COPY_AND_ASSIGN(DelayBasedBweTest); }; diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index 0664ac007c..e67fa76846 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -990,9 +990,10 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { return std::numeric_limits::max(); }; - RateStatistics acked_bitrate(1000, 8000); + RateStatistics acked_bitrate(250, 8000); int64_t time_us = std::min(NextRtpTime(), NextRtcpTime()); + int64_t last_update_us = 0; while (time_us != std::numeric_limits::max()) { clock.AdvanceTimeMicroseconds(time_us - clock.TimeInMicroseconds()); if (clock.TimeInMicroseconds() >= NextRtcpTime()) { @@ -1037,11 +1038,13 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { RTC_DCHECK_EQ(clock.TimeInMicroseconds(), NextProcessTime()); cc.Process(); } - if (observer.GetAndResetBitrateUpdated()) { + if (observer.GetAndResetBitrateUpdated() || + time_us - last_update_us >= 1e6) { uint32_t y = observer.last_bitrate_bps() / 1000; float x = static_cast(clock.TimeInMicroseconds() - begin_time_) / 1000000; time_series.points.emplace_back(x, y); + last_update_us = time_us; } time_us = std::min({NextRtpTime(), NextRtcpTime(), NextProcessTime()}); }