From 214cab5727bafa58aaf270768e2144c02c90231b Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Tue, 16 Aug 2022 09:48:23 +0000 Subject: [PATCH] Move VideoStreamBufferController to packet sequence It no longer has to interact with the decode queue, that will only happen in VideoReceieveStream2. This moves some members in VideoReceieveStream2 to the packet sequence which removes a few post-tasks. Bug: webrtc:14003, webrtc:11993 Change-Id: I4641b593b1a2f68e017c384b73ee4e75d06cf559 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271700 Commit-Queue: Evan Shrubsole Reviewed-by: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#37802} --- test/fake_encoded_frame.cc | 7 +- test/fake_encoded_frame.h | 8 +- video/video_receive_stream2.cc | 180 ++++++++---------- video/video_receive_stream2.h | 30 +-- video/video_stream_buffer_controller.cc | 44 +---- video/video_stream_buffer_controller.h | 10 +- ...video_stream_buffer_controller_unittest.cc | 70 +++---- 7 files changed, 146 insertions(+), 203 deletions(-) diff --git a/test/fake_encoded_frame.cc b/test/fake_encoded_frame.cc index 00d1537bc4..32fa5d8ccf 100644 --- a/test/fake_encoded_frame.cc +++ b/test/fake_encoded_frame.cc @@ -14,7 +14,7 @@ #include "api/video/video_frame_type.h" -namespace webrtc::test { +namespace webrtc { void PrintTo(const EncodedFrame& frame, std::ostream* os) /* no-presubmit-check TODO(webrtc:8982) */ { @@ -26,6 +26,8 @@ void PrintTo(const EncodedFrame& frame, *os << "]"; } +namespace test { + int64_t FakeEncodedFrame::ReceivedTime() const { return received_time_; } @@ -138,4 +140,5 @@ FakeFrameBuilder& FakeFrameBuilder::PacketInfos(RtpPacketInfos packet_infos) { return *this; } -} // namespace webrtc::test +} // namespace test +} // namespace webrtc diff --git a/test/fake_encoded_frame.h b/test/fake_encoded_frame.h index 80502523f7..a5b2aca4a1 100644 --- a/test/fake_encoded_frame.h +++ b/test/fake_encoded_frame.h @@ -17,16 +17,17 @@ #include "api/rtp_packet_infos.h" #include "api/video/encoded_frame.h" -#include "api/video/encoded_image.h" #include "api/video/video_rotation.h" #include "test/gmock.h" -namespace webrtc::test { +namespace webrtc { // For test printing. void PrintTo(const EncodedFrame& frame, std::ostream* os); // no-presubmit-check TODO(webrtc:8982) +namespace test { + class FakeEncodedFrame : public EncodedFrame { public: // Always 10ms delay and on time. @@ -84,6 +85,7 @@ class FakeFrameBuilder { size_t size_ = 10; }; -} // namespace webrtc::test +} // namespace test +} // namespace webrtc #endif // TEST_FAKE_ENCODED_FRAME_H_ diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 23302fc599..1189ea61e7 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -262,9 +262,9 @@ VideoReceiveStream2::VideoReceiveStream2( timing_->set_render_delay(TimeDelta::Millis(config_.render_delay_ms)); buffer_ = VideoStreamBufferController::CreateFromFieldTrial( - clock_, call_->worker_thread(), timing_.get(), &stats_proxy_, - decode_queue_.Get(), this, max_wait_for_keyframe_, max_wait_for_frame_, - decode_sync_, call_->trials()); + clock_, call_->worker_thread(), timing_.get(), &stats_proxy_, this, + max_wait_for_keyframe_, max_wait_for_frame_, decode_sync_, + call_->trials()); if (rtx_ssrc()) { rtx_receive_stream_ = std::make_unique( @@ -434,7 +434,7 @@ void VideoReceiveStream2::Stop() { stats_proxy_.OnUniqueFramesCounted( rtp_video_stream_receiver_.GetUniqueFramesSeen()); - buffer_->StopOnWorker(); + buffer_->Stop(); call_stats_->DeregisterStatsObserver(this); if (decoder_running_) { rtc::Event done; @@ -543,11 +543,8 @@ void VideoReceiveStream2::SetNackHistory(TimeDelta history) { TimeDelta max_wait_for_keyframe = DetermineMaxWaitForFrame(history, true); TimeDelta max_wait_for_frame = DetermineMaxWaitForFrame(history, false); - decode_queue_.PostTask([this, max_wait_for_keyframe, max_wait_for_frame]() { - RTC_DCHECK_RUN_ON(&decode_queue_); - max_wait_for_keyframe_ = max_wait_for_keyframe; - max_wait_for_frame_ = max_wait_for_frame; - }); + max_wait_for_keyframe_ = max_wait_for_keyframe; + max_wait_for_frame_ = max_wait_for_frame; buffer_->SetMaxWaits(max_wait_for_keyframe, max_wait_for_frame); } @@ -738,10 +735,7 @@ void VideoReceiveStream2::RequestKeyFrame(Timestamp now) { // Called from RtpVideoStreamReceiver (rtp_video_stream_receiver_ is // ultimately responsible). rtp_video_stream_receiver_.RequestKeyFrame(); - decode_queue_.PostTask([this, now]() { - RTC_DCHECK_RUN_ON(&decode_queue_); - last_keyframe_request_ = now; - }); + last_keyframe_request_ = now; } void VideoReceiveStream2::OnCompleteFrame(std::unique_ptr frame) { @@ -811,40 +805,57 @@ bool VideoReceiveStream2::SetMinimumPlayoutDelay(int delay_ms) { return true; } -TimeDelta VideoReceiveStream2::GetMaxWait() const { - RTC_DCHECK_RUN_ON(&decode_queue_); - return keyframe_required_ ? max_wait_for_keyframe_ : max_wait_for_frame_; +void VideoReceiveStream2::OnEncodedFrame(std::unique_ptr frame) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + Timestamp now = clock_->CurrentTime(); + const bool keyframe_request_is_due = + !last_keyframe_request_ || + now >= (*last_keyframe_request_ + max_wait_for_keyframe_); + + decode_queue_.PostTask([this, frame = std::move(frame), now, + keyframe_request_is_due, + 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); + }); } -void VideoReceiveStream2::OnEncodedFrame(std::unique_ptr frame) { - RTC_DCHECK_RUN_ON(&decode_queue_); - if (decoder_stopped_) - return; - HandleEncodedFrame(std::move(frame)); +void VideoReceiveStream2::OnDecodableFrameTimeout(TimeDelta wait) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + Timestamp now = clock_->CurrentTime(); + + absl::optional last_packet_ms = + rtp_video_stream_receiver_.LastReceivedPacketMs(); + + // To avoid spamming keyframe requests for a stream that is not active we + // check if we have received a packet within the last 5 seconds. + constexpr TimeDelta kInactiveDuration = TimeDelta::Seconds(5); + const bool stream_is_active = + last_packet_ms && + now - Timestamp::Millis(*last_packet_ms) < kInactiveDuration; + if (!stream_is_active) + stats_proxy_.OnStreamInactive(); + + if (stream_is_active && !IsReceivingKeyFrame(now) && + (!config_.crypto_options.sframe.require_frame_encryption || + rtp_video_stream_receiver_.IsDecryptable())) { + RTC_LOG(LS_WARNING) << "No decodable frame in " << wait + << ", requesting keyframe."; + RequestKeyFrame(now); + } + buffer_->StartNextDecode(keyframe_required_); } -void VideoReceiveStream2::OnDecodableFrameTimeout(TimeDelta wait_time) { +void VideoReceiveStream2::HandleEncodedFrameOnDecodeQueue( + std::unique_ptr frame, + Timestamp now, + bool keyframe_request_is_due, + bool keyframe_required) { RTC_DCHECK_RUN_ON(&decode_queue_); - Timestamp now = clock_->CurrentTime(); - // TODO(bugs.webrtc.org/11993): PostTask to the network thread. - call_->worker_thread()->PostTask(SafeTask( - task_safety_.flag(), - [this, wait_time, now, max_wait_for_keyframe = max_wait_for_keyframe_] { - RTC_DCHECK_RUN_ON(&packet_sequence_checker_); - HandleFrameBufferTimeout(now, wait_time, max_wait_for_keyframe); - - decode_queue_.PostTask([this] { - RTC_DCHECK_RUN_ON(&decode_queue_); - buffer_->StartNextDecode(keyframe_required_); - }); - })); -} - -void VideoReceiveStream2::HandleEncodedFrame( - std::unique_ptr frame) { - RTC_DCHECK_RUN_ON(&decode_queue_); - Timestamp now = clock_->CurrentTime(); // Current OnPreDecode only cares about QP for VP8. int qp = -1; @@ -858,10 +869,6 @@ void VideoReceiveStream2::HandleEncodedFrame( bool force_request_key_frame = false; int64_t decoded_frame_picture_id = -1; - const bool keyframe_request_is_due = - !last_keyframe_request_ || - now >= (*last_keyframe_request_ + max_wait_for_keyframe_); - if (!video_receiver_.IsExternalDecoderRegistered(frame->PayloadType())) { // Look for the decoder with this payload type. for (const Decoder& decoder : config_.decoders) { @@ -878,16 +885,15 @@ void VideoReceiveStream2::HandleEncodedFrame( int decode_result = DecodeAndMaybeDispatchEncodedFrame(std::move(frame)); if (decode_result == WEBRTC_VIDEO_CODEC_OK || decode_result == WEBRTC_VIDEO_CODEC_OK_REQUEST_KEYFRAME) { - keyframe_required_ = false; + keyframe_required = false; frame_decoded_ = true; decoded_frame_picture_id = frame_id; if (decode_result == WEBRTC_VIDEO_CODEC_OK_REQUEST_KEYFRAME) force_request_key_frame = true; - } else if (!frame_decoded_ || !keyframe_required_ || - keyframe_request_is_due) { - keyframe_required_ = true; + } else if (!frame_decoded_ || !keyframe_required || keyframe_request_is_due) { + keyframe_required = true; // TODO(philipel): Remove this keyframe request when downstream project // has been fixed. force_request_key_frame = true; @@ -896,18 +902,19 @@ void VideoReceiveStream2::HandleEncodedFrame( { // 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, - max_wait_for_keyframe = max_wait_for_keyframe_]() { + 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, max_wait_for_keyframe); + HandleKeyFrameGeneration(received_frame_is_keyframe, now, + force_request_key_frame, + keyframe_request_is_due); + buffer_->StartNextDecode(keyframe_required_); })); } } @@ -976,8 +983,7 @@ void VideoReceiveStream2::HandleKeyFrameGeneration( bool received_frame_is_keyframe, Timestamp now, bool always_request_key_frame, - bool keyframe_request_is_due, - TimeDelta max_wait_for_keyframe) { + bool keyframe_request_is_due) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); bool request_key_frame = always_request_key_frame; @@ -986,7 +992,7 @@ void VideoReceiveStream2::HandleKeyFrameGeneration( if (received_frame_is_keyframe) { keyframe_generation_requested_ = false; } else if (keyframe_request_is_due) { - if (!IsReceivingKeyFrame(now, max_wait_for_keyframe)) { + if (!IsReceivingKeyFrame(now)) { request_key_frame = true; } } else { @@ -1002,45 +1008,16 @@ void VideoReceiveStream2::HandleKeyFrameGeneration( } } -void VideoReceiveStream2::HandleFrameBufferTimeout( - Timestamp now, - TimeDelta wait, - TimeDelta max_wait_for_keyframe) { - RTC_DCHECK_RUN_ON(&packet_sequence_checker_); - - absl::optional last_packet_ms = - rtp_video_stream_receiver_.LastReceivedPacketMs(); - - // To avoid spamming keyframe requests for a stream that is not active we - // check if we have received a packet within the last 5 seconds. - constexpr TimeDelta kInactiveDuraction = TimeDelta::Seconds(5); - const bool stream_is_active = - last_packet_ms && - now - Timestamp::Millis(*last_packet_ms) < kInactiveDuraction; - if (!stream_is_active) - stats_proxy_.OnStreamInactive(); - - if (stream_is_active && !IsReceivingKeyFrame(now, max_wait_for_keyframe) && - (!config_.crypto_options.sframe.require_frame_encryption || - rtp_video_stream_receiver_.IsDecryptable())) { - RTC_LOG(LS_WARNING) << "No decodable frame in " << wait - << ", requesting keyframe."; - RequestKeyFrame(now); - } -} - -bool VideoReceiveStream2::IsReceivingKeyFrame( - Timestamp now, - TimeDelta max_wait_for_keyframe) const { +bool VideoReceiveStream2::IsReceivingKeyFrame(Timestamp now) const { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); absl::optional last_keyframe_packet_ms = rtp_video_stream_receiver_.LastReceivedKeyframePacketMs(); // If we recently have been receiving packets belonging to a keyframe then // we assume a keyframe is currently being received. - bool receiving_keyframe = - last_keyframe_packet_ms && - now - Timestamp::Millis(*last_keyframe_packet_ms) < max_wait_for_keyframe; + bool receiving_keyframe = last_keyframe_packet_ms && + now - Timestamp::Millis(*last_keyframe_packet_ms) < + max_wait_for_keyframe_; return receiving_keyframe; } @@ -1100,19 +1077,26 @@ VideoReceiveStream2::SetAndGetRecordingState(RecordingState state, // Save old state, set the new state. RecordingState old_state; + absl::optional last_keyframe_request; + { + // TODO(bugs.webrtc.org/11993): Post this to the network thread. + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + last_keyframe_request = last_keyframe_request_; + last_keyframe_request_ = + generate_key_frame + ? clock_->CurrentTime() + : Timestamp::Millis(state.last_keyframe_request_ms.value_or(0)); + } + decode_queue_.PostTask( [this, &event, &old_state, callback = std::move(state.callback), - generate_key_frame, - last_keyframe_request = - Timestamp::Millis(state.last_keyframe_request_ms.value_or(0))] { + last_keyframe_request = std::move(last_keyframe_request)] { RTC_DCHECK_RUN_ON(&decode_queue_); old_state.callback = std::move(encoded_frame_buffer_function_); encoded_frame_buffer_function_ = std::move(callback); old_state.last_keyframe_request_ms = - last_keyframe_request_.value_or(Timestamp::Zero()).ms(); - last_keyframe_request_ = - generate_key_frame ? clock_->CurrentTime() : last_keyframe_request; + last_keyframe_request.value_or(Timestamp::Zero()).ms(); event.Set(); }); diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index edc514d532..06eab029bb 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -197,27 +197,27 @@ class VideoReceiveStream2 void GenerateKeyFrame() override; private: + // FrameSchedulingReceiver implementation. + // Called on packet sequence. void OnEncodedFrame(std::unique_ptr frame) override; - void OnDecodableFrameTimeout(TimeDelta wait_time) override; + // Called on packet sequence. + void OnDecodableFrameTimeout(TimeDelta wait) override; + void CreateAndRegisterExternalDecoder(const Decoder& decoder); - TimeDelta GetMaxWait() const RTC_RUN_ON(decode_queue_); - void HandleEncodedFrame(std::unique_ptr frame) + void HandleEncodedFrameOnDecodeQueue(std::unique_ptr frame, + Timestamp now, + bool keyframe_request_is_due, + bool keyframe_required) RTC_RUN_ON(decode_queue_); - void HandleFrameBufferTimeout(Timestamp now, - TimeDelta wait, - TimeDelta max_wait_for_keyframe) - RTC_RUN_ON(packet_sequence_checker_); void UpdatePlayoutDelays() const RTC_EXCLUSIVE_LOCKS_REQUIRED(worker_sequence_checker_); void RequestKeyFrame(Timestamp now) RTC_RUN_ON(packet_sequence_checker_); void HandleKeyFrameGeneration(bool received_frame_is_keyframe, Timestamp now, bool always_request_key_frame, - bool keyframe_request_is_due, - TimeDelta max_wait_for_keyframe) + bool keyframe_request_is_due) RTC_RUN_ON(packet_sequence_checker_); - bool IsReceivingKeyFrame(Timestamp timestamp, - TimeDelta max_wait_for_keyframe) const + bool IsReceivingKeyFrame(Timestamp timestamp) const RTC_RUN_ON(packet_sequence_checker_); int DecodeAndMaybeDispatchEncodedFrame(std::unique_ptr frame) RTC_RUN_ON(decode_queue_); @@ -275,17 +275,17 @@ class VideoReceiveStream2 // 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_ RTC_GUARDED_BY(decode_queue_) = true; + bool keyframe_required_ RTC_GUARDED_BY(packet_sequence_checker_) = true; // If we have successfully decoded any frame. bool frame_decoded_ RTC_GUARDED_BY(decode_queue_) = false; absl::optional last_keyframe_request_ - RTC_GUARDED_BY(decode_queue_); + RTC_GUARDED_BY(packet_sequence_checker_); // Keyframe request intervals are configurable through field trials. - TimeDelta max_wait_for_keyframe_ RTC_GUARDED_BY(decode_queue_); - TimeDelta max_wait_for_frame_ RTC_GUARDED_BY(decode_queue_); + TimeDelta max_wait_for_keyframe_ RTC_GUARDED_BY(packet_sequence_checker_); + TimeDelta max_wait_for_frame_ RTC_GUARDED_BY(packet_sequence_checker_); // All of them tries to change current min_playout_delay on `timing_` but // source of the change request is different in each case. Among them the diff --git a/video/video_stream_buffer_controller.cc b/video/video_stream_buffer_controller.cc index e642578c29..037754d404 100644 --- a/video/video_stream_buffer_controller.cc +++ b/video/video_stream_buffer_controller.cc @@ -96,7 +96,6 @@ VideoStreamBufferController::CreateFromFieldTrial( TaskQueueBase* worker_queue, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_proxy, - TaskQueueBase* decode_queue, FrameSchedulingReceiver* receiver, TimeDelta max_wait_for_keyframe, TimeDelta max_wait_for_frame, @@ -116,7 +115,7 @@ VideoStreamBufferController::CreateFromFieldTrial( clock, worker_queue); } return std::make_unique( - clock, worker_queue, timing, stats_proxy, decode_queue, receiver, + clock, worker_queue, timing, stats_proxy, receiver, max_wait_for_keyframe, max_wait_for_frame, std::move(scheduler), field_trials); } @@ -126,7 +125,7 @@ VideoStreamBufferController::CreateFromFieldTrial( auto scheduler = std::make_unique(clock, worker_queue); return std::make_unique( - clock, worker_queue, timing, stats_proxy, decode_queue, receiver, + clock, worker_queue, timing, stats_proxy, receiver, max_wait_for_keyframe, max_wait_for_frame, std::move(scheduler), field_trials); } @@ -138,7 +137,6 @@ VideoStreamBufferController::VideoStreamBufferController( TaskQueueBase* worker_queue, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_proxy, - TaskQueueBase* decode_queue, FrameSchedulingReceiver* receiver, TimeDelta max_wait_for_keyframe, TimeDelta max_wait_for_frame, @@ -146,8 +144,6 @@ VideoStreamBufferController::VideoStreamBufferController( const FieldTrialsView& field_trials) : field_trials_(field_trials), clock_(clock), - worker_queue_(worker_queue), - decode_queue_(decode_queue), stats_proxy_(stats_proxy), receiver_(receiver), timing_(timing), @@ -159,7 +155,7 @@ VideoStreamBufferController::VideoStreamBufferController( decode_timing_(clock_, timing_), timeout_tracker_( clock_, - worker_queue_, + worker_queue, VideoReceiveStreamTimeoutTracker::Timeouts{ .max_wait_for_keyframe = max_wait_for_keyframe, .max_wait_for_frame = max_wait_for_frame}, @@ -167,11 +163,9 @@ VideoStreamBufferController::VideoStreamBufferController( zero_playout_delay_max_decode_queue_size_( "max_decode_queue_size", kZeroPlayoutDelayDefaultMaxDecodeQueueSize) { - RTC_DCHECK(decode_queue_); RTC_DCHECK(stats_proxy_); RTC_DCHECK(receiver_); RTC_DCHECK(timing_); - RTC_DCHECK(worker_queue_); RTC_DCHECK(clock_); RTC_DCHECK(frame_decode_scheduler_); RTC_LOG(LS_WARNING) << "Using FrameBuffer3"; @@ -180,15 +174,11 @@ VideoStreamBufferController::VideoStreamBufferController( field_trials.Lookup("WebRTC-ZeroPlayoutDelay")); } -void VideoStreamBufferController::StopOnWorker() { +void VideoStreamBufferController::Stop() { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); frame_decode_scheduler_->Stop(); timeout_tracker_.Stop(); decoder_ready_for_new_frame_ = false; - decode_queue_->PostTask([this] { - RTC_DCHECK_RUN_ON(decode_queue_); - decode_safety_->SetNotAlive(); - }); } void VideoStreamBufferController::SetProtectionMode( @@ -238,13 +228,6 @@ void VideoStreamBufferController::SetMaxWaits(TimeDelta max_wait_for_keyframe, } void VideoStreamBufferController::StartNextDecode(bool keyframe_required) { - if (!worker_queue_->IsCurrent()) { - worker_queue_->PostTask(SafeTask( - worker_safety_.flag(), - [this, keyframe_required] { StartNextDecode(keyframe_required); })); - return; - } - RTC_DCHECK_RUN_ON(&worker_sequence_checker_); if (!timeout_tracker_.Running()) timeout_tracker_.Start(keyframe_required); @@ -325,28 +308,21 @@ void VideoStreamBufferController::OnFrameReady( timing_->SetLastDecodeScheduledTimestamp(now); decoder_ready_for_new_frame_ = false; - // VideoReceiveStream2 wants frames on the decoder thread. - decode_queue_->PostTask( - SafeTask(decode_safety_, [this, frame = std::move(frame)]() mutable { - RTC_DCHECK_RUN_ON(decode_queue_); - receiver_->OnEncodedFrame(std::move(frame)); - })); + receiver_->OnEncodedFrame(std::move(frame)); } void VideoStreamBufferController::OnTimeout(TimeDelta delay) { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); + + // Stop sending timeouts until receiver starts waiting for a new frame. + timeout_tracker_.Stop(); + // If the stream is paused then ignore the timeout. if (!decoder_ready_for_new_frame_) { - timeout_tracker_.Stop(); return; } - decode_queue_->PostTask(SafeTask(decode_safety_, [this, delay]() { - RTC_DCHECK_RUN_ON(decode_queue_); - receiver_->OnDecodableFrameTimeout(delay); - })); - // Stop sending timeouts until receive starts waiting for a new frame. - timeout_tracker_.Stop(); decoder_ready_for_new_frame_ = false; + receiver_->OnDecodableFrameTimeout(delay); } void VideoStreamBufferController::FrameReadyForDecode(uint32_t rtp_timestamp, diff --git a/video/video_stream_buffer_controller.h b/video/video_stream_buffer_controller.h index f592c8c582..5251545442 100644 --- a/video/video_stream_buffer_controller.h +++ b/video/video_stream_buffer_controller.h @@ -43,7 +43,6 @@ class VideoStreamBufferController { TaskQueueBase* worker_queue, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_proxy, - TaskQueueBase* decode_queue, FrameSchedulingReceiver* receiver, TimeDelta max_wait_for_keyframe, TimeDelta max_wait_for_frame, @@ -55,14 +54,13 @@ class VideoStreamBufferController { TaskQueueBase* worker_queue, VCMTiming* timing, VCMReceiveStatisticsCallback* stats_proxy, - TaskQueueBase* decode_queue, FrameSchedulingReceiver* receiver, TimeDelta max_wait_for_keyframe, TimeDelta max_wait_for_frame, std::unique_ptr frame_decode_scheduler, const FieldTrialsView& field_trials); - void StopOnWorker(); + void Stop(); void SetProtectionMode(VCMVideoProtection protection_mode); void Clear(); absl::optional InsertFrame(std::unique_ptr frame); @@ -90,10 +88,8 @@ class VideoStreamBufferController { const absl::optional rtt_mult_settings_ = RttMultExperiment::GetRttMultValue(); Clock* const clock_; - TaskQueueBase* const worker_queue_; - TaskQueueBase* const decode_queue_; VCMReceiveStatisticsCallback* const stats_proxy_; - FrameSchedulingReceiver* const receiver_ RTC_PT_GUARDED_BY(decode_queue_); + FrameSchedulingReceiver* const receiver_; VCMTiming* const timing_; const std::unique_ptr frame_decode_scheduler_ RTC_GUARDED_BY(&worker_sequence_checker_); @@ -125,8 +121,6 @@ class VideoStreamBufferController { // the frame's render time == 0. FieldTrialParameter zero_playout_delay_max_decode_queue_size_; - rtc::scoped_refptr decode_safety_ = - PendingTaskSafetyFlag::CreateDetached(); ScopedTaskSafety worker_safety_; }; diff --git a/video/video_stream_buffer_controller_unittest.cc b/video/video_stream_buffer_controller_unittest.cc index 81dae65210..7f7d8dbb57 100644 --- a/video/video_stream_buffer_controller_unittest.cc +++ b/video/video_stream_buffer_controller_unittest.cc @@ -30,7 +30,6 @@ #include "test/fake_encoded_frame.h" #include "test/gmock.h" #include "test/gtest.h" -#include "test/run_loop.h" #include "test/scoped_key_value_config.h" #include "test/time_controller/simulated_time_controller.h" #include "video/decode_synchronizer.h" @@ -115,19 +114,17 @@ class VideoStreamBufferControllerFixture : field_trials_(GetParam()), time_controller_(kClockStart), clock_(time_controller_.GetClock()), - decode_queue_(time_controller_.GetTaskQueueFactory()->CreateTaskQueue( - "decode_queue", - TaskQueueFactory::Priority::NORMAL)), fake_metronome_(time_controller_.GetTaskQueueFactory(), TimeDelta::Millis(16)), - decode_sync_(clock_, &fake_metronome_, run_loop_.task_queue()), + decode_sync_(clock_, + &fake_metronome_, + time_controller_.GetMainThread()), timing_(clock_, field_trials_), buffer_(VideoStreamBufferController::CreateFromFieldTrial( clock_, - run_loop_.task_queue(), + time_controller_.GetMainThread(), &timing_, &stats_callback_, - decode_queue_.Get(), this, kMaxWaitForKeyframe, kMaxWaitForFrame, @@ -143,7 +140,7 @@ class VideoStreamBufferControllerFixture ~VideoStreamBufferControllerFixture() override { if (buffer_) { - buffer_->StopOnWorker(); + buffer_->Stop(); } fake_metronome_.Stop(); time_controller_.AdvanceTime(TimeDelta::Zero()); @@ -165,37 +162,35 @@ class VideoStreamBufferControllerFixture if (wait_result_) { return std::move(wait_result_); } - run_loop_.PostTask([&] { time_controller_.AdvanceTime(wait); }); - run_loop_.PostTask([&] { - if (wait_result_) - return; + time_controller_.AdvanceTime(TimeDelta::Zero()); + if (wait_result_) { + return std::move(wait_result_); + } - // If run loop posted to a task queue, flush that if there is no result. - time_controller_.AdvanceTime(TimeDelta::Zero()); - if (wait_result_) - return; + Timestamp now = clock_->CurrentTime(); + // TODO(bugs.webrtc.org/13756): Remove this when rtc::Thread uses uses + // Timestamp instead of an integer milliseconds. This extra wait is needed + // for some tests that use the metronome. This is due to rounding + // milliseconds, affecting the precision of simulated time controller uses + // when posting tasks from threads. + TimeDelta potential_extra_wait = + Timestamp::Millis((now + wait).ms()) - (now + wait); - run_loop_.PostTask([&] { - time_controller_.AdvanceTime(TimeDelta::Zero()); - // Quit if there is no result set. - if (!wait_result_) - run_loop_.Quit(); - }); - }); - run_loop_.Run(); + time_controller_.AdvanceTime(wait); + if (potential_extra_wait > TimeDelta::Zero()) { + time_controller_.AdvanceTime(potential_extra_wait); + } return std::move(wait_result_); } void StartNextDecode() { ResetLastResult(); buffer_->StartNextDecode(false); - time_controller_.AdvanceTime(TimeDelta::Zero()); } void StartNextDecodeForceKeyframe() { ResetLastResult(); buffer_->StartNextDecode(true); - time_controller_.AdvanceTime(TimeDelta::Zero()); } void ResetLastResult() { wait_result_.reset(); } @@ -206,8 +201,6 @@ class VideoStreamBufferControllerFixture test::ScopedKeyValueConfig field_trials_; GlobalSimulatedTimeController time_controller_; Clock* const clock_; - test::RunLoop run_loop_; - rtc::TaskQueue decode_queue_; test::FakeMetronome fake_metronome_; DecodeSynchronizer decode_sync_; VCMTiming timing_; @@ -222,7 +215,6 @@ class VideoStreamBufferControllerFixture RTC_DCHECK(absl::get>(result)); } wait_result_.emplace(std::move(result)); - run_loop_.Quit(); } uint32_t dropped_frames_ = 0; @@ -340,7 +332,7 @@ TEST_P(VideoStreamBufferControllerTest, .AsLast() .Refs({0}) .Build())); - buffer_->StopOnWorker(); + buffer_->Stop(); // Wait for 2x max wait time. Since we stopped, this should cause no timeouts // or frame-ready callbacks. EXPECT_THAT(WaitForFrameOrTimeout(kMaxWaitForFrame * 2), Eq(absl::nullopt)); @@ -580,33 +572,25 @@ TEST_P(VideoStreamBufferControllerTest, SameFrameNotScheduledTwice) { StartNextDecode(); - // Warmup VCMTiming for 30fps. - for (int i = 1; i <= 30; ++i) { - buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp( - test::FakeFrameBuilder().Id(i).Time(i * kFps30Rtp).AsLast().Build())); - EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(i))); - StartNextDecode(); - } - // F2 arrives and is scheduled. buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp( - test::FakeFrameBuilder().Id(32).Time(32 * kFps30Rtp).AsLast().Build())); + test::FakeFrameBuilder().Id(2).Time(2 * kFps30Rtp).AsLast().Build())); // F3 arrives before F2 is extracted. time_controller_.AdvanceTime(kFps30Delay); buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp( - test::FakeFrameBuilder().Id(33).Time(33 * kFps30Rtp).AsLast().Build())); + test::FakeFrameBuilder().Id(3).Time(3 * kFps30Rtp).AsLast().Build())); // F1 arrives and is fast-forwarded since it is too late. // F2 is already scheduled and should not be rescheduled. time_controller_.AdvanceTime(kFps30Delay / 2); buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp( - test::FakeFrameBuilder().Id(31).Time(31 * kFps30Rtp).AsLast().Build())); + test::FakeFrameBuilder().Id(1).Time(1 * kFps30Rtp).AsLast().Build())); - EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(32))); + EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(2))); StartNextDecode(); - EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(33))); + EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(3))); StartNextDecode(); EXPECT_THAT(WaitForFrameOrTimeout(kMaxWaitForFrame), TimedOut()); EXPECT_EQ(dropped_frames(), 1);