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}
diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc
index 01e4f09..073b1a6 100644
--- a/video/receive_statistics_proxy2.cc
+++ b/video/receive_statistics_proxy2.cc
@@ -959,16 +959,12 @@
}
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);
- }
- }));
+ 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() {
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 553f6b4..7e3968a 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -801,15 +801,45 @@
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,
- keyframe_request_is_due,
+ // 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);
+
+ 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,
- keyframe_request_is_due, keyframe_required);
+ DecodeFrameResult result = HandleEncodedFrameOnDecodeQueue(
+ 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 @@
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 @@
}
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 @@
force_request_key_frame = true;
}
- {
- // TODO(bugs.webrtc.org/11993): Make this PostTask to the network thread.
- call_->worker_thread()->PostTask(SafeTask(
- task_safety_.flag(), [this, now, received_frame_is_keyframe,
- 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_);
- }));
- }
+ return DecodeFrameResult{
+ .force_request_key_frame = force_request_key_frame,
+ .decoded_frame_picture_id = std::move(decoded_frame_picture_id),
+ .keyframe_required = keyframe_required,
+ };
}
int VideoReceiveStream2::DecodeAndMaybeDispatchEncodedFrame(
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 4fbb66d..c16e418 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -203,11 +203,33 @@
void OnDecodableFrameTimeout(TimeDelta wait) override;
void CreateAndRegisterExternalDecoder(const Decoder& decoder);
- void HandleEncodedFrameOnDecodeQueue(std::unique_ptr<EncodedFrame> frame,
- Timestamp now,
- bool keyframe_request_is_due,
- bool keyframe_required)
- RTC_RUN_ON(decode_queue_);
+
+ 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) 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_);
