commit c9e532a7ebd3836eb98a6885969af5181cb1bfb1
author Danil Chapovalov <danilchap@webrtc.org> Tue Dec 10 16:03:00 2019
committer Commit Bot <commit-bot@chromium.org> Fri Dec 13 11:36:24 2019
tree bec13186b455542c670fd99792f937ddb7f66f27
parent 5e9cac984ff737894a04ba6134969cd3be441348

Fix PacketBuffer::LastReceivedKeyframePacketMs

to return time of the last receieved packet of a key frame rather than
last received first packet of a key frame.

To match VideoReceiveStream expectation and prevent requesting
a new key frame if a large key frame is currently on the way.

Bug: None
Change-Id: I443a60872a3580d324f050080a9868f7b90d71a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161730
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30084}

modules/video_coding/packet_buffer.cc

diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc
index b6fc521..fb25c0ad 100644
--- a/modules/video_coding/packet_buffer.cc
+++ b/modules/video_coding/packet_buffer.cc
@@ -123,8 +123,11 @@
 
   int64_t now_ms = clock_->TimeInMilliseconds();
   last_received_packet_ms_ = now_ms;
-  if (packet->video_header.frame_type == VideoFrameType::kVideoFrameKey)
+  if (packet->video_header.frame_type == VideoFrameType::kVideoFrameKey ||
+      last_received_keyframe_rtp_timestamp_ == packet->timestamp) {
     last_received_keyframe_packet_ms_ = now_ms;
+    last_received_keyframe_rtp_timestamp_ = packet->timestamp;
+  }
 
   StoredPacket& new_entry = buffer_[index];
   new_entry.continuous = false;

modules/video_coding/packet_buffer.h

diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h
index 8371a37..939168d 100644
--- a/modules/video_coding/packet_buffer.h
+++ b/modules/video_coding/packet_buffer.h
@@ -160,10 +160,12 @@
   // determine continuity between them.
   std::vector<StoredPacket> buffer_ RTC_GUARDED_BY(crit_);
 
-  // Timestamp (not RTP timestamp) of the last received packet/keyframe packet.
+  // Timestamp of the last received packet/keyframe packet.
   absl::optional<int64_t> last_received_packet_ms_ RTC_GUARDED_BY(crit_);
   absl::optional<int64_t> last_received_keyframe_packet_ms_
       RTC_GUARDED_BY(crit_);
+  absl::optional<uint32_t> last_received_keyframe_rtp_timestamp_
+      RTC_GUARDED_BY(crit_);
 
   absl::optional<uint16_t> newest_inserted_seq_num_ RTC_GUARDED_BY(crit_);
   std::set<uint16_t, DescendingSeqNumComp<uint16_t>> missing_packets_

modules/video_coding/packet_buffer_unittest.cc

diff --git a/modules/video_coding/packet_buffer_unittest.cc b/modules/video_coding/packet_buffer_unittest.cc
index 980ac35..0936bf8 100644
--- a/modules/video_coding/packet_buffer_unittest.cc
+++ b/modules/video_coding/packet_buffer_unittest.cc
@@ -755,7 +755,7 @@
   EXPECT_FALSE(packet_keyframe_ms);
 
   int64_t keyframe_ms = clock_.TimeInMilliseconds();
-  Insert(100, kKeyFrame, kFirst, kLast);
+  Insert(100, kKeyFrame, kFirst, kLast, {}, /*timestamp=*/1000);
   packet_ms = packet_buffer_.LastReceivedPacketMs();
   packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
   EXPECT_TRUE(packet_ms);
@@ -765,7 +765,7 @@
 
   clock_.AdvanceTimeMilliseconds(100);
   int64_t delta_ms = clock_.TimeInMilliseconds();
-  Insert(101, kDeltaFrame, kFirst, kLast);
+  Insert(101, kDeltaFrame, kFirst, kLast, {}, /*timestamp=*/2000);
   packet_ms = packet_buffer_.LastReceivedPacketMs();
   packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
   EXPECT_TRUE(packet_ms);
@@ -780,6 +780,32 @@
   EXPECT_FALSE(packet_keyframe_ms);
 }
 
+TEST_F(PacketBufferTest,
+       LastReceivedKeyFrameReturnsReceiveTimeOfALastReceivedPacketOfAKeyFrame) {
+  clock_.AdvanceTimeMilliseconds(100);
+  Insert(/*seq_num=*/100, kKeyFrame, kFirst, kNotLast, {}, /*timestamp=*/1000);
+  EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
+            clock_.TimeInMilliseconds());
+
+  clock_.AdvanceTimeMilliseconds(100);
+  Insert(/*seq_num=*/102, kDeltaFrame, kNotFirst, kLast, {},
+         /*timestamp=*/1000);
+  EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
+            clock_.TimeInMilliseconds());
+
+  clock_.AdvanceTimeMilliseconds(100);
+  Insert(/*seq_num=*/101, kDeltaFrame, kNotFirst, kNotLast, {},
+         /*timestamp=*/1000);
+  EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
+            clock_.TimeInMilliseconds());
+
+  clock_.AdvanceTimeMilliseconds(100);
+  Insert(/*seq_num=*/103, kDeltaFrame, kFirst, kNotLast, {},
+         /*timestamp=*/2000);
+  EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
+            clock_.TimeInMilliseconds() - 100);
+}
+
 TEST_F(PacketBufferTest, IncomingCodecChange) {
   PacketBuffer::Packet packet;
   packet.video_header.is_first_packet_in_frame = true;