Delete AudioEncoderFactory::MakeAudioEncoder

Make AudioEncoderFactory::Create pure virtual.

To finalize migrating AudioEncoderFactory to new interface for creating AudioEncoder and thus guarantee AudioEncoders always have an Environment at construction.

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

api/audio_codecs/audio_encoder_factory.h

@@ -28,9 +28,22 @@ namespace webrtc {
 class AudioEncoderFactory : public RefCountInterface {
  public:
   struct Options {
+    // The encoder will tags its payloads with the specified payload type.
     // TODO(ossu): Try to avoid audio encoders having to know their payload
     // type.
     int payload_type = -1;
+
+    // Links encoders and decoders that talk to the same remote entity: if
+    // a AudioEncoderFactory::Create() and a
+    // AudioDecoderFactory::MakeAudioDecoder() call receive non-null IDs that
+    // compare equal, the factory implementations may assume that the encoder
+    // and decoder form a pair. (The intended use case for this is to set up
+    // communication between the AudioEncoder and AudioDecoder instances, which
+    // is needed for some codecs with built-in bandwidth adaptation.)
+    //
+    // Note: Implementations need to be robust against combinations other than
+    // one encoder, one decoder getting the same ID; such encoders must still
+    // work.
     absl::optional<AudioCodecPairId> codec_pair_id;
   };
 
@@ -43,55 +56,14 @@ class AudioEncoderFactory : public RefCountInterface {
   virtual absl::optional<AudioCodecInfo> QueryAudioEncoder(
       const SdpAudioFormat& format) = 0;
 
-  // Creates an AudioEncoder for the specified format. The encoder will tags its
+  // Creates an AudioEncoder for the specified format.
-  // payloads with the specified payload type. The `codec_pair_id` argument is
-  // used to link encoders and decoders that talk to the same remote entity: if
-  // a AudioEncoderFactory::MakeAudioEncoder() and a
-  // AudioDecoderFactory::MakeAudioDecoder() call receive non-null IDs that
-  // compare equal, the factory implementations may assume that the encoder and
-  // decoder form a pair. (The intended use case for this is to set up
-  // communication between the AudioEncoder and AudioDecoder instances, which is
-  // needed for some codecs with built-in bandwidth adaptation.)
-  //
   // Returns null if the format isn't supported.
-  //
+  virtual absl::Nullable<std::unique_ptr<AudioEncoder>> Create(
-  // Note: Implementations need to be robust against combinations other than
+      const Environment& env,
-  // one encoder, one decoder getting the same ID; such encoders must still
-  // work.
-  // 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);
+      Options options) = 0;
 };
 
-//------------------------------------------------------------------------------
-// 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

@@ -41,12 +41,6 @@ struct Helper<> {
       const SdpAudioFormat& format) {
     return absl::nullopt;
   }
-  static std::unique_ptr<AudioEncoder> MakeAudioEncoder(
-      int payload_type,
-      const SdpAudioFormat& format,
-      absl::optional<AudioCodecPairId> codec_pair_id) {
-    return nullptr;
-  }
   static absl::Nullable<std::unique_ptr<AudioEncoder>> CreateAudioEncoder(
       const Environment& env,
       const SdpAudioFormat& format,
@@ -90,30 +84,6 @@ absl::Nullable<std::unique_ptr<AudioEncoder>> CreateEncoder(
                                  options.codec_pair_id);
 }
 
-template <typename Trait,
-          typename = std::enable_if_t<std::is_convertible_v<
-              decltype(Trait::MakeAudioEncoder(
-                  std::declval<typename Trait::Config>(),
-                  int{},
-                  std::declval<absl::optional<AudioCodecPairId>>())),
-              std::unique_ptr<AudioEncoder>>>>
-absl::Nullable<std::unique_ptr<AudioEncoder>> LegacyCreateEncoder(
-    Rank1,
-    const typename Trait::Config& config,
-    int payload_type,
-    absl::optional<AudioCodecPairId> codec_pair_ids) {
-  return Trait::MakeAudioEncoder(config, payload_type, codec_pair_ids);
-}
-
-template <typename Trait>
-absl::Nullable<std::unique_ptr<AudioEncoder>> LegacyCreateEncoder(
-    Rank0,
-    const typename Trait::Config& config,
-    int payload_type,
-    absl::optional<AudioCodecPairId> codec_pair_ids) {
-  RTC_CHECK_NOTREACHED();
-}
-
 // Inductive case: Called with n + 1 template parameters; calls subroutines
 // with n template parameters.
 template <typename T, typename... Ts>
@@ -133,19 +103,6 @@ struct Helper<T, Ts...> {
                             T::QueryAudioEncoder(*opt_config))
                       : Helper<Ts...>::QueryAudioEncoder(format);
   }
-  static std::unique_ptr<AudioEncoder> MakeAudioEncoder(
-      int payload_type,
-      const SdpAudioFormat& format,
-      absl::optional<AudioCodecPairId> codec_pair_id) {
-    auto opt_config = T::SdpToConfig(format);
-    if (opt_config) {
-      return LegacyCreateEncoder<T>(Rank1{}, *opt_config, payload_type,
-                                    codec_pair_id);
-    } else {
-      return Helper<Ts...>::MakeAudioEncoder(payload_type, format,
-                                             codec_pair_id);
-    }
-  }
 
   static absl::Nullable<std::unique_ptr<AudioEncoder>> CreateAudioEncoder(
       const Environment& env,
@@ -172,13 +129,6 @@ class AudioEncoderFactoryT : public AudioEncoderFactory {
     return Helper<Ts...>::QueryAudioEncoder(format);
   }
 
-  std::unique_ptr<AudioEncoder> MakeAudioEncoder(
-      int payload_type,
-      const SdpAudioFormat& format,
-      absl::optional<AudioCodecPairId> codec_pair_id) override {
-    return Helper<Ts...>::MakeAudioEncoder(payload_type, format, codec_pair_id);
-  }
-
   absl::Nullable<std::unique_ptr<AudioEncoder>> Create(
       const Environment& env,
       const SdpAudioFormat& format,
@@ -224,11 +174,6 @@ class AudioEncoderFactoryT : public AudioEncoderFactory {
 // either be the config type, or an alias for it.
 // When both MakeAudioEncoder signatures are present, 1st one is preferred.
 //
-// Note: AudioEncoderFactory::MakeAudioEncoder factory can't use 1st signature,
-// and thus uses 2nd signature when available, crashes otherwise.
-// TODO: bugs.webrtc.org/343086059 - Remove the note above when MakeAudioEncoder
-// is removed.
-//
 // Whenever it tries to do something, the new factory will try each of the
 // encoders in the order they were specified in the template argument list,
 // stopping at the first one that claims to be able to do the job.

api/audio_codecs/test/audio_encoder_factory_template_unittest.cc

@@ -161,11 +161,6 @@ TEST(AudioEncoderFactoryTemplateTest,
   const Environment env = CreateEnvironment();
   auto factory = CreateAudioEncoderFactory<AudioEncoderApiWithV1Make>();
 
-  EXPECT_THAT(
-      factory->MakeAudioEncoder(17, BaseAudioEncoderApi::AudioFormat(), {}),
-      Pointer(Property(&AudioEncoder::SampleRateHz,
-                       BaseAudioEncoderApi::kV1SameRate)));
-
   EXPECT_THAT(factory->Create(env, BaseAudioEncoderApi::AudioFormat(), {}),
               Pointer(Property(&AudioEncoder::SampleRateHz,
                                BaseAudioEncoderApi::kV1SameRate)));
@@ -180,13 +175,6 @@ TEST(AudioEncoderFactoryTemplateTest,
   EXPECT_THAT(factory->Create(env, BaseAudioEncoderApi::AudioFormat(), {}),
               Pointer(Property(&AudioEncoder::SampleRateHz,
                                BaseAudioEncoderApi::kV2SameRate)));
-
-  // For backward compatibility legacy AudioEncoderFactory::MakeAudioEncoder
-  // still can be used, and uses older signature of the Trait::MakeAudioEncoder.
-  EXPECT_THAT(
-      factory->MakeAudioEncoder(17, BaseAudioEncoderApi::AudioFormat(), {}),
-      Pointer(Property(&AudioEncoder::SampleRateHz,
-                       BaseAudioEncoderApi::kV1SameRate)));
 }
 
 TEST(AudioEncoderFactoryTemplateTest, CanUseTraitWithOnlyV2MakeAudioEncoder) {
@@ -197,17 +185,6 @@ TEST(AudioEncoderFactoryTemplateTest, CanUseTraitWithOnlyV2MakeAudioEncoder) {
                                BaseAudioEncoderApi::kV2SameRate)));
 }
 
-#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
-TEST(AudioEncoderFactoryTemplateTest, CrashesWhenV2OnlyTraitUsedWithOlderApi) {
-  auto factory = CreateAudioEncoderFactory<AudioEncoderApiWithV2Make>();
-  // V2 signature requires Environment that
-  // AudioEncoderFactory::MakeAudioEncoder doesn't provide.
-  EXPECT_DEATH(
-      factory->MakeAudioEncoder(17, BaseAudioEncoderApi::AudioFormat(), {}),
-      "");
-}
-#endif
-
 TEST(AudioEncoderFactoryTemplateTest, NoEncoderTypes) {
   test::ScopedKeyValueConfig field_trials;
   const Environment env = CreateEnvironment(&field_trials);