diff --git a/api/audio_codecs/audio_encoder_factory.h b/api/audio_codecs/audio_encoder_factory.h index 52716939e5..066a15ee0e 100644 --- a/api/audio_codecs/audio_encoder_factory.h +++ b/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 codec_pair_id; }; @@ -43,55 +56,14 @@ class AudioEncoderFactory : public RefCountInterface { virtual absl::optional QueryAudioEncoder( const SdpAudioFormat& format) = 0; - // Creates an AudioEncoder for the specified format. The encoder will tags its - // 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.) - // + // Creates an AudioEncoder for the specified format. // Returns null if the format isn't supported. - // - // Note: Implementations need to be robust against combinations other than - // 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> - 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 MakeAudioEncoder( - int payload_type, + virtual absl::Nullable> Create( + const Environment& env, const SdpAudioFormat& format, - absl::optional codec_pair_id); + Options options) = 0; }; -//------------------------------------------------------------------------------ -// Implementation details follow -//------------------------------------------------------------------------------ - -inline absl::Nullable> -AudioEncoderFactory::Create(const Environment& env, - const SdpAudioFormat& format, - Options options) { - return MakeAudioEncoder(options.payload_type, format, options.codec_pair_id); -} - -inline absl::Nullable> -AudioEncoderFactory::MakeAudioEncoder( - int payload_type, - const SdpAudioFormat& format, - absl::optional 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_ diff --git a/api/audio_codecs/audio_encoder_factory_template.h b/api/audio_codecs/audio_encoder_factory_template.h index 14b1c5ea17..978605e570 100644 --- a/api/audio_codecs/audio_encoder_factory_template.h +++ b/api/audio_codecs/audio_encoder_factory_template.h @@ -41,12 +41,6 @@ struct Helper<> { const SdpAudioFormat& format) { return absl::nullopt; } - static std::unique_ptr MakeAudioEncoder( - int payload_type, - const SdpAudioFormat& format, - absl::optional codec_pair_id) { - return nullptr; - } static absl::Nullable> CreateAudioEncoder( const Environment& env, const SdpAudioFormat& format, @@ -90,30 +84,6 @@ absl::Nullable> CreateEncoder( options.codec_pair_id); } -template (), - int{}, - std::declval>())), - std::unique_ptr>>> -absl::Nullable> LegacyCreateEncoder( - Rank1, - const typename Trait::Config& config, - int payload_type, - absl::optional codec_pair_ids) { - return Trait::MakeAudioEncoder(config, payload_type, codec_pair_ids); -} - -template -absl::Nullable> LegacyCreateEncoder( - Rank0, - const typename Trait::Config& config, - int payload_type, - absl::optional codec_pair_ids) { - RTC_CHECK_NOTREACHED(); -} - // Inductive case: Called with n + 1 template parameters; calls subroutines // with n template parameters. template @@ -133,19 +103,6 @@ struct Helper { T::QueryAudioEncoder(*opt_config)) : Helper::QueryAudioEncoder(format); } - static std::unique_ptr MakeAudioEncoder( - int payload_type, - const SdpAudioFormat& format, - absl::optional codec_pair_id) { - auto opt_config = T::SdpToConfig(format); - if (opt_config) { - return LegacyCreateEncoder(Rank1{}, *opt_config, payload_type, - codec_pair_id); - } else { - return Helper::MakeAudioEncoder(payload_type, format, - codec_pair_id); - } - } static absl::Nullable> CreateAudioEncoder( const Environment& env, @@ -172,13 +129,6 @@ class AudioEncoderFactoryT : public AudioEncoderFactory { return Helper::QueryAudioEncoder(format); } - std::unique_ptr MakeAudioEncoder( - int payload_type, - const SdpAudioFormat& format, - absl::optional codec_pair_id) override { - return Helper::MakeAudioEncoder(payload_type, format, codec_pair_id); - } - absl::Nullable> 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. diff --git a/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc b/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc index 5b8e35dc8d..7b08fbb391 100644 --- a/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc +++ b/api/audio_codecs/test/audio_encoder_factory_template_unittest.cc @@ -161,11 +161,6 @@ TEST(AudioEncoderFactoryTemplateTest, const Environment env = CreateEnvironment(); auto factory = CreateAudioEncoderFactory(); - 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(); - // 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);