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}

webrtc/modules/audio_coding/neteq/decoder_database.cc

@@ -24,8 +24,29 @@ DecoderDatabase::DecoderDatabase()
 
 DecoderDatabase::~DecoderDatabase() {}
 
-DecoderDatabase::DecoderInfo::~DecoderInfo() {
+DecoderDatabase::DecoderInfo::DecoderInfo(NetEqDecoder ct,
-  if (!external) delete decoder;
+                                          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 @@ int DecoderDatabase::RegisterPayload(uint8_t rtp_payload_type,
     return kCodecNotSupported;
   }
   const int fs_hz = CodecSampleRateHz(codec_type);
-  DecoderInfo info(codec_type, name, fs_hz, NULL, false);
+  DecoderInfo info(codec_type, name, fs_hz, nullptr);
-  auto ret = decoders_.insert(std::make_pair(rtp_payload_type, info));
+  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 @@ int DecoderDatabase::InsertExternal(uint8_t rtp_payload_type,
     return kInvalidPointer;
   }
   std::pair<DecoderMap::iterator, bool> ret;
-  DecoderInfo info(codec_type, codec_name, fs_hz, decoder, true);
+  DecoderInfo info(codec_type, codec_name, fs_hz, decoder);
-  ret = decoders_.insert(std::make_pair(rtp_payload_type, info));
+  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 @@ AudioDecoder* DecoderDatabase::GetDecoder(uint8_t rtp_payload_type) {
     return NULL;
   }
   DecoderInfo* info = &(*it).second;
-  if (!info->decoder) {
+  return info->GetDecoder();
-    // 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;
 }
 
 bool DecoderDatabase::IsType(uint8_t rtp_payload_type,
@@ -191,12 +207,7 @@ int DecoderDatabase::SetActiveDecoder(uint8_t rtp_payload_type,
       assert(false);
       return kDecoderNotFound;
     }
-    if (!(*it).second.external) {
+    it->second.DropDecoder();
-      // Delete the AudioDecoder object, unless it is an externally created
-      // decoder.
-      delete (*it).second.decoder;
-      (*it).second.decoder = NULL;
-    }
     *new_decoder = true;
   }
   active_decoder_ = rtp_payload_type;
@@ -226,12 +237,7 @@ int DecoderDatabase::SetActiveCngDecoder(uint8_t rtp_payload_type) {
       assert(false);
       return kDecoderNotFound;
     }
-    if (!(*it).second.external) {
+    it->second.DropDecoder();
-      // Delete the AudioDecoder object, unless it is an externally created
-      // decoder.
-      delete (*it).second.decoder;
-      (*it).second.decoder = NULL;
-    }
   }
   active_cng_decoder_ = rtp_payload_type;
   return kOK;

webrtc/modules/audio_coding/neteq/decoder_database.h

@@ -34,30 +34,31 @@ class DecoderDatabase {
     kInvalidPointer = -6
   };
 
-  // Struct used to store decoder info in the database.
+  // Class that stores decoder info in the database.
-  struct DecoderInfo {
+  class DecoderInfo {
-    DecoderInfo() = default;
+   public:
-    DecoderInfo(NetEqDecoder ct, int fs, AudioDecoder* dec, bool ext)
-        : DecoderInfo(ct, "", fs, dec, ext) {}
     DecoderInfo(NetEqDecoder ct,
                 const std::string& nm,
                 int fs,
-                AudioDecoder* dec,
+                AudioDecoder* ext_dec);
-                bool ext)
+    DecoderInfo(DecoderInfo&&);
-        : codec_type(ct),
-          name(nm),
-          fs_hz(fs),
-          rtp_sample_rate_hz(fs),
-          decoder(dec),
-          external(ext) {}
     ~DecoderInfo();
 
-    NetEqDecoder codec_type = NetEqDecoder::kDecoderArbitrary;
+    // Get the AudioDecoder object, creating it first if necessary.
-    std::string name;
+    AudioDecoder* GetDecoder();
-    int fs_hz = 8000;
+
-    int rtp_sample_rate_hz = 8000;
+    // Delete the AudioDecoder object, unless it's external. (This means we can
-    AudioDecoder* decoder = nullptr;
+    // always recreate it later if we need it.)
-    bool external = false;
+    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

webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc

@@ -53,10 +53,9 @@ TEST(DecoderDatabase, GetDecoderInfo) {
   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 @@ TEST(DecoderDatabase, ExternalDecoder) {
   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);

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc

@@ -301,8 +301,8 @@ TEST_F(NetEqImplTest, InsertPacket) {
       .WillRepeatedly(Return(&mock_decoder));
   EXPECT_CALL(*mock_decoder_database_, IsComfortNoise(kPayloadType))
       .WillRepeatedly(Return(false));  // This is not CNG.
-  DecoderDatabase::DecoderInfo info;
+  DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
-  info.codec_type = NetEqDecoder::kDecoderPCMu;
+                                    nullptr);
   EXPECT_CALL(*mock_decoder_database_, GetDecoderInfo(kPayloadType))
       .WillRepeatedly(Return(&info));
 

webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc

@@ -372,33 +372,33 @@ TEST(AudioPayloadSplitter, NonSplittable) {
   // 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,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISAC, "", 16000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(0))
       .WillRepeatedly(Return(info0.get()));
   std::unique_ptr<DecoderDatabase::DecoderInfo> info1(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISACswb, 32000,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderISACswb, "", 32000,
-                                       NULL, false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(1))
       .WillRepeatedly(Return(info1.get()));
   std::unique_ptr<DecoderDatabase::DecoderInfo> info2(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderRED, 8000, NULL,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderRED, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(2))
       .WillRepeatedly(Return(info2.get()));
   std::unique_ptr<DecoderDatabase::DecoderInfo> info3(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderAVT, 8000, NULL,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderAVT, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(3))
       .WillRepeatedly(Return(info3.get()));
   std::unique_ptr<DecoderDatabase::DecoderInfo> info4(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderCNGnb, 8000, NULL,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderCNGnb, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(4))
       .WillRepeatedly(Return(info4.get()));
   std::unique_ptr<DecoderDatabase::DecoderInfo> info5(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderArbitrary, 8000,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderArbitrary, "",
-                                       NULL, false));
+                                       8000, nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(5))
       .WillRepeatedly(Return(info5.get()));
 
@@ -536,7 +536,7 @@ TEST_P(SplitBySamplesTest, PayloadSizes) {
   // 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 @@ TEST_P(SplitIlbcTest, NumFrames) {
   // 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,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
       .WillRepeatedly(Return(info.get()));
 
@@ -687,8 +687,8 @@ TEST(IlbcPayloadSplitter, TooLargePayload) {
 
   MockDecoderDatabase decoder_database;
   std::unique_ptr<DecoderDatabase::DecoderInfo> info(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, 8000, NULL,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
       .WillRepeatedly(Return(info.get()));
 
@@ -719,8 +719,8 @@ TEST(IlbcPayloadSplitter, UnevenPayload) {
 
   MockDecoderDatabase decoder_database;
   std::unique_ptr<DecoderDatabase::DecoderInfo> info(
-      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, 8000, NULL,
+      new DecoderDatabase::DecoderInfo(NetEqDecoder::kDecoderILBC, "", 8000,
-                                       false));
+                                       nullptr));
   EXPECT_CALL(decoder_database, GetDecoderInfo(kPayloadType))
       .WillRepeatedly(Return(info.get()));
 

webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc

@@ -23,9 +23,9 @@ namespace webrtc {
 
 TEST(TimestampScaler, TestNoScaling) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use PCMu, because it doesn't use scaled timestamps.
-  info.codec_type =
+  const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
-      NetEqDecoder::kDecoderPCMu;  // Does not use scaled timestamps.
+                                          nullptr);
   static const uint8_t kRtpPayloadType = 0;
   EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
       .WillRepeatedly(Return(&info));
@@ -44,9 +44,9 @@ TEST(TimestampScaler, TestNoScaling) {
 
 TEST(TimestampScaler, TestNoScalingLargeStep) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use PCMu, because it doesn't use scaled timestamps.
-  info.codec_type =
+  const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", 8000,
-      NetEqDecoder::kDecoderPCMu;  // Does not use scaled timestamps.
+                                          nullptr);
   static const uint8_t kRtpPayloadType = 0;
   EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
       .WillRepeatedly(Return(&info));
@@ -70,8 +70,9 @@ TEST(TimestampScaler, TestNoScalingLargeStep) {
 
 TEST(TimestampScaler, TestG722) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use G722, which has a factor 2 scaling.
-  info.codec_type = NetEqDecoder::kDecoderG722;  // Uses 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, TestG722) {
 
 TEST(TimestampScaler, TestG722LargeStep) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use G722, which has a factor 2 scaling.
-  info.codec_type = NetEqDecoder::kDecoderG722;  // Uses 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, TestG722LargeStep) {
 
 TEST(TimestampScaler, TestG722WithCng) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info_g722, info_cng;
+  // Use G722, which has a factor 2 scaling.
-  info_g722.codec_type =
+  const DecoderDatabase::DecoderInfo info_g722(NetEqDecoder::kDecoderG722, "",
-      NetEqDecoder::kDecoderG722;  // Uses a factor 2 scaling.
+                                               16000, nullptr);
-  info_cng.codec_type = NetEqDecoder::kDecoderCNGwb;
+  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 @@ TEST(TimestampScaler, TestG722WithCng) {
 // as many tests here.
 TEST(TimestampScaler, TestG722Packet) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use G722, which has a factor 2 scaling.
-  info.codec_type =
+  const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", 16000,
-      NetEqDecoder::kDecoderG722;  // Does uses a factor 2 scaling.
+                                          nullptr);
   static const uint8_t kRtpPayloadType = 17;
   EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
       .WillRepeatedly(Return(&info));
@@ -193,8 +196,9 @@ TEST(TimestampScaler, TestG722Packet) {
 // we are not doing as many tests here.
 TEST(TimestampScaler, TestG722PacketList) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use G722, which has a factor 2 scaling.
-  info.codec_type = NetEqDecoder::kDecoderG722;  // Uses 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, TestG722PacketList) {
 
 TEST(TimestampScaler, TestG722Reset) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  // Use G722, which has a factor 2 scaling.
-  info.codec_type = NetEqDecoder::kDecoderG722;  // Uses 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 @@ TEST(TimestampScaler, TestG722Reset) {
 // timestamp scaler.
 TEST(TimestampScaler, TestOpusLargeStep) {
   MockDecoderDatabase db;
-  DecoderDatabase::DecoderInfo info;
+  const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderOpus, "", 48000,
-  info.codec_type = NetEqDecoder::kDecoderOpus;
+                                          nullptr);
   static const uint8_t kRtpPayloadType = 17;
   EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType))
       .WillRepeatedly(Return(&info));