From 73c8101fcb340163192e44082074b87af9bf67e5 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 12 Apr 2024 15:02:06 +0200 Subject: [PATCH] Require webrtc::Environment to create VP8 encoder Bug: webrtc:15860 Change-Id: I5d2632127e8a701e8cb0fbf3f271f80e3102dbe0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346860 Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#42067} --- modules/video_coding/BUILD.gn | 1 - modules/video_coding/codecs/vp8/include/vp8.h | 9 ---- .../codecs/vp8/libvpx_vp8_encoder.cc | 43 ++++++++----------- .../codecs/vp8/libvpx_vp8_encoder.h | 14 +++--- .../codecs/vp8/libvpx_vp8_simulcast_test.cc | 4 +- .../codecs/vp8/test/vp8_impl_unittest.cc | 41 ++++++++---------- 6 files changed, 46 insertions(+), 66 deletions(-) diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 6e15396372..aee6d608c9 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -587,7 +587,6 @@ rtc_library("webrtc_vp8") { "../../rtc_base/experiments:encoder_info_settings", "../../rtc_base/experiments:field_trial_parser", "../../rtc_base/experiments:rate_control_settings", - "../../system_wrappers:field_trial", "../../system_wrappers:metrics", "svc:scalability_mode_util", "//third_party/libyuv", diff --git a/modules/video_coding/codecs/vp8/include/vp8.h b/modules/video_coding/codecs/vp8/include/vp8.h index e86d758b36..8dd7c57a0c 100644 --- a/modules/video_coding/codecs/vp8/include/vp8.h +++ b/modules/video_coding/codecs/vp8/include/vp8.h @@ -30,15 +30,6 @@ absl::Nonnull> CreateVp8Encoder( const Environment& env, Vp8EncoderSettings settings = {}); -// Deprecated, use CreateVp8Encoder above, bugs.webrtc.org/15860 -class VP8Encoder { - public: - using Settings = Vp8EncoderSettings; - - static std::unique_ptr Create(); - static std::unique_ptr Create(Settings settings); -}; - std::unique_ptr CreateVp8Decoder(const Environment& env); } // namespace webrtc diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 3af0bdfa90..1708d23e01 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -40,7 +40,6 @@ #include "rtc_base/experiments/field_trial_units.h" #include "rtc_base/logging.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" #include "third_party/libyuv/include/libyuv/scale.h" #include "vpx/vp8cx.h" @@ -249,12 +248,13 @@ class FrameDropConfigOverride { const uint32_t original_frame_drop_threshold_; }; -absl::optional ParseFrameDropInterval() { +absl::optional ParseFrameDropInterval( + const FieldTrialsView& field_trials) { FieldTrialFlag disabled = FieldTrialFlag("Disabled"); FieldTrialParameter interval("interval", kDefaultMaxFrameDropInterval); ParseFieldTrial({&disabled, &interval}, - field_trial::FindFullName("WebRTC-VP8-MaxFrameInterval")); + field_trials.Lookup("WebRTC-VP8-MaxFrameInterval")); if (disabled.Get()) { // Kill switch set, don't use any max frame interval. return absl::nullopt; @@ -270,17 +270,6 @@ std::unique_ptr CreateVp8Encoder(const Environment& env, LibvpxInterface::Create()); } -std::unique_ptr VP8Encoder::Create() { - return std::make_unique(LibvpxInterface::Create(), - VP8Encoder::Settings()); -} - -std::unique_ptr VP8Encoder::Create( - VP8Encoder::Settings settings) { - return std::make_unique(LibvpxInterface::Create(), - std::move(settings)); -} - vpx_enc_frame_flags_t LibvpxVp8Encoder::EncodeFlags( const Vp8FrameConfig& references) { RTC_DCHECK(!references.drop_frame); @@ -310,19 +299,23 @@ vpx_enc_frame_flags_t LibvpxVp8Encoder::EncodeFlags( return flags; } -LibvpxVp8Encoder::LibvpxVp8Encoder(std::unique_ptr interface, - VP8Encoder::Settings settings) - : libvpx_(std::move(interface)), - rate_control_settings_(RateControlSettings::ParseFromFieldTrials()), +LibvpxVp8Encoder::LibvpxVp8Encoder(const Environment& env, + Vp8EncoderSettings settings, + std::unique_ptr interface) + : env_(env), + libvpx_(std::move(interface)), + rate_control_settings_( + RateControlSettings::ParseFromKeyValueConfig(&env_.field_trials())), resolution_bitrate_limits_(std::move(settings.resolution_bitrate_limits)), key_frame_request_(kMaxSimulcastStreams, false), last_encoder_output_time_(kMaxSimulcastStreams, Timestamp::MinusInfinity()), variable_framerate_experiment_(ParseVariableFramerateConfig( + env_.field_trials(), "WebRTC-VP8VariableFramerateScreenshare")), framerate_controller_(variable_framerate_experiment_.framerate_limit), - max_frame_drop_interval_(ParseFrameDropInterval()), - android_specific_threading_settings_(webrtc::field_trial::IsEnabled( + max_frame_drop_interval_(ParseFrameDropInterval(env_.field_trials())), + android_specific_threading_settings_(env_.field_trials().IsEnabled( "WebRTC-LibvpxVp8Encoder-AndroidSpecificThreadingSettings")) { // TODO(eladalon/ilnik): These reservations might be wasting memory. // InitEncode() is resizing to the actual size, which might be smaller. @@ -596,7 +589,7 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, // Override the error resilience mode if this is not simulcast, but we are // using temporal layers. - if (field_trial::IsEnabled(kVp8ForcePartitionResilience) && + if (env_.field_trials().IsEnabled(kVp8ForcePartitionResilience) && (number_of_streams == 1) && (SimulcastUtility::NumberOfTemporalLayers(*inst, 0) > 1)) { RTC_LOG(LS_INFO) << "Overriding g_error_resilient from " @@ -800,7 +793,7 @@ int LibvpxVp8Encoder::NumberOfThreads(int width, int height, int cpus) { } #elif defined(WEBRTC_IOS) std::string trial_string = - field_trial::FindFullName(kVP8IosMaxNumberOfThreadFieldTrial); + env_.field_trials().Lookup(kVP8IosMaxNumberOfThreadFieldTrial); FieldTrialParameter max_thread_number( kVP8IosMaxNumberOfThreadFieldTrialParameter, 0); ParseFieldTrial({&max_thread_number}, trial_string); @@ -1504,13 +1497,15 @@ LibvpxVp8Encoder::PrepareBuffers(rtc::scoped_refptr buffer) { // static LibvpxVp8Encoder::VariableFramerateExperiment -LibvpxVp8Encoder::ParseVariableFramerateConfig(std::string group_name) { +LibvpxVp8Encoder::ParseVariableFramerateConfig( + const FieldTrialsView& field_trials, + absl::string_view group_name) { FieldTrialFlag disabled = FieldTrialFlag("Disabled"); FieldTrialParameter framerate_limit("min_fps", 5.0); FieldTrialParameter qp("min_qp", 15); FieldTrialParameter undershoot_percentage("undershoot", 30); ParseFieldTrial({&disabled, &framerate_limit, &qp, &undershoot_percentage}, - field_trial::FindFullName(group_name)); + field_trials.Lookup(group_name)); VariableFramerateExperiment config; config.enabled = !disabled.Get(); config.framerate_limit = framerate_limit.Get(); diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index 989ce5ea2b..c78da360a1 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -16,7 +16,9 @@ #include #include +#include "absl/strings/string_view.h" #include "api/fec_controller_override.h" +#include "api/field_trials_view.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/encoded_image.h" @@ -41,14 +43,8 @@ class LibvpxVp8Encoder : public VideoEncoder { public: LibvpxVp8Encoder(const Environment& env, Vp8EncoderSettings settings, - std::unique_ptr interface) - // TODO: bugs.webrtc.org/15860 - Save `env` and use field trials from it - // when constructor below can be removed. - : LibvpxVp8Encoder(std::move(interface), std::move(settings)) {} + std::unique_ptr interface); - // Deprecated, bugs.webrtc.org/15860 - LibvpxVp8Encoder(std::unique_ptr interface, - VP8Encoder::Settings settings); ~LibvpxVp8Encoder() override; int Release() override; @@ -115,6 +111,7 @@ class LibvpxVp8Encoder : public VideoEncoder { std::vector> PrepareBuffers( rtc::scoped_refptr buffer); + const Environment env_; const std::unique_ptr libvpx_; const CpuSpeedExperiment experimental_cpu_speed_config_arm_; @@ -155,7 +152,8 @@ class LibvpxVp8Encoder : public VideoEncoder { int steady_state_undershoot_percentage = 30; } variable_framerate_experiment_; static VariableFramerateExperiment ParseVariableFramerateConfig( - std::string group_name); + const FieldTrialsView& field_trials, + absl::string_view group_name); FramerateControllerDeprecated framerate_controller_; int num_steady_state_frames_ = 0; diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc index 79bcb5e212..c03f5c7f2f 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc @@ -24,7 +24,9 @@ namespace { std::unique_ptr CreateSpecificSimulcastTestFixture() { std::unique_ptr encoder_factory = std::make_unique( - []() { return VP8Encoder::Create(); }); + [](const Environment& env, const SdpVideoFormat& format) { + return CreateVp8Encoder(env); + }); std::unique_ptr decoder_factory = std::make_unique( [](const Environment& env, const SdpVideoFormat& format) { diff --git a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index 1602c0d314..8c03148157 100644 --- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -13,6 +13,7 @@ #include #include +#include "absl/memory/memory.h" #include "api/environment/environment_factory.h" #include "api/test/create_frame_generator.h" #include "api/test/frame_generator_interface.h" @@ -30,6 +31,7 @@ #include "rtc_base/time_utils.h" #include "test/field_trial.h" #include "test/mappable_native_buffer.h" +#include "test/scoped_key_value_config.h" #include "test/video_codec_settings.h" namespace webrtc { @@ -122,8 +124,7 @@ TEST_F(TestVp8Impl, ErrorResilienceDisabledForNoTemporalLayers) { codec_settings_.simulcastStream[0].numberOfTemporalLayers = 1; auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); EXPECT_CALL(*vpx, codec_enc_init( _, _, Field(&vpx_codec_enc_cfg_t::g_error_resilient, 0), _)); @@ -136,8 +137,7 @@ TEST_F(TestVp8Impl, DefaultErrorResilienceEnabledForTemporalLayers) { codec_settings_.VP8()->numberOfTemporalLayers = 2; auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); EXPECT_CALL(*vpx, codec_enc_init(_, _, Field(&vpx_codec_enc_cfg_t::g_error_resilient, @@ -155,8 +155,7 @@ TEST_F(TestVp8Impl, codec_settings_.VP8()->numberOfTemporalLayers = 2; auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); EXPECT_CALL(*vpx, codec_enc_init(_, _, Field(&vpx_codec_enc_cfg_t::g_error_resilient, @@ -169,8 +168,7 @@ TEST_F(TestVp8Impl, TEST_F(TestVp8Impl, SetRates) { codec_settings_.SetFrameDropEnabled(true); auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder.InitEncode(&codec_settings_, VideoEncoder::Settings(kCapabilities, 1, 1000))); @@ -598,8 +596,7 @@ TEST_F(TestVp8Impl, DontDropKeyframes) { TEST_F(TestVp8Impl, KeepsTimestampOnReencode) { auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); // Settings needed to trigger ScreenshareLayers usage, which is required for // overshoot-drop-reencode logic. @@ -638,8 +635,7 @@ TEST_F(TestVp8Impl, KeepsTimestampOnReencode) { TEST(LibvpxVp8EncoderTest, GetEncoderInfoReturnsStaticInformation) { auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); const auto info = encoder.GetEncoderInfo(); @@ -654,13 +650,13 @@ TEST(LibvpxVp8EncoderTest, GetEncoderInfoReturnsStaticInformation) { } TEST(LibvpxVp8EncoderTest, RequestedResolutionAlignmentFromFieldTrial) { - test::ScopedFieldTrials field_trials( + test::ScopedKeyValueConfig field_trials( "WebRTC-VP8-GetEncoderInfoOverride/" "requested_resolution_alignment:10/"); auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(&field_trials), {}, + absl::WrapUnique(vpx)); EXPECT_EQ(encoder.GetEncoderInfo().requested_resolution_alignment, 10u); EXPECT_FALSE( @@ -669,7 +665,7 @@ TEST(LibvpxVp8EncoderTest, RequestedResolutionAlignmentFromFieldTrial) { } TEST(LibvpxVp8EncoderTest, ResolutionBitrateLimitsFromFieldTrial) { - test::ScopedFieldTrials field_trials( + test::ScopedKeyValueConfig field_trials( "WebRTC-VP8-GetEncoderInfoOverride/" "frame_size_pixels:123|456|789," "min_start_bitrate_bps:11000|22000|33000," @@ -677,8 +673,8 @@ TEST(LibvpxVp8EncoderTest, ResolutionBitrateLimitsFromFieldTrial) { "max_bitrate_bps:77000|88000|99000/"); auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(&field_trials), {}, + absl::WrapUnique(vpx)); EXPECT_THAT( encoder.GetEncoderInfo().resolution_bitrate_limits, @@ -691,8 +687,7 @@ TEST(LibvpxVp8EncoderTest, ResolutionBitrateLimitsFromFieldTrial) { TEST(LibvpxVp8EncoderTest, GetEncoderInfoReturnsEmptyResolutionBitrateLimitsByDefault) { auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - VP8Encoder::Settings()); + LibvpxVp8Encoder encoder(CreateEnvironment(), {}, absl::WrapUnique(vpx)); const auto info = encoder.GetEncoderInfo(); @@ -707,12 +702,12 @@ TEST(LibvpxVp8EncoderTest, /*min_bitrate_bps=*/100, /*max_bitrate_bps=*/1000), VideoEncoder::ResolutionBitrateLimits(320 * 180, 100, 30, 500)}; - VP8Encoder::Settings settings; + Vp8EncoderSettings settings; settings.resolution_bitrate_limits = resolution_bitrate_limits; auto* const vpx = new NiceMock(); - LibvpxVp8Encoder encoder((std::unique_ptr(vpx)), - std::move(settings)); + LibvpxVp8Encoder encoder(CreateEnvironment(), std::move(settings), + absl::WrapUnique(vpx)); const auto info = encoder.GetEncoderInfo();