From 84c016a02404dc0540ba3688d688d1ae950938ad Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 23 Nov 2023 13:41:44 +0100 Subject: [PATCH] FrameCadenceAdapter: use distinct signal for queue overload. The FrameCadenceAdapterInterface::Callback::OnFrame method in VideoStreamEncoder only changed frame handling on frames_scheduled_for_processing being 1. This CL changes the parameter to more explicitly signal queue overload via boolean parameter. Bug: None Change-Id: I1eb46b34fc4d748b7e2f1921642497c939adf197 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327761 Commit-Queue: Markus Handell Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#41226} --- video/frame_cadence_adapter.cc | 18 +++++++++-------- video/frame_cadence_adapter.h | 11 ++++------- video/frame_cadence_adapter_unittest.cc | 26 ++++++++++++------------- video/video_stream_encoder.cc | 4 ++-- video/video_stream_encoder.h | 7 +++---- video/video_stream_encoder_unittest.cc | 4 ++-- 6 files changed, 34 insertions(+), 36 deletions(-) diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index 721b401593..99d4001b72 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -50,7 +50,7 @@ class AdapterMode { // Called on the worker thread for every frame that enters. virtual void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) = 0; // Returns the currently estimated input framerate. @@ -71,10 +71,10 @@ class PassthroughAdapterMode : public AdapterMode { // Adapter overrides. void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) override { RTC_DCHECK_RUN_ON(&sequence_checker_); - callback_->OnFrame(post_time, frames_scheduled_for_processing, frame); + callback_->OnFrame(post_time, queue_overload, frame); } absl::optional GetInputFrameRateFps() override { @@ -119,7 +119,7 @@ class ZeroHertzAdapterMode : public AdapterMode { // Adapter overrides. void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) override; absl::optional GetInputFrameRateFps() override; void UpdateFrameRate() override {} @@ -356,7 +356,7 @@ void ZeroHertzAdapterMode::UpdateLayerStatus(size_t spatial_index, } void ZeroHertzAdapterMode::OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) { RTC_DCHECK_RUN_ON(&sequence_checker_); TRACE_EVENT0("webrtc", "ZeroHertzAdapterMode::OnFrame"); @@ -566,8 +566,10 @@ void ZeroHertzAdapterMode::SendFrameNow(Timestamp post_time, TimeDelta delay = (now - post_time); RTC_HISTOGRAM_COUNTS_10000("WebRTC.Screenshare.ZeroHz.DelayMs", delay.ms()); } - callback_->OnFrame(/*post_time=*/now, /*frames_scheduled_for_processing*/ 1, - frame); + // TODO(crbug.com/1255737): ensure queue_overload is computed from current + // conditions on the encoder queue. + callback_->OnFrame(/*post_time=*/now, + /*queue_overload=*/false, frame); } TimeDelta ZeroHertzAdapterMode::RepeatDuration(bool idle_repeat) const { @@ -689,7 +691,7 @@ void FrameCadenceAdapterImpl::OnFrame(const VideoFrame& frame) { const int frames_scheduled_for_processing = frames_scheduled_for_processing_.fetch_sub(1, std::memory_order_relaxed); - OnFrameOnMainQueue(post_time, frames_scheduled_for_processing, + OnFrameOnMainQueue(post_time, frames_scheduled_for_processing > 1, std::move(frame)); })); } diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index d0eab7e770..37c4a43c73 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -60,14 +60,11 @@ class FrameCadenceAdapterInterface // The |post_time| parameter indicates the current time sampled when // FrameCadenceAdapterInterface::OnFrame was called. // - // |frames_scheduled_for_processing| indicates how many frames that have - // been scheduled for processing. During sequential conditions where - // FrameCadenceAdapterInterface::OnFrame is invoked and subsequently ending - // up in this callback, this value will read 1. Otherwise if the - // |queue| gets stalled for some reason, the value will increase - // beyond 1. + // |queue_overload| is true if the frame cadence adapter notices it's + // not able to deliver the incoming |frame| to the |queue| in the expected + // time. virtual void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) = 0; // Called when the source has discarded a frame. diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 11be45070d..2fd40fe010 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -70,7 +70,7 @@ std::unique_ptr CreateAdapter( class MockCallback : public FrameCadenceAdapterInterface::Callback { public: - MOCK_METHOD(void, OnFrame, (Timestamp, int, const VideoFrame&), (override)); + MOCK_METHOD(void, OnFrame, (Timestamp, bool, const VideoFrame&), (override)); MOCK_METHOD(void, OnDiscardedFrame, (), (override)); MOCK_METHOD(void, RequestRefreshFrame, (), (override)); }; @@ -115,13 +115,13 @@ TEST(FrameCadenceAdapterTest, CountsOutstandingFramesToProcess) { MockCallback callback; auto adapter = CreateAdapter(no_field_trials, time_controller.GetClock()); adapter->Initialize(&callback); - EXPECT_CALL(callback, OnFrame(_, 2, _)).Times(1); - EXPECT_CALL(callback, OnFrame(_, 1, _)).Times(1); + EXPECT_CALL(callback, OnFrame(_, true, _)).Times(1); + EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1); auto frame = CreateFrame(); adapter->OnFrame(frame); adapter->OnFrame(frame); time_controller.AdvanceTime(TimeDelta::Zero()); - EXPECT_CALL(callback, OnFrame(_, 1, _)).Times(1); + EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1); adapter->OnFrame(frame); time_controller.AdvanceTime(TimeDelta::Zero()); } @@ -253,7 +253,7 @@ TEST(FrameCadenceAdapterTest, ForwardsFramesDelayed) { EXPECT_CALL(callback, OnFrame).Times(0); adapter->OnFrame(frame); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), @@ -332,7 +332,7 @@ TEST(FrameCadenceAdapterTest, RepeatsFramesDelayed) { adapter->OnFrame(frame); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), original_timestamp_us); EXPECT_EQ(frame.ntp_time_ms(), original_ntp_time.ToMs()); @@ -341,7 +341,7 @@ TEST(FrameCadenceAdapterTest, RepeatsFramesDelayed) { Mock::VerifyAndClearExpectations(&callback); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), original_timestamp_us + rtc::kNumMicrosecsPerSec); @@ -352,7 +352,7 @@ TEST(FrameCadenceAdapterTest, RepeatsFramesDelayed) { Mock::VerifyAndClearExpectations(&callback); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), original_timestamp_us + 2 * rtc::kNumMicrosecsPerSec); @@ -382,7 +382,7 @@ TEST(FrameCadenceAdapterTest, // Send one frame, expect a repeat. adapter->OnFrame(CreateFrame()); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), 0); EXPECT_EQ(frame.ntp_time_ms(), 0); @@ -390,7 +390,7 @@ TEST(FrameCadenceAdapterTest, time_controller.AdvanceTime(TimeDelta::Seconds(1)); Mock::VerifyAndClearExpectations(&callback); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) { EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime()); EXPECT_EQ(frame.timestamp_us(), 0); EXPECT_EQ(frame.ntp_time_ms(), 0); @@ -422,7 +422,7 @@ TEST(FrameCadenceAdapterTest, StopsRepeatingFramesDelayed) { // Send the new frame at 2.5s, which should appear after 3.5s. adapter->OnFrame(CreateFrameWithTimestamps(&time_controller)); EXPECT_CALL(callback, OnFrame) - .WillOnce(Invoke([&](Timestamp, int, const VideoFrame& frame) { + .WillOnce(Invoke([&](Timestamp, bool, const VideoFrame& frame) { EXPECT_EQ(frame.timestamp_us(), 5 * rtc::kNumMicrosecsPerSec / 2); EXPECT_EQ(frame.ntp_time_ms(), original_ntp_time.ToMs() + 5u * rtc::kNumMillisecsPerSec / 2); @@ -748,7 +748,7 @@ class ZeroHertzLayerQualityConvergenceTest : public ::testing::Test { std::initializer_list list) { Timestamp origin = time_controller_.GetClock()->CurrentTime(); for (auto delay : list) { - EXPECT_CALL(callback_, OnFrame(origin + delay, _, _)); + EXPECT_CALL(callback_, OnFrame(origin + delay, false, _)); time_controller_.AdvanceTime(origin + delay - time_controller_.GetClock()->CurrentTime()); } @@ -892,7 +892,7 @@ TEST(FrameCadenceAdapterRealTimeTest, TimestampsDoNotDrift) { constexpr int kSleepMs = rtc::kNumMillisecsPerSec / 2; EXPECT_CALL(callback, OnFrame) .WillRepeatedly( - Invoke([&](Timestamp, int, const VideoFrame& incoming_frame) { + Invoke([&](Timestamp, bool, const VideoFrame& incoming_frame) { ++frame_counter; // Avoid the first OnFrame and sleep on the second. if (frame_counter == 2) { diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index eeabe049ef..7144b57073 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1482,7 +1482,7 @@ void VideoStreamEncoder::OnEncoderSettingsChanged() { } void VideoStreamEncoder::OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& video_frame) { RTC_DCHECK_RUN_ON(&encoder_queue_); VideoFrame incoming_frame = video_frame; @@ -1541,7 +1541,7 @@ void VideoStreamEncoder::OnFrame(Timestamp post_time, bool cwnd_frame_drop = cwnd_frame_drop_interval_ && (cwnd_frame_counter_++ % cwnd_frame_drop_interval_.value() == 0); - if (frames_scheduled_for_processing == 1 && !cwnd_frame_drop) { + if (!queue_overload && !cwnd_frame_drop) { MaybeEncodeVideoFrame(incoming_frame, post_time.us()); } else { if (cwnd_frame_drop) { diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index c884c7303c..2276d7993f 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -160,10 +160,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, : video_stream_encoder_(video_stream_encoder) {} // FrameCadenceAdapterInterface::Callback overrides. void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& frame) override { - video_stream_encoder_.OnFrame(post_time, frames_scheduled_for_processing, - frame); + video_stream_encoder_.OnFrame(post_time, queue_overload, frame); } void OnDiscardedFrame() override { video_stream_encoder_.OnDiscardedFrame(); @@ -212,7 +211,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void ReconfigureEncoder() RTC_RUN_ON(&encoder_queue_); void OnEncoderSettingsChanged() RTC_RUN_ON(&encoder_queue_); void OnFrame(Timestamp post_time, - int frames_scheduled_for_processing, + bool queue_overload, const VideoFrame& video_frame); void OnDiscardedFrame(); void RequestRefreshFrame(); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index fa28368d68..06e772d9b2 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -136,8 +136,8 @@ void PassAFrame( FrameCadenceAdapterInterface::Callback* video_stream_encoder_callback, int64_t ntp_time_ms) { encoder_queue->PostTask([video_stream_encoder_callback, ntp_time_ms] { - video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms), 1, - CreateSimpleNV12Frame()); + video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms), + false, CreateSimpleNV12Frame()); }); }