From 43c382111dede5cb06a3b5d0d05a4f4ffd350520 Mon Sep 17 00:00:00 2001 From: terelius Date: Thu, 15 Dec 2016 06:42:44 -0800 Subject: [PATCH] Revert of Avoid precision loss in TrendlineEstimator from int64_t -> double conversion (patchset #7 id:120001 of https://codereview.webrtc.org/2577463002/ ) Reason for revert: Multiple definitions of TestEstimator Original issue's description: > Avoid precision loss in TrendlineEstimator by passing the arrival time as an int64_t instead of a double. > > BUG=webrtc:6884 > > Committed: https://crrev.com/c12cbaf9dd0729dd45f3fc45a1938d1b3455e40a > Cr-Commit-Position: refs/heads/master@{#15631} TBR=stefan@webrtc.org,brandtr@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6884 Review-Url: https://codereview.webrtc.org/2582513002 Cr-Commit-Position: refs/heads/master@{#15636} --- .../trendline_estimator.cc | 34 +++---- .../trendline_estimator.h | 10 +- .../trendline_estimator_unittest.cc | 97 +++++++++++++------ 3 files changed, 83 insertions(+), 58 deletions(-) diff --git a/webrtc/modules/congestion_controller/trendline_estimator.cc b/webrtc/modules/congestion_controller/trendline_estimator.cc index ad05a64e1e..a086f6aaa1 100644 --- a/webrtc/modules/congestion_controller/trendline_estimator.cc +++ b/webrtc/modules/congestion_controller/trendline_estimator.cc @@ -13,14 +13,12 @@ #include #include "webrtc/base/checks.h" -#include "webrtc/base/optional.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h" namespace webrtc { namespace { -rtc::Optional LinearFitSlope( - const std::list> points) { +double LinearFitSlope(const std::list> points) { RTC_DCHECK(points.size() >= 2); // Compute the "center of mass". double sum_x = 0; @@ -38,9 +36,7 @@ rtc::Optional LinearFitSlope( numerator += (point.first - x_avg) * (point.second - y_avg); denominator += (point.first - x_avg) * (point.first - x_avg); } - if (denominator == 0) - return rtc::Optional(); - return rtc::Optional(numerator / denominator); + return numerator / denominator; } } // namespace @@ -53,7 +49,6 @@ TrendlineEstimator::TrendlineEstimator(size_t window_size, smoothing_coef_(smoothing_coef), threshold_gain_(threshold_gain), num_of_deltas_(0), - first_arrival_time_ms(-1), accumulated_delay_(0), smoothed_delay_(0), delay_hist_(), @@ -63,35 +58,30 @@ TrendlineEstimator::~TrendlineEstimator() {} void TrendlineEstimator::Update(double recv_delta_ms, double send_delta_ms, - int64_t arrival_time_ms) { + double now_ms) { const double delta_ms = recv_delta_ms - send_delta_ms; ++num_of_deltas_; - if (num_of_deltas_ > kDeltaCounterMax) + if (num_of_deltas_ > kDeltaCounterMax) { num_of_deltas_ = kDeltaCounterMax; - if (first_arrival_time_ms == -1) - first_arrival_time_ms = arrival_time_ms; + } // Exponential backoff filter. accumulated_delay_ += delta_ms; - BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", arrival_time_ms, - accumulated_delay_); + BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", now_ms, accumulated_delay_); smoothed_delay_ = smoothing_coef_ * smoothed_delay_ + (1 - smoothing_coef_) * accumulated_delay_; - BWE_TEST_LOGGING_PLOT(1, "smoothed_delay_ms", arrival_time_ms, - smoothed_delay_); + BWE_TEST_LOGGING_PLOT(1, "smoothed_delay_ms", now_ms, smoothed_delay_); // Simple linear regression. - delay_hist_.push_back(std::make_pair( - static_cast(arrival_time_ms - first_arrival_time_ms), - smoothed_delay_)); - if (delay_hist_.size() > window_size_) + delay_hist_.push_back(std::make_pair(now_ms, smoothed_delay_)); + if (delay_hist_.size() > window_size_) { delay_hist_.pop_front(); + } if (delay_hist_.size() == window_size_) { - // Only update trendline_ if it is possible to fit a line to the data. - trendline_ = LinearFitSlope(delay_hist_).value_or(trendline_); + trendline_ = LinearFitSlope(delay_hist_); } - BWE_TEST_LOGGING_PLOT(1, "trendline_slope", arrival_time_ms, trendline_); + BWE_TEST_LOGGING_PLOT(1, "trendline_slope", now_ms, trendline_); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/trendline_estimator.h b/webrtc/modules/congestion_controller/trendline_estimator.h index 7ca9b7d072..0bf9bf27c7 100644 --- a/webrtc/modules/congestion_controller/trendline_estimator.h +++ b/webrtc/modules/congestion_controller/trendline_estimator.h @@ -10,13 +10,11 @@ #ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_TRENDLINE_ESTIMATOR_H_ #define WEBRTC_MODULES_CONGESTION_CONTROLLER_TRENDLINE_ESTIMATOR_H_ -#include -#include - #include #include #include "webrtc/base/constructormagic.h" +#include "webrtc/common_types.h" namespace webrtc { @@ -35,9 +33,7 @@ class TrendlineEstimator { // Update the estimator with a new sample. The deltas should represent deltas // between timestamp groups as defined by the InterArrival class. - void Update(double recv_delta_ms, - double send_delta_ms, - int64_t arrival_time_ms); + void Update(double recv_delta_ms, double send_delta_ms, double now_ms); // Returns the estimated trend k multiplied by some gain. // 0 < k < 1 -> the delay increases, queues are filling up @@ -55,8 +51,6 @@ class TrendlineEstimator { const double threshold_gain_; // Used by the existing threshold. unsigned int num_of_deltas_; - // Keep the arrival times small by using the change from the first packet. - int64_t first_arrival_time_ms; // Exponential backoff filtering. double accumulated_delay_; double smoothed_delay_; diff --git a/webrtc/modules/congestion_controller/trendline_estimator_unittest.cc b/webrtc/modules/congestion_controller/trendline_estimator_unittest.cc index b923ac5cd6..51778e6cf3 100644 --- a/webrtc/modules/congestion_controller/trendline_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/trendline_estimator_unittest.cc @@ -15,59 +15,100 @@ namespace webrtc { namespace { -constexpr size_t kWindowSize = 20; +constexpr size_t kWindowSize = 15; constexpr double kSmoothing = 0.0; constexpr double kGain = 1; constexpr int64_t kAvgTimeBetweenPackets = 10; -constexpr size_t kPacketCount = 2 * kWindowSize + 1; } // namespace -void TestEstimator(double slope, double jitter_stddev, double tolerance) { +TEST(TrendlineEstimator, PerfectLineSlopeOneHalf) { TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); - Random random(0x1234567); - int64_t send_times[kPacketCount]; - int64_t recv_times[kPacketCount]; - int64_t send_start_time = random.Rand(1000000); - int64_t recv_start_time = random.Rand(1000000); - for (size_t i = 0; i < kPacketCount; ++i) { - send_times[i] = send_start_time + i * kAvgTimeBetweenPackets; - double latency = i * kAvgTimeBetweenPackets / (1 - slope); - double jitter = random.Gaussian(0, jitter_stddev); - recv_times[i] = recv_start_time + latency + jitter; - } - for (size_t i = 1; i < kPacketCount; ++i) { - double recv_delta = recv_times[i] - recv_times[i - 1]; - double send_delta = send_times[i] - send_times[i - 1]; - estimator.Update(recv_delta, send_delta, recv_times[i]); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = 2 * send_delta; + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); if (i < kWindowSize) EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001); else - EXPECT_NEAR(estimator.trendline_slope(), slope, tolerance); + EXPECT_NEAR(estimator.trendline_slope(), 0.5, 0.001); } } -TEST(TrendlineEstimator, PerfectLineSlopeOneHalf) { - TestEstimator(0.5, 0, 0.001); -} - TEST(TrendlineEstimator, PerfectLineSlopeMinusOne) { - TestEstimator(-1, 0, 0.001); + TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = 0.5 * send_delta; + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); + if (i < kWindowSize) + EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001); + else + EXPECT_NEAR(estimator.trendline_slope(), -1, 0.001); + } } TEST(TrendlineEstimator, PerfectLineSlopeZero) { - TestEstimator(0, 0, 0.001); + TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = send_delta; + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); + EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001); + } } TEST(TrendlineEstimator, JitteryLineSlopeOneHalf) { - TestEstimator(0.5, kAvgTimeBetweenPackets / 3.0, 0.01); + TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = 2 * send_delta + rand.Gaussian(0, send_delta / 3); + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); + if (i < kWindowSize) + EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001); + else + EXPECT_NEAR(estimator.trendline_slope(), 0.5, 0.1); + } } TEST(TrendlineEstimator, JitteryLineSlopeMinusOne) { - TestEstimator(-1, kAvgTimeBetweenPackets / 3.0, 0.075); + TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = 0.5 * send_delta + rand.Gaussian(0, send_delta / 25); + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); + if (i < kWindowSize) + EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001); + else + EXPECT_NEAR(estimator.trendline_slope(), -1, 0.1); + } } TEST(TrendlineEstimator, JitteryLineSlopeZero) { - TestEstimator(0, kAvgTimeBetweenPackets / 3.0, 0.02); + TrendlineEstimator estimator(kWindowSize, kSmoothing, kGain); + Random rand(0x1234567); + double now_ms = rand.Rand() * 10000; + for (size_t i = 1; i < 2 * kWindowSize; i++) { + double send_delta = rand.Rand() * 2 * kAvgTimeBetweenPackets; + double recv_delta = send_delta + rand.Gaussian(0, send_delta / 8); + now_ms += recv_delta; + estimator.Update(recv_delta, send_delta, now_ms); + EXPECT_NEAR(estimator.trendline_slope(), 0, 0.1); + } } } // namespace webrtc