diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index b8af3f0695..254ccd6e6f 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -119,8 +119,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(); + // Conditionally requests a refresh frame via + // Callback::RequestRefreshFrame. + void ProcessKeyFrameRequest(); private: // The tracking state of each spatial layer. Used for determining when to @@ -220,7 +221,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; + void ProcessKeyFrameRequest() override; // VideoFrameSink overrides. void OnFrame(const VideoFrame& frame) override; @@ -278,6 +279,9 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // `queue_`. std::atomic frames_scheduled_for_processing_{0}; + // Whether to ask for a refresh frame on activation of zero-hertz mode. + bool should_request_refresh_frame_ RTC_GUARDED_BY(queue_) = false; + ScopedTaskSafetyDetached safety_; }; @@ -368,7 +372,7 @@ absl::optional ZeroHertzAdapterMode::GetInputFrameRateFps() { return max_fps_; } -bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { +void ZeroHertzAdapterMode::ProcessKeyFrameRequest() { RTC_DCHECK_RUN_ON(&sequence_checker_); // If no frame was ever passed to us, request a refresh frame from the source. @@ -376,7 +380,8 @@ bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { RTC_LOG(LS_INFO) << __func__ << " this " << this << " requesting refresh frame due to no frames received yet."; - return true; + callback_->RequestRefreshFrame(); + return; } // The next frame encoded will be a key frame. Reset quality convergence so we @@ -390,7 +395,7 @@ bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { RTC_LOG(LS_INFO) << __func__ << " this " << this << " not requesting refresh frame because of recently " "incoming frame or short repeating."; - return false; + return; } // If the repeat is scheduled within a short (i.e. frame_delay_) interval, we @@ -402,7 +407,7 @@ bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { RTC_LOG(LS_INFO) << __func__ << " this " << this << " not requesting refresh frame because of soon " "happening idle repeat"; - return false; + return; } // Cancel the current repeat and reschedule a short repeat now. No need for a @@ -411,7 +416,7 @@ bool ZeroHertzAdapterMode::ProcessKeyFrameRequest() { << " not requesting refresh frame and scheduling a short " "repeat due to key frame request"; ScheduleRepeat(++current_frame_id_, /*idle_repeat=*/false); - return false; + return; } // RTC_RUN_ON(&sequence_checker_) @@ -590,12 +595,12 @@ void FrameCadenceAdapterImpl::UpdateLayerStatus(int spatial_index, zero_hertz_adapter_->UpdateLayerStatus(spatial_index, enabled); } -bool FrameCadenceAdapterImpl::ProcessKeyFrameRequest() { +void 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; + zero_hertz_adapter_->ProcessKeyFrameRequest(); + else + should_request_refresh_frame_ = true; } void FrameCadenceAdapterImpl::OnFrame(const VideoFrame& frame) { @@ -658,6 +663,12 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( zero_hertz_adapter_.emplace(queue_, clock_, callback_, source_constraints_->max_fps.value()); RTC_LOG(LS_INFO) << "Zero hertz mode activated."; + + if (should_request_refresh_frame_) { + // Ensure we get a first frame to work with. + should_request_refresh_frame_ = false; + callback_->RequestRefreshFrame(); + } } zero_hertz_adapter_->ReconfigureParameters(zero_hertz_params_.value()); current_adapter_mode_ = &zero_hertz_adapter_.value(); diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index c805400654..a881c61533 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -68,6 +68,9 @@ class FrameCadenceAdapterInterface // Called when the source has discarded a frame. virtual void OnDiscardedFrame() = 0; + + // Called when the adapter needs the source to send a refresh frame. + virtual void RequestRefreshFrame() = 0; }; // Factory function creating a production instance. Deletion of the returned @@ -104,9 +107,9 @@ class FrameCadenceAdapterInterface // 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; + // Conditionally requests a refresh frame via + // Callback::RequestRefreshFrame. + virtual void ProcessKeyFrameRequest() = 0; }; } // namespace webrtc diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 320019c24b..b2ddfd8264 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -67,6 +67,7 @@ class MockCallback : public FrameCadenceAdapterInterface::Callback { public: MOCK_METHOD(void, OnFrame, (Timestamp, int, const VideoFrame&), (override)); MOCK_METHOD(void, OnDiscardedFrame, (), (override)); + MOCK_METHOD(void, RequestRefreshFrame, (), (override)); }; class ZeroHertzFieldTrialDisabler : public test::ScopedFieldTrials { @@ -366,7 +367,8 @@ TEST(FrameCadenceAdapterTest, RequestsRefreshFrameOnKeyFrameRequestWhenNew) { FrameCadenceAdapterInterface::ZeroHertzModeParams{}); adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); time_controller.AdvanceTime(TimeDelta::Zero()); - EXPECT_TRUE(adapter->ProcessKeyFrameRequest()); + EXPECT_CALL(callback, RequestRefreshFrame); + adapter->ProcessKeyFrameRequest(); } TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestShortlyAfterFrame) { @@ -380,7 +382,8 @@ TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestShortlyAfterFrame) { adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); adapter->OnFrame(CreateFrame()); time_controller.AdvanceTime(TimeDelta::Zero()); - EXPECT_FALSE(adapter->ProcessKeyFrameRequest()); + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + adapter->ProcessKeyFrameRequest(); } class FrameCadenceAdapterSimulcastLayersParamTest @@ -431,7 +434,8 @@ TEST_P(FrameCadenceAdapterSimulcastLayersParamTest, // Since we're unconverged we assume the process continues. adapter_->OnFrame(CreateFrame()); time_controller_.AdvanceTime(2 * kMinFrameDelay); - EXPECT_FALSE(adapter_->ProcessKeyFrameRequest()); + EXPECT_CALL(callback_, RequestRefreshFrame).Times(0); + adapter_->ProcessKeyFrameRequest(); // Expect short repeating as ususal. EXPECT_CALL(callback_, OnFrame).Times(8); @@ -461,7 +465,8 @@ TEST_P(FrameCadenceAdapterSimulcastLayersParamTest, // We process the key frame request kMinFrameDelay before the first idle // repeat should happen. The resulting repeats should happen spaced by // kMinFrameDelay before we get new convergence info. - EXPECT_FALSE(adapter_->ProcessKeyFrameRequest()); + EXPECT_CALL(callback_, RequestRefreshFrame).Times(0); + adapter_->ProcessKeyFrameRequest(); EXPECT_CALL(callback_, OnFrame).Times(kMaxFpsHz); time_controller_.AdvanceTime(kMaxFpsHz * kMinFrameDelay); } @@ -488,7 +493,8 @@ TEST_P(FrameCadenceAdapterSimulcastLayersParamTest, // We process the key frame request (kMaxFpsHz - 1) * kMinFrameDelay before // the first idle repeat should happen. The resulting repeats should happen // spaced kMinFrameDelay before we get new convergence info. - EXPECT_FALSE(adapter_->ProcessKeyFrameRequest()); + EXPECT_CALL(callback_, RequestRefreshFrame).Times(0); + adapter_->ProcessKeyFrameRequest(); EXPECT_CALL(callback_, OnFrame).Times(kMaxFpsHz); time_controller_.AdvanceTime(kMaxFpsHz * kMinFrameDelay); } diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 3baf10a8ad..40c46ae118 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1817,6 +1817,13 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, } } +void VideoStreamEncoder::RequestRefreshFrame() { + worker_queue_->PostTask(ToQueuedTask(task_safety_, [this] { + RTC_DCHECK_RUN_ON(worker_queue_); + video_source_sink_controller_.RequestRefreshFrame(); + })); +} + void VideoStreamEncoder::SendKeyFrame() { if (!encoder_queue_.IsCurrent()) { encoder_queue_.PostTask([this] { SendKeyFrame(); }); @@ -1826,17 +1833,9 @@ void VideoStreamEncoder::SendKeyFrame() { TRACE_EVENT0("webrtc", "OnKeyFrameRequest"); RTC_DCHECK(!next_frame_types_.empty()); - if (frame_cadence_adapter_) { - if (frame_cadence_adapter_->ProcessKeyFrameRequest()) { - RTC_DLOG(LS_INFO) << __func__ << " RequestRefreshFrame()."; - worker_queue_->PostTask(ToQueuedTask(task_safety_, [this] { - RTC_DCHECK_RUN_ON(worker_queue_); - video_source_sink_controller_.RequestRefreshFrame(); - })); - } else { - RTC_DLOG(LS_INFO) << __func__ << " No RequestRefreshFrame()."; - } - } + if (frame_cadence_adapter_) + frame_cadence_adapter_->ProcessKeyFrameRequest(); + if (!encoder_) { RTC_DLOG(LS_INFO) << __func__ << " no encoder."; return; // Shutting down, or not configured yet. diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index cd181fc6fa..da4747f555 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -155,6 +155,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void OnDiscardedFrame() override { video_stream_encoder_.OnDiscardedFrame(); } + void RequestRefreshFrame() override { + video_stream_encoder_.RequestRefreshFrame(); + } private: VideoStreamEncoder& video_stream_encoder_; @@ -199,6 +202,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, int frames_scheduled_for_processing, const VideoFrame& video_frame); void OnDiscardedFrame(); + void RequestRefreshFrame(); void MaybeEncodeVideoFrame(const VideoFrame& frame, int64_t time_when_posted_in_ms); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index c1d89d73b2..d2b2f3bb35 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -19,6 +19,7 @@ #include "absl/memory/memory.h" #include "api/rtp_parameters.h" #include "api/task_queue/default_task_queue_factory.h" +#include "api/task_queue/task_queue_base.h" #include "api/task_queue/task_queue_factory.h" #include "api/test/mock_fec_controller_override.h" #include "api/test/mock_video_encoder.h" @@ -118,18 +119,21 @@ const uint8_t kCodedFrameVp8Qp25[] = { 0x02, 0x47, 0x08, 0x85, 0x85, 0x88, 0x85, 0x84, 0x88, 0x0c, 0x82, 0x00, 0x0c, 0x0d, 0x60, 0x00, 0xfe, 0xfc, 0x5c, 0xd0}; +VideoFrame CreateSimpleNV12Frame() { + return VideoFrame::Builder() + .set_video_frame_buffer(rtc::make_ref_counted( + /*width=*/16, /*height=*/16)) + .build(); +} + void PassAFrame( TaskQueueBase* encoder_queue, FrameCadenceAdapterInterface::Callback* video_stream_encoder_callback, int64_t ntp_time_ms) { encoder_queue->PostTask( ToQueuedTask([video_stream_encoder_callback, ntp_time_ms] { - video_stream_encoder_callback->OnFrame( - Timestamp::Millis(ntp_time_ms), 1, - VideoFrame::Builder() - .set_video_frame_buffer(rtc::make_ref_counted( - /*width=*/16, /*height=*/16)) - .build()); + video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms), + 1, CreateSimpleNV12Frame()); })); } @@ -672,14 +676,9 @@ class SimpleVideoStreamEncoderFactory { bitrate_allocator_factory_.get(); } - std::unique_ptr Create( + std::unique_ptr CreateWithEncoderQueue( std::unique_ptr zero_hertz_adapter, - TaskQueueBase** encoder_queue_ptr = nullptr) { - auto encoder_queue = - time_controller_.GetTaskQueueFactory()->CreateTaskQueue( - "EncoderQueue", TaskQueueFactory::Priority::NORMAL); - if (encoder_queue_ptr) - *encoder_queue_ptr = encoder_queue.get(); + std::unique_ptr encoder_queue) { auto result = std::make_unique( time_controller_.GetClock(), /*number_of_cores=*/1, @@ -692,9 +691,25 @@ class SimpleVideoStreamEncoderFactory { return result; } + std::unique_ptr Create( + std::unique_ptr zero_hertz_adapter, + TaskQueueBase** encoder_queue_ptr = nullptr) { + auto encoder_queue = + time_controller_.GetTaskQueueFactory()->CreateTaskQueue( + "EncoderQueue", TaskQueueFactory::Priority::NORMAL); + if (encoder_queue_ptr) + *encoder_queue_ptr = encoder_queue.get(); + return CreateWithEncoderQueue(std::move(zero_hertz_adapter), + std::move(encoder_queue)); + } + void DepleteTaskQueues() { time_controller_.AdvanceTime(TimeDelta::Zero()); } MockFakeEncoder& GetMockFakeEncoder() { return mock_fake_encoder_; } + GlobalSimulatedTimeController* GetTimeController() { + return &time_controller_; + } + private: class NullEncoderSink : public VideoStreamEncoderInterface::EncoderSink { public: @@ -750,7 +765,7 @@ class MockFrameCadenceAdapter : public FrameCadenceAdapterInterface { UpdateLayerStatus, (int spatial_index, bool enabled), (override)); - MOCK_METHOD(bool, ProcessKeyFrameRequest, (), (override)); + MOCK_METHOD(void, ProcessKeyFrameRequest, (), (override)); }; class MockEncoderSelector @@ -9002,17 +9017,54 @@ TEST(VideoStreamEncoderFrameCadenceTest, // Ensure the encoder is set up. factory.DepleteTaskQueues(); - EXPECT_CALL(*adapter_ptr, ProcessKeyFrameRequest).WillOnce(Return(true)); + EXPECT_CALL(*adapter_ptr, ProcessKeyFrameRequest) + .WillOnce(Invoke([video_stream_encoder_callback] { + video_stream_encoder_callback->RequestRefreshFrame(); + })); 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(*adapter_ptr, ProcessKeyFrameRequest); EXPECT_CALL(mock_source, RequestRefreshFrame).Times(0); video_stream_encoder->SendKeyFrame(); factory.DepleteTaskQueues(); } +TEST(VideoStreamEncoderFrameCadenceTest, + RequestsRefreshFrameForEarlyZeroHertzKeyFrameRequest) { + SimpleVideoStreamEncoderFactory factory; + auto encoder_queue = + factory.GetTimeController()->GetTaskQueueFactory()->CreateTaskQueue( + "EncoderQueue", TaskQueueFactory::Priority::NORMAL); + + // Enables zero-hertz mode. + test::ScopedFieldTrials field_trials("WebRTC-ZeroHertzScreenshare/Enabled/"); + auto adapter = FrameCadenceAdapterInterface::Create( + factory.GetTimeController()->GetClock(), encoder_queue.get()); + FrameCadenceAdapterInterface* adapter_ptr = adapter.get(); + + MockVideoSourceInterface mock_source; + auto video_stream_encoder = factory.CreateWithEncoderQueue( + std::move(adapter), std::move(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); + + // Eventually expect a refresh frame request when requesting a key frame + // before initializing zero-hertz mode. This can happen in reality because the + // threads invoking key frame requests and constraints setup aren't + // synchronized. + EXPECT_CALL(mock_source, RequestRefreshFrame); + video_stream_encoder->SendKeyFrame(); + adapter_ptr->OnConstraintsChanged(VideoTrackSourceConstraints{0, 30}); + factory.DepleteTaskQueues(); +} + } // namespace webrtc