From 41b4bf97c15b5dce6d5dd7dd0624c232889217ce Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 11 Apr 2024 18:10:31 +0200 Subject: [PATCH] Pass Environment instead of clock to Fake video encoders at construction Some of the fake encoders, FakeVp8Encoder in particular, reuse structures that in turn rely on field trials. Thus fake encoders also can benefit from Environment passed at construction. Bug: webrtc:15860 Change-Id: Ia1542b2663c75fd467e346aad9ead627ff9b3b0f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346780 Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Reviewed-by: Jeremy Leconte Cr-Commit-Position: refs/heads/main@{#42046} --- call/call_unittest.cc | 16 +++--- test/BUILD.gn | 1 + test/call_test.cc | 21 ++++---- test/fake_encoder.cc | 29 ++++++---- test/fake_encoder.h | 20 ++++--- test/fake_vp8_encoder.cc | 4 ++ test/fake_vp8_encoder.h | 4 +- test/fake_vp8_encoder_unittest.cc | 7 +-- test/peer_scenario/peer_scenario_client.cc | 2 +- test/scenario/video_stream.cc | 10 ++-- video/end_to_end_tests/stats_tests.cc | 8 +-- video/video_send_stream_tests.cc | 63 +++++++++++----------- 12 files changed, 106 insertions(+), 79 deletions(-) diff --git a/call/call_unittest.cc b/call/call_unittest.cc index 41394b9689..7fad99c4d6 100644 --- a/call/call_unittest.cc +++ b/call/call_unittest.cc @@ -46,6 +46,8 @@ using ::testing::Contains; using ::testing::MockFunction; using ::testing::NiceMock; using ::testing::StrictMock; +using ::webrtc::test::FakeEncoder; +using ::webrtc::test::FunctionVideoEncoderFactory; using ::webrtc::test::MockAudioDeviceModule; using ::webrtc::test::MockAudioMixer; using ::webrtc::test::MockAudioProcessing; @@ -385,9 +387,10 @@ TEST(CallTest, RecreatingAudioStreamWithSameSsrcReusesRtpState) { TEST(CallTest, AddAdaptationResourceAfterCreatingVideoSendStream) { CallHelper call(true); // Create a VideoSendStream. - test::FunctionVideoEncoderFactory fake_encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + FunctionVideoEncoderFactory fake_encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); auto bitrate_allocator_factory = CreateBuiltinVideoBitrateAllocatorFactory(); MockTransport send_transport; VideoSendStream::Config config(&send_transport); @@ -450,9 +453,10 @@ TEST(CallTest, AddAdaptationResourceBeforeCreatingVideoSendStream) { auto fake_resource = FakeResource::Create("FakeResource"); call->AddAdaptationResource(fake_resource); // Create a VideoSendStream. - test::FunctionVideoEncoderFactory fake_encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + FunctionVideoEncoderFactory fake_encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); auto bitrate_allocator_factory = CreateBuiltinVideoBitrateAllocatorFactory(); MockTransport send_transport; VideoSendStream::Config config(&send_transport); diff --git a/test/BUILD.gn b/test/BUILD.gn index e786592e4b..26ada7fcc3 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1008,6 +1008,7 @@ rtc_library("fake_video_codecs") { "../api:fec_controller_api", "../api:scoped_refptr", "../api:sequence_checker", + "../api/environment", "../api/task_queue", "../api/video:encoded_image", "../api/video:video_bitrate_allocation", diff --git a/test/call_test.cc b/test/call_test.cc index f26a44a341..b3a7e73e82 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -45,16 +45,17 @@ CallTest::CallTest() audio_send_config_(/*send_transport=*/nullptr), audio_send_stream_(nullptr), frame_generator_capturer_(nullptr), - fake_encoder_factory_([this]() { - std::unique_ptr fake_encoder; - if (video_encoder_configs_[0].codec_type == kVideoCodecVP8) { - fake_encoder = std::make_unique(&env_.clock()); - } else { - fake_encoder = std::make_unique(&env_.clock()); - } - fake_encoder->SetMaxBitrate(fake_encoder_max_bitrate_); - return fake_encoder; - }), + fake_encoder_factory_( + [this](const Environment& env, const SdpVideoFormat& format) { + std::unique_ptr fake_encoder; + if (video_encoder_configs_[0].codec_type == kVideoCodecVP8) { + fake_encoder = std::make_unique(env); + } else { + fake_encoder = std::make_unique(env); + } + fake_encoder->SetMaxBitrate(fake_encoder_max_bitrate_); + return fake_encoder; + }), fake_decoder_factory_([]() { return std::make_unique(); }), bitrate_allocator_factory_(CreateBuiltinVideoBitrateAllocatorFactory()), num_video_streams_(1), diff --git a/test/fake_encoder.cc b/test/fake_encoder.cc index 195ff44026..437b12732e 100644 --- a/test/fake_encoder.cc +++ b/test/fake_encoder.cc @@ -17,6 +17,8 @@ #include #include +#include "api/environment/environment.h" +#include "api/task_queue/task_queue_factory.h" #include "api/video/video_content_type.h" #include "modules/video_coding/codecs/h264/include/h264_globals.h" #include "modules/video_coding/include/video_codec_interface.h" @@ -47,8 +49,14 @@ void WriteCounter(unsigned char* payload, uint32_t counter) { } // namespace -FakeEncoder::FakeEncoder(Clock* clock) - : clock_(clock), +FakeEncoder::FakeEncoder(const Environment& env) + : FakeEncoder(env, &env.clock()) {} + +FakeEncoder::FakeEncoder(Clock* clock) : FakeEncoder(absl::nullopt, clock) {} + +FakeEncoder::FakeEncoder(absl::optional env, Clock* clock) + : env_(env), + clock_(clock), num_initializations_(0), callback_(nullptr), max_target_bitrate_kbps_(-1), @@ -309,6 +317,9 @@ const VideoCodec& FakeEncoder::config() const { return config_; } +FakeH264Encoder::FakeH264Encoder(const Environment& env) + : FakeEncoder(env), idr_counter_(0) {} + FakeH264Encoder::FakeH264Encoder(Clock* clock) : FakeEncoder(clock), idr_counter_(0) {} @@ -361,8 +372,8 @@ CodecSpecificInfo FakeH264Encoder::EncodeHook( return codec_specific; } -DelayedEncoder::DelayedEncoder(Clock* clock, int delay_ms) - : test::FakeEncoder(clock), delay_ms_(delay_ms) { +DelayedEncoder::DelayedEncoder(const Environment& env, int delay_ms) + : test::FakeEncoder(env), delay_ms_(delay_ms) { // The encoder could be created on a different thread than // it is being used on. sequence_checker_.Detach(); @@ -383,10 +394,8 @@ int32_t DelayedEncoder::Encode(const VideoFrame& input_image, } MultithreadedFakeH264Encoder::MultithreadedFakeH264Encoder( - Clock* clock, - TaskQueueFactory* task_queue_factory) - : test::FakeH264Encoder(clock), - task_queue_factory_(task_queue_factory), + const Environment& env) + : test::FakeH264Encoder(env), current_queue_(0), queue1_(nullptr), queue2_(nullptr) { @@ -399,9 +408,9 @@ int32_t MultithreadedFakeH264Encoder::InitEncode(const VideoCodec* config, const Settings& settings) { RTC_DCHECK_RUN_ON(&sequence_checker_); - queue1_ = task_queue_factory_->CreateTaskQueue( + queue1_ = env_->task_queue_factory().CreateTaskQueue( "Queue 1", TaskQueueFactory::Priority::NORMAL); - queue2_ = task_queue_factory_->CreateTaskQueue( + queue2_ = env_->task_queue_factory().CreateTaskQueue( "Queue 2", TaskQueueFactory::Priority::NORMAL); return FakeH264Encoder::InitEncode(config, settings); diff --git a/test/fake_encoder.h b/test/fake_encoder.h index b804f2ce35..9c1e87ffad 100644 --- a/test/fake_encoder.h +++ b/test/fake_encoder.h @@ -18,9 +18,10 @@ #include #include "absl/strings/string_view.h" +#include "absl/types/optional.h" +#include "api/environment/environment.h" #include "api/fec_controller_override.h" #include "api/sequence_checker.h" -#include "api/task_queue/task_queue_factory.h" #include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_frame.h" @@ -36,6 +37,9 @@ namespace test { class FakeEncoder : public VideoEncoder { public: + explicit FakeEncoder(const Environment& env_); + // TODO: bugs.webrtc.org/15860 - Delete constructor taking just `Clock` when + // users are migrated to pass full `Environment` explicit FakeEncoder(Clock* clock); virtual ~FakeEncoder() = default; @@ -82,6 +86,8 @@ class FakeEncoder : public VideoEncoder { std::vector layers; }; + FakeEncoder(absl::optional env, Clock* clock); + FrameInfo NextFrame(const std::vector* frame_types, bool keyframe, uint8_t num_simulcast_streams, @@ -99,6 +105,9 @@ class FakeEncoder : public VideoEncoder { void SetRatesLocked(const RateControlParameters& parameters) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + // TODO: bugs.webrtc.org/15860 - Remove constructor that takes just the clock + // and make env_ non-optional. + const absl::optional env_; FrameInfo last_frame_info_ RTC_GUARDED_BY(mutex_); Clock* const clock_; @@ -121,7 +130,8 @@ class FakeEncoder : public VideoEncoder { class FakeH264Encoder : public FakeEncoder { public: - explicit FakeH264Encoder(Clock* clock); + explicit FakeH264Encoder(const Environment& env); + [[deprecated]] explicit FakeH264Encoder(Clock* clock); virtual ~FakeH264Encoder() = default; private: @@ -135,7 +145,7 @@ class FakeH264Encoder : public FakeEncoder { class DelayedEncoder : public test::FakeEncoder { public: - DelayedEncoder(Clock* clock, int delay_ms); + DelayedEncoder(const Environment& env, int delay_ms); virtual ~DelayedEncoder() = default; void SetDelay(int delay_ms); @@ -153,8 +163,7 @@ class DelayedEncoder : public test::FakeEncoder { // as it is called from the task queue in VideoStreamEncoder. class MultithreadedFakeH264Encoder : public test::FakeH264Encoder { public: - MultithreadedFakeH264Encoder(Clock* clock, - TaskQueueFactory* task_queue_factory); + explicit MultithreadedFakeH264Encoder(const Environment& env); virtual ~MultithreadedFakeH264Encoder() = default; int32_t InitEncode(const VideoCodec* config, @@ -169,7 +178,6 @@ class MultithreadedFakeH264Encoder : public test::FakeH264Encoder { int32_t Release() override; protected: - TaskQueueFactory* const task_queue_factory_; int current_queue_ RTC_GUARDED_BY(sequence_checker_); std::unique_ptr queue1_ RTC_GUARDED_BY(sequence_checker_); diff --git a/test/fake_vp8_encoder.cc b/test/fake_vp8_encoder.cc index c16d7c351e..aeb36c9ab0 100644 --- a/test/fake_vp8_encoder.cc +++ b/test/fake_vp8_encoder.cc @@ -49,6 +49,10 @@ FakeVp8Encoder::FakeVp8Encoder(Clock* clock) : FakeEncoder(clock) { sequence_checker_.Detach(); } +FakeVp8Encoder::FakeVp8Encoder(const Environment& env) : FakeEncoder(env) { + sequence_checker_.Detach(); +} + int32_t FakeVp8Encoder::InitEncode(const VideoCodec* config, const Settings& settings) { RTC_DCHECK_RUN_ON(&sequence_checker_); diff --git a/test/fake_vp8_encoder.h b/test/fake_vp8_encoder.h index 6aaf547379..2967db01d8 100644 --- a/test/fake_vp8_encoder.h +++ b/test/fake_vp8_encoder.h @@ -16,6 +16,7 @@ #include +#include "api/environment/environment.h" #include "api/fec_controller_override.h" #include "api/sequence_checker.h" #include "api/video/encoded_image.h" @@ -33,7 +34,8 @@ namespace test { class FakeVp8Encoder : public FakeEncoder { public: - explicit FakeVp8Encoder(Clock* clock); + explicit FakeVp8Encoder(const Environment& env); + [[deprecated]] explicit FakeVp8Encoder(Clock* clock); virtual ~FakeVp8Encoder() = default; int32_t InitEncode(const VideoCodec* config, diff --git a/test/fake_vp8_encoder_unittest.cc b/test/fake_vp8_encoder_unittest.cc index c068be0f43..08982438af 100644 --- a/test/fake_vp8_encoder_unittest.cc +++ b/test/fake_vp8_encoder_unittest.cc @@ -27,9 +27,10 @@ namespace { std::unique_ptr CreateSpecificSimulcastTestFixture() { std::unique_ptr encoder_factory = - std::make_unique([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + std::make_unique( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); std::unique_ptr decoder_factory = std::make_unique( []() { return std::make_unique(); }); diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc index 9d77624afe..4cb45a325f 100644 --- a/test/peer_scenario/peer_scenario_client.cc +++ b/test/peer_scenario/peer_scenario_client.cc @@ -184,7 +184,7 @@ class FakeVideoEncoderFactory : public VideoEncoderFactory { std::unique_ptr Create(const Environment& env, const SdpVideoFormat& format) override { RTC_CHECK_EQ(format.name, "VP8"); - return std::make_unique(&env.clock()); + return std::make_unique(env); } }; diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc index 8e55fa65ef..188e89fd8c 100644 --- a/test/scenario/video_stream.cc +++ b/test/scenario/video_stream.cc @@ -378,16 +378,14 @@ SendVideoStream::SendVideoStream(CallClient* sender, using Codec = VideoStreamConfig::Encoder::Codec; switch (config.encoder.implementation) { case Encoder::Implementation::kFake: - encoder_factory_ = - std::make_unique([this]() { + encoder_factory_ = std::make_unique( + [this](const Environment& env, const SdpVideoFormat& format) { MutexLock lock(&mutex_); std::unique_ptr encoder; if (config_.encoder.codec == Codec::kVideoCodecVP8) { - encoder = std::make_unique( - &sender_->env_.clock()); + encoder = std::make_unique(env); } else if (config_.encoder.codec == Codec::kVideoCodecGeneric) { - encoder = - std::make_unique(&sender_->env_.clock()); + encoder = std::make_unique(env); } else { RTC_DCHECK_NOTREACHED(); } diff --git a/video/end_to_end_tests/stats_tests.cc b/video/end_to_end_tests/stats_tests.cc index d6820eeac2..58f4b30958 100644 --- a/video/end_to_end_tests/stats_tests.cc +++ b/video/end_to_end_tests/stats_tests.cc @@ -53,10 +53,10 @@ TEST_F(StatsEndToEndTest, GetStats) { public: StatsObserver() : EndToEndTest(test::VideoTestConstants::kLongTimeout), - encoder_factory_([]() { - return std::make_unique( - Clock::GetRealTimeClock(), 10); - }) {} + encoder_factory_( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env, 10); + }) {} private: Action OnSendRtp(rtc::ArrayView packet) override { diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index bc82423aac..e56161d583 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -14,7 +14,6 @@ #include "absl/algorithm/container.h" #include "absl/strings/match.h" #include "api/sequence_checker.h" -#include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/task_queue_base.h" #include "api/test/metrics/global_metrics_logger_and_exporter.h" #include "api/test/metrics/metric.h" @@ -278,9 +277,9 @@ TEST_F(VideoSendStreamTest, SupportsTransmissionTimeOffset) { public: TransmissionTimeOffsetObserver() : SendTest(test::VideoTestConstants::kDefaultTimeout), - encoder_factory_([]() { - return std::make_unique( - Clock::GetRealTimeClock(), kEncodeDelayMs); + encoder_factory_([](const Environment& env, + const SdpVideoFormat& format) { + return std::make_unique(env, kEncodeDelayMs); }) { extensions_.Register(kTimestampOffsetExtensionId); } @@ -326,10 +325,10 @@ TEST_F(VideoSendStreamTest, SupportsTransportWideSequenceNumbers) { public: TransportWideSequenceNumberObserver() : SendTest(test::VideoTestConstants::kDefaultTimeout), - encoder_factory_([]() { - return std::make_unique( - Clock::GetRealTimeClock()); - }) { + encoder_factory_( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }) { extensions_.Register(kExtensionId); } @@ -709,9 +708,10 @@ TEST_F(VideoSendStreamWithoutUlpfecTest, NoUlpfecIfDisabledThroughFieldTrial) { // bandwidth since the receiver has to wait for FEC retransmissions to determine // that the received state is actually decodable. TEST_F(VideoSendStreamTest, DoesNotUtilizeUlpfecForH264WithNackEnabled) { - test::FunctionVideoEncoderFactory encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); UlpfecObserver test(false, true, false, false, "H264", &encoder_factory, kReducedTimeout); RunBaseTest(&test); @@ -719,9 +719,10 @@ TEST_F(VideoSendStreamTest, DoesNotUtilizeUlpfecForH264WithNackEnabled) { // Without retransmissions FEC for H264 is fine. TEST_F(VideoSendStreamTest, DoesUtilizeUlpfecForH264WithoutNackEnabled) { - test::FunctionVideoEncoderFactory encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); UlpfecObserver test(false, false, true, true, "H264", &encoder_factory); RunBaseTest(&test); } @@ -749,12 +750,10 @@ TEST_F(VideoSendStreamTest, DoesUtilizeUlpfecForVp9WithNackEnabled) { #endif // defined(RTC_ENABLE_VP9) TEST_F(VideoSendStreamTest, SupportsUlpfecWithMultithreadedH264) { - std::unique_ptr task_queue_factory = - CreateDefaultTaskQueueFactory(); - test::FunctionVideoEncoderFactory encoder_factory([&]() { - return std::make_unique( - Clock::GetRealTimeClock(), task_queue_factory.get()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [&](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); UlpfecObserver test(false, false, true, true, "H264", &encoder_factory); RunBaseTest(&test); } @@ -930,28 +929,28 @@ TEST_F(VideoSendStreamTest, SupportsFlexfecWithNackVp9) { #endif // defined(RTC_ENABLE_VP9) TEST_F(VideoSendStreamTest, SupportsFlexfecH264) { - test::FunctionVideoEncoderFactory encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); FlexfecObserver test(false, false, "H264", &encoder_factory, 1); RunBaseTest(&test); } TEST_F(VideoSendStreamTest, SupportsFlexfecWithNackH264) { - test::FunctionVideoEncoderFactory encoder_factory([]() { - return std::make_unique(Clock::GetRealTimeClock()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); FlexfecObserver test(false, true, "H264", &encoder_factory, 1); RunBaseTest(&test); } TEST_F(VideoSendStreamTest, SupportsFlexfecWithMultithreadedH264) { - std::unique_ptr task_queue_factory = - CreateDefaultTaskQueueFactory(); - test::FunctionVideoEncoderFactory encoder_factory([&]() { - return std::make_unique( - Clock::GetRealTimeClock(), task_queue_factory.get()); - }); + test::FunctionVideoEncoderFactory encoder_factory( + [&](const Environment& env, const SdpVideoFormat& format) { + return std::make_unique(env); + }); FlexfecObserver test(false, false, "H264", &encoder_factory, 1); RunBaseTest(&test);