Implement Create instead of MakeAudioDecoder in AudioDecoderFactory template

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

api/audio_codecs/audio_decoder_factory_template.h

@@ -20,6 +20,7 @@
 #include "api/audio_codecs/audio_decoder.h"
 #include "api/audio_codecs/audio_decoder_factory.h"
 #include "api/audio_codecs/audio_format.h"
+#include "api/environment/environment.h"
 #include "api/make_ref_counted.h"
 #include "api/scoped_refptr.h"
 
@@ -35,13 +36,48 @@ template <>
 struct Helper<> {
   static void AppendSupportedDecoders(std::vector<AudioCodecSpec>* specs) {}
   static bool IsSupportedDecoder(const SdpAudioFormat& format) { return false; }
-  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+
+  static absl::Nullable<std::unique_ptr<AudioDecoder>> MakeAudioDecoder(
+      const Environment& env,
       const SdpAudioFormat& format,
       absl::optional<AudioCodecPairId> codec_pair_id) {
     return nullptr;
   }
 };
 
+// Use ranked overloads (abseil.io/tips/229) for dispatching.
+struct Rank0 {};
+struct Rank1 : Rank0 {};
+
+template <typename Trait,
+          typename = std::enable_if_t<std::is_convertible_v<
+              decltype(Trait::MakeAudioDecoder(
+                  std::declval<Environment>(),
+                  std::declval<typename Trait::Config>(),
+                  std::declval<absl::optional<AudioCodecPairId>>())),
+              std::unique_ptr<AudioDecoder>>>>
+absl::Nullable<std::unique_ptr<AudioDecoder>> CreateDecoder(
+    Rank1,
+    const Environment& env,
+    const typename Trait::Config& config,
+    absl::optional<AudioCodecPairId> codec_pair_id) {
+  return Trait::MakeAudioDecoder(env, config, codec_pair_id);
+}
+
+template <typename Trait,
+          typename = std::enable_if_t<std::is_convertible_v<
+              decltype(Trait::MakeAudioDecoder(
+                  std::declval<typename Trait::Config>(),
+                  std::declval<absl::optional<AudioCodecPairId>>())),
+              std::unique_ptr<AudioDecoder>>>>
+absl::Nullable<std::unique_ptr<AudioDecoder>> CreateDecoder(
+    Rank0,
+    const Environment& env,
+    const typename Trait::Config& config,
+    absl::optional<AudioCodecPairId> codec_pair_id) {
+  return Trait::MakeAudioDecoder(config, codec_pair_id);
+}
+
 // Inductive case: Called with n + 1 template parameters; calls subroutines
 // with n template parameters.
 template <typename T, typename... Ts>
@@ -58,17 +94,15 @@ struct Helper<T, Ts...> {
                   "absl::optional<T::Config>");
     return opt_config ? true : Helper<Ts...>::IsSupportedDecoder(format);
   }
-  // TODO: bugs.webrtc.org/356878416 - Migrate to `Create`
-  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+
+  static absl::Nullable<std::unique_ptr<AudioDecoder>> MakeAudioDecoder(
+      const Environment& env,
       const SdpAudioFormat& format,
       absl::optional<AudioCodecPairId> codec_pair_id) {
     auto opt_config = T::SdpToConfig(format);
-    return opt_config ?
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-                      T::MakeAudioDecoder(*opt_config, codec_pair_id)
-#pragma clang diagnostic pop
-                      : Helper<Ts...>::MakeAudioDecoder(format, codec_pair_id);
+    return opt_config.has_value()
+               ? CreateDecoder<T>(Rank1{}, env, *opt_config, codec_pair_id)
+               : Helper<Ts...>::MakeAudioDecoder(env, format, codec_pair_id);
   }
 };
 
@@ -85,10 +119,11 @@ class AudioDecoderFactoryT : public AudioDecoderFactory {
     return Helper<Ts...>::IsSupportedDecoder(format);
   }
 
-  std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+  absl::Nullable<std::unique_ptr<AudioDecoder>> Create(
+      const Environment& env,
       const SdpAudioFormat& format,
       absl::optional<AudioCodecPairId> codec_pair_id) override {
-    return Helper<Ts...>::MakeAudioDecoder(format, codec_pair_id);
+    return Helper<Ts...>::MakeAudioDecoder(env, format, codec_pair_id);
   }
 };
 
@@ -109,7 +144,12 @@ class AudioDecoderFactoryT : public AudioDecoderFactory {
 //   void AppendSupportedDecoders(std::vector<AudioCodecSpec>* specs);
 //
 //   // Creates an AudioDecoder for the specified format. Used to implement
-//   // AudioDecoderFactory::MakeAudioDecoder().
+//   // AudioDecoderFactory::Create().
+//   std::unique_ptr<AudioEncoder> MakeAudioDecoder(
+//       const Environment& env,
+//       const ConfigType& config,
+//       absl::optional<AudioCodecPairId> codec_pair_id);
+//   or
 //   std::unique_ptr<AudioDecoder> MakeAudioDecoder(
 //       const ConfigType& config,
 //       absl::optional<AudioCodecPairId> codec_pair_id);

api/audio_codecs/opus/audio_decoder_opus.h

@@ -44,6 +44,12 @@ struct RTC_EXPORT AudioDecoderOpus {
 
   static std::unique_ptr<AudioDecoder> MakeAudioDecoder(const Environment& env,
                                                         Config config);
+  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+      const Environment& env,
+      Config config,
+      absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
+    return MakeAudioDecoder(env, config);
+  }
 };
 
 }  // namespace webrtc

api/audio_codecs/test/audio_decoder_factory_template_unittest.cc

@@ -33,9 +33,14 @@
 #include "test/mock_audio_decoder.h"
 
 namespace webrtc {
-
 namespace {
 
+using ::testing::NiceMock;
+using ::testing::NotNull;
+using ::testing::Pointer;
+using ::testing::Property;
+using ::testing::Return;
+
 struct BogusParams {
   static SdpAudioFormat AudioFormat() { return {"bogus", 8000, 1}; }
   static AudioCodecInfo CodecInfo() { return {8000, 1, 12345}; }
@@ -83,7 +88,73 @@ struct AudioDecoderFakeApi {
   }
 };
 
-}  // namespace
+// Trait to pass as template parameter to `CreateAudioDecoderFactory` with
+// all the functions except the functions to create the audio decoder.
+struct BaseAudioDecoderApi {
+  struct Config {};
+
+  static SdpAudioFormat AudioFormat() { return {"fake", 16'000, 2, {}}; }
+
+  static absl::optional<Config> SdpToConfig(
+      const SdpAudioFormat& audio_format) {
+    return Config();
+  }
+
+  static void AppendSupportedDecoders(std::vector<AudioCodecSpec>* specs) {
+    specs->push_back({.format = AudioFormat(), .info = {16'000, 2, 23456}});
+  }
+};
+
+struct TraitWithTwoMakeAudioDecoders : BaseAudioDecoderApi {
+  // Create Decoders with different sample rates depending if it is created
+  // through one or another `MAkeAudioDecoder` so that a test may detect which
+  // method was used.
+  static constexpr int kRateWithoutEnv = 10'000;
+  static constexpr int kRateWithEnv = 20'000;
+
+  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+      const Config& config,
+      absl::optional<AudioCodecPairId> codec_pair_id) {
+    auto decoder = std::make_unique<NiceMock<MockAudioDecoder>>();
+    ON_CALL(*decoder, SampleRateHz).WillByDefault(Return(kRateWithoutEnv));
+    return decoder;
+  }
+
+  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+      const Environment& env,
+      const Config& config,
+      absl::optional<AudioCodecPairId> codec_pair_id) {
+    auto decoder = std::make_unique<NiceMock<MockAudioDecoder>>();
+    ON_CALL(*decoder, SampleRateHz).WillByDefault(Return(kRateWithEnv));
+    return decoder;
+  }
+};
+
+TEST(AudioDecoderFactoryTemplateTest,
+     PrefersToPassEnvironmentToMakeAudioDecoder) {
+  const Environment env = CreateEnvironment();
+  auto factory = CreateAudioDecoderFactory<TraitWithTwoMakeAudioDecoders>();
+
+  EXPECT_THAT(factory->Create(env, BaseAudioDecoderApi::AudioFormat(), {}),
+              Pointer(Property(&AudioDecoder::SampleRateHz,
+                               TraitWithTwoMakeAudioDecoders::kRateWithEnv)));
+}
+
+struct AudioDecoderApiWithV1Make : BaseAudioDecoderApi {
+  static std::unique_ptr<AudioDecoder> MakeAudioDecoder(
+      const Config& config,
+      absl::optional<AudioCodecPairId> codec_pair_id) {
+    return std::make_unique<NiceMock<MockAudioDecoder>>();
+  }
+};
+
+TEST(AudioDecoderFactoryTemplateTest,
+     CanUseMakeAudioDecoderWithoutPassingEnvironment) {
+  const Environment env = CreateEnvironment();
+  auto factory = CreateAudioDecoderFactory<AudioDecoderApiWithV1Make>();
+  EXPECT_THAT(factory->Create(env, BaseAudioDecoderApi::AudioFormat(), {}),
+              NotNull());
+}
 
 TEST(AudioDecoderFactoryTemplateTest, NoDecoderTypes) {
   const Environment env = CreateEnvironment();
@@ -226,4 +297,5 @@ TEST(AudioDecoderFactoryTemplateTest, Opus) {
   EXPECT_EQ(48000, dec->SampleRateHz());
 }
 
+}  // namespace
 }  // namespace webrtc