From b158537a4f3ee38b6e87dd538ecdd3be2fd297e7 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 12 Feb 2024 15:28:35 +0100 Subject: [PATCH] Allow to propagate field trials into Vp8 Decoder Bug: webrtc:15791 Change-Id: I0cd279006924c7a4859697b26a2271c3dc63ea6d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/337400 Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41741} --- api/video_codecs/test/BUILD.gn | 3 ++ ...oder_software_fallback_wrapper_unittest.cc | 10 +++++-- modules/video_coding/BUILD.gn | 3 ++ modules/video_coding/codecs/vp8/include/vp8.h | 5 ++++ .../codecs/vp8/libvpx_vp8_decoder.cc | 30 +++++++++++++------ .../codecs/vp8/libvpx_vp8_decoder.h | 6 ++++ .../codecs/vp8/libvpx_vp8_simulcast_test.cc | 4 ++- .../codecs/vp8/test/vp8_impl_unittest.cc | 3 +- 8 files changed, 50 insertions(+), 14 deletions(-) diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn index 7bfe86e9f4..d338406bc4 100644 --- a/api/video_codecs/test/BUILD.gn +++ b/api/video_codecs/test/BUILD.gn @@ -41,8 +41,11 @@ if (rtc_include_tests) { "../../../rtc_base:rtc_base_tests_utils", "../../../test:fake_video_codecs", "../../../test:field_trial", + "../../../test:scoped_key_value_config", "../../../test:test_support", "../../../test:video_test_common", + "../../environment", + "../../environment:environment_factory", "../../video:encoded_image", "../../video:video_bitrate_allocation", "../../video:video_frame", diff --git a/api/video_codecs/test/video_decoder_software_fallback_wrapper_unittest.cc b/api/video_codecs/test/video_decoder_software_fallback_wrapper_unittest.cc index 97be6250db..de9293bbe0 100644 --- a/api/video_codecs/test/video_decoder_software_fallback_wrapper_unittest.cc +++ b/api/video_codecs/test/video_decoder_software_fallback_wrapper_unittest.cc @@ -13,6 +13,8 @@ #include #include "absl/types/optional.h" +#include "api/environment/environment.h" +#include "api/environment/environment_factory.h" #include "api/video/encoded_image.h" #include "api/video/video_frame.h" #include "api/video_codecs/video_decoder.h" @@ -20,8 +22,8 @@ #include "modules/video_coding/include/video_codec_interface.h" #include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/checks.h" -#include "test/field_trial.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" namespace webrtc { @@ -32,9 +34,10 @@ class VideoDecoderSoftwareFallbackWrapperTest : public ::testing::Test { explicit VideoDecoderSoftwareFallbackWrapperTest( const std::string& field_trials) : override_field_trials_(field_trials), + env_(CreateEnvironment(&override_field_trials_)), fake_decoder_(new CountingFakeDecoder()), fallback_wrapper_(CreateVideoDecoderSoftwareFallbackWrapper( - std::unique_ptr(VP8Decoder::Create()), + CreateVp8Decoder(env_), std::unique_ptr(fake_decoder_))) {} class CountingFakeDecoder : public VideoDecoder { @@ -71,7 +74,8 @@ class VideoDecoderSoftwareFallbackWrapperTest : public ::testing::Test { int release_count_ = 0; int reset_count_ = 0; }; - test::ScopedFieldTrials override_field_trials_; + test::ScopedKeyValueConfig override_field_trials_; + const Environment env_; // `fake_decoder_` is owned and released by `fallback_wrapper_`. CountingFakeDecoder* fake_decoder_; std::unique_ptr fallback_wrapper_; diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 6e62b9ace3..35fb2b49bc 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -585,7 +585,10 @@ rtc_library("webrtc_vp8") { ":webrtc_vp8_scalability", ":webrtc_vp8_temporal_layers", "../../api:fec_controller_api", + "../../api:field_trials_view", "../../api:scoped_refptr", + "../../api/environment", + "../../api/transport:field_trial_based_config", "../../api/units:time_delta", "../../api/units:timestamp", "../../api/video:encoded_image", diff --git a/modules/video_coding/codecs/vp8/include/vp8.h b/modules/video_coding/codecs/vp8/include/vp8.h index 2fc647874f..45b7cee00a 100644 --- a/modules/video_coding/codecs/vp8/include/vp8.h +++ b/modules/video_coding/codecs/vp8/include/vp8.h @@ -14,6 +14,7 @@ #include #include +#include "api/environment/environment.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/vp8_frame_buffer_controller.h" #include "modules/video_coding/include/video_codec_interface.h" @@ -40,11 +41,15 @@ class VP8Encoder { static std::unique_ptr Create(Settings settings); }; +// TODO: bugs.webrtc.org/15791 - Deprecate and delete in favor of the +// CreateVp8Decoder function. class VP8Decoder { public: static std::unique_ptr Create(); }; +std::unique_ptr CreateVp8Decoder(const Environment& env); + } // namespace webrtc #endif // MODULES_VIDEO_CODING_CODECS_VP8_INCLUDE_VP8_H_ diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc index 5afd105949..3466080c83 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.cc @@ -18,7 +18,10 @@ #include #include "absl/types/optional.h" +#include "api/environment/environment.h" +#include "api/field_trials_view.h" #include "api/scoped_refptr.h" +#include "api/transport/field_trial_based_config.h" #include "api/video/i420_buffer.h" #include "api/video/video_frame.h" #include "api/video/video_frame_buffer.h" @@ -28,7 +31,6 @@ #include "rtc_base/checks.h" #include "rtc_base/numerics/exp_filter.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" #include "third_party/libyuv/include/libyuv/convert.h" #include "vpx/vp8.h" @@ -59,9 +61,9 @@ absl::optional DefaultDeblockParams() { } absl::optional -GetPostProcParamsFromFieldTrialGroup() { - std::string group = webrtc::field_trial::FindFullName( - kIsArm ? kVp8PostProcArmFieldTrial : kVp8PostProcFieldTrial); +GetPostProcParamsFromFieldTrialGroup(const FieldTrialsView& field_trials) { + std::string group = field_trials.Lookup(kIsArm ? kVp8PostProcArmFieldTrial + : kVp8PostProcFieldTrial); if (group.empty()) { return DefaultDeblockParams(); } @@ -89,6 +91,10 @@ std::unique_ptr VP8Decoder::Create() { return std::make_unique(); } +std::unique_ptr CreateVp8Decoder(const Environment& env) { + return std::make_unique(env); +} + class LibvpxVp8Decoder::QpSmoother { public: QpSmoother() : last_sample_ms_(rtc::TimeMillis()), smoother_(kAlpha) {} @@ -114,9 +120,14 @@ class LibvpxVp8Decoder::QpSmoother { }; LibvpxVp8Decoder::LibvpxVp8Decoder() - : use_postproc_( - kIsArm ? webrtc::field_trial::IsEnabled(kVp8PostProcArmFieldTrial) - : true), + : LibvpxVp8Decoder(FieldTrialBasedConfig()) {} + +LibvpxVp8Decoder::LibvpxVp8Decoder(const Environment& env) + : LibvpxVp8Decoder(env.field_trials()) {} + +LibvpxVp8Decoder::LibvpxVp8Decoder(const FieldTrialsView& field_trials) + : use_postproc_(kIsArm ? field_trials.IsEnabled(kVp8PostProcArmFieldTrial) + : true), buffer_pool_(false, 300 /* max_number_of_buffers*/), decode_complete_callback_(NULL), inited_(false), @@ -124,8 +135,9 @@ LibvpxVp8Decoder::LibvpxVp8Decoder() last_frame_width_(0), last_frame_height_(0), key_frame_required_(true), - deblock_params_(use_postproc_ ? GetPostProcParamsFromFieldTrialGroup() - : absl::nullopt), + deblock_params_(use_postproc_ + ? GetPostProcParamsFromFieldTrialGroup(field_trials) + : absl::nullopt), qp_smoother_(use_postproc_ ? new QpSmoother() : nullptr) {} LibvpxVp8Decoder::~LibvpxVp8Decoder() { diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.h index 74f4dc7c89..8ed8e7ca88 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_decoder.h @@ -14,6 +14,8 @@ #include #include "absl/types/optional.h" +#include "api/environment/environment.h" +#include "api/field_trials_view.h" #include "api/video/encoded_image.h" #include "api/video_codecs/video_decoder.h" #include "common_video/include/video_frame_buffer_pool.h" @@ -26,7 +28,10 @@ namespace webrtc { class LibvpxVp8Decoder : public VideoDecoder { public: + // TODO: bugs.webrtc.org/15791 - Delete default constructor when + // Environment is always propagated. LibvpxVp8Decoder(); + explicit LibvpxVp8Decoder(const Environment& env); ~LibvpxVp8Decoder() override; bool Configure(const Settings& settings) override; @@ -56,6 +61,7 @@ class LibvpxVp8Decoder : public VideoDecoder { private: class QpSmoother; + explicit LibvpxVp8Decoder(const FieldTrialsView& field_trials); int ReturnFrame(const vpx_image_t* img, uint32_t timeStamp, int qp, 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 4ca3de20d5..3f13066892 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_simulcast_test.cc @@ -27,7 +27,9 @@ std::unique_ptr CreateSpecificSimulcastTestFixture() { []() { return VP8Encoder::Create(); }); std::unique_ptr decoder_factory = std::make_unique( - []() { return VP8Decoder::Create(); }); + [](const Environment& env, const SdpVideoFormat& format) { + return CreateVp8Decoder(env); + }); return CreateSimulcastTestFixture(std::move(encoder_factory), std::move(decoder_factory), SdpVideoFormat("VP8")); 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 a6f570f855..514d3d7e1d 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 "api/environment/environment_factory.h" #include "api/test/create_frame_generator.h" #include "api/test/frame_generator_interface.h" #include "api/test/mock_video_decoder.h" @@ -70,7 +71,7 @@ class TestVp8Impl : public VideoCodecUnitTest { } std::unique_ptr CreateDecoder() override { - return VP8Decoder::Create(); + return CreateVp8Decoder(CreateEnvironment()); } void ModifyCodecSettings(VideoCodec* codec_settings) override {