From 3e86e7eec727b9c2348a92ca3282d9066b4f9e9a Mon Sep 17 00:00:00 2001
From: sprang <sprang@webrtc.org>
Date: Tue, 22 Aug 2017 09:23:28 -0700
Subject: [PATCH] Ignore inter-frame delay stats samples when stream is
 inactive

BUG=webrtc:7694

Review-Url: https://codereview.webrtc.org/3002103002
Cr-Commit-Position: refs/heads/master@{#19453}
---
 webrtc/video/receive_statistics_proxy.cc      |  8 ++++
 webrtc/video/receive_statistics_proxy.h       |  5 ++-
 .../receive_statistics_proxy_unittest.cc      | 41 +++++++++++++++++++
 webrtc/video/video_receive_stream.cc          |  2 +
 4 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc
index 7852a05913..61c264fcbe 100644
--- a/webrtc/video/receive_statistics_proxy.cc
+++ b/webrtc/video/receive_statistics_proxy.cc
@@ -648,6 +648,14 @@ void ReceiveStatisticsProxy::OnPreDecode(
   }
 }
 
+void ReceiveStatisticsProxy::OnStreamInactive() {
+  // TODO(sprang): Figure out any other state that should be reset.
+
+  rtc::CritScope lock(&crit_);
+  // Don't report inter-frame delay if stream was paused.
+  last_decoded_frame_time_ms_.reset();
+}
+
 void ReceiveStatisticsProxy::SampleCounter::Add(int sample) {
   sum += sample;
   ++num_samples;
diff --git a/webrtc/video/receive_statistics_proxy.h b/webrtc/video/receive_statistics_proxy.h
index 0cd0b529aa..1ed95cdd28 100644
--- a/webrtc/video/receive_statistics_proxy.h
+++ b/webrtc/video/receive_statistics_proxy.h
@@ -58,6 +58,9 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   void OnPreDecode(const EncodedImage& encoded_image,
                    const CodecSpecificInfo* codec_specific_info);
 
+  // Indicates video stream has been paused (no incoming packets).
+  void OnStreamInactive();
+
   // Overrides VCMReceiveStatisticsCallback.
   void OnReceiveRatesUpdated(uint32_t bitRate, uint32_t frameRate) override;
   void OnFrameCountsUpdated(const FrameCounts& frame_counts) override;
@@ -160,7 +163,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   int64_t avg_rtt_ms_ GUARDED_BY(crit_);
   mutable std::map<int64_t, size_t> frame_window_ GUARDED_BY(&crit_);
   VideoContentType last_content_type_ GUARDED_BY(&crit_);
-  rtc::Optional<int64_t> last_decoded_frame_time_ms_;
+  rtc::Optional<int64_t> last_decoded_frame_time_ms_ GUARDED_BY(&crit_);
   rtc::Optional<TimingFrameInfo> timing_frame_info_ GUARDED_BY(&crit_);
 };
 
diff --git a/webrtc/video/receive_statistics_proxy_unittest.cc b/webrtc/video/receive_statistics_proxy_unittest.cc
index d37d203548..8b3bc0aa7e 100644
--- a/webrtc/video/receive_statistics_proxy_unittest.cc
+++ b/webrtc/video/receive_statistics_proxy_unittest.cc
@@ -779,4 +779,45 @@ TEST_P(ReceiveStatisticsProxyTest, MaxInterFrameDelayOnlyWithValidAverage) {
                    "WebRTC.Video.Screenshare.InterframeDelayMaxInMs"));
 }
 
+TEST_P(ReceiveStatisticsProxyTest, MaxInterFrameDelayOnlyWithPause) {
+  const VideoContentType content_type = GetParam();
+  const int kInterFrameDelayMs = 33;
+  for (int i = 0; i <= kMinRequiredSamples; ++i) {
+    statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
+    fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs);
+  }
+
+  // At this state, we should have a valid inter-frame delay.
+  // Indicate stream paused and make a large jump in time.
+  statistics_proxy_->OnStreamInactive();
+  fake_clock_.AdvanceTimeMilliseconds(5000);
+
+  // Insert two more frames. The interval during the pause should be disregarded
+  // in the stats.
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
+  fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs);
+  statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
+
+  statistics_proxy_.reset();
+  if (content_type == VideoContentType::SCREENSHARE) {
+    EXPECT_EQ(
+        1, metrics::NumSamples("WebRTC.Video.Screenshare.InterframeDelayInMs"));
+    EXPECT_EQ(1, metrics::NumSamples(
+                     "WebRTC.Video.Screenshare.InterframeDelayMaxInMs"));
+    EXPECT_EQ(
+        kInterFrameDelayMs,
+        metrics::MinSample("WebRTC.Video.Screenshare.InterframeDelayInMs"));
+    EXPECT_EQ(
+        kInterFrameDelayMs,
+        metrics::MinSample("WebRTC.Video.Screenshare.InterframeDelayMaxInMs"));
+  } else {
+    EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.InterframeDelayInMs"));
+    EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.InterframeDelayMaxInMs"));
+    EXPECT_EQ(kInterFrameDelayMs,
+              metrics::MinSample("WebRTC.Video.InterframeDelayInMs"));
+    EXPECT_EQ(kInterFrameDelayMs,
+              metrics::MinSample("WebRTC.Video.InterframeDelayMaxInMs"));
+  }
+}
+
 }  // namespace webrtc
diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc
index f702742925..56264d9b1f 100644
--- a/webrtc/video/video_receive_stream.cc
+++ b/webrtc/video/video_receive_stream.cc
@@ -431,6 +431,8 @@ bool VideoReceiveStream::Decode() {
     // To avoid spamming keyframe requests for a stream that is not active we
     // check if we have received a packet within the last 5 seconds.
     bool stream_is_active = last_packet_ms && now_ms - *last_packet_ms < 5000;
+    if (!stream_is_active)
+      stats_proxy_.OnStreamInactive();
 
     // If we recently have been receiving packets belonging to a keyframe then
     // we assume a keyframe is currently being received.