Change LastProcessedRtt (used in the rtp/rtcp module) to return the average RTT (instead of max RTT) to get a smooth estimate of the nack interval.
R=mflodman@webrtc.org, stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/27349004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@7863 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/video_engine/call_stats.cc b/webrtc/video_engine/call_stats.cc
index 05385c3..ccc97fa 100644
--- a/webrtc/video_engine/call_stats.cc
+++ b/webrtc/video_engine/call_stats.cc
@@ -17,11 +17,57 @@
#include "webrtc/system_wrappers/interface/tick_util.h"
namespace webrtc {
-
+namespace {
// A rtt report is considered valid for this long.
const int kRttTimeoutMs = 1500;
// Time interval for updating the observers.
const int kUpdateIntervalMs = 1000;
+// Weight factor to apply to the average rtt.
+const float kWeightFactor = 0.3f;
+
+void RemoveOldReports(int64_t now, std::list<CallStats::RttTime>* reports) {
+ while (!reports->empty() &&
+ (now - reports->front().time) > kRttTimeoutMs) {
+ reports->pop_front();
+ }
+}
+
+uint32_t GetMaxRttMs(std::list<CallStats::RttTime>* reports) {
+ uint32_t max_rtt_ms = 0;
+ for (std::list<CallStats::RttTime>::const_iterator it = reports->begin();
+ it != reports->end(); ++it) {
+ max_rtt_ms = std::max(it->rtt, max_rtt_ms);
+ }
+ return max_rtt_ms;
+}
+
+uint32_t GetAvgRttMs(std::list<CallStats::RttTime>* reports) {
+ if (reports->empty()) {
+ return 0;
+ }
+ uint32_t sum = 0;
+ for (std::list<CallStats::RttTime>::const_iterator it = reports->begin();
+ it != reports->end(); ++it) {
+ sum += it->rtt;
+ }
+ return sum / reports->size();
+}
+
+void UpdateAvgRttMs(std::list<CallStats::RttTime>* reports, uint32_t* avg_rtt) {
+ uint32_t cur_rtt_ms = GetAvgRttMs(reports);
+ if (cur_rtt_ms == 0) {
+ // Reset.
+ *avg_rtt = 0;
+ return;
+ }
+ if (*avg_rtt == 0) {
+ // Initialize.
+ *avg_rtt = cur_rtt_ms;
+ return;
+ }
+ *avg_rtt = *avg_rtt * (1.0f - kWeightFactor) + cur_rtt_ms * kWeightFactor;
+}
+} // namespace
class RtcpObserver : public RtcpRttStats {
public:
@@ -32,8 +78,9 @@
owner_->OnRttUpdate(rtt);
}
+ // Returns the average RTT.
virtual uint32_t LastProcessedRtt() const {
- return owner_->last_processed_rtt_ms();
+ return owner_->avg_rtt_ms();
}
private:
@@ -46,7 +93,8 @@
: crit_(CriticalSectionWrapper::CreateCriticalSection()),
rtcp_rtt_stats_(new RtcpObserver(this)),
last_process_time_(TickTime::MillisecondTimestamp()),
- last_processed_rtt_ms_(0) {
+ max_rtt_ms_(0),
+ avg_rtt_ms_(0) {
}
CallStats::~CallStats() {
@@ -60,39 +108,30 @@
int32_t CallStats::Process() {
CriticalSectionScoped cs(crit_.get());
- if (TickTime::MillisecondTimestamp() < last_process_time_ + kUpdateIntervalMs)
+ int64_t now = TickTime::MillisecondTimestamp();
+ if (now < last_process_time_ + kUpdateIntervalMs)
return 0;
- // Remove invalid, as in too old, rtt values.
- int64_t time_now = TickTime::MillisecondTimestamp();
- while (!reports_.empty() && reports_.front().time + kRttTimeoutMs <
- time_now) {
- reports_.pop_front();
- }
+ last_process_time_ = now;
- // Find the max stored RTT.
- uint32_t max_rtt = 0;
- for (std::list<RttTime>::const_iterator it = reports_.begin();
- it != reports_.end(); ++it) {
- if (it->rtt > max_rtt)
- max_rtt = it->rtt;
- }
+ RemoveOldReports(now, &reports_);
+ max_rtt_ms_ = GetMaxRttMs(&reports_);
+ UpdateAvgRttMs(&reports_, &avg_rtt_ms_);
- // If there is a valid rtt, update all observers.
- if (max_rtt > 0) {
+ // If there is a valid rtt, update all observers with the max rtt.
+ // TODO(asapersson): Consider changing this to report the average rtt.
+ if (max_rtt_ms_ > 0) {
for (std::list<CallStatsObserver*>::iterator it = observers_.begin();
it != observers_.end(); ++it) {
- (*it)->OnRttUpdate(max_rtt);
+ (*it)->OnRttUpdate(max_rtt_ms_);
}
}
- last_processed_rtt_ms_ = max_rtt;
- last_process_time_ = time_now;
return 0;
}
-uint32_t CallStats::last_processed_rtt_ms() const {
+uint32_t CallStats::avg_rtt_ms() const {
CriticalSectionScoped cs(crit_.get());
- return last_processed_rtt_ms_;
+ return avg_rtt_ms_;
}
RtcpRttStats* CallStats::rtcp_rtt_stats() const {
@@ -122,8 +161,7 @@
void CallStats::OnRttUpdate(uint32_t rtt) {
CriticalSectionScoped cs(crit_.get());
- int64_t time_now = TickTime::MillisecondTimestamp();
- reports_.push_back(RttTime(rtt, time_now));
+ reports_.push_back(RttTime(rtt, TickTime::MillisecondTimestamp()));
}
} // namespace webrtc
diff --git a/webrtc/video_engine/call_stats.h b/webrtc/video_engine/call_stats.h
index 95f4fee..f516ebf 100644
--- a/webrtc/video_engine/call_stats.h
+++ b/webrtc/video_engine/call_stats.h
@@ -43,12 +43,6 @@
void RegisterStatsObserver(CallStatsObserver* observer);
void DeregisterStatsObserver(CallStatsObserver* observer);
- protected:
- void OnRttUpdate(uint32_t rtt);
-
- uint32_t last_processed_rtt_ms() const;
-
- private:
// Helper struct keeping track of the time a rtt value is reported.
struct RttTime {
RttTime(uint32_t new_rtt, int64_t rtt_time)
@@ -57,6 +51,12 @@
const int64_t time;
};
+ protected:
+ void OnRttUpdate(uint32_t rtt);
+
+ uint32_t avg_rtt_ms() const;
+
+ private:
// Protecting all members.
scoped_ptr<CriticalSectionWrapper> crit_;
// Observer receiving statistics updates.
@@ -64,7 +64,8 @@
// The last time 'Process' resulted in statistic update.
int64_t last_process_time_;
// The last RTT in the statistics update (zero if there is no valid estimate).
- uint32_t last_processed_rtt_ms_;
+ uint32_t max_rtt_ms_;
+ uint32_t avg_rtt_ms_;
// All Rtt reports within valid time interval, oldest first.
std::list<RttTime> reports_;
diff --git a/webrtc/video_engine/call_stats_unittest.cc b/webrtc/video_engine/call_stats_unittest.cc
index f504094..1196ada 100644
--- a/webrtc/video_engine/call_stats_unittest.cc
+++ b/webrtc/video_engine/call_stats_unittest.cc
@@ -101,42 +101,42 @@
TEST_F(CallStatsTest, MultipleObservers) {
MockStatsObserver stats_observer_1;
call_stats_->RegisterStatsObserver(&stats_observer_1);
- // Add the secondobserver twice, there should still be only one report to the
+ // Add the second observer twice, there should still be only one report to the
// observer.
MockStatsObserver stats_observer_2;
call_stats_->RegisterStatsObserver(&stats_observer_2);
call_stats_->RegisterStatsObserver(&stats_observer_2);
RtcpRttStats* rtcp_rtt_stats = call_stats_->rtcp_rtt_stats();
- uint32_t rtt = 100;
- rtcp_rtt_stats->OnRttUpdate(rtt);
+ const uint32_t kRtt = 100;
+ rtcp_rtt_stats->OnRttUpdate(kRtt);
// Verify both observers are updated.
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
.Times(1);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
.Times(1);
call_stats_->Process();
// Deregister the second observer and verify update is only sent to the first
// observer.
call_stats_->DeregisterStatsObserver(&stats_observer_2);
- rtcp_rtt_stats->OnRttUpdate(rtt);
+ rtcp_rtt_stats->OnRttUpdate(kRtt);
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
.Times(1);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
.Times(0);
call_stats_->Process();
// Deregister the first observer.
call_stats_->DeregisterStatsObserver(&stats_observer_1);
- rtcp_rtt_stats->OnRttUpdate(rtt);
+ rtcp_rtt_stats->OnRttUpdate(kRtt);
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer_1, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_1, OnRttUpdate(kRtt))
.Times(0);
- EXPECT_CALL(stats_observer_2, OnRttUpdate(rtt))
+ EXPECT_CALL(stats_observer_2, OnRttUpdate(kRtt))
.Times(0);
call_stats_->Process();
}
@@ -151,17 +151,17 @@
TickTime::AdvanceFakeClock(1000);
// Set a first value and verify the callback is triggered.
- const uint32_t first_rtt = 100;
- rtcp_rtt_stats->OnRttUpdate(first_rtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(first_rtt))
+ const uint32_t kFirstRtt = 100;
+ rtcp_rtt_stats->OnRttUpdate(kFirstRtt);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kFirstRtt))
.Times(1);
call_stats_->Process();
// Increase rtt and verify the new value is reported.
TickTime::AdvanceFakeClock(1000);
- const uint32_t high_rtt = first_rtt + 20;
- rtcp_rtt_stats->OnRttUpdate(high_rtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(high_rtt))
+ const uint32_t kHighRtt = kFirstRtt + 20;
+ rtcp_rtt_stats->OnRttUpdate(kHighRtt);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kHighRtt))
.Times(1);
call_stats_->Process();
@@ -169,20 +169,50 @@
// rtt invalid. Report a lower rtt and verify the old/high value still is sent
// in the callback.
TickTime::AdvanceFakeClock(1000);
- const uint32_t low_rtt = first_rtt - 20;
- rtcp_rtt_stats->OnRttUpdate(low_rtt);
- EXPECT_CALL(stats_observer, OnRttUpdate(high_rtt))
+ const uint32_t kLowRtt = kFirstRtt - 20;
+ rtcp_rtt_stats->OnRttUpdate(kLowRtt);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kHighRtt))
.Times(1);
call_stats_->Process();
// Advance time to make the high report invalid, the lower rtt should now be
// in the callback.
TickTime::AdvanceFakeClock(1000);
- EXPECT_CALL(stats_observer, OnRttUpdate(low_rtt))
+ EXPECT_CALL(stats_observer, OnRttUpdate(kLowRtt))
.Times(1);
call_stats_->Process();
call_stats_->DeregisterStatsObserver(&stats_observer);
}
+TEST_F(CallStatsTest, LastProcessedRtt) {
+ MockStatsObserver stats_observer;
+ call_stats_->RegisterStatsObserver(&stats_observer);
+ RtcpRttStats* rtcp_rtt_stats = call_stats_->rtcp_rtt_stats();
+ TickTime::AdvanceFakeClock(1000);
+
+ // Set a first values and verify that LastProcessedRtt initially returns the
+ // average rtt.
+ const uint32_t kRttLow = 10;
+ const uint32_t kRttHigh = 30;
+ const uint32_t kAvgRtt = 20;
+ rtcp_rtt_stats->OnRttUpdate(kRttLow);
+ rtcp_rtt_stats->OnRttUpdate(kRttHigh);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kRttHigh))
+ .Times(1);
+ call_stats_->Process();
+ EXPECT_EQ(kAvgRtt, rtcp_rtt_stats->LastProcessedRtt());
+
+ // Update values and verify LastProcessedRtt.
+ TickTime::AdvanceFakeClock(1000);
+ rtcp_rtt_stats->OnRttUpdate(kRttLow);
+ rtcp_rtt_stats->OnRttUpdate(kRttHigh);
+ EXPECT_CALL(stats_observer, OnRttUpdate(kRttHigh))
+ .Times(1);
+ call_stats_->Process();
+ EXPECT_EQ(kAvgRtt, rtcp_rtt_stats->LastProcessedRtt());
+
+ call_stats_->DeregisterStatsObserver(&stats_observer);
+}
+
} // namespace webrtc
