Convert CNG into C++ and remove it from AudioDecoder

webrtc/modules/audio_coding/neteq/decoder_database.cc

Index: webrtc/modules/audio_coding/neteq/decoder_database.cc
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc
index 92d4bab1e4a573fb3c84ef53fa724ebfeabe860c..03e79960d4abc7c430977edf966859cf6900969d 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.cc
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc
@@ -20,9 +20,13 @@
 namespace webrtc {
 
 DecoderDatabase::DecoderDatabase()
-    : active_decoder_(-1), active_cng_decoder_(-1) {}
+    : active_decoder_(-1), active_cng_decoder_(-1),
+      active_cng_dec_inst_(nullptr) { }
 
-DecoderDatabase::~DecoderDatabase() {}
+DecoderDatabase::~DecoderDatabase() {
+  if (active_cng_dec_inst_)
+    WebRtcCng_FreeDec(active_cng_dec_inst_);

[kwiberg-webrtc, 2016/04/09 07:34:49]

Having to write destructor code manually is almost always a sign that you're
doing something wrong... Please use something like unique_ptr to own the CNG
decoder. (You'll either have to ensure that the decoder's destructor calls
WebRtcCng_FreeDec, or have a custom deleter that does that.)

(Having looked through the rest of the CL, I think wrapping the CNG struct in a
C++ class with a proper destructor would be best. That way, you could also wrap
the CNG methods you use and give them more convenient signatures.)

[ossu, 2016/04/11 08:31:10]

I think wrapping it is probably the best. This is just the minimal amount needed
to get CNG out of the AudioDecoder interface. I was hoping to just put it aside
for a while, do the whole external decoder thing, then see how one would fold
CNG back into that. Still, doesn't hurt to have better code during that work. :)

[hlundin-webrtc, 2016/04/11 11:33:00]

I think it is ok to leave some of this debt behind for now; the ends justify the
means.

As I said in another place, I'm not so sure the CNG functionality should be
folded back in with the AudioDecoder interface. The calling pattern is different
between a regular decoder and a CNG decoder.

+}
 
 DecoderDatabase::DecoderInfo::~DecoderInfo() {
   if (!external) delete decoder;
@@ -122,7 +126,8 @@ uint8_t DecoderDatabase::GetRtpPayloadType(
 }
 
 AudioDecoder* DecoderDatabase::GetDecoder(uint8_t rtp_payload_type) {
-  if (IsDtmf(rtp_payload_type) || IsRed(rtp_payload_type)) {
+  if (IsDtmf(rtp_payload_type) || IsRed(rtp_payload_type) ||
+      IsComfortNoise(rtp_payload_type)) {
     // These are not real decoders.
     return NULL;
   }
@@ -178,6 +183,7 @@ int DecoderDatabase::SetActiveDecoder(uint8_t rtp_payload_type,
     // Decoder not found.
     return kDecoderNotFound;
   }
+  RTC_CHECK(!IsComfortNoise(rtp_payload_type));
   assert(new_decoder);
   *new_decoder = false;
   if (active_decoder_ < 0) {
@@ -226,23 +232,26 @@ int DecoderDatabase::SetActiveCngDecoder(uint8_t rtp_payload_type) {
       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;
+    RTC_CHECK(!(*it).second.external);

[kwiberg-webrtc, 2016/04/09 07:34:49]

Since you're rewriting this line anyway, please change (*a).b to a->b.

[ossu, 2016/04/11 08:31:10]

Acknowledged.

+    if (active_cng_dec_inst_) {
+      WebRtcCng_FreeDec(active_cng_dec_inst_);
+      active_cng_dec_inst_ = nullptr;
     }
   }
   active_cng_decoder_ = rtp_payload_type;
   return kOK;
 }
 
-AudioDecoder* DecoderDatabase::GetActiveCngDecoder() {
+CNG_dec_inst* DecoderDatabase::GetActiveCngDecoder() {
   if (active_cng_decoder_ < 0) {
     // No active CNG decoder.
     return NULL;
   }
-  return GetDecoder(active_cng_decoder_);
+  if (!active_cng_dec_inst_) {
+    RTC_CHECK_EQ(0, WebRtcCng_CreateDec(&active_cng_dec_inst_));
+    WebRtcCng_InitDec(active_cng_dec_inst_);
+  }
+  return active_cng_dec_inst_;
 }
 
 int DecoderDatabase::CheckPayloadTypes(const PacketList& packet_list) const {