Dont request keyframes if the stream is inactive or if we are currently receiving a keyframe.

BUG=webrtc:7520

Review-Url: https://codereview.webrtc.org/2853503002
Cr-Commit-Position: refs/heads/master@{#18199}

webrtc/modules/video_coding/packet_buffer.cc

@@ -59,6 +59,7 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) {
   std::vector<std::unique_ptr<RtpFrameObject>> 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<int64_t>(now_ms);
+    if (packet->frameType == kVideoFrameKey)
+      last_received_keyframe_packet_ms_ = rtc::Optional<int64_t>(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<int64_t>();
+  last_received_keyframe_packet_ms_ = rtc::Optional<int64_t>();
+}
+
+rtc::Optional<int64_t> PacketBuffer::LastReceivedPacketMs() const {
+  rtc::CritScope lock(&crit_);
+  return last_received_packet_ms_;
+}
+
+rtc::Optional<int64_t> PacketBuffer::LastReceivedKeyframePacketMs() const {
+  rtc::CritScope lock(&crit_);
+  return last_received_keyframe_packet_ms_;
 }
 
 bool PacketBuffer::ExpandBufferSize() {

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<int64_t> LastReceivedPacketMs() const;
+  rtc::Optional<int64_t> 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<int64_t> last_received_packet_ms_ GUARDED_BY(crit_);
+  rtc::Optional<int64_t> last_received_keyframe_packet_ms_ GUARDED_BY(crit_);
+
   mutable volatile int ref_count_ = 0;
 };
 

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> clock_;
+  std::unique_ptr<SimulatedClock> clock_;
   rtc::scoped_refptr<PacketBuffer> packet_buffer_;
   std::map<uint16_t, std::unique_ptr<RtpFrameObject>> frames_from_callback_;
 };
@@ -501,5 +502,40 @@ TEST_F(TestPacketBuffer, OneH264FrameFillBuffer) {
   CheckFrame(0);
 }
 
+TEST_F(TestPacketBuffer, PacketTimestamps) {
+  rtc::Optional<int64_t> packet_ms;
+  rtc::Optional<int64_t> 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

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<VideoReceiveStream*>& 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)

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<int64_t> RtpStreamReceiver::LastReceivedPacketMs() const {
+  return packet_buffer_->LastReceivedPacketMs();
+}
+
+rtc::Optional<int64_t> RtpStreamReceiver::LastReceivedKeyframePacketMs() const {
+  return packet_buffer_->LastReceivedKeyframePacketMs();
+}
+
 // TODO(nisse): Drop return value.
 bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
                                       size_t packet_length,

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<int64_t> LastReceivedPacketMs() const;
+  rtc::Optional<int64_t> LastReceivedKeyframePacketMs() const;
+
  private:
   bool AddReceiveCodec(const VideoCodec& video_codec);
   bool ReceivePacket(const uint8_t* packet,

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<int64_t> last_packet_ms =
+        rtp_stream_receiver_.LastReceivedPacketMs();
+    rtc::Optional<int64_t> 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;
 }