From 2e0f4f0f37c1a6a9d2969ca489bd887db3b74f85 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Tue, 21 Dec 2021 19:14:58 +0100 Subject: [PATCH] ZeroHertzAdapterMode: handle key frame requests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under zero-hertz mode, provided that a frame arrived to the VideoStreamEncoder, the receiver may experience up to a second between incoming frames. This results in key frame requests getting serviced with that delay, which is undesired. What's worse is also the fact that if no frame ever arrived to the VideoStreamEncoder, it will not service the keyframe requests at all until the first frame comes. This change introduces VideoSourceInterface::RequestRefreshFrame which results in a refresh frame being sent from complying sources. The method is used under zero-hertz mode from the VideoStreamEncoder when frames didn't arrive to it yet (with changes to the zero-hertz adapter). With this change, when the frame adapter has received at least one frame, it will conditionally repeat the last frame in response to the key frame request. go/rtc-0hz-present Bug: chromium:1255737 Change-Id: I6f97813b3a938747357d45e5dda54f759129b44d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/242361 Reviewed-by: Erik Språng Reviewed-by: Niels Moller Reviewed-by: Henrik Boström Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#35562} --- api/video/video_source_interface.h | 4 + pc/video_track_source_proxy.h | 1 + video/BUILD.gn | 5 +- video/frame_cadence_adapter.cc | 154 ++++++++++++++---- video/frame_cadence_adapter.h | 7 + video/frame_cadence_adapter_unittest.cc | 105 ++++++++++++ video/video_source_sink_controller.cc | 6 + video/video_source_sink_controller.h | 3 + .../video_source_sink_controller_unittest.cc | 15 ++ video/video_stream_encoder.cc | 8 + video/video_stream_encoder_unittest.cc | 54 ++++++ 11 files changed, 325 insertions(+), 37 deletions(-) diff --git a/api/video/video_source_interface.h b/api/video/video_source_interface.h index d66a235da0..5eb4ebfd75 100644 --- a/api/video/video_source_interface.h +++ b/api/video/video_source_interface.h @@ -97,6 +97,10 @@ class VideoSourceInterface { // RemoveSink must guarantee that at the time the method returns, // there is no current and no future calls to VideoSinkInterface::OnFrame. virtual void RemoveSink(VideoSinkInterface* sink) = 0; + + // Request underlying source to capture a new frame. + // TODO(crbug/1255737): make pure virtual once downstream projects adapt. + virtual void RequestRefreshFrame() {} }; } // namespace rtc diff --git a/pc/video_track_source_proxy.h b/pc/video_track_source_proxy.h index 6e71bb1615..1f6d976ba8 100644 --- a/pc/video_track_source_proxy.h +++ b/pc/video_track_source_proxy.h @@ -32,6 +32,7 @@ PROXY_SECONDARY_METHOD2(void, rtc::VideoSinkInterface*, const rtc::VideoSinkWants&) PROXY_SECONDARY_METHOD1(void, RemoveSink, rtc::VideoSinkInterface*) +PROXY_SECONDARY_METHOD0(void, RequestRefreshFrame) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) PROXY_CONSTMETHOD0(bool, SupportsEncodedOutput) diff --git a/video/BUILD.gn b/video/BUILD.gn index c1e7292675..b504d3dea6 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -280,7 +280,10 @@ rtc_library("frame_cadence_adapter") { "../system_wrappers:field_trial", "../system_wrappers:metrics", ] - absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/algorithm:container", + "//third_party/abseil-cpp/absl/base:core_headers", + ] } rtc_library("video_stream_encoder_impl") { diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index 18b99f5383..36ef740079 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -17,6 +17,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/base/attributes.h" #include "api/sequence_checker.h" #include "api/task_queue/task_queue_base.h" #include "api/units/time_delta.h" @@ -115,6 +116,9 @@ class ZeroHertzAdapterMode : public AdapterMode { absl::optional GetInputFrameRateFps() override; void UpdateFrameRate() override {} + // Returns true if a refresh frame should be requested from the video source. + ABSL_MUST_USE_RESULT bool ProcessKeyFrameRequest(); + private: // The tracking state of each spatial layer. Used for determining when to // stop repeating frames. @@ -123,20 +127,41 @@ class ZeroHertzAdapterMode : public AdapterMode { // convergence status of the layer. absl::optional quality_converged; }; + // The state of a scheduled repeat. + struct ScheduledRepeat { + ScheduledRepeat(Timestamp scheduled, bool idle) + : scheduled(scheduled), idle(idle) {} + // The instant when the repeat was scheduled. + Timestamp scheduled; + // True if the repeat was scheduled as an idle repeat (long), false + // otherwise. + bool idle; + }; + // Returns true if all spatial layers can be considered to be converged in + // terms of quality. + // Convergence means QP has dropped to a low-enough level to warrant ceasing + // to send identical frames at high frequency. + bool HasQualityConverged() const RTC_RUN_ON(sequence_checker_); // Processes incoming frames on a delayed cadence. void ProcessOnDelayedCadence() RTC_RUN_ON(sequence_checker_); // Schedules a later repeat with delay depending on state of layer trackers. - void ScheduleRepeat(int frame_id) RTC_RUN_ON(sequence_checker_); + // If true is passed in `idle_repeat`, the repeat is going to be + // kZeroHertzIdleRepeatRatePeriod. Otherwise it'll be the value of + // `frame_delay`. + void ScheduleRepeat(int frame_id, bool idle_repeat) + RTC_RUN_ON(sequence_checker_); // Repeats a frame in the abscence of incoming frames. Slows down when quality // convergence is attained, and stops the cadence terminally when new frames - // have arrived. `scheduled_delay` specifies the delay by which to modify the - // repeate frame's timestamps when it's sent. - void ProcessRepeatedFrameOnDelayedCadence(int frame_id, - TimeDelta scheduled_delay) + // have arrived. + void ProcessRepeatedFrameOnDelayedCadence(int frame_id) RTC_RUN_ON(sequence_checker_); // Sends a frame, updating the timestamp to the current time. - void SendFrameNow(const VideoFrame& frame); + void SendFrameNow(const VideoFrame& frame) const + RTC_RUN_ON(sequence_checker_); + // Returns the repeat duration depending on if it's an idle repeat or not. + TimeDelta RepeatDuration(bool idle_repeat) const + RTC_RUN_ON(sequence_checker_); TaskQueueBase* const queue_; Clock* const clock_; @@ -153,8 +178,9 @@ class ZeroHertzAdapterMode : public AdapterMode { // The current frame ID to use when starting to repeat frames. This is used // for cancelling deferred repeated frame processing happening. int current_frame_id_ RTC_GUARDED_BY(sequence_checker_) = 0; - // True when we are repeating frames. - bool is_repeating_ RTC_GUARDED_BY(sequence_checker_) = false; + // Has content when we are repeating frames. + absl::optional scheduled_repeat_ + RTC_GUARDED_BY(sequence_checker_); // Convergent state of each of the configured simulcast layers. std::vector layer_trackers_ RTC_GUARDED_BY(sequence_checker_); @@ -175,6 +201,7 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { void UpdateLayerQualityConvergence(int spatial_index, bool quality_converged) override; void UpdateLayerStatus(int spatial_index, bool enabled) override; + ABSL_MUST_USE_RESULT bool ProcessKeyFrameRequest() override; // VideoFrameSink overrides. void OnFrame(const VideoFrame& frame) override; @@ -293,7 +320,7 @@ void ZeroHertzAdapterMode::OnFrame(Timestamp post_time, } // Remove stored repeating frame if needed. - if (is_repeating_) { + if (scheduled_repeat_.has_value()) { RTC_DCHECK(queued_frames_.size() == 1); RTC_DLOG(LS_VERBOSE) << __func__ << " this " << this << " cancel repeat and restart with original"; @@ -303,7 +330,7 @@ void ZeroHertzAdapterMode::OnFrame(Timestamp post_time, // Store the frame in the queue and schedule deferred processing. queued_frames_.push_back(frame); current_frame_id_++; - is_repeating_ = false; + scheduled_repeat_ = absl::nullopt; queue_->PostDelayedTask(ToQueuedTask(safety_, [this] { RTC_DCHECK_RUN_ON(&sequence_checker_); @@ -317,6 +344,55 @@ absl::optional ZeroHertzAdapterMode::GetInputFrameRateFps() { return max_fps_; } +bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { + RTC_DCHECK_RUN_ON(&sequence_checker_); + + // If no frame was ever passed to us, request a refresh frame from the source. + if (current_frame_id_ == 0) { + RTC_LOG(LS_INFO) << __func__ << " this " << this + << " recommending requesting refresh frame due to no " + "frames received yet."; + return true; + } + + // If we're not repeating, or we're repeating with non-idle duration, we will + // very soon send out a frame and don't need a refresh frame. + if (!scheduled_repeat_.has_value() || !scheduled_repeat_->idle) { + RTC_LOG(LS_INFO) << __func__ << " this " << this + << " ignoring key frame request because of recently " + "incoming frame or short repeating."; + return false; + } + + // If the repeat is scheduled within a short (i.e. frame_delay_) interval, we + // will very soon send out a frame and don't need a refresh frame. + Timestamp now = clock_->CurrentTime(); + if (scheduled_repeat_->scheduled + RepeatDuration(/*idle_repeat=*/true) - + now <= + frame_delay_) { + RTC_LOG(LS_INFO) + << __func__ << " this " << this + << " ignoring key frame request because of soon happening idle repeat"; + return false; + } + + // Cancel the current repeat and reschedule a short repeat now. No need for a + // new refresh frame. + RTC_LOG(LS_INFO) << __func__ << " this " << this + << " scheduling a short repeat due to key frame request"; + ScheduleRepeat(++current_frame_id_, /*idle_repeat=*/false); + return false; +} + +// RTC_RUN_ON(&sequence_checker_) +bool ZeroHertzAdapterMode::HasQualityConverged() const { + const bool quality_converged = + absl::c_all_of(layer_trackers_, [](const SpatialLayerTracker& tracker) { + return tracker.quality_converged.value_or(true); + }); + return quality_converged; +} + // RTC_RUN_ON(&sequence_checker_) void ZeroHertzAdapterMode::ProcessOnDelayedCadence() { RTC_DCHECK(!queued_frames_.empty()); @@ -334,38 +410,26 @@ void ZeroHertzAdapterMode::ProcessOnDelayedCadence() { // There's only one frame to send. Schedule a repeat sequence, which is // cancelled by `current_frame_id_` getting incremented should new frames // arrive. - is_repeating_ = true; - ScheduleRepeat(current_frame_id_); + ScheduleRepeat(current_frame_id_, HasQualityConverged()); } // RTC_RUN_ON(&sequence_checker_) -void ZeroHertzAdapterMode::ScheduleRepeat(int frame_id) { +void ZeroHertzAdapterMode::ScheduleRepeat(int frame_id, bool idle_repeat) { RTC_DLOG(LS_VERBOSE) << __func__ << " this " << this << " frame_id " << frame_id; - // Determine if quality has converged. Adjust the time for the next repeat - // accordingly. - const bool quality_converged = - absl::c_all_of(layer_trackers_, [](const SpatialLayerTracker& tracker) { - return !tracker.quality_converged.has_value() || - tracker.quality_converged.value(); - }); - TimeDelta repeat_delay = - quality_converged - ? FrameCadenceAdapterInterface::kZeroHertzIdleRepeatRatePeriod - : frame_delay_; - queue_->PostDelayedTask(ToQueuedTask(safety_, - [this, frame_id, repeat_delay] { - RTC_DCHECK_RUN_ON(&sequence_checker_); - ProcessRepeatedFrameOnDelayedCadence( - frame_id, repeat_delay); - }), - repeat_delay.ms()); + scheduled_repeat_.emplace(clock_->CurrentTime(), idle_repeat); + TimeDelta repeat_delay = RepeatDuration(idle_repeat); + queue_->PostDelayedTask( + ToQueuedTask(safety_, + [this, frame_id] { + RTC_DCHECK_RUN_ON(&sequence_checker_); + ProcessRepeatedFrameOnDelayedCadence(frame_id); + }), + repeat_delay.ms()); } // RTC_RUN_ON(&sequence_checker_) -void ZeroHertzAdapterMode::ProcessRepeatedFrameOnDelayedCadence( - int frame_id, - TimeDelta scheduled_delay) { +void ZeroHertzAdapterMode::ProcessRepeatedFrameOnDelayedCadence(int frame_id) { RTC_DLOG(LS_VERBOSE) << __func__ << " this " << this << " frame_id " << frame_id; RTC_DCHECK(!queued_frames_.empty()); @@ -373,6 +437,7 @@ void ZeroHertzAdapterMode::ProcessRepeatedFrameOnDelayedCadence( // Cancel this invocation if new frames turned up. if (frame_id != current_frame_id_) return; + RTC_DCHECK(scheduled_repeat_.has_value()); VideoFrame& frame = queued_frames_.front(); @@ -385,6 +450,7 @@ void ZeroHertzAdapterMode::ProcessRepeatedFrameOnDelayedCadence( // scheduling this method. // NOTE: No need to update the RTP timestamp as the VideoStreamEncoder // overwrites it based on its chosen NTP timestamp source. + TimeDelta scheduled_delay = RepeatDuration(scheduled_repeat_->idle); if (frame.timestamp_us() > 0) frame.set_timestamp_us(frame.timestamp_us() + scheduled_delay.us()); if (frame.ntp_time_ms()) @@ -392,10 +458,11 @@ void ZeroHertzAdapterMode::ProcessRepeatedFrameOnDelayedCadence( SendFrameNow(frame); // Schedule another repeat. - ScheduleRepeat(frame_id); + ScheduleRepeat(frame_id, HasQualityConverged()); } -void ZeroHertzAdapterMode::SendFrameNow(const VideoFrame& frame) { +// RTC_RUN_ON(&sequence_checker_) +void ZeroHertzAdapterMode::SendFrameNow(const VideoFrame& frame) const { RTC_DLOG(LS_VERBOSE) << __func__ << " this " << this; // TODO(crbug.com/1255737): figure out if frames_scheduled_for_processing // makes sense to compute in this implementation. @@ -403,6 +470,13 @@ void ZeroHertzAdapterMode::SendFrameNow(const VideoFrame& frame) { /*frames_scheduled_for_processing=*/1, frame); } +// RTC_RUN_ON(&sequence_checker_) +TimeDelta ZeroHertzAdapterMode::RepeatDuration(bool idle_repeat) const { + return idle_repeat + ? FrameCadenceAdapterInterface::kZeroHertzIdleRepeatRatePeriod + : frame_delay_; +} + FrameCadenceAdapterImpl::FrameCadenceAdapterImpl(Clock* clock, TaskQueueBase* queue) : clock_(clock), @@ -453,6 +527,14 @@ void FrameCadenceAdapterImpl::UpdateLayerStatus(int spatial_index, zero_hertz_adapter_->UpdateLayerStatus(spatial_index, enabled); } +bool FrameCadenceAdapterImpl::ProcessKeyFrameRequest() { + RTC_DCHECK_RUN_ON(queue_); + if (zero_hertz_adapter_) + return zero_hertz_adapter_->ProcessKeyFrameRequest(); + // We depend on min_fps not being zero for passthrough. + return false; +} + void FrameCadenceAdapterImpl::OnFrame(const VideoFrame& frame) { // This method is called on the network thread under Chromium, or other // various contexts in test. diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index aeecea992d..c805400654 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -13,6 +13,7 @@ #include +#include "absl/base/attributes.h" #include "api/task_queue/task_queue_base.h" #include "api/units/time_delta.h" #include "api/video/video_frame.h" @@ -95,11 +96,17 @@ class FrameCadenceAdapterInterface virtual void UpdateFrameRate() = 0; // Updates quality convergence status for an enabled spatial layer. + // Convergence means QP has dropped to a low-enough level to warrant ceasing + // to send identical frames at high frequency. virtual void UpdateLayerQualityConvergence(int spatial_index, bool converged) = 0; // Updates spatial layer enabled status. virtual void UpdateLayerStatus(int spatial_index, bool enabled) = 0; + + // Returns true if a key frame request should cause generation of a new frame + // from the source. + virtual ABSL_MUST_USE_RESULT bool ProcessKeyFrameRequest() = 0; }; } // namespace webrtc diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 38827d6e64..8932c7f410 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -352,6 +352,111 @@ TEST(FrameCadenceAdapterTest, StopsRepeatingFramesDelayed) { time_controller.AdvanceTime(TimeDelta::Seconds(1)); } +TEST(FrameCadenceAdapterTest, RequestsRefreshFrameOnKeyFrameRequestWhenNew) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); + time_controller.AdvanceTime(TimeDelta::Zero()); + EXPECT_TRUE(adapter->ProcessKeyFrameRequest()); +} + +TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestShortlyAfterFrame) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Zero()); + EXPECT_FALSE(adapter->ProcessKeyFrameRequest()); +} + +TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestWhileShortRepeating) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{ + /*num_simulcast_layers=*/1}); + constexpr int kMaxFpsHz = 10; + constexpr TimeDelta kMinFrameDelay = TimeDelta::Millis(1000 / kMaxFpsHz); + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFpsHz}); + time_controller.AdvanceTime(TimeDelta::Zero()); + adapter->UpdateLayerStatus(0, true); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(2 * kMinFrameDelay); + EXPECT_FALSE(adapter->ProcessKeyFrameRequest()); + + // Expect repeating as ususal. + EXPECT_CALL(callback, OnFrame).Times(8); + time_controller.AdvanceTime(8 * kMinFrameDelay); +} + +TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestJustBeforeIdleRepeating) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + constexpr int kMaxFpsHz = 10; + constexpr TimeDelta kMinFrameDelay = TimeDelta::Millis(1000 / kMaxFpsHz); + constexpr TimeDelta kIdleFrameDelay = + FrameCadenceAdapterInterface::kZeroHertzIdleRepeatRatePeriod; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFpsHz}); + time_controller.AdvanceTime(TimeDelta::Zero()); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(kIdleFrameDelay); + + // We process the key frame request kMinFrameDelay before the first idle + // repeat should happen. The repeat should happen at T = kMinFrameDelay + + // kIdleFrameDelay as originally expected. + EXPECT_FALSE(adapter->ProcessKeyFrameRequest()); + EXPECT_CALL(callback, OnFrame); + time_controller.AdvanceTime(kMinFrameDelay); + EXPECT_CALL(callback, OnFrame); + time_controller.AdvanceTime(kIdleFrameDelay); +} + +TEST(FrameCadenceAdapterTest, + IgnoresKeyFrameRequestShortRepeatsBeforeIdleRepeat) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + constexpr int kMaxFpsHz = 10; + constexpr TimeDelta kMinFrameDelay = TimeDelta::Millis(1000 / kMaxFpsHz); + constexpr TimeDelta kIdleFrameDelay = + FrameCadenceAdapterInterface::kZeroHertzIdleRepeatRatePeriod; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFpsHz}); + time_controller.AdvanceTime(TimeDelta::Zero()); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(2 * kMinFrameDelay); + + // We process the key frame request 9 * kMinFrameDelay before the first idle + // repeat should happen. We should get a short repeat in kMinFrameDelay and an + // idle repeat after that. + EXPECT_FALSE(adapter->ProcessKeyFrameRequest()); + EXPECT_CALL(callback, OnFrame); + time_controller.AdvanceTime(kMinFrameDelay); + EXPECT_CALL(callback, OnFrame); + time_controller.AdvanceTime(kIdleFrameDelay); +} + class ZeroHertzLayerQualityConvergenceTest : public ::testing::Test { public: static constexpr TimeDelta kMinFrameDelay = TimeDelta::Millis(100); diff --git a/video/video_source_sink_controller.cc b/video/video_source_sink_controller.cc index 810a4ff1f5..cf3b649f70 100644 --- a/video/video_source_sink_controller.cc +++ b/video/video_source_sink_controller.cc @@ -75,6 +75,12 @@ bool VideoSourceSinkController::HasSource() const { return source_ != nullptr; } +void VideoSourceSinkController::RequestRefreshFrame() { + RTC_DCHECK_RUN_ON(&sequence_checker_); + if (source_) + source_->RequestRefreshFrame(); +} + void VideoSourceSinkController::PushSourceSinkSettings() { RTC_DCHECK_RUN_ON(&sequence_checker_); if (!source_) diff --git a/video/video_source_sink_controller.h b/video/video_source_sink_controller.h index d2e3267a89..e2a7eb7c78 100644 --- a/video/video_source_sink_controller.h +++ b/video/video_source_sink_controller.h @@ -38,6 +38,9 @@ class VideoSourceSinkController { void SetSource(rtc::VideoSourceInterface* source); bool HasSource() const; + // Requests a refresh frame from the current source, if set. + void RequestRefreshFrame(); + // Must be called in order for changes to settings to have an effect. This // allows you to modify multiple properties in a single push to the sink. void PushSourceSinkSettings(); diff --git a/video/video_source_sink_controller_unittest.cc b/video/video_source_sink_controller_unittest.cc index 93388637f3..2db7b5663f 100644 --- a/video/video_source_sink_controller_unittest.cc +++ b/video/video_source_sink_controller_unittest.cc @@ -48,6 +48,7 @@ class MockVideoSourceWithVideoFrame RemoveSink, (rtc::VideoSinkInterface*), (override)); + MOCK_METHOD(void, RequestRefreshFrame, (), (override)); }; } // namespace @@ -147,4 +148,18 @@ TEST(VideoSourceSinkControllerTest, controller.PushSourceSinkSettings(); } +TEST(VideoSourceSinkControllerTest, RequestsRefreshFrameWithSource) { + MockVideoSinkWithVideoFrame sink; + MockVideoSourceWithVideoFrame source; + VideoSourceSinkController controller(&sink, &source); + EXPECT_CALL(source, RequestRefreshFrame); + controller.RequestRefreshFrame(); +} + +TEST(VideoSourceSinkControllerTest, + RequestsRefreshFrameWithoutSourceDoesNotCrash) { + MockVideoSinkWithVideoFrame sink; + VideoSourceSinkController controller(&sink, nullptr); + controller.RequestRefreshFrame(); +} } // namespace webrtc diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index d581f0847c..572f82cb99 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1818,6 +1818,14 @@ void VideoStreamEncoder::SendKeyFrame() { if (!encoder_) return; // Shutting down. + if (frame_cadence_adapter_ && + frame_cadence_adapter_->ProcessKeyFrameRequest()) { + worker_queue_->PostTask(ToQueuedTask(task_safety_, [this] { + RTC_DCHECK_RUN_ON(worker_queue_); + video_source_sink_controller_.RequestRefreshFrame(); + })); + } + // TODO(webrtc:10615): Map keyframe request to spatial layer. std::fill(next_frame_types_.begin(), next_frame_types_.end(), VideoFrameType::kVideoFrameKey); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 0a8c2a6c9d..3d6956ebf7 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -752,6 +752,7 @@ class MockFrameCadenceAdapter : public FrameCadenceAdapterInterface { UpdateLayerStatus, (int spatial_index, bool enabled), (override)); + MOCK_METHOD(bool, ProcessKeyFrameRequest, (), (override)); }; class MockEncoderSelector @@ -768,6 +769,20 @@ class MockEncoderSelector MOCK_METHOD(absl::optional, OnEncoderBroken, (), (override)); }; +class MockVideoSourceInterface : public rtc::VideoSourceInterface { + public: + MOCK_METHOD(void, + AddOrUpdateSink, + (rtc::VideoSinkInterface*, + const rtc::VideoSinkWants&), + (override)); + MOCK_METHOD(void, + RemoveSink, + (rtc::VideoSinkInterface*), + (override)); + MOCK_METHOD(void, RequestRefreshFrame, (), (override)); +}; + } // namespace class VideoStreamEncoderTest : public ::testing::Test { @@ -8949,4 +8964,43 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) { Mock::VerifyAndClearExpectations(&factory.GetMockFakeEncoder()); } +TEST(VideoStreamEncoderFrameCadenceTest, + RequestsRefreshFramesWhenCadenceAdapterInstructs) { + auto adapter = std::make_unique(); + auto* adapter_ptr = adapter.get(); + MockVideoSourceInterface mock_source; + SimpleVideoStreamEncoderFactory factory; + FrameCadenceAdapterInterface::Callback* video_stream_encoder_callback = + nullptr; + EXPECT_CALL(*adapter_ptr, Initialize) + .WillOnce(Invoke([&video_stream_encoder_callback]( + FrameCadenceAdapterInterface::Callback* callback) { + video_stream_encoder_callback = callback; + })); + TaskQueueBase* encoder_queue = nullptr; + auto video_stream_encoder = + factory.Create(std::move(adapter), &encoder_queue); + video_stream_encoder->SetSource( + &mock_source, webrtc::DegradationPreference::MAINTAIN_FRAMERATE); + VideoEncoderConfig config; + config.content_type = VideoEncoderConfig::ContentType::kScreen; + test::FillEncoderConfiguration(kVideoCodecVP8, 1, &config); + video_stream_encoder->ConfigureEncoder(std::move(config), 0); + PassAFrame(encoder_queue, video_stream_encoder_callback, /*ntp_time_ms=*/2); + // Ensure the encoder is set up. + factory.DepleteTaskQueues(); + + EXPECT_CALL(*adapter_ptr, ProcessKeyFrameRequest).WillOnce(Return(true)); + EXPECT_CALL(mock_source, RequestRefreshFrame); + video_stream_encoder->SendKeyFrame(); + factory.DepleteTaskQueues(); + Mock::VerifyAndClearExpectations(adapter_ptr); + Mock::VerifyAndClearExpectations(&mock_source); + + EXPECT_CALL(*adapter_ptr, ProcessKeyFrameRequest).WillOnce(Return(false)); + EXPECT_CALL(mock_source, RequestRefreshFrame).Times(0); + video_stream_encoder->SendKeyFrame(); + factory.DepleteTaskQueues(); +} + } // namespace webrtc