From dd28f1364b7be0c01ad896ac15087f253db82ca3 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 14 Mar 2024 18:39:44 +0100 Subject: [PATCH] In VideoEncoderFactoryTemplate pass webrtc::Environment to individual traits when supported Bug: webrtc:15860 Change-Id: I022887e57855c072ddfb0edaf37cd96e9fc64ea6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342981 Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41909} --- api/video_codecs/BUILD.gn | 3 +- api/video_codecs/test/BUILD.gn | 2 + .../video_encoder_factory_template_tests.cc | 51 +++++++++++++------ .../video_encoder_factory_template.h | 51 ++++++++++++++++++- 4 files changed, 89 insertions(+), 18 deletions(-) diff --git a/api/video_codecs/BUILD.gn b/api/video_codecs/BUILD.gn index 6ea5995d04..ddfdc376a3 100644 --- a/api/video_codecs/BUILD.gn +++ b/api/video_codecs/BUILD.gn @@ -166,8 +166,9 @@ rtc_source_set("video_encoder_factory_template") { deps = [ ":video_codecs_api", - "../../api:array_view", + "..:array_view", "../../modules/video_coding/svc:scalability_mode_util", + "../environment", ] absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container" ] diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn index 6b749aa749..2fd9bc1460 100644 --- a/api/video_codecs/test/BUILD.gn +++ b/api/video_codecs/test/BUILD.gn @@ -66,6 +66,8 @@ if (rtc_include_tests) { "..:video_encoder_factory_template_open_h264_adapter", "../../:mock_video_encoder", "../../../test:test_support", + "../../environment", + "../../environment:environment_factory", "//testing/gtest", ] } diff --git a/api/video_codecs/test/video_encoder_factory_template_tests.cc b/api/video_codecs/test/video_encoder_factory_template_tests.cc index 91b02aa905..ea4bdf9238 100644 --- a/api/video_codecs/test/video_encoder_factory_template_tests.cc +++ b/api/video_codecs/test/video_encoder_factory_template_tests.cc @@ -8,6 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "api/environment/environment.h" +#include "api/environment/environment_factory.h" #include "api/test/mock_video_encoder.h" #include "api/video_codecs/video_encoder_factory_template.h" #include "api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h" @@ -17,6 +19,9 @@ #include "test/gmock.h" #include "test/gtest.h" +namespace webrtc { +namespace { + using ::testing::Contains; using ::testing::Each; using ::testing::Eq; @@ -25,11 +30,10 @@ using ::testing::IsEmpty; using ::testing::IsNull; using ::testing::Not; using ::testing::NotNull; +using ::testing::StrictMock; using ::testing::UnorderedElementsAre; - -namespace webrtc { -namespace { using CodecSupport = VideoEncoderFactory::CodecSupport; + const SdpVideoFormat kFooSdp("Foo"); const SdpVideoFormat kBarLowSdp("Bar", {{"profile", "low"}}); const SdpVideoFormat kBarHighSdp("Bar", {{"profile", "high"}}); @@ -39,7 +43,7 @@ struct FooEncoderTemplateAdapter { static std::unique_ptr CreateEncoder( const SdpVideoFormat& format) { - return std::make_unique>(); + return std::make_unique>(); } static bool IsScalabilityModeSupported(ScalabilityMode scalability_mode) { @@ -54,8 +58,9 @@ struct BarEncoderTemplateAdapter { } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { - return std::make_unique>(); + return std::make_unique>(); } static bool IsScalabilityModeSupported(ScalabilityMode scalability_mode) { @@ -67,10 +72,11 @@ struct BarEncoderTemplateAdapter { }; TEST(VideoEncoderFactoryTemplate, OneTemplateAdapterCreateEncoder) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; EXPECT_THAT(factory.GetSupportedFormats(), UnorderedElementsAre(kFooSdp)); - EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), NotNull()); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), IsNull()); + EXPECT_THAT(factory.Create(env, kFooSdp), NotNull()); + EXPECT_THAT(factory.Create(env, SdpVideoFormat("FooX")), IsNull()); } TEST(VideoEncoderFactoryTemplate, OneTemplateAdapterCodecSupport) { @@ -93,16 +99,27 @@ TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersNoDuplicates) { } TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCreateEncoders) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; EXPECT_THAT(factory.GetSupportedFormats(), UnorderedElementsAre(kFooSdp, kBarLowSdp, kBarHighSdp)); + EXPECT_THAT(factory.Create(env, kFooSdp), NotNull()); + EXPECT_THAT(factory.Create(env, kBarLowSdp), NotNull()); + EXPECT_THAT(factory.Create(env, kBarHighSdp), NotNull()); + EXPECT_THAT(factory.Create(env, SdpVideoFormat("FooX")), IsNull()); + EXPECT_THAT(factory.Create(env, SdpVideoFormat("Bar")), NotNull()); +} + +TEST(VideoEncoderFactoryTemplate, + CreatesEncoderWithoutEnvironmentWhenNotNeeded) { + // FooEncoderTemplateAdapter::CreateEncoder doesn't take Environment parameter + // Expect it can be created both with newer and older api. + VideoEncoderFactoryTemplate factory; + EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), NotNull()); - EXPECT_THAT(factory.CreateVideoEncoder(kBarLowSdp), NotNull()); - EXPECT_THAT(factory.CreateVideoEncoder(kBarHighSdp), NotNull()); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), IsNull()); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("Bar")), NotNull()); + EXPECT_THAT(factory.Create(CreateEnvironment(), kFooSdp), NotNull()); } TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCodecSupport) { @@ -126,47 +143,51 @@ TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCodecSupport) { } TEST(VideoEncoderFactoryTemplate, LibvpxVp8) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; auto formats = factory.GetSupportedFormats(); EXPECT_THAT(formats.size(), 1); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::name, "VP8")); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL1T3))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); + EXPECT_THAT(factory.Create(env, formats[0]), NotNull()); } TEST(VideoEncoderFactoryTemplate, LibvpxVp9) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; auto formats = factory.GetSupportedFormats(); EXPECT_THAT(formats, Not(IsEmpty())); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::name, "VP9"))); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL3T3_KEY)))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); + EXPECT_THAT(factory.Create(env, formats[0]), NotNull()); } // TODO(bugs.webrtc.org/13573): When OpenH264 is no longer a conditional build // target remove this #ifdef. #if defined(WEBRTC_USE_H264) TEST(VideoEncoderFactoryTemplate, OpenH264) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; auto formats = factory.GetSupportedFormats(); EXPECT_THAT(formats, Not(IsEmpty())); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::name, "H264"))); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL1T3)))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); + EXPECT_THAT(factory.Create(env, formats[0]), NotNull()); } #endif // defined(WEBRTC_USE_H264) TEST(VideoEncoderFactoryTemplate, LibaomAv1) { + const Environment env = CreateEnvironment(); VideoEncoderFactoryTemplate factory; auto formats = factory.GetSupportedFormats(); EXPECT_THAT(formats.size(), 1); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::name, "AV1")); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL3T3_KEY))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); + EXPECT_THAT(factory.Create(env, formats[0]), NotNull()); } } // namespace diff --git a/api/video_codecs/video_encoder_factory_template.h b/api/video_codecs/video_encoder_factory_template.h index 10212ac816..46fb4528fa 100644 --- a/api/video_codecs/video_encoder_factory_template.h +++ b/api/video_codecs/video_encoder_factory_template.h @@ -13,10 +13,12 @@ #include #include +#include #include #include "absl/algorithm/container.h" #include "api/array_view.h" +#include "api/environment/environment.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" @@ -34,7 +36,8 @@ namespace webrtc { // // // Creates an encoder instance for the given format. // static std::unique_ptr -// CreateEncoder(const SdpVideoFormat& format); +// CreateEncoder(const Environment& env, +// const SdpVideoFormat& format); // // // Returns true if the encoder supports the given scalability mode. // static bool @@ -64,6 +67,20 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { return CreateVideoEncoderInternal(matched.value_or(format)); } + std::unique_ptr Create(const Environment& env, + const SdpVideoFormat& format) override { + // We fuzzy match the specified format for both valid and not so valid + // reasons. The valid reason is that there are many standardized codec + // specific fmtp parameters that have not been implemented, and in those + // cases we should not fail to instantiate an encoder just because we don't + // recognize the parameter. The not so valid reason is that we have started + // adding parameters completely unrelated to the SDP to the SdpVideoFormat. + // TODO: bugs.webrtc.org/13868 - Remove FuzzyMatchSdpVideoFormat + absl::optional matched = + FuzzyMatchSdpVideoFormat(GetSupportedFormats(), format); + return CreateInternal(env, matched.value_or(format)); + } + CodecSupport QueryCodecSupport( const SdpVideoFormat& format, absl::optional scalability_mode) const override { @@ -114,7 +131,16 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { std::unique_ptr CreateVideoEncoderInternal( const SdpVideoFormat& format) { if (IsFormatInList(format, V::SupportedFormats())) { - return V::CreateEncoder(format); + if constexpr (std::is_invocable_r_v, + decltype(V::CreateEncoder), + const Environment&, + const SdpVideoFormat&>) { + // Newer code shouldn't call `CreateVideoEncoder` function, + // Older code should only use traits where Environmnet is not required. + RTC_CHECK_NOTREACHED(); + } else { + return V::CreateEncoder(format); + } } if constexpr (sizeof...(Vs) > 0) { @@ -124,6 +150,27 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { return nullptr; } + template + std::unique_ptr CreateInternal(const Environment& env, + const SdpVideoFormat& format) { + if (IsFormatInList(format, V::SupportedFormats())) { + if constexpr (std::is_invocable_r_v, + decltype(V::CreateEncoder), + const Environment&, + const SdpVideoFormat&>) { + return V::CreateEncoder(env, format); + } else { + return V::CreateEncoder(format); + } + } + + if constexpr (sizeof...(Vs) > 0) { + return CreateInternal(env, format); + } + + return nullptr; + } + template CodecSupport QueryCodecSupportInternal( const SdpVideoFormat& format,