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

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 @@ class RtcpObserver : public RtcpRttStats {
     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 @@ CallStats::CallStats()
     : 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::TimeUntilNextProcess() {
 
 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.
+  last_process_time_ = now;
-  int64_t time_now = TickTime::MillisecondTimestamp();
-  while (!reports_.empty() && reports_.front().time + kRttTimeoutMs <
-         time_now) {
-    reports_.pop_front();
-  }
 
-  // Find the max stored RTT.
+  RemoveOldReports(now, &reports_);
-  uint32_t max_rtt = 0;
+  max_rtt_ms_ = GetMaxRttMs(&reports_);
-  for (std::list<RttTime>::const_iterator it = reports_.begin();
+  UpdateAvgRttMs(&reports_, &avg_rtt_ms_);
-       it != reports_.end(); ++it) {
-    if (it->rtt > max_rtt)
-      max_rtt = it->rtt;
-  }
 
-  // If there is a valid rtt, update all observers.
+  // If there is a valid rtt, update all observers with the max rtt.
-  if (max_rtt > 0) {
+  // 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::DeregisterStatsObserver(CallStatsObserver* observer) {
 
 void CallStats::OnRttUpdate(uint32_t rtt) {
   CriticalSectionScoped cs(crit_.get());
-  int64_t time_now = TickTime::MillisecondTimestamp();
+  reports_.push_back(RttTime(rtt, TickTime::MillisecondTimestamp()));
-  reports_.push_back(RttTime(rtt, time_now));
 }
 
 }  // namespace webrtc

webrtc/video_engine/call_stats.h

@@ -43,12 +43,6 @@ class CallStats : public Module {
   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 @@ class CallStats : public Module {
     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 @@ class CallStats : public Module {
   // 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_;

webrtc/video_engine/call_stats_unittest.cc

@@ -101,42 +101,42 @@ TEST_F(CallStatsTest, ProcessTime) {
 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;
+  const uint32_t kRtt = 100;
-  rtcp_rtt_stats->OnRttUpdate(rtt);
+  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 @@ TEST_F(CallStatsTest, ChangeRtt) {
   TickTime::AdvanceFakeClock(1000);
 
   // Set a first value and verify the callback is triggered.
-  const uint32_t first_rtt = 100;
+  const uint32_t kFirstRtt = 100;
-  rtcp_rtt_stats->OnRttUpdate(first_rtt);
+  rtcp_rtt_stats->OnRttUpdate(kFirstRtt);
-  EXPECT_CALL(stats_observer, OnRttUpdate(first_rtt))
+  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;
+  const uint32_t kHighRtt = kFirstRtt + 20;
-  rtcp_rtt_stats->OnRttUpdate(high_rtt);
+  rtcp_rtt_stats->OnRttUpdate(kHighRtt);
-  EXPECT_CALL(stats_observer, OnRttUpdate(high_rtt))
+  EXPECT_CALL(stats_observer, OnRttUpdate(kHighRtt))
       .Times(1);
   call_stats_->Process();
 
@@ -169,20 +169,50 @@ TEST_F(CallStatsTest, ChangeRtt) {
   // 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;
+  const uint32_t kLowRtt = kFirstRtt - 20;
-  rtcp_rtt_stats->OnRttUpdate(low_rtt);
+  rtcp_rtt_stats->OnRttUpdate(kLowRtt);
-  EXPECT_CALL(stats_observer, OnRttUpdate(high_rtt))
+  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