From c9e532a7ebd3836eb98a6885969af5181cb1bfb1 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 10 Dec 2019 17:03:00 +0100 Subject: [PATCH] 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 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#30084} --- modules/video_coding/packet_buffer.cc | 5 +++- modules/video_coding/packet_buffer.h | 4 ++- .../video_coding/packet_buffer_unittest.cc | 30 +++++++++++++++++-- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index b6fc521d04..fb25c0ad0f 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -123,8 +123,11 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket( 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; diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h index 8371a3737d..939168d017 100644 --- a/modules/video_coding/packet_buffer.h +++ b/modules/video_coding/packet_buffer.h @@ -160,10 +160,12 @@ class PacketBuffer { // determine continuity between them. std::vector 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 last_received_packet_ms_ RTC_GUARDED_BY(crit_); absl::optional last_received_keyframe_packet_ms_ RTC_GUARDED_BY(crit_); + absl::optional last_received_keyframe_rtp_timestamp_ + RTC_GUARDED_BY(crit_); absl::optional newest_inserted_seq_num_ RTC_GUARDED_BY(crit_); std::set> missing_packets_ diff --git a/modules/video_coding/packet_buffer_unittest.cc b/modules/video_coding/packet_buffer_unittest.cc index 980ac35ef7..0936bf8ab0 100644 --- a/modules/video_coding/packet_buffer_unittest.cc +++ b/modules/video_coding/packet_buffer_unittest.cc @@ -755,7 +755,7 @@ TEST_F(PacketBufferTest, PacketTimestamps) { 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 @@ TEST_F(PacketBufferTest, PacketTimestamps) { 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 @@ TEST_F(PacketBufferTest, PacketTimestamps) { 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;