Change AudioEncoderFactory api to provide Environment to construct AudioEncoders

Update AudioEncoderFactoryTemplate implementation to expand unit tests to the new api.

Bug: webrtc:343086059
Change-Id: Ib63640de38aa15cc36067d5a3d1de2bf42cec313
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/353981
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42500}

api/audio_codecs/BUILD.gn

@@ -42,6 +42,7 @@ rtc_library("audio_codecs_api") {
     "../../rtc_base:refcount",
     "../../rtc_base:sanitizer",
     "../../rtc_base/system:rtc_export",
+    "../environment",
     "../units:data_rate",
     "../units:time_delta",
     "//third_party/abseil-cpp/absl/base:core_headers",

api/audio_codecs/audio_encoder_factory.h

@@ -18,6 +18,8 @@
 #include "api/audio_codecs/audio_codec_pair_id.h"
 #include "api/audio_codecs/audio_encoder.h"
 #include "api/audio_codecs/audio_format.h"
+#include "api/environment/environment.h"
+#include "rtc_base/checks.h"
 #include "rtc_base/ref_count.h"
 
 namespace webrtc {
@@ -25,6 +27,13 @@ namespace webrtc {
 // A factory that creates AudioEncoders.
 class AudioEncoderFactory : public RefCountInterface {
  public:
+  struct Options {
+    // TODO(ossu): Try to avoid audio encoders having to know their payload
+    // type.
+    int payload_type = -1;
+    absl::optional<AudioCodecPairId> codec_pair_id;
+  };
+
   // Returns a prioritized list of audio codecs, to use for signaling etc.
   virtual std::vector<AudioCodecSpec> GetSupportedEncoders() = 0;
 
@@ -49,14 +58,40 @@ class AudioEncoderFactory : public RefCountInterface {
   // Note: Implementations need to be robust against combinations other than
   // one encoder, one decoder getting the same ID; such encoders must still
   // work.
-  //
-  // TODO(ossu): Try to avoid audio encoders having to know their payload type.
+  // TODO: bugs.webrtc.org/343086059 - make pure virtual when all
+  // implementations of the `AudioEncoderFactory` are updated.
+  virtual absl::Nullable<std::unique_ptr<AudioEncoder>>
+  Create(const Environment& env, const SdpAudioFormat& format, Options options);
+
+  // TODO: bugs.webrtc.org/343086059 - Update all callers to use `Create`
+  // instead, update implementations not to override it, then delete.
   virtual std::unique_ptr<AudioEncoder> MakeAudioEncoder(
       int payload_type,
       const SdpAudioFormat& format,
-      absl::optional<AudioCodecPairId> codec_pair_id) = 0;
+      absl::optional<AudioCodecPairId> codec_pair_id);
 };
 
+//------------------------------------------------------------------------------
+// Implementation details follow
+//------------------------------------------------------------------------------
+
+inline absl::Nullable<std::unique_ptr<AudioEncoder>>
+AudioEncoderFactory::Create(const Environment& env,
+                            const SdpAudioFormat& format,
+                            Options options) {
+  return MakeAudioEncoder(options.payload_type, format, options.codec_pair_id);
+}
+
+inline absl::Nullable<std::unique_ptr<AudioEncoder>>
+AudioEncoderFactory::MakeAudioEncoder(
+    int payload_type,
+    const SdpAudioFormat& format,
+    absl::optional<AudioCodecPairId> codec_pair_id) {
+  // Newer shouldn't call it.
+  // Older code should implement it.
+  RTC_CHECK_NOTREACHED();
+}
+
 }  // namespace webrtc
 
 #endif  // API_AUDIO_CODECS_AUDIO_ENCODER_FACTORY_H_

api/audio_codecs/audio_encoder_factory_template.h

@@ -15,6 +15,7 @@
 #include <vector>
 
 #include "api/audio_codecs/audio_encoder_factory.h"
+#include "api/environment/environment.h"
 #include "api/field_trials_view.h"
 #include "api/make_ref_counted.h"
 #include "api/scoped_refptr.h"
@@ -41,6 +42,12 @@ struct Helper<> {
       const FieldTrialsView* field_trials) {
     return nullptr;
   }
+  static absl::Nullable<std::unique_ptr<AudioEncoder>> CreateAudioEncoder(
+      const Environment& env,
+      const SdpAudioFormat& format,
+      const AudioEncoderFactory::Options& options) {
+    return nullptr;
+  }
 };
 
 // Inductive case: Called with n + 1 template parameters; calls subroutines
@@ -75,6 +82,19 @@ struct Helper<T, Ts...> {
                                              codec_pair_id, field_trials);
     }
   }
+
+  static absl::Nullable<std::unique_ptr<AudioEncoder>> CreateAudioEncoder(
+      const Environment& env,
+      const SdpAudioFormat& format,
+      const AudioEncoderFactory::Options& options) {
+    if (auto opt_config = T::SdpToConfig(format); opt_config.has_value()) {
+      // TODO: bugs.webrtc.org/343086059 - propagate `env` to the individual
+      // encoder factory functions.
+      return T::MakeAudioEncoder(*opt_config, options.payload_type,
+                                 options.codec_pair_id);
+    }
+    return Helper<Ts...>::CreateAudioEncoder(env, format, options);
+  }
 };
 
 template <typename... Ts>
@@ -103,6 +123,13 @@ class AudioEncoderFactoryT : public AudioEncoderFactory {
                                            field_trials_);
   }
 
+  absl::Nullable<std::unique_ptr<AudioEncoder>> Create(
+      const Environment& env,
+      const SdpAudioFormat& format,
+      Options options) override {
+    return Helper<Ts...>::CreateAudioEncoder(env, format, options);
+  }
+
   const FieldTrialsView* field_trials_;
 };
 

api/audio_codecs/test/BUILD.gn

@@ -24,6 +24,8 @@ if (rtc_include_tests) {
       "../../../test:audio_codec_mocks",
       "../../../test:scoped_key_value_config",
       "../../../test:test_support",
+      "../../environment",
+      "../../environment:environment_factory",
       "../L16:audio_decoder_L16",
       "../L16:audio_encoder_L16",
       "../g711:audio_decoder_g711",

api/audio_codecs/test/audio_encoder_factory_template_unittest.cc

@@ -17,15 +17,20 @@
 #include "api/audio_codecs/g722/audio_encoder_g722.h"
 #include "api/audio_codecs/ilbc/audio_encoder_ilbc.h"
 #include "api/audio_codecs/opus/audio_encoder_opus.h"
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/mock_audio_encoder.h"
 #include "test/scoped_key_value_config.h"
 
 namespace webrtc {
-
 namespace {
 
+using ::testing::IsNull;
+using ::testing::Pointer;
+using ::testing::Property;
+
 struct BogusParams {
   static SdpAudioFormat AudioFormat() { return {"bogus", 8000, 1}; }
   static AudioCodecInfo CodecInfo() { return {8000, 1, 12345}; }
@@ -73,10 +78,9 @@ struct AudioEncoderFakeApi {
   }
 };
 
-}  // namespace
-
 TEST(AudioEncoderFactoryTemplateTest, NoEncoderTypes) {
   test::ScopedKeyValueConfig field_trials;
+  const Environment env = CreateEnvironment(&field_trials);
   rtc::scoped_refptr<AudioEncoderFactory> factory(
       rtc::make_ref_counted<
           audio_encoder_factory_template_impl::AudioEncoderFactoryT<>>(
@@ -85,9 +89,12 @@ TEST(AudioEncoderFactoryTemplateTest, NoEncoderTypes) {
   EXPECT_EQ(absl::nullopt, factory->QueryAudioEncoder({"foo", 8000, 1}));
   EXPECT_EQ(nullptr,
             factory->MakeAudioEncoder(17, {"bar", 16000, 1}, absl::nullopt));
+
+  EXPECT_THAT(factory->Create(env, {"bar", 16000, 1}, {}), IsNull());
 }
 
 TEST(AudioEncoderFactoryTemplateTest, OneEncoderType) {
+  const Environment env = CreateEnvironment();
   auto factory = CreateAudioEncoderFactory<AudioEncoderFakeApi<BogusParams>>();
   EXPECT_THAT(factory->GetSupportedEncoders(),
               ::testing::ElementsAre(
@@ -100,9 +107,14 @@ TEST(AudioEncoderFactoryTemplateTest, OneEncoderType) {
   auto enc = factory->MakeAudioEncoder(17, {"bogus", 8000, 1}, absl::nullopt);
   ASSERT_NE(nullptr, enc);
   EXPECT_EQ(8000, enc->SampleRateHz());
+
+  EXPECT_THAT(factory->Create(env, {"bar", 16000, 1}, {}), IsNull());
+  EXPECT_THAT(factory->Create(env, {"bogus", 8000, 1}, {}),
+              Pointer(Property(&AudioEncoder::SampleRateHz, 8000)));
 }
 
 TEST(AudioEncoderFactoryTemplateTest, TwoEncoderTypes) {
+  const Environment env = CreateEnvironment();
   auto factory = CreateAudioEncoderFactory<AudioEncoderFakeApi<BogusParams>,
                                            AudioEncoderFakeApi<ShamParams>>();
   EXPECT_THAT(factory->GetSupportedEncoders(),
@@ -127,6 +139,13 @@ TEST(AudioEncoderFactoryTemplateTest, TwoEncoderTypes) {
       17, {"sham", 16000, 2, {{"param", "value"}}}, absl::nullopt);
   ASSERT_NE(nullptr, enc2);
   EXPECT_EQ(16000, enc2->SampleRateHz());
+
+  EXPECT_THAT(factory->Create(env, {"bar", 16000, 1}, {}), IsNull());
+  EXPECT_THAT(factory->Create(env, {"bogus", 8000, 1}, {}),
+              Pointer(Property(&AudioEncoder::SampleRateHz, 8000)));
+  EXPECT_THAT(
+      factory->Create(env, {"sham", 16000, 2, {{"param", "value"}}}, {}),
+      Pointer(Property(&AudioEncoder::SampleRateHz, 16000)));
 }
 
 TEST(AudioEncoderFactoryTemplateTest, G711) {
@@ -221,4 +240,5 @@ TEST(AudioEncoderFactoryTemplateTest, Opus) {
   EXPECT_EQ(48000, enc->SampleRateHz());
 }
 
+}  // namespace
 }  // namespace webrtc