From 41bb792ce47ead4e137d20aa4d87a30683992ac7 Mon Sep 17 00:00:00 2001
From: philipel <philipel@webrtc.org>
Date: Mon, 20 Feb 2017 07:53:23 -0800
Subject: [PATCH] Advance picture id of keyframe if the stream has been
 continuous without a new keyframe for a while.

BUG=webrtc:5514

Review-Url: https://codereview.webrtc.org/2708593003
Cr-Commit-Position: refs/heads/master@{#16725}
---
 .../video_coding/rtp_frame_reference_finder.cc | 18 +++++++++++++++---
 .../rtp_frame_reference_finder_unittest.cc     | 13 +++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/webrtc/modules/video_coding/rtp_frame_reference_finder.cc b/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
index 3d5283378f..e49a81d758 100644
--- a/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
+++ b/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
@@ -112,6 +112,16 @@ void RtpFrameReferenceFinder::UpdateLastPictureIdWithPadding(uint16_t seq_num) {
     ++next_seq_num_with_padding;
     padding_seq_num_it = stashed_padding_.erase(padding_seq_num_it);
   }
+
+  // In the case where the stream has been continuous without any new keyframes
+  // for a while there is a risk that new frames will appear to be older than
+  // the keyframe they belong to due to wrapping sequence number. In order
+  // to prevent this we advance the picture id of the keyframe every so often.
+  if (ForwardDiff(gop_seq_num_it->first, seq_num) > 10000) {
+    RTC_DCHECK_EQ(1ul, last_seq_num_gop_.size());
+    last_seq_num_gop_[seq_num] = gop_seq_num_it->second;
+    last_seq_num_gop_.erase(gop_seq_num_it);
+  }
 }
 
 void RtpFrameReferenceFinder::RetryStashedFrames() {
@@ -164,8 +174,10 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
   // Clean up info for old keyframes but make sure to keep info
   // for the last keyframe.
   auto clean_to = last_seq_num_gop_.lower_bound(frame->last_seq_num() - 100);
-  if (clean_to != last_seq_num_gop_.end())
-    last_seq_num_gop_.erase(last_seq_num_gop_.begin(), clean_to);
+  for (auto it = last_seq_num_gop_.begin();
+       it != clean_to && last_seq_num_gop_.size() > 1;) {
+    it = last_seq_num_gop_.erase(it);
+  }
 
   // Find the last sequence number of the last frame for the keyframe
   // that this frame indirectly references.
@@ -173,7 +185,7 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
   if (seq_num_it == last_seq_num_gop_.begin()) {
     LOG(LS_WARNING) << "Generic frame with packet range ["
                     << frame->first_seq_num() << ", " << frame->last_seq_num()
-                    << "] has no Gop, dropping frame.";
+                    << "] has no GoP, dropping frame.";
     return;
   }
   seq_num_it--;
diff --git a/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc b/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
index d67e1baafe..5fa88802a6 100644
--- a/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
+++ b/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
@@ -300,6 +300,19 @@ TEST_F(TestRtpFrameReferenceFinder, PaddingPacketsReorderedMultipleKeyframes) {
   EXPECT_EQ(4UL, frames_from_callback_.size());
 }
 
+TEST_F(TestRtpFrameReferenceFinder, AdvanceSavedKeyframe) {
+  uint16_t sn = Rand();
+
+  InsertGeneric(sn, sn, true);
+  InsertGeneric(sn + 1, sn + 1, true);
+  InsertGeneric(sn + 2, sn + 10000, false);
+  InsertGeneric(sn + 10001, sn + 20000, false);
+  InsertGeneric(sn + 20001, sn + 30000, false);
+  InsertGeneric(sn + 30001, sn + 40000, false);
+
+  EXPECT_EQ(6UL, frames_from_callback_.size());
+}
+
 TEST_F(TestRtpFrameReferenceFinder, ClearTo) {
   uint16_t sn = Rand();