From e0754305aa8787c483c2a4fd319fa383b9bf1493 Mon Sep 17 00:00:00 2001
From: philipel <philipel@webrtc.org>
Date: Wed, 25 Jan 2017 08:56:23 -0800
Subject: [PATCH] Don't update the jitter estimate with frames containing
 retransmitted packets.

BUG=chromium:682636

Review-Url: https://codereview.webrtc.org/2645343002
Cr-Commit-Position: refs/heads/master@{#16273}
---
 webrtc/modules/video_coding/frame_buffer2.cc | 22 +++++++++++---------
 webrtc/modules/video_coding/frame_object.cc  |  4 ++++
 webrtc/modules/video_coding/frame_object.h   |  6 ++++++
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index 58c41bf1d0..0831c0cd5f 100644
--- a/webrtc/modules/video_coding/frame_buffer2.cc
+++ b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -118,18 +118,20 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame(
   rtc::CritScope lock(&crit_);
   if (next_frame_it != frames_.end()) {
     std::unique_ptr<FrameObject> frame = std::move(next_frame_it->second.frame);
-    int64_t received_time = frame->ReceivedTime();
-    uint32_t timestamp = frame->timestamp;
 
-    int64_t frame_delay;
-    if (inter_frame_delay_.CalculateDelay(timestamp, &frame_delay,
-                                          received_time)) {
-      jitter_estimator_->UpdateEstimate(frame_delay, frame->size());
+    if (!frame->delayed_by_retransmission()) {
+      int64_t frame_delay;
+
+      if (inter_frame_delay_.CalculateDelay(frame->timestamp, &frame_delay,
+                                            frame->ReceivedTime())) {
+        jitter_estimator_->UpdateEstimate(frame_delay, frame->size());
+      }
+
+      float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0;
+      timing_->SetJitterDelay(jitter_estimator_->GetJitterEstimate(rtt_mult));
+      timing_->UpdateCurrentDelay(frame->RenderTime(),
+                                  clock_->TimeInMilliseconds());
     }
-    float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0;
-    timing_->SetJitterDelay(jitter_estimator_->GetJitterEstimate(rtt_mult));
-    timing_->UpdateCurrentDelay(frame->RenderTime(),
-                                clock_->TimeInMilliseconds());
 
     UpdateJitterDelay();
 
diff --git a/webrtc/modules/video_coding/frame_object.cc b/webrtc/modules/video_coding/frame_object.cc
index d593d4bef4..70b0a02868 100644
--- a/webrtc/modules/video_coding/frame_object.cc
+++ b/webrtc/modules/video_coding/frame_object.cc
@@ -121,6 +121,10 @@ int64_t RtpFrameObject::RenderTime() const {
   return _renderTimeMs;
 }
 
+bool RtpFrameObject::delayed_by_retransmission() const {
+  return times_nacked() > 0;
+}
+
 rtc::Optional<RTPVideoTypeHeader> RtpFrameObject::GetCodecHeader() const {
   rtc::CritScope lock(&packet_buffer_->crit_);
   VCMPacket* packet = packet_buffer_->GetPacket(first_seq_num_);
diff --git a/webrtc/modules/video_coding/frame_object.h b/webrtc/modules/video_coding/frame_object.h
index 915224ede8..413cc33910 100644
--- a/webrtc/modules/video_coding/frame_object.h
+++ b/webrtc/modules/video_coding/frame_object.h
@@ -37,6 +37,11 @@ class FrameObject : public webrtc::VCMEncodedFrame {
   // When this frame should be rendered.
   virtual int64_t RenderTime() const = 0;
 
+  // This information is currently needed by the timing calculation class.
+  // TODO(philipel): Remove this function when a new timing class has
+  //                 been implemented.
+  virtual bool delayed_by_retransmission() const { return 0; }
+
   size_t size() { return _length; }
 
   // The tuple (|picture_id|, |spatial_layer|) uniquely identifies a frame
@@ -72,6 +77,7 @@ class RtpFrameObject : public FrameObject {
   uint32_t Timestamp() const override;
   int64_t ReceivedTime() const override;
   int64_t RenderTime() const override;
+  bool delayed_by_retransmission() const override;
   rtc::Optional<RTPVideoTypeHeader> GetCodecHeader() const;
 
  private: