diff --git a/api/audio_codecs/audio_decoder_factory_template.h b/api/audio_codecs/audio_decoder_factory_template.h index 6490f4c2a9..f72531c448 100644 --- a/api/audio_codecs/audio_decoder_factory_template.h +++ b/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* specs) {} static bool IsSupportedDecoder(const SdpAudioFormat& format) { return false; } - static std::unique_ptr MakeAudioDecoder( + + static absl::Nullable> MakeAudioDecoder( + const Environment& env, const SdpAudioFormat& format, absl::optional codec_pair_id) { return nullptr; } }; +// Use ranked overloads (abseil.io/tips/229) for dispatching. +struct Rank0 {}; +struct Rank1 : Rank0 {}; + +template (), + std::declval(), + std::declval>())), + std::unique_ptr>>> +absl::Nullable> CreateDecoder( + Rank1, + const Environment& env, + const typename Trait::Config& config, + absl::optional codec_pair_id) { + return Trait::MakeAudioDecoder(env, config, codec_pair_id); +} + +template (), + std::declval>())), + std::unique_ptr>>> +absl::Nullable> CreateDecoder( + Rank0, + const Environment& env, + const typename Trait::Config& config, + absl::optional 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 @@ -58,17 +94,15 @@ struct Helper { "absl::optional"); return opt_config ? true : Helper::IsSupportedDecoder(format); } - // TODO: bugs.webrtc.org/356878416 - Migrate to `Create` - static std::unique_ptr MakeAudioDecoder( + + static absl::Nullable> MakeAudioDecoder( + const Environment& env, const SdpAudioFormat& format, absl::optional 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::MakeAudioDecoder(format, codec_pair_id); + return opt_config.has_value() + ? CreateDecoder(Rank1{}, env, *opt_config, codec_pair_id) + : Helper::MakeAudioDecoder(env, format, codec_pair_id); } }; @@ -85,10 +119,11 @@ class AudioDecoderFactoryT : public AudioDecoderFactory { return Helper::IsSupportedDecoder(format); } - std::unique_ptr MakeAudioDecoder( + absl::Nullable> Create( + const Environment& env, const SdpAudioFormat& format, absl::optional codec_pair_id) override { - return Helper::MakeAudioDecoder(format, codec_pair_id); + return Helper::MakeAudioDecoder(env, format, codec_pair_id); } }; @@ -109,7 +144,12 @@ class AudioDecoderFactoryT : public AudioDecoderFactory { // void AppendSupportedDecoders(std::vector* specs); // // // Creates an AudioDecoder for the specified format. Used to implement -// // AudioDecoderFactory::MakeAudioDecoder(). +// // AudioDecoderFactory::Create(). +// std::unique_ptr MakeAudioDecoder( +// const Environment& env, +// const ConfigType& config, +// absl::optional codec_pair_id); +// or // std::unique_ptr MakeAudioDecoder( // const ConfigType& config, // absl::optional codec_pair_id); diff --git a/api/audio_codecs/opus/audio_decoder_opus.h b/api/audio_codecs/opus/audio_decoder_opus.h index b4d84cb2d5..3ab430db3a 100644 --- a/api/audio_codecs/opus/audio_decoder_opus.h +++ b/api/audio_codecs/opus/audio_decoder_opus.h @@ -44,6 +44,12 @@ struct RTC_EXPORT AudioDecoderOpus { static std::unique_ptr MakeAudioDecoder(const Environment& env, Config config); + static std::unique_ptr MakeAudioDecoder( + const Environment& env, + Config config, + absl::optional /*codec_pair_id*/) { + return MakeAudioDecoder(env, config); + } }; } // namespace webrtc diff --git a/api/audio_codecs/test/audio_decoder_factory_template_unittest.cc b/api/audio_codecs/test/audio_decoder_factory_template_unittest.cc index d42ab484fe..134e0f5785 100644 --- a/api/audio_codecs/test/audio_decoder_factory_template_unittest.cc +++ b/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 SdpToConfig( + const SdpAudioFormat& audio_format) { + return Config(); + } + + static void AppendSupportedDecoders(std::vector* 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 MakeAudioDecoder( + const Config& config, + absl::optional codec_pair_id) { + auto decoder = std::make_unique>(); + ON_CALL(*decoder, SampleRateHz).WillByDefault(Return(kRateWithoutEnv)); + return decoder; + } + + static std::unique_ptr MakeAudioDecoder( + const Environment& env, + const Config& config, + absl::optional codec_pair_id) { + auto decoder = std::make_unique>(); + ON_CALL(*decoder, SampleRateHz).WillByDefault(Return(kRateWithEnv)); + return decoder; + } +}; + +TEST(AudioDecoderFactoryTemplateTest, + PrefersToPassEnvironmentToMakeAudioDecoder) { + const Environment env = CreateEnvironment(); + auto factory = CreateAudioDecoderFactory(); + + EXPECT_THAT(factory->Create(env, BaseAudioDecoderApi::AudioFormat(), {}), + Pointer(Property(&AudioDecoder::SampleRateHz, + TraitWithTwoMakeAudioDecoders::kRateWithEnv))); +} + +struct AudioDecoderApiWithV1Make : BaseAudioDecoderApi { + static std::unique_ptr MakeAudioDecoder( + const Config& config, + absl::optional codec_pair_id) { + return std::make_unique>(); + } +}; + +TEST(AudioDecoderFactoryTemplateTest, + CanUseMakeAudioDecoderWithoutPassingEnvironment) { + const Environment env = CreateEnvironment(); + auto factory = CreateAudioDecoderFactory(); + 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