diff --git a/webrtc/modules/video_coding/packet_buffer.cc b/webrtc/modules/video_coding/packet_buffer.cc index ff013e0010..6c3cfacac5 100644 --- a/webrtc/modules/video_coding/packet_buffer.cc +++ b/webrtc/modules/video_coding/packet_buffer.cc @@ -59,6 +59,7 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) { std::vector> found_frames; { rtc::CritScope lock(&crit_); + uint16_t seq_num = packet->seqNum; size_t index = seq_num % size_; @@ -107,6 +108,11 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) { data_buffer_[index] = *packet; packet->dataPtr = nullptr; + int64_t now_ms = clock_->TimeInMilliseconds(); + last_received_packet_ms_ = rtc::Optional(now_ms); + if (packet->frameType == kVideoFrameKey) + last_received_keyframe_packet_ms_ = rtc::Optional(now_ms); + found_frames = FindFrames(seq_num); } @@ -143,6 +149,18 @@ void PacketBuffer::Clear() { first_packet_received_ = false; is_cleared_to_first_seq_num_ = false; + last_received_packet_ms_ = rtc::Optional(); + last_received_keyframe_packet_ms_ = rtc::Optional(); +} + +rtc::Optional PacketBuffer::LastReceivedPacketMs() const { + rtc::CritScope lock(&crit_); + return last_received_packet_ms_; +} + +rtc::Optional PacketBuffer::LastReceivedKeyframePacketMs() const { + rtc::CritScope lock(&crit_); + return last_received_keyframe_packet_ms_; } bool PacketBuffer::ExpandBufferSize() { diff --git a/webrtc/modules/video_coding/packet_buffer.h b/webrtc/modules/video_coding/packet_buffer.h index 001f8bd89d..3d9eb9cb42 100644 --- a/webrtc/modules/video_coding/packet_buffer.h +++ b/webrtc/modules/video_coding/packet_buffer.h @@ -55,6 +55,10 @@ class PacketBuffer { void ClearTo(uint16_t seq_num); void Clear(); + // Timestamp (not RTP timestamp) of the last received packet/keyframe packet. + rtc::Optional LastReceivedPacketMs() const; + rtc::Optional LastReceivedKeyframePacketMs() const; + int AddRef() const; int Release() const; @@ -142,6 +146,10 @@ class PacketBuffer { // Called when a received frame is found. OnReceivedFrameCallback* const received_frame_callback_; + // Timestamp (not RTP timestamp) of the last received packet/keyframe packet. + rtc::Optional last_received_packet_ms_ GUARDED_BY(crit_); + rtc::Optional last_received_keyframe_packet_ms_ GUARDED_BY(crit_); + mutable volatile int ref_count_ = 0; }; diff --git a/webrtc/modules/video_coding/video_packet_buffer_unittest.cc b/webrtc/modules/video_coding/video_packet_buffer_unittest.cc index 86d54ebafe..e418d12466 100644 --- a/webrtc/modules/video_coding/video_packet_buffer_unittest.cc +++ b/webrtc/modules/video_coding/video_packet_buffer_unittest.cc @@ -58,7 +58,8 @@ class TestPacketBuffer : public ::testing::Test, VCMPacket packet; packet.codec = kVideoCodecGeneric; packet.seqNum = seq_num; - packet.frameType = keyframe ? kVideoFrameKey : kVideoFrameDelta; + packet.frameType = + keyframe == kKeyFrame ? kVideoFrameKey : kVideoFrameDelta; packet.is_first_packet_in_frame = first == kFirst; packet.markerBit = last == kLast; packet.sizeBytes = data_size; @@ -78,7 +79,7 @@ class TestPacketBuffer : public ::testing::Test, const int kMaxSize = 64; Random rand_; - std::unique_ptr clock_; + std::unique_ptr clock_; rtc::scoped_refptr packet_buffer_; std::map> frames_from_callback_; }; @@ -501,5 +502,40 @@ TEST_F(TestPacketBuffer, OneH264FrameFillBuffer) { CheckFrame(0); } +TEST_F(TestPacketBuffer, PacketTimestamps) { + rtc::Optional packet_ms; + rtc::Optional packet_keyframe_ms; + + packet_ms = packet_buffer_->LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + EXPECT_FALSE(packet_ms); + EXPECT_FALSE(packet_keyframe_ms); + + int64_t keyframe_ms = clock_->TimeInMilliseconds(); + EXPECT_TRUE(Insert(100, kKeyFrame, kFirst, kLast)); + packet_ms = packet_buffer_->LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + EXPECT_TRUE(packet_ms); + EXPECT_TRUE(packet_keyframe_ms); + EXPECT_EQ(keyframe_ms, *packet_ms); + EXPECT_EQ(keyframe_ms, *packet_keyframe_ms); + + clock_->AdvanceTimeMilliseconds(100); + int64_t delta_ms = clock_->TimeInMilliseconds(); + EXPECT_TRUE(Insert(101, kDeltaFrame, kFirst, kLast)); + packet_ms = packet_buffer_->LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + EXPECT_TRUE(packet_ms); + EXPECT_TRUE(packet_keyframe_ms); + EXPECT_EQ(delta_ms, *packet_ms); + EXPECT_EQ(keyframe_ms, *packet_keyframe_ms); + + packet_buffer_->Clear(); + packet_ms = packet_buffer_->LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + EXPECT_FALSE(packet_ms); + EXPECT_FALSE(packet_keyframe_ms); +} + } // namespace video_coding } // namespace webrtc diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 99e5279424..6e721b60ee 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -2229,6 +2229,45 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { RunBaseTest(&test); } +TEST_F(EndToEndTest, StopSendingKeyframeRequestsForInactiveStream) { + class KeyframeRequestObserver : public test::EndToEndTest { + public: + KeyframeRequestObserver() : clock_(Clock::GetRealTimeClock()) {} + + void OnVideoStreamsCreated( + VideoSendStream* send_stream, + const std::vector& receive_streams) override { + RTC_DCHECK_EQ(1, receive_streams.size()); + send_stream_ = send_stream; + receive_stream_ = receive_streams[0]; + } + + void PerformTest() override { + bool frame_decoded = false; + int64_t start_time = clock_->TimeInMilliseconds(); + while (clock_->TimeInMilliseconds() - start_time <= 5000) { + if (receive_stream_->GetStats().frames_decoded > 0) { + frame_decoded = true; + break; + } + SleepMs(100); + } + ASSERT_TRUE(frame_decoded); + send_stream_->Stop(); + SleepMs(10000); + ASSERT_EQ( + 1U, receive_stream_->GetStats().rtcp_packet_type_counts.pli_packets); + } + + private: + Clock* clock_; + VideoSendStream* send_stream_; + VideoReceiveStream* receive_stream_; + } test; + + RunBaseTest(&test); +} + class ProbingTest : public test::EndToEndTest { public: explicit ProbingTest(int start_bitrate_bps) diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index 9b9272dad2..99d0cc893f 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -420,6 +420,14 @@ void RtpStreamReceiver::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { nack_module_->UpdateRtt(max_rtt_ms); } +rtc::Optional RtpStreamReceiver::LastReceivedPacketMs() const { + return packet_buffer_->LastReceivedPacketMs(); +} + +rtc::Optional RtpStreamReceiver::LastReceivedKeyframePacketMs() const { + return packet_buffer_->LastReceivedKeyframePacketMs(); +} + // TODO(nisse): Drop return value. bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet, size_t packet_length, diff --git a/webrtc/video/rtp_stream_receiver.h b/webrtc/video/rtp_stream_receiver.h index 89fc4f6b93..14edb61388 100644 --- a/webrtc/video/rtp_stream_receiver.h +++ b/webrtc/video/rtp_stream_receiver.h @@ -135,6 +135,9 @@ class RtpStreamReceiver : public RtpData, void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; + rtc::Optional LastReceivedPacketMs() const; + rtc::Optional LastReceivedKeyframePacketMs() const; + private: bool AddReceiveCodec(const VideoCodec& video_codec); bool ReceivePacket(const uint8_t* packet, diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 0ce3794eb5..6e7ea13b2a 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -477,9 +477,28 @@ bool VideoReceiveStream::Decode() { if (video_receiver_.Decode(frame.get()) == VCM_OK) rtp_stream_receiver_.FrameDecoded(frame->picture_id); } else { - LOG(LS_WARNING) << "No decodable frame in " << kMaxWaitForFrameMs - << " ms, requesting keyframe."; - RequestKeyFrame(); + int64_t now_ms = clock_->TimeInMilliseconds(); + rtc::Optional last_packet_ms = + rtp_stream_receiver_.LastReceivedPacketMs(); + rtc::Optional last_keyframe_packet_ms = + rtp_stream_receiver_.LastReceivedKeyframePacketMs(); + + // To avoid spamming keyframe requests for a stream that is not active we + // check if we have received a packet within the last 5 seconds. + bool stream_is_active = last_packet_ms && now_ms - *last_packet_ms < 5000; + + // If we recently (within |kMaxWaitForFrameMs|) have been receiving packets + // belonging to a keyframe then we assume a keyframe is being received right + // now. + bool receiving_keyframe = + last_keyframe_packet_ms && + now_ms - *last_keyframe_packet_ms < kMaxWaitForFrameMs; + + if (stream_is_active && !receiving_keyframe) { + LOG(LS_WARNING) << "No decodable frame in " << kMaxWaitForFrameMs + << " ms, requesting keyframe."; + RequestKeyFrame(); + } } return true; }