NetEq: Use a BuiltinAudioDecoderFactory to create decoders

webrtc/modules/audio_coding/neteq/decoder_database.h

Index: webrtc/modules/audio_coding/neteq/decoder_database.h
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h
index ec8a5d69bc27dfb1b19e472dc5121dbadedffbbf..3a40e08c8a64a2aade03dfcef6e555b8a63635a7 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.h
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.h
@@ -17,6 +17,8 @@
 
 #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"
@@ -47,7 +49,7 @@ class DecoderDatabase {
     ~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.)
@@ -59,6 +61,7 @@ class DecoderDatabase {
     AudioDecoder* const external_decoder;
 
    private:
+    const rtc::Optional<SdpAudioFormat> audio_format_;
     std::unique_ptr<AudioDecoder> decoder_;
   };
 
@@ -66,7 +69,7 @@ class DecoderDatabase {
   // only 7 bits).
   static const uint8_t kRtpPayloadTypeError = 0xFF;
 
-  DecoderDatabase();
+  DecoderDatabase(std::unique_ptr<AudioDecoderFactory> decoder_factory);
 
   virtual ~DecoderDatabase();
 
@@ -157,6 +160,7 @@ class DecoderDatabase {
   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);
 };