Remove FrameBuffer::ReturnReason

This was a remenant leftover from a previous design, which was no longer
valid after the switch to TaskQueues. ReturnReason::kStopped was not
used at all, and so Timeout or FrameFound can be inferred from whether
the frame is null or not.

Bug: webrtc:13343, webrtc:13346
Change-Id: Ib0f847b1e1192e32ea11208e48f5a3892703521e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239651
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35490}

modules/video_coding/frame_buffer2.cc

@@ -78,11 +78,10 @@ FrameBuffer::~FrameBuffer() {
   RTC_DCHECK_RUN_ON(&construction_checker_);
 }
 
-void FrameBuffer::NextFrame(
+void FrameBuffer::NextFrame(int64_t max_wait_time_ms,
-    int64_t max_wait_time_ms,
+                            bool keyframe_required,
-    bool keyframe_required,
+                            rtc::TaskQueue* callback_queue,
-    rtc::TaskQueue* callback_queue,
+                            NextFrameCallback handler) {
-    std::function<void(std::unique_ptr<EncodedFrame>, ReturnReason)> 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<EncodedFrame> frame;
-        std::function<void(std::unique_ptr<EncodedFrame>, ReturnReason)>
+        NextFrameCallback frame_handler;
-            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));
-        frame_handler(std::move(frame), reason);
         return TimeDelta::Zero();  // Ignored.
       });
 }

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<EncodedFrame> frame);
 
-  // Get the next frame for decoding. Will return at latest after
+  using NextFrameCallback = std::function<void(std::unique_ptr<EncodedFrame>)>;
-  // `max_wait_time_ms`.
+  // Get the next frame for decoding. `handler` is invoked with the next frame
-  void NextFrame(
+  // or with nullptr if no frame is ready for decoding after `max_wait_time_ms`.
-      int64_t max_wait_time_ms,
+  void NextFrame(int64_t max_wait_time_ms,
-      bool keyframe_required,
+                 bool keyframe_required,
-      rtc::TaskQueue* callback_queue,
+                 rtc::TaskQueue* callback_queue,
-      std::function<void(std::unique_ptr<EncodedFrame>, ReturnReason)> handler);
+                 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<void(std::unique_ptr<EncodedFrame>, ReturnReason)>
+  NextFrameCallback frame_handler_ RTC_GUARDED_BY(mutex_);
-      frame_handler_ RTC_GUARDED_BY(mutex_);
   int64_t latest_return_time_ms_ RTC_GUARDED_BY(mutex_);
   bool keyframe_required_ RTC_GUARDED_BY(mutex_);
 

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(
+      buffer_->NextFrame(max_wait_time, keyframe_required, &time_task_queue_,
-          max_wait_time, keyframe_required, &time_task_queue_,
+                         [this](std::unique_ptr<EncodedFrame> frame) {
-          [this](std::unique_ptr<EncodedFrame> frame,
+                           frames_.emplace_back(std::move(frame));
-                 video_coding::FrameBuffer::ReturnReason reason) {
+                         });
-            if (reason != FrameBuffer::ReturnReason::kStopped) {
-              frames_.emplace_back(std::move(frame));
-            }
-          });
     });
     if (max_wait_time == 0) {
       time_controller_.AdvanceTime(TimeDelta::Millis(0));

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](
+              [&next_frame_task_running](std::unique_ptr<EncodedFrame> frame) {
-                  std::unique_ptr<EncodedFrame> frame,
-                  video_coding::FrameBuffer::ReturnReason res) {
                 next_frame_task_running = false;
               });
         });

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(
+  frame_buffer_->NextFrame(GetWaitMs(), keyframe_required_, &decode_queue_,
-      GetWaitMs(), keyframe_required_, &decode_queue_,
+                           /* encoded frame handler */
-      /* encoded frame handler */
+                           [this](std::unique_ptr<EncodedFrame> frame) {
-      [this](std::unique_ptr<EncodedFrame> frame, ReturnReason res) {
+                             RTC_DCHECK_RUN_ON(&decode_queue_);
-        RTC_DCHECK_EQ(frame == nullptr, res == ReturnReason::kTimeout);
+                             if (decoder_stopped_)
-        RTC_DCHECK_EQ(frame != nullptr, res == ReturnReason::kFrameFound);
+                               return;
-        RTC_DCHECK_RUN_ON(&decode_queue_);
+                             if (frame) {
-        if (decoder_stopped_)
+                               HandleEncodedFrame(std::move(frame));
-          return;
+                             } else {
-        if (frame) {
+                               HandleFrameBufferTimeout();
-          HandleEncodedFrame(std::move(frame));
+                             }
-        } else {
+                             StartNextDecode();
-          HandleFrameBufferTimeout();
+                           });
-        }
-        StartNextDecode();
-      });
 }
 
 void VideoReceiveStream::HandleEncodedFrame(

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<EncodedFrame> frame, ReturnReason res) {
+      [this](std::unique_ptr<EncodedFrame> frame) {
-        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;

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(
+  frame_buffer_.NextFrame(max_wait_time, keyframe_required_,
-      max_wait_time, keyframe_required_, &bookkeeping_queue_,
+                          &bookkeeping_queue_,
-      [this](std::unique_ptr<EncodedFrame> frame,
+                          [this](std::unique_ptr<EncodedFrame> frame) {
-             video_coding::FrameBuffer::ReturnReason res) mutable {
+                            RTC_DCHECK_RUN_ON(&bookkeeping_queue_);
-        RTC_DCHECK_RUN_ON(&bookkeeping_queue_);
+                            OnNextFrameCallback(std::move(frame));
-        OnNextFrameCallback(std::move(frame), res);
+                          });
-      });
 }
 
 void VideoStreamDecoderImpl::OnNextFrameCallback(
-    std::unique_ptr<EncodedFrame> frame,
+    std::unique_ptr<EncodedFrame> frame) {
-    video_coding::FrameBuffer::ReturnReason result) {
+  if (frame) {
-  switch (result) {
+    RTC_DCHECK(frame);
-    case video_coding::FrameBuffer::kFrameFound: {
+    SaveFrameInfo(*frame);
-      RTC_DCHECK(frame);
-      SaveFrameInfo(*frame);
 
-      MutexLock lock(&shut_down_mutex_);
+    MutexLock lock(&shut_down_mutex_);
-      if (shut_down_) {
+    if (shut_down_) {
-        return;
+      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;
     }
-    case video_coding::FrameBuffer::kTimeout: {
+
-      callbacks_->OnNonDecodableState();
+    decode_queue_.PostTask([this, frame = std::move(frame)]() mutable {
-      // The `frame_buffer_` requires the frame callback function to complete
+      RTC_DCHECK_RUN_ON(&decode_queue_);
-      // before NextFrame is called again. For this reason we call
+      DecodeResult decode_result = DecodeFrame(std::move(frame));
-      // StartNextDecode in a later task to allow this task to complete first.
+      bookkeeping_queue_.PostTask([this, decode_result]() {
-      bookkeeping_queue_.PostTask([this]() {
         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;
+    });
-    }
+  } else {
-    case video_coding::FrameBuffer::kStopped: {
+    callbacks_->OnNonDecodableState();
-      // We are shutting down, do nothing.
+    // The `frame_buffer_` requires the frame callback function to complete
-      break;
+    // 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();
+    });
   }
 }
 

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<EncodedFrame> frame,
+  void OnNextFrameCallback(std::unique_ptr<EncodedFrame> frame)
-                           video_coding::FrameBuffer::ReturnReason res)
       RTC_RUN_ON(bookkeeping_queue_);
   void OnDecodedFrameCallback(VideoFrame& decodedImage,  // NOLINT
                               absl::optional<int32_t> decode_time_ms,