ZeroHertzAdapterMode: handle pending key frame requests.
The frame cadence adapter ignores key frame processing that happens
before the point where zero-hertz mode is activated, which leads to
no refresh frame requests if the key frame request comes too early.
Fix this to register a pending refresh frame request that gets
serviced when zero-hertz mode is activated. The CL changes the
FrameCadenceAdapterInterface::ProcessKeyFrameRequest from returning
whether to request a refresh frame into the frame cadence adapter
actively doing so itself via a new Callback::RequestRefreshFrame
API.
go/rtc-0hz-present
Bug: chromium:1255737
Change-Id: I53c2dbf6468e883eb2a2e81498e7134b1b35c336
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/242963
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35598}
diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc
index b8af3f0..254ccd6 100644
--- a/video/frame_cadence_adapter.cc
+++ b/video/frame_cadence_adapter.cc
@@ -119,8 +119,9 @@
absl::optional<uint32_t> 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 @@
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 @@
// `queue_`.
std::atomic<int> 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 @@
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 @@
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 @@
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 @@
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 @@
<< " 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 @@
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 @@
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 c805400..a881c61 100644
--- a/video/frame_cadence_adapter.h
+++ b/video/frame_cadence_adapter.h
@@ -68,6 +68,9 @@
// 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 @@
// 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 320019c..b2ddfd8 100644
--- a/video/frame_cadence_adapter_unittest.cc
+++ b/video/frame_cadence_adapter_unittest.cc
@@ -67,6 +67,7 @@
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
// 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 3baf10a..40c46ae 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1817,6 +1817,13 @@
}
}
+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 @@
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 cd181fc..da4747f 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -155,6 +155,9 @@
void OnDiscardedFrame() override {
video_stream_encoder_.OnDiscardedFrame();
}
+ void RequestRefreshFrame() override {
+ video_stream_encoder_.RequestRefreshFrame();
+ }
private:
VideoStreamEncoder& video_stream_encoder_;
@@ -199,6 +202,7 @@
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 c1d89d7..d2b2f3b 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 @@
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<NV12Buffer>(
+ /*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<NV12Buffer>(
- /*width=*/16, /*height=*/16))
- .build());
+ video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms),
+ 1, CreateSimpleNV12Frame());
}));
}
@@ -672,14 +676,9 @@
bitrate_allocator_factory_.get();
}
- std::unique_ptr<AdaptedVideoStreamEncoder> Create(
+ std::unique_ptr<AdaptedVideoStreamEncoder> CreateWithEncoderQueue(
std::unique_ptr<FrameCadenceAdapterInterface> 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<TaskQueueBase, TaskQueueDeleter> encoder_queue) {
auto result = std::make_unique<AdaptedVideoStreamEncoder>(
time_controller_.GetClock(),
/*number_of_cores=*/1,
@@ -692,9 +691,25 @@
return result;
}
+ std::unique_ptr<AdaptedVideoStreamEncoder> Create(
+ std::unique_ptr<FrameCadenceAdapterInterface> 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 @@
UpdateLayerStatus,
(int spatial_index, bool enabled),
(override));
- MOCK_METHOD(bool, ProcessKeyFrameRequest, (), (override));
+ MOCK_METHOD(void, ProcessKeyFrameRequest, (), (override));
};
class MockEncoderSelector
@@ -9002,17 +9017,54 @@
// 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
