diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc index 016a24e807..2540ae5941 100644 --- a/webrtc/modules/video_coding/frame_buffer2.cc +++ b/webrtc/modules/video_coding/frame_buffer2.cc @@ -59,8 +59,7 @@ FrameBuffer::~FrameBuffer() {} FrameBuffer::ReturnReason FrameBuffer::NextFrame( int64_t max_wait_time_ms, - std::unique_ptr* frame_out, - bool keyframe_required) { + std::unique_ptr* frame_out) { TRACE_EVENT0("webrtc", "FrameBuffer::NextFrame"); int64_t latest_return_time_ms = clock_->TimeInMilliseconds() + max_wait_time_ms; @@ -106,10 +105,6 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame( } FrameObject* frame = frame_it->second.frame.get(); - - if (keyframe_required && !frame->is_keyframe()) - continue; - next_frame_it_ = frame_it; if (frame->RenderTime() == -1) frame->SetRenderTime(timing_->RenderTimeMs(frame->timestamp, now_ms)); @@ -284,7 +279,7 @@ int FrameBuffer::InsertFrame(std::unique_ptr frame) { TRACE_EVENT0("webrtc", "FrameBuffer::InsertFrame"); RTC_DCHECK(frame); if (stats_callback_) - stats_callback_->OnCompleteFrame(frame->is_keyframe(), frame->size()); + stats_callback_->OnCompleteFrame(frame->num_references == 0, frame->size()); FrameKey key(frame->picture_id, frame->spatial_layer); rtc::CritScope lock(&crit_); @@ -312,7 +307,7 @@ int FrameBuffer::InsertFrame(std::unique_ptr frame) { if (last_decoded_frame_it_ != frames_.end() && key <= last_decoded_frame_it_->first) { if (AheadOf(frame->timestamp, last_decoded_frame_timestamp_) && - frame->is_keyframe()) { + frame->num_references == 0) { // If this frame has a newer timestamp but an earlier picture id then we // assume there has been a jump in the picture id due to some encoder // reconfiguration or some other reason. Even though this is not according diff --git a/webrtc/modules/video_coding/frame_buffer2.h b/webrtc/modules/video_coding/frame_buffer2.h index 8de0680ef9..4950dac3ff 100644 --- a/webrtc/modules/video_coding/frame_buffer2.h +++ b/webrtc/modules/video_coding/frame_buffer2.h @@ -57,8 +57,7 @@ class FrameBuffer { // kTimeout. // - If the FrameBuffer is stopped then it will return kStopped. ReturnReason NextFrame(int64_t max_wait_time_ms, - std::unique_ptr* frame_out, - bool keyframe_required = false); + std::unique_ptr* frame_out); // Tells the FrameBuffer which protection mode that is in use. Affects // the frame timing. diff --git a/webrtc/modules/video_coding/frame_buffer2_unittest.cc b/webrtc/modules/video_coding/frame_buffer2_unittest.cc index 88f841048c..122dd3d8de 100644 --- a/webrtc/modules/video_coding/frame_buffer2_unittest.cc +++ b/webrtc/modules/video_coding/frame_buffer2_unittest.cc @@ -168,12 +168,11 @@ class TestFrameBuffer2 : public ::testing::Test { return buffer_.InsertFrame(std::move(frame)); } - void ExtractFrame(int64_t max_wait_time = 0, bool keyframe_required = false) { + void ExtractFrame(int64_t max_wait_time = 0) { crit_.Enter(); if (max_wait_time == 0) { std::unique_ptr frame; - FrameBuffer::ReturnReason res = - buffer_.NextFrame(0, &frame, keyframe_required); + FrameBuffer::ReturnReason res = buffer_.NextFrame(0, &frame); if (res != FrameBuffer::ReturnReason::kStopped) frames_.emplace_back(std::move(frame)); crit_.Leave(); @@ -542,18 +541,5 @@ TEST_F(TestFrameBuffer2, InvalidReferences) { EXPECT_EQ(2, InsertFrame(2, 0, 3000, false, 1)); } -TEST_F(TestFrameBuffer2, KeyframeRequired) { - EXPECT_EQ(1, InsertFrame(1, 0, 1000, false)); - EXPECT_EQ(2, InsertFrame(2, 0, 2000, false, 1)); - EXPECT_EQ(3, InsertFrame(3, 0, 3000, false)); - ExtractFrame(); - ExtractFrame(0, true); - ExtractFrame(); - - CheckFrame(0, 1, 0); - CheckFrame(1, 3, 0); - CheckNoFrame(2); -} - } // namespace video_coding } // namespace webrtc diff --git a/webrtc/modules/video_coding/frame_object.h b/webrtc/modules/video_coding/frame_object.h index 5299ed8152..0751077041 100644 --- a/webrtc/modules/video_coding/frame_object.h +++ b/webrtc/modules/video_coding/frame_object.h @@ -44,8 +44,6 @@ class FrameObject : public webrtc::VCMEncodedFrame { size_t size() const { return _length; } - bool is_keyframe() const { return num_references == 0; } - // The tuple (|picture_id|, |spatial_layer|) uniquely identifies a frame // object. For codec types that don't necessarily have picture ids they // have to be constructed from the header data relevant to that codec. diff --git a/webrtc/modules/video_coding/video_receiver.cc b/webrtc/modules/video_coding/video_receiver.cc index de6764f9b5..3d3e155dc3 100644 --- a/webrtc/modules/video_coding/video_receiver.cc +++ b/webrtc/modules/video_coding/video_receiver.cc @@ -320,7 +320,24 @@ int32_t VideoReceiver::Decode(const VCMEncodedFrame& frame) { if (decoder == nullptr) { return VCM_NO_CODEC_REGISTERED; } - return decoder->Decode(frame, clock_->TimeInMilliseconds()); + // Decode a frame + int32_t ret = decoder->Decode(frame, clock_->TimeInMilliseconds()); + + // Check for failed decoding, run frame type request callback if needed. + bool request_key_frame = false; + if (ret < 0) { + request_key_frame = true; + } + + if (!frame.Complete() || frame.MissingFrame()) { + request_key_frame = true; + ret = VCM_OK; + } + if (request_key_frame) { + rtc::CritScope cs(&process_crit_); + _scheduleKeyRequest = true; + } + return ret; } // Register possible receive codecs, can be called multiple times diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 18cba03e21..d40bcb336f 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -497,14 +497,9 @@ void VideoReceiveStream::DecodeThreadFunction(void* ptr) { bool VideoReceiveStream::Decode() { TRACE_EVENT0("webrtc", "VideoReceiveStream::Decode"); static const int kMaxWaitForFrameMs = 3000; - static const int kMaxWaitForKeyFrameMs = 200; - - int wait_ms = keyframe_required_ ? kMaxWaitForKeyFrameMs : kMaxWaitForFrameMs; std::unique_ptr frame; - // TODO(philipel): Call NextFrame with |keyframe_required| argument when - // downstream project has been fixed. video_coding::FrameBuffer::ReturnReason res = - frame_buffer_->NextFrame(wait_ms, &frame); + frame_buffer_->NextFrame(kMaxWaitForFrameMs, &frame); if (res == video_coding::FrameBuffer::ReturnReason::kStopped) { video_receiver_.DecodingStopped(); @@ -513,15 +508,8 @@ bool VideoReceiveStream::Decode() { if (frame) { RTC_DCHECK_EQ(res, video_coding::FrameBuffer::ReturnReason::kFrameFound); - if (video_receiver_.Decode(frame.get()) == VCM_OK) { - keyframe_required_ = false; + if (video_receiver_.Decode(frame.get()) == VCM_OK) rtp_video_stream_receiver_.FrameDecoded(frame->picture_id); - } else if (keyframe_required_ == false) { - keyframe_required_ = true; - // TODO(philipel): Remove this keyframe request when downstream project - // has been fixed. - RequestKeyFrame(); - } } else { RTC_DCHECK_EQ(res, video_coding::FrameBuffer::ReturnReason::kTimeout); int64_t now_ms = clock_->TimeInMilliseconds(); @@ -534,14 +522,15 @@ bool VideoReceiveStream::Decode() { // 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 have been receiving packets belonging to a keyframe then - // we assume a keyframe is currently being received. + // 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 < kMaxWaitForKeyFrameMs; + now_ms - *last_keyframe_packet_ms < kMaxWaitForFrameMs; if (stream_is_active && !receiving_keyframe) { - LOG(LS_WARNING) << "No decodable frame in " << wait_ms + LOG(LS_WARNING) << "No decodable frame in " << kMaxWaitForFrameMs << " ms, requesting keyframe."; RequestKeyFrame(); } diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index 89c324ce0b..3ae092fe6e 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -148,10 +148,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, std::unique_ptr media_receiver_; std::unique_ptr rtx_receiver_; - - // Whenever we are in an undecodable state (stream has just started or due to - // a decoding error) we require a keyframe to restart the stream. - bool keyframe_required_ = true; }; } // namespace internal } // namespace webrtc