Report max interframe delay over window insdead of interframe delay sum

Maximum of interframe delay is calculated over moving window in
ReceiveStatistics proxy now and reported via GetStats. Name of a metric
is also changed.

BUG=none

Review-Url: https://codereview.webrtc.org/2995143002
Cr-Commit-Position: refs/heads/master@{#19463}

webrtc/api/statstypes.cc

@@ -538,8 +538,8 @@ const char* StatsReport::Value::display_name() const {
       return "googFrameWidthSent";
     case kStatsValueNameInitiator:
       return "googInitiator";
-    case kStatsValueNameInterframeDelaySumMs:
-      return "googInterframeDelaySum";
+    case kStatsValueNameInterframeDelayMaxMs:
+      return "googInterframeDelayMax";
     case kStatsValueNameIssuerId:
       return "googIssuerId";
     case kStatsValueNameJitterReceived:

webrtc/api/statstypes.h

@@ -107,7 +107,7 @@ class StatsReport {
     kStatsValueNameDataChannelId,
     kStatsValueNameFramesDecoded,
     kStatsValueNameFramesEncoded,
-    kStatsValueNameInterframeDelaySumMs,
+    kStatsValueNameInterframeDelayMaxMs,  // Max over last 10 seconds.
     kStatsValueNameMediaType,
     kStatsValueNamePacketsLost,
     kStatsValueNamePacketsReceived,

webrtc/call/video_receive_stream.h

@@ -75,7 +75,7 @@ class VideoReceiveStream {
     int jitter_buffer_ms = 0;
     int min_playout_delay_ms = 0;
     int render_delay_ms = 10;
-    uint64_t interframe_delay_sum_ms = 0;
+    int64_t interframe_delay_max_ms = -1;
     uint32_t frames_decoded = 0;
     rtc::Optional<uint64_t> qp_sum;
 

webrtc/media/base/mediachannel.h

@@ -753,7 +753,7 @@ struct VideoReceiverInfo : public MediaReceiverInfo {
         frames_received(0),
         frames_decoded(0),
         frames_rendered(0),
-        interframe_delay_sum_ms(0),
+        interframe_delay_max_ms(-1),
         decode_ms(0),
         max_decode_ms(0),
         jitter_buffer_ms(0),
@@ -783,7 +783,7 @@ struct VideoReceiverInfo : public MediaReceiverInfo {
   uint32_t frames_decoded;
   uint32_t frames_rendered;
   rtc::Optional<uint64_t> qp_sum;
-  uint64_t interframe_delay_sum_ms;
+  int64_t interframe_delay_max_ms;
 
   // All stats below are gathered per-VideoReceiver, but some will be correlated
   // across MediaStreamTracks.  NOTE(hta): when sinking stats into per-SSRC

webrtc/media/engine/webrtcvideoengine.cc

@@ -2464,7 +2464,7 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo(
   info.frames_rendered = stats.frames_rendered;
   info.qp_sum = stats.qp_sum;
 
-  info.interframe_delay_sum_ms = stats.interframe_delay_sum_ms;
+  info.interframe_delay_max_ms = stats.interframe_delay_max_ms;
 
   info.codec_name = GetCodecNameFromPayloadType(stats.current_payload_type);
 

webrtc/pc/statscollector.cc

@@ -268,8 +268,8 @@ void ExtractStats(const cricket::VideoReceiverInfo& info, StatsReport* report) {
                       info.timing_frame_info->ToString());
   }
 
-  report->AddInt64(StatsReport::kStatsValueNameInterframeDelaySumMs,
-                   info.interframe_delay_sum_ms);
+  report->AddInt64(StatsReport::kStatsValueNameInterframeDelayMaxMs,
+                   info.interframe_delay_max_ms);
 }
 
 void ExtractStats(const cricket::VideoSenderInfo& info, StatsReport* report) {

webrtc/rtc_base/BUILD.gn

@@ -135,6 +135,7 @@ rtc_static_library("rtc_base_approved") {
     "location.cc",
     "location.h",
     "mod_ops.h",
+    "moving_max_counter.h",
     "onetimeevent.h",
     "optional.cc",
     "optional.h",
@@ -824,6 +825,7 @@ if (rtc_include_tests) {
       "logging_unittest.cc",
       "md5digest_unittest.cc",
       "mod_ops_unittest.cc",
+      "moving_max_counter_unittest.cc",
       "onetimeevent_unittest.cc",
       "optional_unittest.cc",
       "pathutils_unittest.cc",

webrtc/rtc_base/moving_max_counter.h

@@ -0,0 +1,116 @@
+/*
+ *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_RTC_BASE_MOVING_MAX_COUNTER_H_
+#define WEBRTC_RTC_BASE_MOVING_MAX_COUNTER_H_
+
+#include <stdint.h>
+
+#include <deque>
+#include <limits>
+#include <utility>
+
+#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/constructormagic.h"
+#include "webrtc/rtc_base/optional.h"
+
+namespace rtc {
+
+// Implements moving max: can add samples to it and calculate maximum over some
+// fixed moving window.
+//
+// Window size is configured at constructor.
+// Samples can be added with |Add()| and max over current window is returned by
+// |MovingMax|. |current_time_ms| in successive calls to Add and MovingMax
+// should never decrease as if it's a wallclock time.
+template <class T>
+class MovingMaxCounter {
+ public:
+  explicit MovingMaxCounter(int64_t window_length_ms);
+  // Advances the current time, and adds a new sample. The new current time must
+  // be at least as large as the old current time.
+  void Add(const T& sample, int64_t current_time_ms);
+  // Advances the current time, and returns the maximum sample in the time
+  // window ending at the current time. The new current time must be at least as
+  // large as the old current time.
+  rtc::Optional<T> Max(int64_t current_time_ms);
+  void Reset();
+
+ private:
+  // Throws out obsolete samples.
+  void RollWindow(int64_t new_time_ms);
+  const int64_t window_length_ms_;
+  // This deque stores (timestamp, sample) pairs in chronological order; new
+  // pairs are only ever added at the end. However, because they can't affect
+  // the Max() calculation, pairs older than window_length_ms_ are discarded,
+  // and if an older pair has a sample that's smaller than that of a younger
+  // pair, the older pair is discarded. As a result, the sequence of timestamps
+  // is strictly increasing, and the sequence of samples is strictly decreasing.
+  std::deque<std::pair<int64_t, T>> samples_;
+#if RTC_DCHECK_IS_ON
+  int64_t last_call_time_ms_ = std::numeric_limits<int64_t>::min();
+#endif
+  RTC_DISALLOW_COPY_AND_ASSIGN(MovingMaxCounter);
+};
+
+template <class T>
+MovingMaxCounter<T>::MovingMaxCounter(int64_t window_length_ms)
+    : window_length_ms_(window_length_ms) {}
+
+template <class T>
+void MovingMaxCounter<T>::Add(const T& sample, int64_t current_time_ms) {
+  RollWindow(current_time_ms);
+  // Remove samples that will never be maximum in any window: newly added sample
+  // will always be in all windows the previous samples are. Thus, smaller or
+  // equal samples could be removed. This will maintain the invariant - deque
+  // contains strictly decreasing sequence of values.
+  while (!samples_.empty() && samples_.back().second <= sample) {
+    samples_.pop_back();
+  }
+  // Add the new sample but only if there's no existing sample at the same time.
+  // Due to checks above, the already existing element will be larger, so the
+  // new sample will never be the maximum in any window.
+  if (samples_.empty() || samples_.back().first < current_time_ms) {
+    samples_.emplace_back(std::make_pair(current_time_ms, sample));
+  }
+}
+
+template <class T>
+rtc::Optional<T> MovingMaxCounter<T>::Max(int64_t current_time_ms) {
+  RollWindow(current_time_ms);
+  rtc::Optional<T> res;
+  if (!samples_.empty()) {
+    res.emplace(samples_.front().second);
+  }
+  return res;
+}
+
+template <class T>
+void MovingMaxCounter<T>::Reset() {
+  samples_.clear();
+}
+
+template <class T>
+void MovingMaxCounter<T>::RollWindow(int64_t new_time_ms) {
+#if RTC_DCHECK_IS_ON
+  RTC_DCHECK_GE(new_time_ms, last_call_time_ms_);
+  last_call_time_ms_ = new_time_ms;
+#endif
+  const int64_t window_begin_ms = new_time_ms - window_length_ms_;
+  auto it = samples_.begin();
+  while (it != samples_.end() && it->first < window_begin_ms) {
+    ++it;
+  }
+  samples_.erase(samples_.begin(), it);
+}
+
+}  // namespace rtc
+
+#endif  // WEBRTC_RTC_BASE_MOVING_MAX_COUNTER_H_

webrtc/rtc_base/moving_max_counter_unittest.cc

@@ -0,0 +1,57 @@
+/*
+ *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/rtc_base/moving_max_counter.h"
+#include "webrtc/test/gtest.h"
+
+TEST(MovingMaxCounter, ReportsMaximumInTheWindow) {
+  rtc::MovingMaxCounter<int> counter(100);
+  counter.Add(1, 1);
+  EXPECT_EQ(counter.Max(1), rtc::Optional<int>(1));
+  counter.Add(2, 30);
+  EXPECT_EQ(counter.Max(30), rtc::Optional<int>(2));
+  counter.Add(100, 60);
+  EXPECT_EQ(counter.Max(60), rtc::Optional<int>(100));
+  counter.Add(4, 70);
+  EXPECT_EQ(counter.Max(70), rtc::Optional<int>(100));
+  counter.Add(5, 90);
+  EXPECT_EQ(counter.Max(90), rtc::Optional<int>(100));
+}
+
+TEST(MovingMaxCounter, IgnoresOldElements) {
+  rtc::MovingMaxCounter<int> counter(100);
+  counter.Add(1, 1);
+  counter.Add(2, 30);
+  counter.Add(100, 60);
+  counter.Add(4, 70);
+  counter.Add(5, 90);
+  EXPECT_EQ(counter.Max(160), rtc::Optional<int>(100));
+  // 100 is now out of the window. Next maximum is 5.
+  EXPECT_EQ(counter.Max(161), rtc::Optional<int>(5));
+}
+
+TEST(MovingMaxCounter, HandlesEmptyWindow) {
+  rtc::MovingMaxCounter<int> counter(100);
+  counter.Add(123, 1);
+  EXPECT_TRUE(counter.Max(101).has_value());
+  EXPECT_FALSE(counter.Max(102).has_value());
+}
+
+TEST(MovingMaxCounter, HandlesSamplesWithEqualTimestamps) {
+  rtc::MovingMaxCounter<int> counter(100);
+  counter.Add(2, 30);
+  EXPECT_EQ(counter.Max(30), rtc::Optional<int>(2));
+  counter.Add(5, 30);
+  EXPECT_EQ(counter.Max(30), rtc::Optional<int>(5));
+  counter.Add(4, 30);
+  EXPECT_EQ(counter.Max(30), rtc::Optional<int>(5));
+  counter.Add(1, 90);
+  EXPECT_EQ(counter.Max(150), rtc::Optional<int>(1));
+}

webrtc/video/receive_statistics_proxy.cc

@@ -43,6 +43,9 @@ const int kHighQpThresholdVp8 = 70;
 const int kLowVarianceThreshold = 1;
 const int kHighVarianceThreshold = 2;
 
+// Some metrics are reported as a maximum over this period.
+const int kMovingMaxWindowMs = 10000;
+
 // How large window we use to calculate the framerate/bitrate.
 const int kRateStatisticsWindowSizeMs = 1000;
 }  // namespace
@@ -78,6 +81,7 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy(
       e2e_delay_max_ms_screenshare_(-1),
       interframe_delay_max_ms_video_(-1),
       interframe_delay_max_ms_screenshare_(-1),
+      interframe_delay_max_moving_(kMovingMaxWindowMs),
       freq_offset_counter_(clock, nullptr, kFreqOffsetProcessIntervalMs),
       first_report_block_time_ms_(-1),
       avg_rtt_ms_(0),
@@ -394,6 +398,8 @@ VideoReceiveStream::Stats ReceiveStatisticsProxy::GetStats() const {
   stats_.decode_frame_rate = decode_fps_estimator_.Rate(now_ms).value_or(0);
   stats_.total_bitrate_bps =
       static_cast<int>(total_byte_tracker_.ComputeRate() * 8);
+  stats_.interframe_delay_max_ms =
+      interframe_delay_max_moving_.Max(now_ms).value_or(-1);
   return stats_;
 }
 
@@ -544,7 +550,7 @@ void ReceiveStatisticsProxy::OnDecodedFrame(rtc::Optional<uint8_t> qp,
   if (last_decoded_frame_time_ms_) {
     int64_t interframe_delay_ms = now - *last_decoded_frame_time_ms_;
     RTC_DCHECK_GE(interframe_delay_ms, 0);
-    stats_.interframe_delay_sum_ms += interframe_delay_ms;
+    interframe_delay_max_moving_.Add(interframe_delay_ms, now);
     if (last_content_type_ == VideoContentType::SCREENSHARE) {
       interframe_delay_counter_screenshare_.Add(interframe_delay_ms);
       if (interframe_delay_max_ms_screenshare_ < interframe_delay_ms) {

webrtc/video/receive_statistics_proxy.h

@@ -19,6 +19,7 @@
 #include "webrtc/common_video/include/frame_callback.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
 #include "webrtc/rtc_base/criticalsection.h"
+#include "webrtc/rtc_base/moving_max_counter.h"
 #include "webrtc/rtc_base/rate_statistics.h"
 #include "webrtc/rtc_base/ratetracker.h"
 #include "webrtc/rtc_base/thread_annotations.h"
@@ -155,6 +156,8 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   int64_t e2e_delay_max_ms_screenshare_ GUARDED_BY(crit_);
   int64_t interframe_delay_max_ms_video_  GUARDED_BY(crit_);
   int64_t interframe_delay_max_ms_screenshare_  GUARDED_BY(crit_);
+  mutable rtc::MovingMaxCounter<int> interframe_delay_max_moving_
+      GUARDED_BY(crit_);
   MaxCounter freq_offset_counter_ GUARDED_BY(crit_);
   int64_t first_report_block_time_ms_ GUARDED_BY(crit_);
   ReportBlockStats report_block_stats_ GUARDED_BY(crit_);

webrtc/video/receive_statistics_proxy_unittest.cc

@@ -98,26 +98,63 @@ TEST_F(ReceiveStatisticsProxyTest, OnDecodedFrameIncreasesQpSum) {
             statistics_proxy_->GetStats().qp_sum);
 }
 
-TEST_F(ReceiveStatisticsProxyTest,
-       OnDecodedFrameIncreasesInterframeDelayMsSum) {
-  const uint64_t kInterframeDelayMs1 = 100;
-  const uint64_t kInterframeDelayMs2 = 200;
-  EXPECT_EQ(0u, statistics_proxy_->GetStats().interframe_delay_sum_ms);
+TEST_F(ReceiveStatisticsProxyTest, ReportsMaxInterframeDelay) {
+  const int64_t kInterframeDelayMs1 = 100;
+  const int64_t kInterframeDelayMs2 = 200;
+  const int64_t kInterframeDelayMs3 = 100;
+  EXPECT_EQ(-1, statistics_proxy_->GetStats().interframe_delay_max_ms);
   statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(3u),
                                     VideoContentType::UNSPECIFIED);
-  EXPECT_EQ(0u, statistics_proxy_->GetStats().interframe_delay_sum_ms);
+  EXPECT_EQ(-1, statistics_proxy_->GetStats().interframe_delay_max_ms);
 
   fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs1);
   statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
                                     VideoContentType::UNSPECIFIED);
   EXPECT_EQ(kInterframeDelayMs1,
-            statistics_proxy_->GetStats().interframe_delay_sum_ms);
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
 
   fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs2);
   statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
                                     VideoContentType::UNSPECIFIED);
-  EXPECT_EQ(kInterframeDelayMs1 + kInterframeDelayMs2,
-            statistics_proxy_->GetStats().interframe_delay_sum_ms);
+  EXPECT_EQ(kInterframeDelayMs2,
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
+
+  fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs3);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
+                                    VideoContentType::UNSPECIFIED);
+  // kInterframeDelayMs3 is smaller than kInterframeDelayMs2.
+  EXPECT_EQ(kInterframeDelayMs2,
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
+}
+
+TEST_F(ReceiveStatisticsProxyTest, ReportInterframeDelayInWindow) {
+  const int64_t kInterframeDelayMs1 = 9000;
+  const int64_t kInterframeDelayMs2 = 7500;
+  const int64_t kInterframeDelayMs3 = 7000;
+  EXPECT_EQ(-1, statistics_proxy_->GetStats().interframe_delay_max_ms);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(3u),
+                                    VideoContentType::UNSPECIFIED);
+  EXPECT_EQ(-1, statistics_proxy_->GetStats().interframe_delay_max_ms);
+
+  fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs1);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
+                                    VideoContentType::UNSPECIFIED);
+  EXPECT_EQ(kInterframeDelayMs1,
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
+
+  fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs2);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
+                                    VideoContentType::UNSPECIFIED);
+  // Still first delay is the maximum
+  EXPECT_EQ(kInterframeDelayMs1,
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
+
+  fake_clock_.AdvanceTimeMilliseconds(kInterframeDelayMs3);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(127u),
+                                    VideoContentType::UNSPECIFIED);
+  // Now the first sample is out of the window, so the second is the maximum.
+  EXPECT_EQ(kInterframeDelayMs2,
+            statistics_proxy_->GetStats().interframe_delay_max_ms);
 }
 
 TEST_F(ReceiveStatisticsProxyTest, OnDecodedFrameWithoutQpQpSumWontExist) {