NetEq: Simplify DecoderDatabase::DecoderInfo
By eliminating one of the two constructors, handling decoder ownership
with a unique_ptr instead of a raw pointer, and making all member
variables const (except one, which is made private instead).
BUG=webrtc:5801
Review URL: https://codereview.webrtc.org/1899733002
Cr-Commit-Position: refs/heads/master@{#12425}
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc
index 92d4bab..44e0b4d 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.cc
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc
@@ -24,8 +24,29 @@
DecoderDatabase::~DecoderDatabase() {}
-DecoderDatabase::DecoderInfo::~DecoderInfo() {
- if (!external) delete decoder;
+DecoderDatabase::DecoderInfo::DecoderInfo(NetEqDecoder ct,
+ const std::string& nm,
+ int fs,
+ AudioDecoder* ext_dec)
+ : codec_type(ct),
+ name(nm),
+ fs_hz(fs),
+ rtp_sample_rate_hz(fs),
+ external_decoder(ext_dec) {}
+
+DecoderDatabase::DecoderInfo::DecoderInfo(DecoderInfo&&) = default;
+DecoderDatabase::DecoderInfo::~DecoderInfo() = default;
+
+AudioDecoder* DecoderDatabase::DecoderInfo::GetDecoder() {
+ if (external_decoder) {
+ RTC_DCHECK(!decoder_);
+ return external_decoder;
+ }
+ if (!decoder_) {
+ decoder_.reset(CreateAudioDecoder(codec_type));
+ }
+ RTC_DCHECK(decoder_);
+ return decoder_.get();
}
bool DecoderDatabase::Empty() const { return decoders_.empty(); }
@@ -48,8 +69,9 @@
return kCodecNotSupported;
}
const int fs_hz = CodecSampleRateHz(codec_type);
- DecoderInfo info(codec_type, name, fs_hz, NULL, false);
- auto ret = decoders_.insert(std::make_pair(rtp_payload_type, info));
+ DecoderInfo info(codec_type, name, fs_hz, nullptr);
+ auto ret =
+ decoders_.insert(std::make_pair(rtp_payload_type, std::move(info)));
if (ret.second == false) {
// Database already contains a decoder with type |rtp_payload_type|.
return kDecoderExists;
@@ -75,8 +97,8 @@
return kInvalidPointer;
}
std::pair<DecoderMap::iterator, bool> ret;
- DecoderInfo info(codec_type, codec_name, fs_hz, decoder, true);
- ret = decoders_.insert(std::make_pair(rtp_payload_type, info));
+ DecoderInfo info(codec_type, codec_name, fs_hz, decoder);
+ ret = decoders_.insert(std::make_pair(rtp_payload_type, std::move(info)));
if (ret.second == false) {
// Database already contains a decoder with type |rtp_payload_type|.
return kDecoderExists;
@@ -132,13 +154,7 @@
return NULL;
}
DecoderInfo* info = &(*it).second;
- if (!info->decoder) {
- // Create the decoder object.
- AudioDecoder* decoder = CreateAudioDecoder(info->codec_type);
- assert(decoder); // Should not be able to have an unsupported codec here.
- info->decoder = decoder;
- }
- return info->decoder;
+ return info->GetDecoder();
}
bool DecoderDatabase::IsType(uint8_t rtp_payload_type,
@@ -191,12 +207,7 @@
assert(false);
return kDecoderNotFound;
}
- if (!(*it).second.external) {
- // Delete the AudioDecoder object, unless it is an externally created
- // decoder.
- delete (*it).second.decoder;
- (*it).second.decoder = NULL;
- }
+ it->second.DropDecoder();
*new_decoder = true;
}
active_decoder_ = rtp_payload_type;
@@ -226,12 +237,7 @@
assert(false);
return kDecoderNotFound;
}
- if (!(*it).second.external) {
- // Delete the AudioDecoder object, unless it is an externally created
- // decoder.
- delete (*it).second.decoder;
- (*it).second.decoder = NULL;
- }
+ it->second.DropDecoder();
}
active_cng_decoder_ = rtp_payload_type;
return kOK;
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h
index 01ff0c9..3706b90 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.h
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.h
@@ -34,30 +34,31 @@
kInvalidPointer = -6
};
- // Struct used to store decoder info in the database.
- struct DecoderInfo {
- DecoderInfo() = default;
- DecoderInfo(NetEqDecoder ct, int fs, AudioDecoder* dec, bool ext)
- : DecoderInfo(ct, "", fs, dec, ext) {}
+ // Class that stores decoder info in the database.
+ class DecoderInfo {
+ public:
DecoderInfo(NetEqDecoder ct,
const std::string& nm,
int fs,
- AudioDecoder* dec,
- bool ext)
- : codec_type(ct),
- name(nm),
- fs_hz(fs),
- rtp_sample_rate_hz(fs),
- decoder(dec),
- external(ext) {}
+ AudioDecoder* ext_dec);
+ DecoderInfo(DecoderInfo&&);
~DecoderInfo();
- NetEqDecoder codec_type = NetEqDecoder::kDecoderArbitrary;
- std::string name;
- int fs_hz = 8000;
- int rtp_sample_rate_hz = 8000;
- AudioDecoder* decoder = nullptr;
- bool external = false;
+ // Get the AudioDecoder object, creating it first if necessary.
+ AudioDecoder* GetDecoder();
+
+ // Delete the AudioDecoder object, unless it's external. (This means we can
+ // always recreate it later if we need it.)
+ void DropDecoder() { decoder_.reset(); }
+
+ const NetEqDecoder codec_type;
+ const std::string name;
+ const int fs_hz;
+ const int rtp_sample_rate_hz;
+ AudioDecoder* const external_decoder;
+
+ private:
+ std::unique_ptr<AudioDecoder> decoder_;
};
// Maximum value for 8 bits, and an invalid RTP payload type (since it is
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc
index 85aaef1..929f844 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc
@@ -53,10 +53,9 @@
info = db.GetDecoderInfo(kPayloadType);
ASSERT_TRUE(info != NULL);
EXPECT_EQ(NetEqDecoder::kDecoderPCMu, info->codec_type);
- EXPECT_EQ(NULL, info->decoder);
+ EXPECT_EQ(nullptr, info->external_decoder);
EXPECT_EQ(8000, info->fs_hz);
EXPECT_EQ(kCodecName, info->name);
- EXPECT_FALSE(info->external);
info = db.GetDecoderInfo(kPayloadType + 1); // Other payload type.
EXPECT_TRUE(info == NULL); // Should not be found.
}
@@ -139,9 +138,8 @@
ASSERT_TRUE(info != NULL);
EXPECT_EQ(NetEqDecoder::kDecoderPCMu, info->codec_type);
EXPECT_EQ(kCodecName, info->name);
- EXPECT_EQ(&decoder, info->decoder);
+ EXPECT_EQ(&decoder, info->external_decoder);
EXPECT_EQ(8000, info->fs_hz);
- EXPECT_TRUE(info->external);
// Expect not to delete the decoder when removing it from the database, since
// it was declared externally.
EXPECT_CALL(decoder, Die()).Times(0);
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
index 561c045..e9291d1 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -301,8 +301,8 @@
.WillRepeatedly(Return(&mock_decoder));
EXPECT_CALL(*mock_decoder_database_, IsComfortNoise(kPayloadType))
.WillRepeatedly(Return(false)); // This is not CNG.
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderPCMu;
+ DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
+ nullptr);
EXPECT_CALL(*mock_decoder_database_, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(&info));
diff --git a/webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc b/webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc
index a68e8d6..98d9961 100644
--- a/webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc
@@ -372,33 +372,33 @@
// codec types.
// Use scoped pointers to avoid having to delete them later.
std::unique_ptr<DecoderDatabase::DecoderInfo> info0(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISAC, 16000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISAC, "", 16000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(0))
.WillRepeatedly(Return(info0.get()));
std::unique_ptr<DecoderDatabase::DecoderInfo> info1(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISACswb, 32000,
- NULL, false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISACswb, "", 32000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(1))
.WillRepeatedly(Return(info1.get()));
std::unique_ptr<DecoderDatabase::DecoderInfo> info2(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderRED, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderRED, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(2))
.WillRepeatedly(Return(info2.get()));
std::unique_ptr<DecoderDatabase::DecoderInfo> info3(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderAVT, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderAVT, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(3))
.WillRepeatedly(Return(info3.get()));
std::unique_ptr<DecoderDatabase::DecoderInfo> info4(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderCNGnb, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderCNGnb, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(4))
.WillRepeatedly(Return(info4.get()));
std::unique_ptr<DecoderDatabase::DecoderInfo> info5(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderArbitrary, 8000,
- NULL, false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderArbitrary, "",
+ 8000, nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(5))
.WillRepeatedly(Return(info5.get()));
@@ -536,7 +536,7 @@
// Use scoped pointers to avoid having to delete them later.
// (Sample rate is set to 8000 Hz, but does not matter.)
std::unique_ptr<DecoderDatabase::DecoderInfo> info(
- new DecoderDatabase::DecoderInfo(decoder_type_, 8000, NULL, false));
+ new DecoderDatabase::DecoderInfo(decoder_type_, "", 8000, nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(info.get()));
@@ -623,8 +623,8 @@
// codec types.
// Use scoped pointers to avoid having to delete them later.
std::unique_ptr<DecoderDatabase::DecoderInfo> info(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(info.get()));
@@ -687,8 +687,8 @@
MockDecoderDatabase decoder_database;
std::unique_ptr<DecoderDatabase::DecoderInfo> info(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(info.get()));
@@ -719,8 +719,8 @@
MockDecoderDatabase decoder_database;
std::unique_ptr<DecoderDatabase::DecoderInfo> info(
- new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, 8000, NULL,
- false));
+ new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
+ nullptr));
EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(info.get()));
diff --git a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc
index b1cb45d..adaf162 100644
--- a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc
@@ -23,9 +23,9 @@
TEST(TimestampScaler, TestNoScaling) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type =
- NetEqDecoder::kDecoderPCMu; // Does not use scaled timestamps.
+ // Use PCMu, because it doesn't use scaled timestamps.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
+ nullptr);
static const uint8_t kRtpPayloadType = 0;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -44,9 +44,9 @@
TEST(TimestampScaler, TestNoScalingLargeStep) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type =
- NetEqDecoder::kDecoderPCMu; // Does not use scaled timestamps.
+ // Use PCMu, because it doesn't use scaled timestamps.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
+ nullptr);
static const uint8_t kRtpPayloadType = 0;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -70,8 +70,9 @@
TEST(TimestampScaler, TestG722) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderG722; // Uses a factor 2 scaling.
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -94,8 +95,9 @@
TEST(TimestampScaler, TestG722LargeStep) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderG722; // Uses a factor 2 scaling.
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -122,10 +124,11 @@
TEST(TimestampScaler, TestG722WithCng) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info_g722, info_cng;
- info_g722.codec_type =
- NetEqDecoder::kDecoderG722; // Uses a factor 2 scaling.
- info_cng.codec_type = NetEqDecoder::kDecoderCNGwb;
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info_g722(NetEqDecoder::kDecoderG722, "",
+ 16000, nullptr);
+ const DecoderDatabase::DecoderInfo info_cng(NetEqDecoder::kDecoderCNGwb, "",
+ 16000, nullptr);
static const uint8_t kRtpPayloadTypeG722 = 17;
static const uint8_t kRtpPayloadTypeCng = 13;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadTypeG722))
@@ -164,9 +167,9 @@
// as many tests here.
TEST(TimestampScaler, TestG722Packet) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type =
- NetEqDecoder::kDecoderG722; // Does uses a factor 2 scaling.
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -193,8 +196,9 @@
// we are not doing as many tests here.
TEST(TimestampScaler, TestG722PacketList) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderG722; // Uses a factor 2 scaling.
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -222,8 +226,9 @@
TEST(TimestampScaler, TestG722Reset) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderG722; // Uses a factor 2 scaling.
+ // Use G722, which has a factor 2 scaling.
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));
@@ -262,8 +267,8 @@
// timestamp scaler.
TEST(TimestampScaler, TestOpusLargeStep) {
MockDecoderDatabase db;
- DecoderDatabase::DecoderInfo info;
- info.codec_type = NetEqDecoder::kDecoderOpus;
+ const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderOpus, "", 48000,
+ nullptr);
static const uint8_t kRtpPayloadType = 17;
EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
.WillRepeatedly(Return(&info));