From 8d2f5c59cdb09ab70f2a51ea5b6cc98775bfcb55 Mon Sep 17 00:00:00 2001 From: "mikhal@webrtc.org" Date: Fri, 26 Oct 2012 22:53:21 +0000 Subject: [PATCH] Fixing BWE estimation reordering issue BUG=1009 TBR=stefan Review URL: https://webrtc-codereview.appspot.com/936006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3007 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../overuse_detector.cc | 22 +++++++++++++------ .../remote_bitrate_estimator_unittest.cc | 6 ++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc b/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc index 084fa156d..433a55228 100644 --- a/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc +++ b/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc @@ -110,6 +110,14 @@ void OveruseDetector::Update(uint16_t packet_size, #endif bool new_timestamp = (timestamp != current_frame_.timestamp); + if (current_frame_.timestamp_ms == -1) { + const uint32_t timestamp_diff = timestamp - current_frame_.timestamp; + if (timestamp_diff > 0x80000000) { + // Assume that a diff this big must be due to reordering. Don't update + // with reordered samples. + return; + } + } if (timestamp_ms >= 0) { if (prev_frame_.timestamp_ms == -1 && current_frame_.timestamp_ms == -1) { SwitchTimeBase(); @@ -126,16 +134,16 @@ void OveruseDetector::Update(uint16_t packet_size, if (prev_frame_.complete_time_ms >= 0) { // This is our second frame int64_t t_delta = 0; double ts_delta = 0; - if (!TimeDeltas(current_frame_, prev_frame_, + if (TimeDeltas(current_frame_, prev_frame_, &t_delta, &ts_delta)) { - // Frame reordering, dropping this sample. - return; + // No reordering. + UpdateKalman(t_delta, ts_delta, current_frame_.size, prev_frame_.size); + prev_frame_ = current_frame_; } - UpdateKalman(t_delta, ts_delta, current_frame_.size, prev_frame_.size); + } else { + prev_frame_ = current_frame_; } - // The new timestamp is now the current frame, - // and the old timestamp becomes the previous frame. - prev_frame_ = current_frame_; + // The new timestamp is now the current frame. current_frame_.timestamp = timestamp; current_frame_.timestamp_ms = timestamp_ms; current_frame_.size = 0; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc index 59c1285d8..cdc0a466e 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc @@ -512,7 +512,7 @@ TEST_F(RemoteBitrateEstimatorTest, TestCapacityDropRtpTimestampsWrap) { bitrate_observer_->Reset(); } } - EXPECT_EQ(8299, bitrate_drop_time); + EXPECT_EQ(8366, bitrate_drop_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -552,7 +552,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TestCapacityDropRtpTimestampsWrap) { bitrate_observer_->Reset(); } } - EXPECT_EQ(8299, bitrate_drop_time); + EXPECT_EQ(8366, bitrate_drop_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -666,7 +666,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) { bitrate_observer_->Reset(); } } - EXPECT_EQ(3933, bitrate_drop_time); + EXPECT_EQ(3900, bitrate_drop_time); } } // namespace webrtc