Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(563)

Unified Diff: webrtc/modules/congestion_controller/median_slope_estimator.cc

Issue 2578543002: Avoid precision loss in MedianSlopeEstimator from int64_t -> double conversion (Closed)
Patch Set: Move test function to anonymous namespace Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/modules/congestion_controller/median_slope_estimator.cc
diff --git a/webrtc/modules/congestion_controller/median_slope_estimator.cc b/webrtc/modules/congestion_controller/median_slope_estimator.cc
index 488831e1738f6c876653eb9bb22aaf210cdf20ed..328aa6b5d4262c98efc10b4fba8078bb459dde10 100644
--- a/webrtc/modules/congestion_controller/median_slope_estimator.cc
+++ b/webrtc/modules/congestion_controller/median_slope_estimator.cc
@@ -35,15 +35,15 @@ MedianSlopeEstimator::~MedianSlopeEstimator() {}
void MedianSlopeEstimator::Update(double recv_delta_ms,
double send_delta_ms,
- double now_ms) {
+ int64_t arrival_time_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;
- }
accumulated_delay_ += delta_ms;
- BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", now_ms, accumulated_delay_);
+ BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", arrival_time_ms,
+ accumulated_delay_);
// If the window is full, remove the |window_size_| - 1 slopes that belong to
// the oldest point.
@@ -56,7 +56,7 @@ void MedianSlopeEstimator::Update(double recv_delta_ms,
}
// Add |window_size_| - 1 new slopes.
for (auto& old_delay : delay_hist_) {
- if (now_ms - old_delay.time != 0) {
+ if (arrival_time_ms - old_delay.time != 0) {
// The C99 standard explicitly states that casts and assignments must
// perform the associated conversions. This means that |slope| will be
// a 64-bit double even if the division is computed using, e.g., 80-bit
@@ -64,20 +64,21 @@ void MedianSlopeEstimator::Update(double recv_delta_ms,
// C++11 standard isn't as explicit. Furthermore, there are good reasons
// to believe that compilers couldn't perform optimizations that break
// this assumption even if they wanted to.
- double slope =
- (accumulated_delay_ - old_delay.delay) / (now_ms - old_delay.time);
+ double slope = (accumulated_delay_ - old_delay.delay) /
+ static_cast<double>(arrival_time_ms - old_delay.time);
median_filter_.Insert(slope);
// We want to avoid issues with different rounding mode / precision
// which we might get if we recomputed the slope when we remove it.
old_delay.slopes.push_back(slope);
}
}
- delay_hist_.emplace_back(now_ms, accumulated_delay_, window_size_ - 1);
+ delay_hist_.emplace_back(arrival_time_ms, accumulated_delay_,
+ window_size_ - 1);
// Recompute the median slope.
if (delay_hist_.size() == window_size_)
trendline_ = median_filter_.GetPercentileValue();
- BWE_TEST_LOGGING_PLOT(1, "trendline_slope", now_ms, trendline_);
+ BWE_TEST_LOGGING_PLOT(1, "trendline_slope", arrival_time_ms, trendline_);
}
} // namespace webrtc

Powered by Google App Engine
This is Rietveld 408576698