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 <[email protected]> Commit-Queue: Danil Chapovalov <[email protected]> Cr-Commit-Position: refs/heads/main@{#41990}
basharalbashier · Apr 3, 2024 · 71566bc · 71566bc
1 parent abf1e0b
commit 71566bc
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 61 deletions.
diff --git 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,15 +193,22 @@ 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") {
   visibility = [ "*" ]
   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
@@ -42,6 +42,7 @@ struct FooEncoderTemplateAdapter {
   static std::vector<SdpVideoFormat> SupportedFormats() { return {kFooSdp}; }
 
   static std::unique_ptr<VideoEncoder> CreateEncoder(
+      const Environment& env,
       const SdpVideoFormat& format) {
     return std::make_unique<StrictMock<MockVideoEncoder>>();
   }
@@ -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<FooEncoderTemplateAdapter> factory;
-
-  EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), NotNull());
-  EXPECT_THAT(factory.Create(CreateEnvironment(), kFooSdp), NotNull());
-}
-
 TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCodecSupport) {
   VideoEncoderFactoryTemplate<FooEncoderTemplateAdapter,
                               BarEncoderTemplateAdapter>

diff --git 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<Ts...>();
   }
 
-  std::unique_ptr<VideoEncoder> 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<SdpVideoFormat> matched =
-        FuzzyMatchSdpVideoFormat(GetSupportedFormats(), format);
-    return CreateVideoEncoderInternal<Ts...>(matched.value_or(format));
-  }
-
   std::unique_ptr<VideoEncoder> 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 <typename V, typename... Vs>
-  std::unique_ptr<VideoEncoder> CreateVideoEncoderInternal(
-      const SdpVideoFormat& format) {
-    if (IsFormatInList(format, V::SupportedFormats())) {
-      if constexpr (std::is_invocable_r_v<std::unique_ptr<VideoEncoder>,
-                                          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<Vs...>(format);
-    }
-
-    return nullptr;
-  }
-
   template <typename V, typename... Vs>
   std::unique_ptr<VideoEncoder> CreateInternal(const Environment& env,
                                                const SdpVideoFormat& format) {
     if (IsFormatInList(format, V::SupportedFormats())) {
-      if constexpr (std::is_invocable_r_v<std::unique_ptr<VideoEncoder>,
-                                          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
@@ -15,6 +15,7 @@
 #include <vector>
 
 #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<VideoEncoder> 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
@@ -15,6 +15,7 @@
 #include <vector>
 
 #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<VideoEncoder> 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
@@ -14,6 +14,8 @@
 #include <memory>
 #include <vector>
 
+#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<VideoEncoder> 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
@@ -14,6 +14,7 @@
 #include <memory>
 #include <vector>
 
+#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<VideoEncoder> 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