From 71566bc802ccc6c5a0313593802eea83f0e279ff Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 22 Mar 2024 15:58:15 +0100 Subject: [PATCH] In VideoEncoderFactoryTemplate pass webrtc::Environment to individual traits Bug: webrtc:15860 Change-Id: I8727491e60247433db4753678c69d16b8a1d5a72 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343781 Reviewed-by: Philip Eliasson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41990} --- api/video_codecs/BUILD.gn | 13 +++++- .../video_encoder_factory_template_tests.cc | 11 +---- .../video_encoder_factory_template.h | 46 +------------------ ...oder_factory_template_libaom_av1_adapter.h | 4 +- ...oder_factory_template_libvpx_vp8_adapter.h | 4 +- ...oder_factory_template_libvpx_vp9_adapter.h | 7 ++- ...coder_factory_template_open_h264_adapter.h | 4 +- 7 files changed, 28 insertions(+), 61 deletions(-) diff --git a/api/video_codecs/BUILD.gn b/api/video_codecs/BUILD.gn index dbdc23ad54..bcf11cb7a1 100644 --- a/api/video_codecs/BUILD.gn +++ b/api/video_codecs/BUILD.gn @@ -182,6 +182,7 @@ rtc_source_set("video_encoder_factory_template_libvpx_vp8_adapter") { ":video_codecs_api", "../../modules/video_coding:webrtc_vp8", "../../modules/video_coding:webrtc_vp8_scalability", + "../environment", ] absl_deps = [ "//third_party/abseil-cpp/absl/container:inlined_vector" ] @@ -192,7 +193,11 @@ rtc_source_set("video_encoder_factory_template_libvpx_vp9_adapter") { allow_poison = [ "software_video_codecs" ] public = [ "video_encoder_factory_template_libvpx_vp9_adapter.h" ] - deps = [ "../../modules/video_coding:webrtc_vp9" ] + deps = [ + ":video_codecs_api", + "../../modules/video_coding:webrtc_vp9", + "../environment", + ] } rtc_source_set("video_encoder_factory_template_open_h264_adapter") { @@ -200,7 +205,10 @@ rtc_source_set("video_encoder_factory_template_open_h264_adapter") { allow_poison = [ "software_video_codecs" ] public = [ "video_encoder_factory_template_open_h264_adapter.h" ] - deps = [ "../../modules/video_coding:webrtc_h264" ] + deps = [ + "../../modules/video_coding:webrtc_h264", + "../environment", + ] } rtc_source_set("video_encoder_factory_template_libaom_av1_adapter") { @@ -214,6 +222,7 @@ rtc_source_set("video_encoder_factory_template_libaom_av1_adapter") { "../../modules/video_coding/codecs/av1:av1_svc_config", "../../modules/video_coding/codecs/av1:libaom_av1_encoder", "../../modules/video_coding/svc:scalability_mode_util", + "../environment", ] absl_deps = [ "//third_party/abseil-cpp/absl/container:inlined_vector" ] } 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 ea4bdf9238..569cb5c6bb 100644 --- a/api/video_codecs/test/video_encoder_factory_template_tests.cc +++ b/api/video_codecs/test/video_encoder_factory_template_tests.cc @@ -42,6 +42,7 @@ struct FooEncoderTemplateAdapter { static std::vector SupportedFormats() { return {kFooSdp}; } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { return std::make_unique>(); } @@ -112,16 +113,6 @@ TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCreateEncoders) { 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.Create(CreateEnvironment(), kFooSdp), NotNull()); -} - TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCodecSupport) { VideoEncoderFactoryTemplate diff --git a/api/video_codecs/video_encoder_factory_template.h b/api/video_codecs/video_encoder_factory_template.h index 46fb4528fa..0772ff9367 100644 --- a/api/video_codecs/video_encoder_factory_template.h +++ b/api/video_codecs/video_encoder_factory_template.h @@ -53,20 +53,6 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { return GetSupportedFormatsInternal(); } - std::unique_ptr CreateVideoEncoder( - 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 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 @@ -127,41 +113,11 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { return supported_formats; } - template - std::unique_ptr CreateVideoEncoderInternal( - const SdpVideoFormat& format) { - if (IsFormatInList(format, V::SupportedFormats())) { - 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) { - return CreateVideoEncoderInternal(format); - } - - 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); - } + return V::CreateEncoder(env, format); } if constexpr (sizeof...(Vs) > 0) { diff --git a/api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h b/api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h index 0f801ad3c7..70789ebccd 100644 --- a/api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h +++ b/api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h @@ -15,6 +15,7 @@ #include #include "absl/container/inlined_vector.h" +#include "api/environment/environment.h" #include "api/video_codecs/sdp_video_format.h" #include "modules/video_coding/codecs/av1/av1_svc_config.h" #include "modules/video_coding/codecs/av1/libaom_av1_encoder.h" @@ -28,8 +29,9 @@ struct LibaomAv1EncoderTemplateAdapter { } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { - return CreateLibaomAv1Encoder(); + return CreateLibaomAv1Encoder(env); } static bool IsScalabilityModeSupported(ScalabilityMode scalability_mode) { diff --git a/api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h b/api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h index c60aa04795..26c4c866a6 100644 --- a/api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h +++ b/api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h @@ -15,6 +15,7 @@ #include #include "absl/container/inlined_vector.h" +#include "api/environment/environment.h" #include "api/video_codecs/sdp_video_format.h" #include "modules/video_coding/codecs/vp8/include/vp8.h" #include "modules/video_coding/codecs/vp8/vp8_scalability.h" @@ -32,8 +33,9 @@ struct LibvpxVp8EncoderTemplateAdapter { } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { - return VP8Encoder::Create(); + return CreateVp8Encoder(env); } static bool IsScalabilityModeSupported(ScalabilityMode scalability_mode) { diff --git a/api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h b/api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h index b16989c8c7..a46e38bf4f 100644 --- a/api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h +++ b/api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h @@ -14,6 +14,8 @@ #include #include +#include "api/environment/environment.h" +#include "api/video_codecs/vp9_profile.h" #include "modules/video_coding/codecs/vp9/include/vp9.h" namespace webrtc { @@ -23,8 +25,11 @@ struct LibvpxVp9EncoderTemplateAdapter { } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { - return VP9Encoder::Create(cricket::CreateVideoCodec(format)); + return CreateVp9Encoder(env, + {.profile = ParseSdpForVP9Profile(format.parameters) + .value_or(VP9Profile::kProfile0)}); } static bool IsScalabilityModeSupported(ScalabilityMode scalability_mode) { diff --git a/api/video_codecs/video_encoder_factory_template_open_h264_adapter.h b/api/video_codecs/video_encoder_factory_template_open_h264_adapter.h index 6995b27800..041a6344e8 100644 --- a/api/video_codecs/video_encoder_factory_template_open_h264_adapter.h +++ b/api/video_codecs/video_encoder_factory_template_open_h264_adapter.h @@ -14,6 +14,7 @@ #include #include +#include "api/environment/environment.h" #include "modules/video_coding/codecs/h264/include/h264.h" namespace webrtc { @@ -29,9 +30,10 @@ struct OpenH264EncoderTemplateAdapter { } static std::unique_ptr CreateEncoder( + const Environment& env, const SdpVideoFormat& format) { #if defined(WEBRTC_USE_H264) - return H264Encoder::Create(cricket::CreateVideoCodec(format)); + return CreateH264Encoder(env, H264EncoderSettings::Parse(format)); #else return nullptr; #endif