From 45cb8f65baefadf4ba77c4935571e8786cf1a943 Mon Sep 17 00:00:00 2001
From: philipel <philipel@webrtc.org>
Date: Thu, 19 Mar 2020 15:54:23 +0100
Subject: [PATCH] Drop duplicate VP8 frames in the RtpFrameReferenceFinder.

Bug: webrtc:11431
Change-Id: Ib74c05e7cf12b9a3f60423a268a5d459407fd00b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171060
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30834}
---
 .../rtp_frame_reference_finder.cc             | 22 +++++++++-
 .../rtp_frame_reference_finder_unittest.cc    | 44 +++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/modules/video_coding/rtp_frame_reference_finder.cc b/modules/video_coding/rtp_frame_reference_finder.cc
index e67ac664b1..ce0811b83e 100644
--- a/modules/video_coding/rtp_frame_reference_finder.cc
+++ b/modules/video_coding/rtp_frame_reference_finder.cc
@@ -341,7 +341,16 @@ RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp8(
     layer_info_it =
         layer_info_.emplace(unwrapped_tl0, layer_info_it->second).first;
     frame->num_references = 1;
-    frame->references[0] = layer_info_it->second[0];
+    int64_t last_pid_on_layer = layer_info_it->second[0];
+
+    // Is this an old frame that has already been used to update the state? If
+    // so, drop it.
+    if (AheadOrAt<uint16_t, kPicIdLength>(last_pid_on_layer,
+                                          frame->id.picture_id)) {
+      return kDrop;
+    }
+
+    frame->references[0] = last_pid_on_layer;
     UpdateLayerInfoVp8(frame, unwrapped_tl0, codec_header.temporalIdx);
     return kHandOff;
   }
@@ -349,8 +358,17 @@ RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp8(
   // Layer sync frame, this frame only references its base layer frame.
   if (codec_header.layerSync) {
     frame->num_references = 1;
-    frame->references[0] = layer_info_it->second[0];
+    int64_t last_pid_on_layer = layer_info_it->second[codec_header.temporalIdx];
 
+    // Is this an old frame that has already been used to update the state? If
+    // so, drop it.
+    if (last_pid_on_layer != -1 &&
+        AheadOrAt<uint16_t, kPicIdLength>(last_pid_on_layer,
+                                          frame->id.picture_id)) {
+      return kDrop;
+    }
+
+    frame->references[0] = layer_info_it->second[0];
     UpdateLayerInfoVp8(frame, unwrapped_tl0, codec_header.temporalIdx);
     return kHandOff;
   }
diff --git a/modules/video_coding/rtp_frame_reference_finder_unittest.cc b/modules/video_coding/rtp_frame_reference_finder_unittest.cc
index e103023420..ca3c7138c4 100644
--- a/modules/video_coding/rtp_frame_reference_finder_unittest.cc
+++ b/modules/video_coding/rtp_frame_reference_finder_unittest.cc
@@ -406,6 +406,50 @@ TEST_F(TestRtpFrameReferenceFinder, Vp8KeyFrameReferences) {
   CheckReferencesVp8(sn);
 }
 
+TEST_F(TestRtpFrameReferenceFinder, Vp8RepeatedFrame_0) {
+  uint16_t pid = Rand();
+  uint16_t sn = Rand();
+
+  InsertVp8(sn, sn, true, pid, 0, 1);
+  InsertVp8(sn + 1, sn + 1, false, pid + 1, 0, 2);
+  InsertVp8(sn + 1, sn + 1, false, pid + 1, 0, 2);
+
+  ASSERT_EQ(2UL, frames_from_callback_.size());
+  CheckReferencesVp8(pid);
+  CheckReferencesVp8(pid + 1, pid);
+}
+
+TEST_F(TestRtpFrameReferenceFinder, Vp8RepeatedFrameLayerSync_01) {
+  uint16_t pid = Rand();
+  uint16_t sn = Rand();
+
+  InsertVp8(sn, sn, true, pid, 0, 1);
+  InsertVp8(sn + 1, sn + 1, false, pid + 1, 1, 1, true);
+  ASSERT_EQ(2UL, frames_from_callback_.size());
+  InsertVp8(sn + 1, sn + 1, false, pid + 1, 1, 1, true);
+
+  ASSERT_EQ(2UL, frames_from_callback_.size());
+  CheckReferencesVp8(pid);
+  CheckReferencesVp8(pid + 1, pid);
+}
+
+TEST_F(TestRtpFrameReferenceFinder, Vp8RepeatedFrame_01) {
+  uint16_t pid = Rand();
+  uint16_t sn = Rand();
+
+  InsertVp8(sn, sn, true, pid, 0, 1);
+  InsertVp8(sn + 1, sn + 1, false, pid + 1, 0, 2, true);
+  InsertVp8(sn + 2, sn + 2, false, pid + 2, 0, 3);
+  InsertVp8(sn + 3, sn + 3, false, pid + 3, 0, 4);
+  InsertVp8(sn + 3, sn + 3, false, pid + 3, 0, 4);
+
+  ASSERT_EQ(4UL, frames_from_callback_.size());
+  CheckReferencesVp8(pid);
+  CheckReferencesVp8(pid + 1, pid);
+  CheckReferencesVp8(pid + 2, pid + 1);
+  CheckReferencesVp8(pid + 3, pid + 2);
+}
+
 // Test with 1 temporal layer.
 TEST_F(TestRtpFrameReferenceFinder, Vp8TemporalLayers_0) {
   uint16_t pid = Rand();