From 78a57efb29162c94baac9833adc682a143089b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Terelius?= Date: Thu, 4 Jan 2024 19:44:07 +0000 Subject: [PATCH] Revert "FrameCadenceAdapter: align video encoding to metronome" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b39c2a8464c48306a495f14beccf431b91e51efd. Reason for revert: Breaks downstream build Original change's description: > FrameCadenceAdapter: align video encoding to metronome > > This CL aligns the video encoding tasks to metronome tick which > similar with the metronome decoding. > > Design doc: https://docs.google.com/document/d/18PvEgS-DehClK6twCSCATOlX-j9acmXd-3vjb0tR9-Y > > Bug: b/304158952 > Change-Id: I262bd4a5097fdaeed559b9d7391a059ae86e2d63 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327460 > Reviewed-by: Markus Handell > Reviewed-by: Harald Alvestrand > Reviewed-by: Henrik Boström > Commit-Queue: Zhaoliang Ma > Cr-Commit-Position: refs/heads/main@{#41469} Bug: b/304158952 Change-Id: I6f7a3d45cc24b63bc1fe92a93bf5c8d5058f32a8 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/333482 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Harald Alvestrand Commit-Queue: Björn Terelius Auto-Submit: Björn Terelius Cr-Commit-Position: refs/heads/main@{#41471} --- api/metronome/test/fake_metronome.cc | 4 - api/metronome/test/fake_metronome.h | 4 +- api/peer_connection_interface.h | 4 - call/call.cc | 9 +- call/call_config.h | 1 - pc/peer_connection_factory.cc | 5 +- pc/peer_connection_factory.h | 1 - video/BUILD.gn | 3 - video/frame_cadence_adapter.cc | 197 ++---------------------- video/frame_cadence_adapter.h | 3 - video/frame_cadence_adapter_unittest.cc | 85 +--------- video/video_send_stream.cc | 8 +- video/video_send_stream.h | 2 - video/video_stream_encoder_unittest.cc | 7 +- 14 files changed, 23 insertions(+), 310 deletions(-) diff --git a/api/metronome/test/fake_metronome.cc b/api/metronome/test/fake_metronome.cc index 37001fe4a1..025f7ce5a6 100644 --- a/api/metronome/test/fake_metronome.cc +++ b/api/metronome/test/fake_metronome.cc @@ -49,10 +49,6 @@ void ForcedTickMetronome::Tick() { FakeMetronome::FakeMetronome(TimeDelta tick_period) : tick_period_(tick_period) {} -void FakeMetronome::SetTickPeriod(TimeDelta tick_period) { - tick_period_ = tick_period; -} - void FakeMetronome::RequestCallOnNextTick( absl::AnyInvocable callback) { TaskQueueBase* current = TaskQueueBase::Current(); diff --git a/api/metronome/test/fake_metronome.h b/api/metronome/test/fake_metronome.h index e1aff4f4c0..73c938e9cd 100644 --- a/api/metronome/test/fake_metronome.h +++ b/api/metronome/test/fake_metronome.h @@ -55,14 +55,12 @@ class FakeMetronome : public Metronome { public: explicit FakeMetronome(TimeDelta tick_period); - void SetTickPeriod(TimeDelta tick_period); - // Metronome implementation. void RequestCallOnNextTick(absl::AnyInvocable callback) override; TimeDelta TickPeriod() const override; private: - TimeDelta tick_period_; + const TimeDelta tick_period_; std::vector> callbacks_; }; diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 3c225eb28a..c44e741a43 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1442,10 +1442,6 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final { transport_controller_send_factory; // Metronome used for decoding, must be called on the worker thread. std::unique_ptr decode_metronome; - // Metronome used for encoding, must be called on the worker thread. - // TODO(b/304158952): Consider merging into a single metronome for all codec - // usage. - std::unique_ptr encode_metronome; // Media specific dependencies. Unused when `media_factory == nullptr`. rtc::scoped_refptr adm; diff --git a/call/call.cc b/call/call.cc index 7db8cf7b3a..e35502f202 100644 --- a/call/call.cc +++ b/call/call.cc @@ -889,11 +889,10 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( VideoSendStream* send_stream = new VideoSendStream( &env_.clock(), num_cpu_cores_, &env_.task_queue_factory(), network_thread_, call_stats_->AsRtcpRttStats(), transport_send_.get(), - config_.encode_metronome, bitrate_allocator_.get(), - video_send_delay_stats_.get(), &env_.event_log(), std::move(config), - std::move(encoder_config), suspended_video_send_ssrcs_, - suspended_video_payload_states_, std::move(fec_controller), - env_.field_trials()); + bitrate_allocator_.get(), video_send_delay_stats_.get(), + &env_.event_log(), std::move(config), std::move(encoder_config), + suspended_video_send_ssrcs_, suspended_video_payload_states_, + std::move(fec_controller), env_.field_trials()); for (uint32_t ssrc : ssrcs) { RTC_DCHECK(video_send_ssrcs_.find(ssrc) == video_send_ssrcs_.end()); diff --git a/call/call_config.h b/call/call_config.h index 6fd9179a43..b99c644dae 100644 --- a/call/call_config.h +++ b/call/call_config.h @@ -69,7 +69,6 @@ struct CallConfig { rtp_transport_controller_send_factory = nullptr; Metronome* decode_metronome = nullptr; - Metronome* encode_metronome = nullptr; // The burst interval of the pacer, see TaskQueuePacedSender constructor. absl::optional pacer_burst_interval; diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index 6bf0ef944a..06ca790ea2 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -103,8 +103,7 @@ PeerConnectionFactory::PeerConnectionFactory( (dependencies->transport_controller_send_factory) ? std::move(dependencies->transport_controller_send_factory) : std::make_unique()), - decode_metronome_(std::move(dependencies->decode_metronome)), - encode_metronome_(std::move(dependencies->encode_metronome)) {} + decode_metronome_(std::move(dependencies->decode_metronome)) {} PeerConnectionFactory::PeerConnectionFactory( PeerConnectionFactoryDependencies dependencies) @@ -120,7 +119,6 @@ PeerConnectionFactory::~PeerConnectionFactory() { worker_thread()->BlockingCall([this] { RTC_DCHECK_RUN_ON(worker_thread()); decode_metronome_ = nullptr; - encode_metronome_ = nullptr; }); } @@ -346,7 +344,6 @@ std::unique_ptr PeerConnectionFactory::CreateCall_w( call_config.rtp_transport_controller_send_factory = transport_controller_send_factory_.get(); call_config.decode_metronome = decode_metronome_.get(); - call_config.encode_metronome = encode_metronome_.get(); call_config.pacer_burst_interval = configuration.pacer_burst_interval; return context_->call_factory()->CreateCall(call_config); } diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h index a8af9f5efa..66afebb37b 100644 --- a/pc/peer_connection_factory.h +++ b/pc/peer_connection_factory.h @@ -148,7 +148,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { const std::unique_ptr transport_controller_send_factory_; std::unique_ptr decode_metronome_ RTC_GUARDED_BY(worker_thread()); - std::unique_ptr encode_metronome_ RTC_GUARDED_BY(worker_thread()); }; } // namespace webrtc diff --git a/video/BUILD.gn b/video/BUILD.gn index 21e681cba0..79edee5aa6 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -92,7 +92,6 @@ rtc_library("video") { "../api/crypto:frame_decryptor_interface", "../api/crypto:options", "../api/environment", - "../api/metronome", "../api/task_queue", "../api/task_queue:pending_task_safety_flag", "../api/units:data_rate", @@ -231,7 +230,6 @@ rtc_library("frame_cadence_adapter") { deps = [ "../api:field_trials_view", "../api:sequence_checker", - "../api/metronome", "../api/task_queue", "../api/task_queue:pending_task_safety_flag", "../api/units:time_delta", @@ -255,7 +253,6 @@ rtc_library("frame_cadence_adapter") { absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:core_headers", - "//third_party/abseil-cpp/absl/cleanup:cleanup", ] } diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index 929b712597..8c881f1010 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -19,7 +19,6 @@ #include "absl/algorithm/container.h" #include "absl/base/attributes.h" -#include "absl/cleanup/cleanup.h" #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/task_queue/task_queue_base.h" @@ -235,81 +234,10 @@ class ZeroHertzAdapterMode : public AdapterMode { ScopedTaskSafety safety_; }; -// Implements a frame cadence adapter supporting VSync aligned encoding. -class VSyncEncodeAdapterMode : public AdapterMode { - public: - VSyncEncodeAdapterMode( - Clock* clock, - TaskQueueBase* queue, - rtc::scoped_refptr queue_safety_flag, - Metronome* metronome, - TaskQueueBase* worker_queue, - FrameCadenceAdapterInterface::Callback* callback) - : clock_(clock), - queue_(queue), - queue_safety_flag_(queue_safety_flag), - callback_(callback), - metronome_(metronome), - worker_queue_(worker_queue) { - queue_sequence_checker_.Detach(); - worker_sequence_checker_.Detach(); - } - - // Adapter overrides. - void OnFrame(Timestamp post_time, - bool queue_overload, - const VideoFrame& frame) override; - - absl::optional GetInputFrameRateFps() override { - RTC_DCHECK_RUN_ON(&queue_sequence_checker_); - return input_framerate_.Rate(clock_->TimeInMilliseconds()); - } - - void UpdateFrameRate() override { - RTC_DCHECK_RUN_ON(&queue_sequence_checker_); - input_framerate_.Update(1, clock_->TimeInMilliseconds()); - } - - void EncodeAllEnqueuedFrames(); - - private: - // Holds input frames coming from the client ready to be encoded. - struct InputFrameRef { - InputFrameRef(const VideoFrame& video_frame, Timestamp time_when_posted_us) - : time_when_posted_us(time_when_posted_us), - video_frame(std::move(video_frame)) {} - Timestamp time_when_posted_us; - const VideoFrame video_frame; - }; - - Clock* const clock_; - TaskQueueBase* queue_; - RTC_NO_UNIQUE_ADDRESS SequenceChecker queue_sequence_checker_; - rtc::scoped_refptr queue_safety_flag_; - // Input frame rate statistics for use when not in zero-hertz mode. - RateStatistics input_framerate_ RTC_GUARDED_BY(queue_sequence_checker_){ - FrameCadenceAdapterInterface::kFrameRateAveragingWindowSizeMs, 1000}; - FrameCadenceAdapterInterface::Callback* const callback_; - - Metronome* metronome_; - TaskQueueBase* const worker_queue_; - RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_sequence_checker_; - // `worker_safety_` protects tasks on the worker queue related to `metronome_` - // since metronome usage must happen on worker thread. - ScopedTaskSafetyDetached worker_safety_; - Timestamp expected_next_tick_ RTC_GUARDED_BY(worker_sequence_checker_) = - Timestamp::PlusInfinity(); - // Vector of input frames to be encoded. - std::vector input_queue_ - RTC_GUARDED_BY(worker_sequence_checker_); -}; - class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { public: FrameCadenceAdapterImpl(Clock* clock, TaskQueueBase* queue, - Metronome* metronome, - TaskQueueBase* worker_queue, const FieldTrialsView& field_trials); ~FrameCadenceAdapterImpl(); @@ -345,10 +273,6 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // - zero-hertz mode enabled bool IsZeroHertzScreenshareEnabled() const RTC_RUN_ON(queue_); - // Configures current adapter on non-ZeroHertz mode, called when Initialize or - // MaybeReconfigureAdapters. - void ConfigureCurrentAdapterWithoutZeroHertz(); - // Handles adapter creation on configuration changes. void MaybeReconfigureAdapters(bool was_zero_hertz_enabled) RTC_RUN_ON(queue_); @@ -359,21 +283,14 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // 0 Hz. const bool zero_hertz_screenshare_enabled_; - // The three possible modes we're under. + // The two possible modes we're under. absl::optional passthrough_adapter_; absl::optional zero_hertz_adapter_; - // The `vsync_encode_adapter_` must be destroyed on the worker queue since - // VSync metronome needs to happen on worker thread. - std::unique_ptr vsync_encode_adapter_; // If set, zero-hertz mode has been enabled. absl::optional zero_hertz_params_; // Cache for the current adapter mode. AdapterMode* current_adapter_mode_ = nullptr; - // VSync encoding is used when this valid. - Metronome* const metronome_; - TaskQueueBase* const worker_queue_; - // Timestamp for statistics reporting. absl::optional zero_hertz_adapter_created_timestamp_ RTC_GUARDED_BY(queue_); @@ -703,98 +620,23 @@ void ZeroHertzAdapterMode::MaybeStartRefreshFrameRequester() { } } -void VSyncEncodeAdapterMode::OnFrame(Timestamp post_time, - bool queue_overload, - const VideoFrame& frame) { - // We expect `metronome_` and `EncodeAllEnqueuedFrames()` runs on - // `worker_queue_`. - if (!worker_queue_->IsCurrent()) { - worker_queue_->PostTask(SafeTask( - worker_safety_.flag(), [this, post_time, queue_overload, frame] { - OnFrame(post_time, queue_overload, frame); - })); - return; - } - - RTC_DCHECK_RUN_ON(&worker_sequence_checker_); - TRACE_EVENT0("webrtc", "VSyncEncodeAdapterMode::OnFrame"); - - input_queue_.emplace_back(std::move(frame), post_time); - - // The `metronome_` tick period maybe throttled in some case, so here we only - // align encode task to VSync event when `metronome_` tick period is less - // than 34ms (30Hz). - static constexpr TimeDelta kMaxAllowedDelay = TimeDelta::Millis(34); - if (metronome_->TickPeriod() <= kMaxAllowedDelay) { - // The metronome is ticking frequently enough that it is worth the extra - // delay. - metronome_->RequestCallOnNextTick( - SafeTask(worker_safety_.flag(), [this] { EncodeAllEnqueuedFrames(); })); - } else { - // The metronome is ticking too infrequently, encode immediately. - EncodeAllEnqueuedFrames(); - } -} - -void VSyncEncodeAdapterMode::EncodeAllEnqueuedFrames() { - RTC_DCHECK_RUN_ON(&worker_sequence_checker_); - TRACE_EVENT0("webrtc", "VSyncEncodeAdapterMode::EncodeAllEnqueuedFrames"); - - // Local time in webrtc time base. - Timestamp post_time = clock_->CurrentTime(); - - for (auto& input : input_queue_) { - uint32_t vsync_encode_delay = (post_time - input.time_when_posted_us).ms(); - TRACE_EVENT1("webrtc", "FrameCadenceAdapterImpl::EncodeAllEnqueuedFrames", - "VSyncEncodeDelay", vsync_encode_delay); - - const VideoFrame frame = std::move(input.video_frame); - queue_->PostTask(SafeTask(queue_safety_flag_, [this, post_time, frame] { - RTC_DCHECK_RUN_ON(queue_); - - // TODO(b/304158952): Support more refined queue overload control. - callback_->OnFrame(post_time, /*queue_overload=*/false, frame); - })); - } - - input_queue_.clear(); -} - FrameCadenceAdapterImpl::FrameCadenceAdapterImpl( Clock* clock, TaskQueueBase* queue, - Metronome* metronome, - TaskQueueBase* worker_queue, const FieldTrialsView& field_trials) : clock_(clock), queue_(queue), zero_hertz_screenshare_enabled_( - !field_trials.IsDisabled("WebRTC-ZeroHertzScreenshare")), - metronome_(metronome), - worker_queue_(worker_queue) {} + !field_trials.IsDisabled("WebRTC-ZeroHertzScreenshare")) {} FrameCadenceAdapterImpl::~FrameCadenceAdapterImpl() { RTC_DLOG(LS_VERBOSE) << __func__ << " this " << this; - - // VSync adapter needs to be destroyed on worker queue when metronome is - // valid. - if (metronome_) { - absl::Cleanup cleanup = [adapter = std::move(vsync_encode_adapter_)] {}; - worker_queue_->PostTask([cleanup = std::move(cleanup)] {}); - } } void FrameCadenceAdapterImpl::Initialize(Callback* callback) { callback_ = callback; - // Use VSync encode mode if metronome is valid, otherwise passthrough mode - // would be used. - if (metronome_) { - vsync_encode_adapter_ = std::make_unique( - clock_, queue_, safety_.flag(), metronome_, worker_queue_, callback_); - } else { - passthrough_adapter_.emplace(clock_, callback); - } - ConfigureCurrentAdapterWithoutZeroHertz(); + passthrough_adapter_.emplace(clock_, callback); + current_adapter_mode_ = &passthrough_adapter_.value(); } void FrameCadenceAdapterImpl::SetZeroHertzModeEnabled( @@ -813,16 +655,9 @@ absl::optional FrameCadenceAdapterImpl::GetInputFrameRateFps() { void FrameCadenceAdapterImpl::UpdateFrameRate() { RTC_DCHECK_RUN_ON(queue_); // The frame rate need not be updated for the zero-hertz adapter. The - // vsync encode and passthrough adapter however uses it. Always pass frames - // into the vsync encode or passthrough to keep the estimation alive should - // there be an adapter switch. - if (metronome_) { - RTC_CHECK(vsync_encode_adapter_); - vsync_encode_adapter_->UpdateFrameRate(); - } else { - RTC_CHECK(passthrough_adapter_); - passthrough_adapter_->UpdateFrameRate(); - } + // passthrough adapter however uses it. Always pass frames into the + // passthrough to keep the estimation alive should there be an adapter switch. + passthrough_adapter_->UpdateFrameRate(); } void FrameCadenceAdapterImpl::UpdateLayerQualityConvergence( @@ -922,17 +757,6 @@ bool FrameCadenceAdapterImpl::IsZeroHertzScreenshareEnabled() const { zero_hertz_params_.has_value(); } -void FrameCadenceAdapterImpl::ConfigureCurrentAdapterWithoutZeroHertz() { - // Enable VSyncEncodeAdapterMode if metronome is valid. - if (metronome_) { - RTC_CHECK(vsync_encode_adapter_); - current_adapter_mode_ = vsync_encode_adapter_.get(); - } else { - RTC_CHECK(passthrough_adapter_); - current_adapter_mode_ = &passthrough_adapter_.value(); - } -} - void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( bool was_zero_hertz_enabled) { RTC_DCHECK_RUN_ON(queue_); @@ -956,7 +780,7 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( zero_hertz_adapter_ = absl::nullopt; RTC_LOG(LS_INFO) << "Zero hertz mode disabled."; } - ConfigureCurrentAdapterWithoutZeroHertz(); + current_adapter_mode_ = &passthrough_adapter_.value(); } } @@ -965,11 +789,8 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( std::unique_ptr FrameCadenceAdapterInterface::Create(Clock* clock, TaskQueueBase* queue, - Metronome* metronome, - TaskQueueBase* worker_queue, const FieldTrialsView& field_trials) { - return std::make_unique(clock, queue, metronome, - worker_queue, field_trials); + return std::make_unique(clock, queue, field_trials); } } // namespace webrtc diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index ec8e667b04..2b62bb26cd 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -15,7 +15,6 @@ #include "absl/base/attributes.h" #include "api/field_trials_view.h" -#include "api/metronome/metronome.h" #include "api/task_queue/task_queue_base.h" #include "api/units/time_delta.h" #include "api/video/video_frame.h" @@ -82,8 +81,6 @@ class FrameCadenceAdapterInterface static std::unique_ptr Create( Clock* clock, TaskQueueBase* queue, - Metronome* metronome, - TaskQueueBase* worker_queue, const FieldTrialsView& field_trials); // Call before using the rest of the API. diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 13fc1830d0..0fef2400f0 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -14,7 +14,6 @@ #include #include "absl/functional/any_invocable.h" -#include "api/metronome/test/fake_metronome.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" @@ -65,9 +64,8 @@ VideoFrame CreateFrameWithTimestamps( std::unique_ptr CreateAdapter( const FieldTrialsView& field_trials, Clock* clock) { - return FrameCadenceAdapterInterface::Create( - clock, TaskQueueBase::Current(), /*metronome=*/nullptr, - /*worker_queue=*/nullptr, field_trials); + return FrameCadenceAdapterInterface::Create(clock, TaskQueueBase::Current(), + field_trials); } class MockCallback : public FrameCadenceAdapterInterface::Callback { @@ -595,8 +593,7 @@ TEST(FrameCadenceAdapterTest, IgnoresDropInducedCallbacksPostDestruction) { auto queue = time_controller.GetTaskQueueFactory()->CreateTaskQueue( "queue", TaskQueueFactory::Priority::NORMAL); auto adapter = FrameCadenceAdapterInterface::Create( - time_controller.GetClock(), queue.get(), /*metronome=*/nullptr, - /*worker_queue=*/nullptr, enabler); + time_controller.GetClock(), queue.get(), enabler); queue->PostTask([&adapter, &callback] { adapter->Initialize(callback.get()); adapter->SetZeroHertzModeEnabled( @@ -612,82 +609,6 @@ TEST(FrameCadenceAdapterTest, IgnoresDropInducedCallbacksPostDestruction) { time_controller.AdvanceTime(3 * TimeDelta::Seconds(1) / kMaxFps); } -TEST(FrameCadenceAdapterTest, EncodeFramesAreAlignedWithMetronomeTick) { - ZeroHertzFieldTrialEnabler enabler; - GlobalSimulatedTimeController time_controller(Timestamp::Zero()); - // Here the metronome interval is 33ms, because the metronome is not - // infrequent then the encode tasks are aligned with the tick period. - static constexpr TimeDelta kTickPeriod = TimeDelta::Millis(33); - auto queue = time_controller.GetTaskQueueFactory()->CreateTaskQueue( - "queue", TaskQueueFactory::Priority::NORMAL); - auto worker_queue = time_controller.GetTaskQueueFactory()->CreateTaskQueue( - "work_queue", TaskQueueFactory::Priority::NORMAL); - static test::FakeMetronome metronome(kTickPeriod); - auto adapter = FrameCadenceAdapterInterface::Create( - time_controller.GetClock(), queue.get(), &metronome, worker_queue.get(), - enabler); - MockCallback callback; - adapter->Initialize(&callback); - auto frame = CreateFrame(); - - // `callback->OnFrame()` would not be called if only 32ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(0); - adapter->OnFrame(frame); - time_controller.AdvanceTime(TimeDelta::Millis(32)); - Mock::VerifyAndClearExpectations(&callback); - - // `callback->OnFrame()` should be called if 33ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1); - time_controller.AdvanceTime(TimeDelta::Millis(1)); - Mock::VerifyAndClearExpectations(&callback); - - // `callback->OnFrame()` would not be called if only 32ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(0); - // Send two frame before next tick. - adapter->OnFrame(frame); - adapter->OnFrame(frame); - time_controller.AdvanceTime(TimeDelta::Millis(32)); - Mock::VerifyAndClearExpectations(&callback); - - // `callback->OnFrame()` should be called if 33ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(2); - time_controller.AdvanceTime(TimeDelta::Millis(1)); - Mock::VerifyAndClearExpectations(&callback); - - // Change the metronome tick period to 67ms (15Hz). - metronome.SetTickPeriod(TimeDelta::Millis(67)); - // Expect the encode would happen immediately. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1); - adapter->OnFrame(frame); - time_controller.AdvanceTime(TimeDelta::Zero()); - Mock::VerifyAndClearExpectations(&callback); - - // Change the metronome tick period to 16ms (60Hz). - metronome.SetTickPeriod(TimeDelta::Millis(16)); - // Expect the encode would not happen if only 15ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(0); - adapter->OnFrame(frame); - time_controller.AdvanceTime(TimeDelta::Millis(15)); - Mock::VerifyAndClearExpectations(&callback); - // `callback->OnFrame()` should be called if 16ms went by after - // `adapter->OnFrame()`. - EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1); - time_controller.AdvanceTime(TimeDelta::Millis(1)); - Mock::VerifyAndClearExpectations(&callback); - - rtc::Event finalized; - queue->PostTask([&] { - adapter = nullptr; - finalized.Set(); - }); - finalized.Wait(rtc::Event::kForever); -} - class FrameCadenceAdapterSimulcastLayersParamTest : public ::testing::TestWithParam { public: diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index f336a61301..b99b08eefb 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -114,7 +114,6 @@ std::unique_ptr CreateVideoStreamEncoder( VideoStreamEncoder::BitrateAllocationCallbackType bitrate_allocation_callback_type, const FieldTrialsView& field_trials, - Metronome* metronome, webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { std::unique_ptr encoder_queue = task_queue_factory->CreateTaskQueue("EncoderQueue", @@ -123,9 +122,8 @@ std::unique_ptr CreateVideoStreamEncoder( return std::make_unique( clock, num_cpu_cores, stats_proxy, encoder_settings, std::make_unique(stats_proxy), - FrameCadenceAdapterInterface::Create( - clock, encoder_queue_ptr, metronome, - /*worker_queue=*/TaskQueueBase::Current(), field_trials), + FrameCadenceAdapterInterface::Create(clock, encoder_queue_ptr, + field_trials), std::move(encoder_queue), bitrate_allocation_callback_type, field_trials, encoder_selector); } @@ -141,7 +139,6 @@ VideoSendStream::VideoSendStream( TaskQueueBase* network_queue, RtcpRttStats* call_stats, RtpTransportControllerSendInterface* transport, - Metronome* metronome, BitrateAllocatorInterface* bitrate_allocator, SendDelayStats* send_delay_stats, RtcEventLog* event_log, @@ -164,7 +161,6 @@ VideoSendStream::VideoSendStream( config_.encoder_settings, GetBitrateAllocationCallbackType(config_, field_trials), field_trials, - metronome, config_.encoder_selector)), encoder_feedback_( clock, diff --git a/video/video_send_stream.h b/video/video_send_stream.h index ff1bf0c0a1..05970d619e 100644 --- a/video/video_send_stream.h +++ b/video/video_send_stream.h @@ -18,7 +18,6 @@ #include "api/fec_controller.h" #include "api/field_trials_view.h" -#include "api/metronome/metronome.h" #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "call/bitrate_allocator.h" @@ -63,7 +62,6 @@ class VideoSendStream : public webrtc::VideoSendStream { TaskQueueBase* network_queue, RtcpRttStats* call_stats, RtpTransportControllerSendInterface* transport, - Metronome* metronome, BitrateAllocatorInterface* bitrate_allocator, SendDelayStats* send_delay_stats, RtcEventLog* event_log, diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index d752e1b23b..6fa99081cd 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -875,9 +875,8 @@ class VideoStreamEncoderTest : public ::testing::Test { "EncoderQueue", TaskQueueFactory::Priority::NORMAL); TaskQueueBase* encoder_queue_ptr = encoder_queue.get(); std::unique_ptr cadence_adapter = - FrameCadenceAdapterInterface::Create( - time_controller_.GetClock(), encoder_queue_ptr, - /*metronome=*/nullptr, /*worker_queue=*/nullptr, field_trials_); + FrameCadenceAdapterInterface::Create(time_controller_.GetClock(), + encoder_queue_ptr, field_trials_); video_stream_encoder_ = std::make_unique( &time_controller_, std::move(cadence_adapter), std::move(encoder_queue), stats_proxy_.get(), video_send_config_.encoder_settings, @@ -9557,7 +9556,7 @@ TEST(VideoStreamEncoderFrameCadenceTest, "WebRTC-ZeroHertzScreenshare/Enabled/"); auto adapter = FrameCadenceAdapterInterface::Create( factory.GetTimeController()->GetClock(), encoder_queue.get(), - /*metronome=*/nullptr, /*worker_queue=*/nullptr, field_trials); + field_trials); FrameCadenceAdapterInterface* adapter_ptr = adapter.get(); MockVideoSourceInterface mock_source;