Clean VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue

Encapsulate the results in a struct and move the post back to the packet
sequence in the original method. This allows the calls to
ReceiveStatisticsProxy::OnPreDecode to be moved to the main thread
always.

Bug: webrtc:11993
Change-Id: I37516444efeabdb330868733bccb1e842ea4d54f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271285
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37823}

video/receive_statistics_proxy2.cc

@@ -959,16 +959,12 @@ void ReceiveStatisticsProxy::OnDroppedFrames(uint32_t frames_dropped) {
 }
 
 void ReceiveStatisticsProxy::OnPreDecode(VideoCodecType codec_type, int qp) {
-  RTC_DCHECK_RUN_ON(&decode_queue_);
-  worker_thread_->PostTask(
-      SafeTask(task_safety_.flag(), [codec_type, qp, this]() {
   RTC_DCHECK_RUN_ON(&main_thread_);
   last_codec_type_ = codec_type;
   if (last_codec_type_ == kVideoCodecVP8 && qp != -1) {
     qp_counters_.vp8.Add(qp);
     qp_sample_.Add(qp);
   }
-      }));
 }
 
 void ReceiveStatisticsProxy::OnStreamInactive() {

video/video_receive_stream2.cc

@@ -801,15 +801,45 @@ void VideoReceiveStream2::OnEncodedFrame(std::unique_ptr<EncodedFrame> frame) {
   const bool keyframe_request_is_due =
       !last_keyframe_request_ ||
       now >= (*last_keyframe_request_ + max_wait_for_keyframe_);
+  const bool received_frame_is_keyframe =
+      frame->FrameType() == VideoFrameType::kVideoFrameKey;
 
-  decode_queue_.PostTask([this, frame = std::move(frame), now,
+  // Current OnPreDecode only cares about QP for VP8.
-                          keyframe_request_is_due,
+  int qp = -1;
+  if (frame->CodecSpecific()->codecType == kVideoCodecVP8) {
+    if (!vp8::GetQp(frame->data(), frame->size(), &qp)) {
+      RTC_LOG(LS_WARNING) << "Failed to extract QP from VP8 video frame";
+    }
+  }
+  stats_proxy_.OnPreDecode(frame->CodecSpecific()->codecType, qp);
+
+  decode_queue_.PostTask([this, now, keyframe_request_is_due,
+                          received_frame_is_keyframe, frame = std::move(frame),
                           keyframe_required = keyframe_required_]() mutable {
     RTC_DCHECK_RUN_ON(&decode_queue_);
     if (decoder_stopped_)
       return;
-    HandleEncodedFrameOnDecodeQueue(std::move(frame), now,
+    DecodeFrameResult result = HandleEncodedFrameOnDecodeQueue(
-                                    keyframe_request_is_due, keyframe_required);
+        std::move(frame), keyframe_request_is_due, keyframe_required);
+
+    // TODO(bugs.webrtc.org/11993): Make this PostTask to the network thread.
+    call_->worker_thread()->PostTask(
+        SafeTask(task_safety_.flag(),
+                 [this, now, result = std::move(result),
+                  received_frame_is_keyframe, keyframe_request_is_due]() {
+                   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+                   keyframe_required_ = result.keyframe_required;
+
+                   if (result.decoded_frame_picture_id) {
+                     rtp_video_stream_receiver_.FrameDecoded(
+                         *result.decoded_frame_picture_id);
+                   }
+
+                   HandleKeyFrameGeneration(received_frame_is_keyframe, now,
+                                            result.force_request_key_frame,
+                                            keyframe_request_is_due);
+                   buffer_->StartNextDecode(keyframe_required_);
+                 }));
   });
 }
 
@@ -840,24 +870,15 @@ void VideoReceiveStream2::OnDecodableFrameTimeout(TimeDelta wait) {
   buffer_->StartNextDecode(keyframe_required_);
 }
 
-void VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue(
+VideoReceiveStream2::DecodeFrameResult
+VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue(
     std::unique_ptr<EncodedFrame> frame,
-    Timestamp now,
     bool keyframe_request_is_due,
     bool keyframe_required) {
   RTC_DCHECK_RUN_ON(&decode_queue_);
 
-  // Current OnPreDecode only cares about QP for VP8.
-  int qp = -1;
-  if (frame->CodecSpecific()->codecType == kVideoCodecVP8) {
-    if (!vp8::GetQp(frame->data(), frame->size(), &qp)) {
-      RTC_LOG(LS_WARNING) << "Failed to extract QP from VP8 video frame";
-    }
-  }
-  stats_proxy_.OnPreDecode(frame->CodecSpecific()->codecType, qp);
-
   bool force_request_key_frame = false;
-  int64_t decoded_frame_picture_id = -1;
+  absl::optional<int64_t> decoded_frame_picture_id;
 
   if (!video_receiver_.IsExternalDecoderRegistered(frame->PayloadType())) {
     // Look for the decoder with this payload type.
@@ -870,8 +891,6 @@ void VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue(
   }
 
   int64_t frame_id = frame->Id();
-  bool received_frame_is_keyframe =
-      frame->FrameType() == VideoFrameType::kVideoFrameKey;
   int decode_result = DecodeAndMaybeDispatchEncodedFrame(std::move(frame));
   if (decode_result == WEBRTC_VIDEO_CODEC_OK ||
       decode_result == WEBRTC_VIDEO_CODEC_OK_REQUEST_KEYFRAME) {
@@ -889,24 +908,11 @@ void VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue(
     force_request_key_frame = true;
   }
 
-  {
+  return DecodeFrameResult{
-    // TODO(bugs.webrtc.org/11993): Make this PostTask to the network thread.
+      .force_request_key_frame = force_request_key_frame,
-    call_->worker_thread()->PostTask(SafeTask(
+      .decoded_frame_picture_id = std::move(decoded_frame_picture_id),
-        task_safety_.flag(), [this, now, received_frame_is_keyframe,
+      .keyframe_required = keyframe_required,
-                              force_request_key_frame, decoded_frame_picture_id,
+  };
-                              keyframe_request_is_due, keyframe_required]() {
-          RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
-          keyframe_required_ = keyframe_required;
-
-          if (decoded_frame_picture_id != -1)
-            rtp_video_stream_receiver_.FrameDecoded(decoded_frame_picture_id);
-
-          HandleKeyFrameGeneration(received_frame_is_keyframe, now,
-                                   force_request_key_frame,
-                                   keyframe_request_is_due);
-          buffer_->StartNextDecode(keyframe_required_);
-        }));
-  }
 }
 
 int VideoReceiveStream2::DecodeAndMaybeDispatchEncodedFrame(

video/video_receive_stream2.h

@@ -203,11 +203,33 @@ class VideoReceiveStream2
   void OnDecodableFrameTimeout(TimeDelta wait) override;
 
   void CreateAndRegisterExternalDecoder(const Decoder& decoder);
-  void HandleEncodedFrameOnDecodeQueue(std::unique_ptr<EncodedFrame> frame,
+
-                                       Timestamp now,
+  struct DecodeFrameResult {
+    DecodeFrameResult(const DecodeFrameResult&) = delete;
+    DecodeFrameResult& operator=(const DecodeFrameResult&) = delete;
+    DecodeFrameResult(DecodeFrameResult&&) = default;
+    DecodeFrameResult& operator=(DecodeFrameResult&&) = default;
+
+    // True if the decoder returned code WEBRTC_VIDEO_CODEC_OK_REQUEST_KEYFRAME,
+    // or if the decoder failed and a keyframe is required. When true, a
+    // keyframe request should be sent even if a keyframe request was sent
+    // recently.
+    bool force_request_key_frame;
+
+    // The picture id of the frame that was decoded, or nullopt if the frame was
+    // not decoded.
+    absl::optional<int64_t> decoded_frame_picture_id;
+
+    // True if the next frame decoded must be a keyframe. This value will set
+    // the value of `keyframe_required_`, which will force the frame buffer to
+    // drop all frames that are not keyframes.
+    bool keyframe_required;
+  };
+
+  DecodeFrameResult HandleEncodedFrameOnDecodeQueue(
+      std::unique_ptr<EncodedFrame> frame,
       bool keyframe_request_is_due,
-                                       bool keyframe_required)
+      bool keyframe_required) RTC_RUN_ON(decode_queue_);
-      RTC_RUN_ON(decode_queue_);
   void UpdatePlayoutDelays() const
       RTC_EXCLUSIVE_LOCKS_REQUIRED(worker_sequence_checker_);
   void RequestKeyFrame(Timestamp now) RTC_RUN_ON(packet_sequence_checker_);