NetEq: Use a BuiltinAudioDecoderFactory to create decoders

webrtc/modules/audio_coding/neteq/decoder_database.h

 /*
  *  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.
  */
 
 #ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_DECODER_DATABASE_H_
 #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_DECODER_DATABASE_H_
 
 #include <map>
 #include <memory>
 #include <string>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/common_types.h"  // NULL
+#include "webrtc/modules/audio_coding/codecs/audio_decoder_factory.h"
+#include "webrtc/modules/audio_coding/codecs/audio_format.h"
 #include "webrtc/modules/audio_coding/codecs/cng/webrtc_cng.h"
 #include "webrtc/modules/audio_coding/neteq/audio_decoder_impl.h"
 #include "webrtc/modules/audio_coding/neteq/packet.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 
 class DecoderDatabase {
  public:
   enum DatabaseReturnCodes {
@@ skipping 10 matching lines @@
   class DecoderInfo {
    public:
     DecoderInfo(NetEqDecoder ct,
                 const std::string& nm,
                 int fs,
                 AudioDecoder* ext_dec);
     DecoderInfo(DecoderInfo&&);
     ~DecoderInfo();
 
     // Get the AudioDecoder object, creating it first if necessary.
-    AudioDecoder* GetDecoder();
+    AudioDecoder* GetDecoder(AudioDecoderFactory* factory);
 
     // 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;
     AudioDecoder* const external_decoder;
 
    private:
+    const rtc::Optional<SdpAudioFormat> audio_format_;
     std::unique_ptr<AudioDecoder> decoder_;
   };
 
   // Maximum value for 8 bits, and an invalid RTP payload type (since it is
   // only 7 bits).
   static const uint8_t kRtpPayloadTypeError = 0xFF;
 
-  DecoderDatabase();
+  DecoderDatabase(std::unique_ptr<AudioDecoderFactory> decoder_factory);
 
   virtual ~DecoderDatabase();
 
   // Returns true if the database is empty.
   virtual bool Empty() const;
 
   // Returns the number of decoders registered in the database.
   virtual int Size() const;
 
   // Resets the database, erasing all registered payload types, and deleting
@@ skipping 70 matching lines @@
   // registered in the database. Otherwise, returns kDecoderNotFound.
   virtual int CheckPayloadTypes(const PacketList& packet_list) const;
 
  private:
   typedef std::map<uint8_t, DecoderInfo> DecoderMap;
 
   DecoderMap decoders_;
   int active_decoder_type_;
   int active_cng_decoder_type_;
   std::unique_ptr<ComfortNoiseDecoder> active_cng_decoder_;
+  const std::unique_ptr<AudioDecoderFactory> decoder_factory_;

[ossu, 2016/04/29 12:21:33]

Since the decoder_factory_ will (eventually) be created somewhere far from here,
and be pushed down into possibly many a DecoderFactory (and other places), it's
not going to fly to have a unique_ptr here. In my work of moving the decoder
factory further and further away, I've been using shared_ptr, though I'm
beginning to think we should maybe only keep it refcounted much earlier, and
keep a reference here. The lifetime of this DecoderDatabase ought to be
controlled by the WebRtcVoiceEngine, for example, so it might be enough to keep
ownership there.

I'm aware we're not supposed to use references as function parameters, but we
could always send them down as pointers. The key is we really need a non-null
AudioDecoderFactory here.

[kwiberg-webrtc, 2016/04/29 23:10:13]

I know. I'm just using unique_ptr for now so that I don't have to make the hard
choice about what else to use. I'd like shared_ptr, but I suspect we'll end up
having to use scoped_refptr which is sooo ugly.
I'm not fond of that idea. There's no machine making sure that we don't mess it
up. shared_ptr would be much safer.

Hey---Idea for a new type of smart pointer for this use case: master_ptr and
slave_ptr. master_ptr behaves like unique_ptr (i.e., moveable but not copyable),
but additionally you can fork off slave_ptrs that point to the same object.
slave_ptr behaves like shared_ptr (i.e., copyable). When the master_ptr releases
the object, it CHECKs that no slave_ptrs are still live. (This is easily
implemented by making both master_ptr and slave_ptr use a shared_ptr under the
hood; ~master_ptr will simply CHECK that it owns the last remaining reference.)

Maybe that'd be overkill though, and we should stick with shared_ptr.

For this CL right now, however, I'd like to keep using unique_ptr, to avoid
blocking on the decision about what type of smart pointer we should use.

[ossu, 2016/05/02 10:59:40]

Yeah, keep it like this for now. No reason to block the CL over it.
Unfortunately, shared_ptr/scoped_refptr won't solve ownership by itself. I've
made that mistake in a previous project. In the end, we had to figure out a
proper ownership structure and once we had that, keeping shared_ptrs was just
superfluous.

 
   RTC_DISALLOW_COPY_AND_ASSIGN(DecoderDatabase);
 };
 
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_AUDIO_CODING_NETEQ_DECODER_DATABASE_H_