Convert CNG into C++ and remove it from AudioDecoder

webrtc/modules/audio_coding/neteq/decoder_database.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/modules/audio_coding/neteq/decoder_database.h"
 
 #include <assert.h>
 #include <utility>  // pair
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/modules/audio_coding/codecs/audio_decoder.h"
 
 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;
 }
 
 bool DecoderDatabase::Empty() const { return decoders_.empty(); }
 
 int DecoderDatabase::Size() const { return static_cast<int>(decoders_.size()); }
 
 void DecoderDatabase::Reset() {
@@ skipping 79 matching lines @@
     if ((*it).second.codec_type == codec_type) {
       // Match found.
       return (*it).first;
     }
   }
   // No match.
   return kRtpPayloadTypeError;
 }
 
 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;
   }
   DecoderMap::iterator it = decoders_.find(rtp_payload_type);
   if (it == decoders_.end()) {
     // Decoder not found.
     return NULL;
   }
   DecoderInfo* info = &(*it).second;
   if (!info->decoder) {
@@ skipping 35 matching lines @@
 }
 
 int DecoderDatabase::SetActiveDecoder(uint8_t rtp_payload_type,
                                       bool* new_decoder) {
   // Check that |rtp_payload_type| exists in the database.
   DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
   if (it == decoders_.end()) {
     // Decoder not found.
     return kDecoderNotFound;
   }
+  RTC_CHECK(!IsComfortNoise(rtp_payload_type));
   assert(new_decoder);
   *new_decoder = false;
   if (active_decoder_ < 0) {
     // This is the first active decoder.
     *new_decoder = true;
   } else if (active_decoder_ != rtp_payload_type) {
     // Moving from one active decoder to another. Delete the first one.
     DecoderMap::iterator it = decoders_.find(active_decoder_);
     if (it == decoders_.end()) {
       // Decoder not found. This should not be possible.
@@ skipping 28 matching lines @@
     return kDecoderNotFound;
   }
   if (active_cng_decoder_ >= 0 && active_cng_decoder_ != rtp_payload_type) {
     // Moving from one active CNG decoder to another. Delete the first one.
     DecoderMap::iterator it = decoders_.find(active_cng_decoder_);
     if (it == decoders_.end()) {
       // Decoder not found. This should not be possible.
       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 {
   PacketList::const_iterator it;
   for (it = packet_list.begin(); it != packet_list.end(); ++it) {
     if (decoders_.find((*it)->header.payloadType) == decoders_.end()) {
       // Payload type is not found.
       LOG(LS_WARNING) << "CheckPayloadTypes: unknown RTP payload type "
                       << static_cast<int>((*it)->header.payloadType);
       return kDecoderNotFound;
     }
   }
   return kOK;
 }
 
 
 }  // namespace webrtc