diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 463e62cc02..ba46327a7b 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -78,11 +78,10 @@ FrameBuffer::~FrameBuffer() { RTC_DCHECK_RUN_ON(&construction_checker_); } -void FrameBuffer::NextFrame( - int64_t max_wait_time_ms, - bool keyframe_required, - rtc::TaskQueue* callback_queue, - std::function, ReturnReason)> handler) { +void FrameBuffer::NextFrame(int64_t max_wait_time_ms, + bool keyframe_required, + rtc::TaskQueue* callback_queue, + NextFrameCallback handler) { RTC_DCHECK_RUN_ON(&callback_checker_); RTC_DCHECK(callback_queue->IsCurrent()); TRACE_EVENT0("webrtc", "FrameBuffer::NextFrame"); @@ -110,8 +109,7 @@ void FrameBuffer::StartWaitForNextFrameOnQueue() { // If this task has not been cancelled, we did not get any new frames // while waiting. Continue with frame delivery. std::unique_ptr frame; - std::function, ReturnReason)> - frame_handler; + NextFrameCallback frame_handler; { MutexLock lock(&mutex_); if (!frames_to_decode_.empty()) { @@ -130,8 +128,7 @@ void FrameBuffer::StartWaitForNextFrameOnQueue() { CancelCallback(); } // Deliver frame, if any. Otherwise signal timeout. - ReturnReason reason = frame ? kFrameFound : kTimeout; - frame_handler(std::move(frame), reason); + frame_handler(std::move(frame)); return TimeDelta::Zero(); // Ignored. }); } diff --git a/modules/video_coding/frame_buffer2.h b/modules/video_coding/frame_buffer2.h index 1a957a671a..411c69cefd 100644 --- a/modules/video_coding/frame_buffer2.h +++ b/modules/video_coding/frame_buffer2.h @@ -45,8 +45,6 @@ namespace video_coding { class FrameBuffer { public: - enum ReturnReason { kFrameFound, kTimeout, kStopped }; - FrameBuffer(Clock* clock, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_callback); @@ -61,13 +59,13 @@ class FrameBuffer { // of the last continuous frame or -1 if there is no continuous frame. int64_t InsertFrame(std::unique_ptr frame); - // Get the next frame for decoding. Will return at latest after - // `max_wait_time_ms`. - void NextFrame( - int64_t max_wait_time_ms, - bool keyframe_required, - rtc::TaskQueue* callback_queue, - std::function, ReturnReason)> handler); + using NextFrameCallback = std::function)>; + // Get the next frame for decoding. `handler` is invoked with the next frame + // or with nullptr if no frame is ready for decoding after `max_wait_time_ms`. + void NextFrame(int64_t max_wait_time_ms, + bool keyframe_required, + rtc::TaskQueue* callback_queue, + NextFrameCallback handler); // Tells the FrameBuffer which protection mode that is in use. Affects // the frame timing. @@ -171,8 +169,7 @@ class FrameBuffer { rtc::TaskQueue* callback_queue_ RTC_GUARDED_BY(mutex_); RepeatingTaskHandle callback_task_ RTC_GUARDED_BY(mutex_); - std::function, ReturnReason)> - frame_handler_ RTC_GUARDED_BY(mutex_); + NextFrameCallback frame_handler_ RTC_GUARDED_BY(mutex_); int64_t latest_return_time_ms_ RTC_GUARDED_BY(mutex_); bool keyframe_required_ RTC_GUARDED_BY(mutex_); diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc index e498afd3cd..be838c748e 100644 --- a/modules/video_coding/frame_buffer2_unittest.cc +++ b/modules/video_coding/frame_buffer2_unittest.cc @@ -199,14 +199,10 @@ class TestFrameBuffer2 : public ::testing::Test { void ExtractFrame(int64_t max_wait_time = 0, bool keyframe_required = false) { time_task_queue_.PostTask([this, max_wait_time, keyframe_required]() { - buffer_->NextFrame( - max_wait_time, keyframe_required, &time_task_queue_, - [this](std::unique_ptr frame, - video_coding::FrameBuffer::ReturnReason reason) { - if (reason != FrameBuffer::ReturnReason::kStopped) { - frames_.emplace_back(std::move(frame)); - } - }); + buffer_->NextFrame(max_wait_time, keyframe_required, &time_task_queue_, + [this](std::unique_ptr frame) { + frames_.emplace_back(std::move(frame)); + }); }); if (max_wait_time == 0) { time_controller_.AdvanceTime(TimeDelta::Millis(0)); diff --git a/test/fuzzers/frame_buffer2_fuzzer.cc b/test/fuzzers/frame_buffer2_fuzzer.cc index 0572675f71..a20efaecc3 100644 --- a/test/fuzzers/frame_buffer2_fuzzer.cc +++ b/test/fuzzers/frame_buffer2_fuzzer.cc @@ -97,9 +97,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { max_wait_time_ms] { frame_buffer.NextFrame( max_wait_time_ms, keyframe_required, &task_queue, - [&next_frame_task_running]( - std::unique_ptr frame, - video_coding::FrameBuffer::ReturnReason res) { + [&next_frame_task_running](std::unique_ptr frame) { next_frame_task_running = false; }); }); diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index fcd4d7d313..27f86cfaa8 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -59,8 +59,6 @@ constexpr int VideoReceiveStream::kMaxWaitForKeyFrameMs; namespace { -using ReturnReason = video_coding::FrameBuffer::ReturnReason; - constexpr int kMinBaseMinimumDelayMs = 0; constexpr int kMaxBaseMinimumDelayMs = 10000; @@ -609,22 +607,19 @@ int64_t VideoReceiveStream::GetWaitMs() const { void VideoReceiveStream::StartNextDecode() { TRACE_EVENT0("webrtc", "VideoReceiveStream::StartNextDecode"); - frame_buffer_->NextFrame( - GetWaitMs(), keyframe_required_, &decode_queue_, - /* encoded frame handler */ - [this](std::unique_ptr frame, ReturnReason res) { - RTC_DCHECK_EQ(frame == nullptr, res == ReturnReason::kTimeout); - RTC_DCHECK_EQ(frame != nullptr, res == ReturnReason::kFrameFound); - RTC_DCHECK_RUN_ON(&decode_queue_); - if (decoder_stopped_) - return; - if (frame) { - HandleEncodedFrame(std::move(frame)); - } else { - HandleFrameBufferTimeout(); - } - StartNextDecode(); - }); + frame_buffer_->NextFrame(GetWaitMs(), keyframe_required_, &decode_queue_, + /* encoded frame handler */ + [this](std::unique_ptr frame) { + RTC_DCHECK_RUN_ON(&decode_queue_); + if (decoder_stopped_) + return; + if (frame) { + HandleEncodedFrame(std::move(frame)); + } else { + HandleFrameBufferTimeout(); + } + StartNextDecode(); + }); } void VideoReceiveStream::HandleEncodedFrame( diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 87117eac82..528b2998e9 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -58,8 +58,6 @@ constexpr int VideoReceiveStream2::kMaxWaitForKeyFrameMs; namespace { -using ReturnReason = video_coding::FrameBuffer::ReturnReason; - constexpr int kMinBaseMinimumDelayMs = 0; constexpr int kMaxBaseMinimumDelayMs = 10000; @@ -723,9 +721,7 @@ void VideoReceiveStream2::StartNextDecode() { frame_buffer_->NextFrame( GetMaxWaitMs(), keyframe_required_, &decode_queue_, /* encoded frame handler */ - [this](std::unique_ptr frame, ReturnReason res) { - RTC_DCHECK_EQ(frame == nullptr, res == ReturnReason::kTimeout); - RTC_DCHECK_EQ(frame != nullptr, res == ReturnReason::kFrameFound); + [this](std::unique_ptr frame) { RTC_DCHECK_RUN_ON(&decode_queue_); if (decoder_stopped_) return; diff --git a/video/video_stream_decoder_impl.cc b/video/video_stream_decoder_impl.cc index 3b65d17cca..bbd67ee257 100644 --- a/video/video_stream_decoder_impl.cc +++ b/video/video_stream_decoder_impl.cc @@ -135,69 +135,58 @@ void VideoStreamDecoderImpl::SaveFrameInfo(const EncodedFrame& frame) { void VideoStreamDecoderImpl::StartNextDecode() { int64_t max_wait_time = keyframe_required_ ? 200 : 3000; - frame_buffer_.NextFrame( - max_wait_time, keyframe_required_, &bookkeeping_queue_, - [this](std::unique_ptr frame, - video_coding::FrameBuffer::ReturnReason res) mutable { - RTC_DCHECK_RUN_ON(&bookkeeping_queue_); - OnNextFrameCallback(std::move(frame), res); - }); + frame_buffer_.NextFrame(max_wait_time, keyframe_required_, + &bookkeeping_queue_, + [this](std::unique_ptr frame) { + RTC_DCHECK_RUN_ON(&bookkeeping_queue_); + OnNextFrameCallback(std::move(frame)); + }); } void VideoStreamDecoderImpl::OnNextFrameCallback( - std::unique_ptr frame, - video_coding::FrameBuffer::ReturnReason result) { - switch (result) { - case video_coding::FrameBuffer::kFrameFound: { - RTC_DCHECK(frame); - SaveFrameInfo(*frame); + std::unique_ptr frame) { + if (frame) { + RTC_DCHECK(frame); + SaveFrameInfo(*frame); - MutexLock lock(&shut_down_mutex_); - if (shut_down_) { - return; - } - - decode_queue_.PostTask([this, frame = std::move(frame)]() mutable { - RTC_DCHECK_RUN_ON(&decode_queue_); - DecodeResult decode_result = DecodeFrame(std::move(frame)); - bookkeeping_queue_.PostTask([this, decode_result]() { - RTC_DCHECK_RUN_ON(&bookkeeping_queue_); - switch (decode_result) { - case kOk: { - keyframe_required_ = false; - break; - } - case kOkRequestKeyframe: { - callbacks_->OnNonDecodableState(); - keyframe_required_ = false; - break; - } - case kDecodeFailure: { - callbacks_->OnNonDecodableState(); - keyframe_required_ = true; - break; - } - } - StartNextDecode(); - }); - }); - break; + MutexLock lock(&shut_down_mutex_); + if (shut_down_) { + return; } - case video_coding::FrameBuffer::kTimeout: { - callbacks_->OnNonDecodableState(); - // The `frame_buffer_` requires the frame callback function to complete - // before NextFrame is called again. For this reason we call - // StartNextDecode in a later task to allow this task to complete first. - bookkeeping_queue_.PostTask([this]() { + + decode_queue_.PostTask([this, frame = std::move(frame)]() mutable { + RTC_DCHECK_RUN_ON(&decode_queue_); + DecodeResult decode_result = DecodeFrame(std::move(frame)); + bookkeeping_queue_.PostTask([this, decode_result]() { RTC_DCHECK_RUN_ON(&bookkeeping_queue_); + switch (decode_result) { + case kOk: { + keyframe_required_ = false; + break; + } + case kOkRequestKeyframe: { + callbacks_->OnNonDecodableState(); + keyframe_required_ = false; + break; + } + case kDecodeFailure: { + callbacks_->OnNonDecodableState(); + keyframe_required_ = true; + break; + } + } StartNextDecode(); }); - break; - } - case video_coding::FrameBuffer::kStopped: { - // We are shutting down, do nothing. - break; - } + }); + } else { + callbacks_->OnNonDecodableState(); + // The `frame_buffer_` requires the frame callback function to complete + // before NextFrame is called again. For this reason we call + // StartNextDecode in a later task to allow this task to complete first. + bookkeeping_queue_.PostTask([this]() { + RTC_DCHECK_RUN_ON(&bookkeeping_queue_); + StartNextDecode(); + }); } } diff --git a/video/video_stream_decoder_impl.h b/video/video_stream_decoder_impl.h index 2cb6513574..9d028a2d6e 100644 --- a/video/video_stream_decoder_impl.h +++ b/video/video_stream_decoder_impl.h @@ -72,8 +72,7 @@ class VideoStreamDecoderImpl : public VideoStreamDecoderInterface { void SaveFrameInfo(const EncodedFrame& frame) RTC_RUN_ON(bookkeeping_queue_); FrameInfo* GetFrameInfo(int64_t timestamp) RTC_RUN_ON(bookkeeping_queue_); void StartNextDecode() RTC_RUN_ON(bookkeeping_queue_); - void OnNextFrameCallback(std::unique_ptr frame, - video_coding::FrameBuffer::ReturnReason res) + void OnNextFrameCallback(std::unique_ptr frame) RTC_RUN_ON(bookkeeping_queue_); void OnDecodedFrameCallback(VideoFrame& decodedImage, // NOLINT absl::optional decode_time_ms,