Request keyframe if the first received frame is not a keyframe.

webrtc/video/rtp_stream_receiver.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 93 matching lines @@
       receiving_(false),
       restored_packet_in_use_(false),
       last_packet_log_ms_(-1),
       rtp_rtcp_(CreateRtpRtcpModule(rtp_receive_statistics_.get(),
                                     transport,
                                     rtt_stats,
                                     receive_stats_proxy,
                                     packet_router)),
       complete_frame_callback_(complete_frame_callback),
       keyframe_request_sender_(keyframe_request_sender),
-      timing_(timing) {
+      timing_(timing),
+      has_received_frame_(false) {
   packet_router_->AddReceiveRtpModule(rtp_rtcp_.get());
   rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy);
   rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
 
   RTC_DCHECK(config_.rtp.rtcp_mode != RtcpMode::kOff)
       << "A stream should not be configured with RTCP disabled. This value is "
          "reserved for internal usage.";
   RTC_DCHECK(config_.rtp.remote_ssrc != 0);
   // TODO(pbos): What's an appropriate local_ssrc for receive-only streams?
   RTC_DCHECK(config_.rtp.local_ssrc != 0);
@@ skipping 248 matching lines @@
   rtp_rtcp_->SendNack(sequence_numbers);
 }
 
 int32_t RtpStreamReceiver::ResendPackets(const uint16_t* sequence_numbers,
                                          uint16_t length) {
   return rtp_rtcp_->SendNACK(sequence_numbers, length);
 }
 
 void RtpStreamReceiver::OnReceivedFrame(
     std::unique_ptr<video_coding::RtpFrameObject> frame) {
+
+  if (!has_received_frame_) {
+    has_received_frame_ = true;

[mflodman, 2017/05/11 13:11:36]

What happens if:
1. The frame buffer is reset later in a call, for some reason.
2. The first key frame is lost after the reset.

I guess that will lead to a 3 second freeze then?

This code will work for the first frame of a call, but not later in a call.
Right?

[philipel, 2017/05/11 13:38:58]

If things are working as they should (the incoming stream follow the spec on the
most basic level) the frame buffer should never be reset, the reseting of the
frame buffer is another hack to allow random jumps in the stream.
This hack is to minimize the problem of recreating a receive stream while
receiving a stream. I'm not sure what you mean by reset in this case, I thought
recreating was the only way.
Exactly, we only do this check for the first frame.

[mflodman, 2017/05/15 08:46:29]

I was thinking about a jitter buffer flush, or the equivalent in the new jitter
buffer. And I have to admit I don't know the details of how the new JB works and
maybe it's best to discuss this offline.

[stefan-webrtc, 2017/05/15 11:01:27]

Philip, I think the question is if the PacketBuffer, FrameBuffer or
RtpFrameReferenceFinder can be reset without us knowing it here, thus still
leading to a 3 second delay. I think whenever we have to reset the jitter buffer
in some way we should make sure we have a key frame as soon as possible. I'm
actually a bit surprised that this method is even called when we haven't
received a key frame. How can we have something decodable if that's the case?

BTW, this will send a single PLI. Maybe it's better to only set
has_received_frame_ to true if it's a key frame? That way if the RTCP is lost we
will try again.

[philipel, 2017/05/15 11:43:22]

This function is called when the packet buffer finds a complete frame, it has
yet to go through the reference finder and the frame buffer, so this function
being called before we have received a keyframe is not that strange (reordering,
recreating the video receive stream)
We will continuously retry to get a keyframe every 3000 ms if we don't have a
decodable stream, we could of course change that interval.

[stefan-webrtc, 2017/05/15 14:43:38]

Ah, I see.
Right, but every 3000 ms is a very low frequency. I'd expect it to be worth
sending way more often if it's a matter of actually getting the stream started.

Doesn't it make sense to set has_received_frame_ to false whenever the state of
the jitter buffer is reset? That's what I would've expected. Or is there some
other way we can get key frames more often until we have a decodable stream?
Maybe increase the frequency until we have something?

[philipel, 2017/05/16 11:26:23]

We might want to tweak how keyframe requests are sent before the first frame has
been decoded, but I don't think we should do that in this CL.

This CL was meant to implement a mechanism similar to that of the old  jitter
buffer (sending a keyframe request if the first frame sent for decoding isn't a
keyframe).

+    if (frame->FrameType() != kVideoFrameKey)
+      keyframe_request_sender_->RequestKeyFrame();

[brandtr, 2017/05/11 12:37:39]

Except for it being a heuristic, are there any drawbacks by doing this?

[philipel, 2017/05/11 13:38:58]

The worst drawback I can think of is on really bad networks where keyframes is a
big cost, and in the case of reordering this could potentially cause an extra
keyframe request. Other then that I can't think of any major drawback.

[stefan-webrtc, 2017/05/15 11:01:27]

Are these key frame requests throttled somewhere, or will this automatically
lead to an RTCP being sent?

[philipel, 2017/05/15 11:43:22]

Quickly looking through the code I can't find any throttle.

+  }
+
   if (!frame->delayed_by_retransmission())
     timing_->IncomingTimestamp(frame->timestamp, clock_->TimeInMilliseconds());
   reference_finder_->ManageFrame(std::move(frame));
 }
 
 void RtpStreamReceiver::OnCompleteFrame(
     std::unique_ptr<video_coding::FrameObject> frame) {
   {
     rtc::CritScope lock(&last_seq_num_cs_);
     video_coding::RtpFrameObject* rtp_frame =
@@ skipping 256 matching lines @@
     return;
 
   if (!sprop_decoder.DecodeSprop(sprop_base64_it->second.c_str()))
     return;
 
   tracker_.InsertSpsPpsNalus(sprop_decoder.sps_nalu(),
                              sprop_decoder.pps_nalu());
 }
 
 }  // namespace webrtc