Propagate Environment to DecoderDatabase::DecoderInfo to make it available for creating AudioDecoders Bug: webrtc:356878416 Change-Id: Ibd24a55df70985dfe02d924da037618f13661032 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359241 Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42771}
diff --git a/modules/audio_coding/neteq/decoder_database.cc b/modules/audio_coding/neteq/decoder_database.cc index 3447ced..c765ec5 100644 --- a/modules/audio_coding/neteq/decoder_database.cc +++ b/modules/audio_coding/neteq/decoder_database.cc
@@ -20,6 +20,7 @@ #include "absl/strings/match.h" #include "absl/strings/string_view.h" #include "api/audio_codecs/audio_decoder.h" +#include "api/environment/environment.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/strings/audio_format_to_string.h" @@ -27,33 +28,29 @@ namespace webrtc { DecoderDatabase::DecoderDatabase( - const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory, + const Environment& env, + scoped_refptr<AudioDecoderFactory> decoder_factory, absl::optional<AudioCodecPairId> codec_pair_id) - : active_decoder_type_(-1), + : env_(env), + active_decoder_type_(-1), active_cng_decoder_type_(-1), - decoder_factory_(decoder_factory), + decoder_factory_(std::move(decoder_factory)), codec_pair_id_(codec_pair_id) {} DecoderDatabase::~DecoderDatabase() = default; DecoderDatabase::DecoderInfo::DecoderInfo( + const Environment& env, const SdpAudioFormat& audio_format, absl::optional<AudioCodecPairId> codec_pair_id, - AudioDecoderFactory* factory, - absl::string_view codec_name) - : name_(codec_name), + AudioDecoderFactory* factory) + : env_(env), audio_format_(audio_format), codec_pair_id_(codec_pair_id), factory_(factory), cng_decoder_(CngDecoder::Create(audio_format)), subtype_(SubtypeFromFormat(audio_format)) {} -DecoderDatabase::DecoderInfo::DecoderInfo( - const SdpAudioFormat& audio_format, - absl::optional<AudioCodecPairId> codec_pair_id, - AudioDecoderFactory* factory) - : DecoderInfo(audio_format, codec_pair_id, factory, audio_format.name) {} - DecoderDatabase::DecoderInfo::DecoderInfo(DecoderInfo&&) = default; DecoderDatabase::DecoderInfo::~DecoderInfo() = default; @@ -133,8 +130,8 @@ RTC_DCHECK_LE(rtp_payload_type, 0x7f); if (decoders_.count(rtp_payload_type) == 0) { decoders_.insert(std::make_pair( - rtp_payload_type, - DecoderInfo(audio_format, codec_pair_id_, decoder_factory_.get()))); + rtp_payload_type, DecoderInfo(env_, audio_format, codec_pair_id_, + decoder_factory_.get()))); } else { // The mapping for this payload type hasn't changed. } @@ -150,7 +147,7 @@ } const auto ret = decoders_.insert(std::make_pair( rtp_payload_type, - DecoderInfo(audio_format, codec_pair_id_, decoder_factory_.get()))); + DecoderInfo(env_, audio_format, codec_pair_id_, decoder_factory_.get()))); if (ret.second == false) { // Database already contains a decoder with type `rtp_payload_type`. return kDecoderExists;
diff --git a/modules/audio_coding/neteq/decoder_database.h b/modules/audio_coding/neteq/decoder_database.h index 8cf2019..f70d255 100644 --- a/modules/audio_coding/neteq/decoder_database.h +++ b/modules/audio_coding/neteq/decoder_database.h
@@ -18,6 +18,7 @@ #include "absl/strings/string_view.h" #include "api/audio_codecs/audio_decoder_factory.h" #include "api/audio_codecs/audio_format.h" +#include "api/environment/environment.h" #include "api/scoped_refptr.h" #include "modules/audio_coding/codecs/cng/webrtc_cng.h" #include "modules/audio_coding/neteq/packet.h" @@ -39,13 +40,10 @@ // Class that stores decoder info in the database. class DecoderInfo { public: - DecoderInfo(const SdpAudioFormat& audio_format, + DecoderInfo(const Environment& env, + const SdpAudioFormat& audio_format, absl::optional<AudioCodecPairId> codec_pair_id, - AudioDecoderFactory* factory, - absl::string_view codec_name); - explicit DecoderInfo(const SdpAudioFormat& audio_format, - absl::optional<AudioCodecPairId> codec_pair_id, - AudioDecoderFactory* factory = nullptr); + AudioDecoderFactory* factory); DecoderInfo(DecoderInfo&&); ~DecoderInfo(); @@ -83,14 +81,10 @@ // Returns true if the decoder's format is named `name`. bool IsType(absl::string_view name) const; - const std::string& get_name() const { return name_; } + const std::string& get_name() const { return audio_format_.name; } private: - // TODO(ossu): `name_` is kept here while we retain the old external - // decoder interface. Remove this once using an - // AudioDecoderFactory has supplanted the old functionality. - const std::string name_; - + const Environment env_; const SdpAudioFormat audio_format_; const absl::optional<AudioCodecPairId> codec_pair_id_; AudioDecoderFactory* const factory_; @@ -114,9 +108,9 @@ // only 7 bits). static const uint8_t kRtpPayloadTypeError = 0xFF; - DecoderDatabase( - const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory, - absl::optional<AudioCodecPairId> codec_pair_id); + DecoderDatabase(const Environment& env, + scoped_refptr<AudioDecoderFactory> decoder_factory, + absl::optional<AudioCodecPairId> codec_pair_id); virtual ~DecoderDatabase(); @@ -192,11 +186,12 @@ private: typedef std::map<uint8_t, DecoderInfo> DecoderMap; + const Environment env_; DecoderMap decoders_; int active_decoder_type_; int active_cng_decoder_type_; mutable std::unique_ptr<ComfortNoiseDecoder> active_cng_decoder_; - rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_; + scoped_refptr<AudioDecoderFactory> decoder_factory_; const absl::optional<AudioCodecPairId> codec_pair_id_; };
diff --git a/modules/audio_coding/neteq/decoder_database_unittest.cc b/modules/audio_coding/neteq/decoder_database_unittest.cc index 445c219..8460d69 100644 --- a/modules/audio_coding/neteq/decoder_database_unittest.cc +++ b/modules/audio_coding/neteq/decoder_database_unittest.cc
@@ -15,6 +15,7 @@ #include <string> #include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/environment/environment_factory.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/mock_audio_decoder.h" @@ -26,15 +27,17 @@ namespace webrtc { TEST(DecoderDatabase, CreateAndDestroy) { - DecoderDatabase db(rtc::make_ref_counted<MockAudioDecoderFactory>(), + DecoderDatabase db(CreateEnvironment(), + make_ref_counted<MockAudioDecoderFactory>(), absl::nullopt); EXPECT_EQ(0, db.Size()); EXPECT_TRUE(db.Empty()); } TEST(DecoderDatabase, InsertAndRemove) { - auto factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); - DecoderDatabase db(factory, absl::nullopt); + DecoderDatabase db(CreateEnvironment(), + make_ref_counted<MockAudioDecoderFactory>(), + absl::nullopt); const uint8_t kPayloadType = 0; const std::string kCodecName = "Robert\'); DROP TABLE Students;"; EXPECT_EQ( @@ -48,8 +51,9 @@ } TEST(DecoderDatabase, InsertAndRemoveAll) { - auto factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); - DecoderDatabase db(factory, absl::nullopt); + DecoderDatabase db(CreateEnvironment(), + make_ref_counted<MockAudioDecoderFactory>(), + absl::nullopt); const std::string kCodecName1 = "Robert\'); DROP TABLE Students;"; const std::string kCodecName2 = "https://xkcd.com/327/"; EXPECT_EQ(DecoderDatabase::kOK, @@ -73,7 +77,7 @@ EXPECT_EQ("pcmu", format.name); dec->reset(decoder); })); - DecoderDatabase db(factory, absl::nullopt); + DecoderDatabase db(CreateEnvironment(), std::move(factory), absl::nullopt); const uint8_t kPayloadType = 0; const std::string kCodecName = "pcmu"; EXPECT_EQ( @@ -90,7 +94,8 @@ } TEST(DecoderDatabase, GetDecoder) { - DecoderDatabase db(CreateBuiltinAudioDecoderFactory(), absl::nullopt); + DecoderDatabase db(CreateEnvironment(), CreateBuiltinAudioDecoderFactory(), + absl::nullopt); const uint8_t kPayloadType = 0; EXPECT_EQ(DecoderDatabase::kOK, db.RegisterPayload(kPayloadType, SdpAudioFormat("l16", 8000, 1))); @@ -99,8 +104,9 @@ } TEST(DecoderDatabase, TypeTests) { - auto factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); - DecoderDatabase db(factory, absl::nullopt); + DecoderDatabase db(CreateEnvironment(), + make_ref_counted<MockAudioDecoderFactory>(), + absl::nullopt); const uint8_t kPayloadTypePcmU = 0; const uint8_t kPayloadTypeCng = 13; const uint8_t kPayloadTypeDtmf = 100; @@ -133,8 +139,9 @@ TEST(DecoderDatabase, CheckPayloadTypes) { constexpr int kNumPayloads = 10; - auto factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); - DecoderDatabase db(factory, absl::nullopt); + DecoderDatabase db(CreateEnvironment(), + make_ref_counted<MockAudioDecoderFactory>(), + absl::nullopt); // Load a number of payloads into the database. Payload types are 0, 1, ..., // while the decoder type is the same for all payload types (this does not // matter for the test). @@ -175,7 +182,8 @@ // Test the methods for setting and getting active speech and CNG decoders. TEST(DecoderDatabase, IF_ISAC(ActiveDecoders)) { - DecoderDatabase db(CreateBuiltinAudioDecoderFactory(), absl::nullopt); + DecoderDatabase db(CreateEnvironment(), CreateBuiltinAudioDecoderFactory(), + absl::nullopt); // Load payload types. ASSERT_EQ(DecoderDatabase::kOK, db.RegisterPayload(0, SdpAudioFormat("pcmu", 8000, 1)));
diff --git a/modules/audio_coding/neteq/mock/mock_decoder_database.h b/modules/audio_coding/neteq/mock/mock_decoder_database.h index 2394120..3adae87 100644 --- a/modules/audio_coding/neteq/mock/mock_decoder_database.h +++ b/modules/audio_coding/neteq/mock/mock_decoder_database.h
@@ -13,6 +13,7 @@ #include <string> +#include "api/environment/environment_factory.h" #include "modules/audio_coding/neteq/decoder_database.h" #include "test/gmock.h" @@ -20,9 +21,10 @@ class MockDecoderDatabase : public DecoderDatabase { public: - explicit MockDecoderDatabase( - rtc::scoped_refptr<AudioDecoderFactory> factory = nullptr) - : DecoderDatabase(factory, absl::nullopt) {} + MockDecoderDatabase() + : DecoderDatabase(CreateEnvironment(), + /*decoder_factory=*/nullptr, + /*codec_pair_id=*/absl::nullopt) {} ~MockDecoderDatabase() override { Die(); } MOCK_METHOD(void, Die, ()); MOCK_METHOD(bool, Empty, (), (const, override));
diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index 01a4d7f..cb16820 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -115,7 +115,9 @@ tick_timer(new TickTimer), stats(new StatisticsCalculator), decoder_database( - new DecoderDatabase(decoder_factory, config.codec_pair_id)), + std::make_unique<DecoderDatabase>(env, + std::move(decoder_factory), + config.codec_pair_id)), dtmf_buffer(new DtmfBuffer(config.sample_rate_hz)), dtmf_tone_generator(new DtmfToneGenerator), packet_buffer(new PacketBuffer(config.max_packets_in_buffer,
diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index c28bd8c..888c459 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -307,6 +307,7 @@ fake_packet.sequence_number = kFirstSequenceNumber; fake_packet.timestamp = kFirstTimestamp; + const Environment env = CreateEnvironment(); auto mock_decoder_factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); EXPECT_CALL(*mock_decoder_factory, MakeAudioDecoderMock(_, _, _)) .WillOnce(Invoke([&](const SdpAudioFormat& format, @@ -321,7 +322,7 @@ *dec = std::move(mock_decoder); })); - DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1), + DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("pcmu", 8000, 1), absl::nullopt, mock_decoder_factory.get()); // Expectations for decoder database. @@ -1644,6 +1645,7 @@ AudioDecoder* decoder = nullptr; + const Environment env = CreateEnvironment(); auto mock_decoder_factory = rtc::make_ref_counted<MockAudioDecoderFactory>(); EXPECT_CALL(*mock_decoder_factory, MakeAudioDecoderMock(_, _, _)) .WillOnce(Invoke([&](const SdpAudioFormat& format, @@ -1653,7 +1655,7 @@ *dec = std::make_unique<AudioDecoderPcmU>(1000); decoder = dec->get(); })); - DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1), + DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("pcmu", 8000, 1), absl::nullopt, mock_decoder_factory.get()); // Expectations for decoder database. EXPECT_CALL(*mock_decoder_database_, GetDecoderInfo(kPayloadType))
diff --git a/modules/audio_coding/neteq/red_payload_splitter_unittest.cc b/modules/audio_coding/neteq/red_payload_splitter_unittest.cc index 55f9bee..5e84c75 100644 --- a/modules/audio_coding/neteq/red_payload_splitter_unittest.cc +++ b/modules/audio_coding/neteq/red_payload_splitter_unittest.cc
@@ -16,6 +16,8 @@ #include <utility> // pair #include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/environment/environment.h" +#include "api/environment/environment_factory.h" #include "modules/audio_coding/neteq/decoder_database.h" #include "modules/audio_coding/neteq/packet.h" #include "rtc_base/numerics/safe_conversions.h" @@ -286,6 +288,7 @@ // is a non-CNG, non-DTMF payload of another type than the first speech payload // found in the list (which is PCMu). TEST(RedPayloadSplitter, CheckRedPayloads) { + const Environment env = CreateEnvironment(); PacketList packet_list; for (uint8_t i = 0; i <= 3; ++i) { // Create packet with payload type `i`, payload length 10 bytes, all 0. @@ -296,7 +299,7 @@ // easier to just register the payload types and let the actual implementation // do its job. DecoderDatabase decoder_database( - rtc::make_ref_counted<MockAudioDecoderFactory>(), absl::nullopt); + env, make_ref_counted<MockAudioDecoderFactory>(), absl::nullopt); decoder_database.RegisterPayload(0, SdpAudioFormat("cn", 8000, 1)); decoder_database.RegisterPayload(1, SdpAudioFormat("pcmu", 8000, 1)); decoder_database.RegisterPayload(2, @@ -321,6 +324,7 @@ // for RED. That is, some kind of weird nested RED packet. This is not supported // and the splitter should discard all packets. TEST(RedPayloadSplitter, CheckRedPayloadsRecursiveRed) { + const Environment env = CreateEnvironment(); PacketList packet_list; for (uint8_t i = 0; i <= 3; ++i) { // Create packet with RED payload type, payload length 10 bytes, all 0. @@ -331,7 +335,7 @@ // easier to just register the payload types and let the actual implementation // do its job. DecoderDatabase decoder_database( - rtc::make_ref_counted<MockAudioDecoderFactory>(), absl::nullopt); + env, make_ref_counted<MockAudioDecoderFactory>(), absl::nullopt); decoder_database.RegisterPayload(kRedPayloadType, SdpAudioFormat("red", 8000, 1));
diff --git a/modules/audio_coding/neteq/timestamp_scaler_unittest.cc b/modules/audio_coding/neteq/timestamp_scaler_unittest.cc index c2bb4dd..804e4d8 100644 --- a/modules/audio_coding/neteq/timestamp_scaler_unittest.cc +++ b/modules/audio_coding/neteq/timestamp_scaler_unittest.cc
@@ -11,6 +11,8 @@ #include "modules/audio_coding/neteq/timestamp_scaler.h" #include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/environment/environment.h" +#include "api/environment/environment_factory.h" #include "modules/audio_coding/neteq/mock/mock_decoder_database.h" #include "modules/audio_coding/neteq/packet.h" #include "test/gmock.h" @@ -23,10 +25,11 @@ namespace webrtc { TEST(TimestampScaler, TestNoScaling) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use PCMu, because it doesn't use scaled timestamps. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("pcmu", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 0; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -45,10 +48,11 @@ } TEST(TimestampScaler, TestNoScalingLargeStep) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use PCMu, because it doesn't use scaled timestamps. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("pcmu", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 0; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -72,10 +76,11 @@ } TEST(TimestampScaler, TestG722) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("g722", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -98,10 +103,11 @@ } TEST(TimestampScaler, TestG722LargeStep) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("g722", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -128,13 +134,14 @@ } TEST(TimestampScaler, TestG722WithCng) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info_g722(SdpAudioFormat("g722", 8000, 1), - absl::nullopt, factory.get()); - const DecoderDatabase::DecoderInfo info_cng(SdpAudioFormat("cn", 16000, 1), - absl::nullopt, factory.get()); + const DecoderDatabase::DecoderInfo info_g722( + env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); + const DecoderDatabase::DecoderInfo info_cng( + env, SdpAudioFormat("cn", 16000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadTypeG722 = 17; static const uint8_t kRtpPayloadTypeCng = 13; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadTypeG722)) @@ -172,10 +179,11 @@ // Since it is simply calling the other ToInternal method, we are not doing // as many tests here. TEST(TimestampScaler, TestG722Packet) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("g722", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -202,10 +210,11 @@ // correctly. Since it is simply calling the ToInternal(Packet* packet) method, // we are not doing as many tests here. TEST(TimestampScaler, TestG722PacketList) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("g722", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -236,10 +245,11 @@ } TEST(TimestampScaler, TestG722Reset) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("g722", 8000, 1), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("g722", 8000, 1), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) @@ -278,9 +288,10 @@ // keep it, since it can be taken as a test case for the situation of a trivial // timestamp scaler. TEST(TimestampScaler, TestOpusLargeStep) { + const Environment env = CreateEnvironment(); MockDecoderDatabase db; auto factory = CreateBuiltinAudioDecoderFactory(); - const DecoderDatabase::DecoderInfo info(SdpAudioFormat("opus", 48000, 2), + const DecoderDatabase::DecoderInfo info(env, SdpAudioFormat("opus", 48000, 2), absl::nullopt, factory.get()); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))