WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

NOTE: This CL has been split up into the following constituent CLs, in order:
https://codereview.webrtc.org/1990803004/
https://codereview.webrtc.org/1992763002/
https://codereview.webrtc.org/1993783002/
https://codereview.webrtc.org/1991233004/

Original description follows:
The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a parameter. The CreatePeerConnectionFactory call has not been changed yet, and provides our builtin AudioDecoderFactory for that parameter (i.e. CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which creates an AudioCodingModule, which creates an AcmReceiver which in turn creates a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an AudioCodingModule it creates and owns itself. At that point, the path we pass the AudioDecoderFactory along will go straight through AudioReceiveStream, instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the AudioDecoderFactory must be done earlier. There is instead a check here, to make sure the factories being sent down both paths are one and the same.

BUG=webrtc:5805

[ossu, 2016-05-03 16:13:22]

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/webrtcvid...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (left):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/webrtcvid...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:100: :
WebRtcVideoEngine2Test(nullptr, field_trials) {}
This got removed when updating tests to the changes in voice engine. It turns
out this test didn't use the voice engine it was given.

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:24: #include
"webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h"
This one should go.

[the sun, 2016-05-03 21:07:29]

solenberg@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[the sun, 2016-05-03 21:07:30]

This is a good proof of concept; I don't think we need to do this much, and once
you've narrowed it down, perhaps we should break it up into smaller pieces as
well.

https://codereview.webrtc.org/1949533002/diff/1/webrtc/api/peerconnectionfact...
File webrtc/api/peerconnectionfactory.h (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/api/peerconnectionfact...
webrtc/api/peerconnectionfactory.h:102:
std::shared_ptr<webrtc::AudioDecoderFactory> audio_decoder_factory,
We should use rtc::scoped_refptr instead.

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/fakewebrt...
File webrtc/media/engine/fakewebrtcvoiceengine.h (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/fakewebrt...
webrtc/media/engine/fakewebrtcvoiceengine.h:235:
std::shared_ptr<webrtc::AudioDecoderFactory> decoder_factory)) {
This is changing the depended-on legacy VoE interface, which we want to avoid if
possible. What are the options w/ regards to the lower level code?

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine_unittest.cc (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3468: // TODO(ossu): Should
probably use a mock audio decoder factory.
Yes!

https://codereview.webrtc.org/1949533002/diff/1/webrtc/modules/audio_coding/a...
File webrtc/modules/audio_coding/acm2/audio_coding_module.cc (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/modules/audio_coding/a...
webrtc/modules/audio_coding/acm2/audio_coding_module.cc:28:
config.decoder_factory = CreateBuiltinAudioDecoderFactory();
That's what I was thinking - until the legacy VoE API is deprecated, can we
avoid touching that and instantiate the built-in decoder factory here. For the
use case with AudioReceiveStream, we'd instead need a function on voe::Channel
(and ACM) to set the decoder factory, or possibly recreate the voe::Channel's
ACM.

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:731: std::shared_ptr<AudioDecoderFactory>
decoder_factory)
I think I'd rather avoid this and instead add a setter function on Channel, to
allow setting the ADF from AudioReceiveStream, via ChannelProxy, by recreating
the ACM.

[ossu, 2016-05-11 11:18:37]

Patchset #2 (id:20001) has been deleted

[ossu, 2016-05-11 11:22:32]

I've put the previous CreateChannel signatures back and added versions that take
audio decoder factories as overloads.

I've also put in the plumbing for eventually sending this down
AudioReceiveStream. At this point, however, it just RTC_CHECKs that a factory is
provided, and that it is the same object as the one provided when creating the
Channel.

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.cc
File webrtc/voice_engine/channel.cc (right):

https://codereview.webrtc.org/1949533002/diff/1/webrtc/voice_engine/channel.c...
webrtc/voice_engine/channel.cc:24: #include
"webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h"
On 2016/05/03 16:13:22, ossu wrote:
> This one should go.

Not anymore!

[ossu, 2016-05-11 12:59:38]

Description was changed from

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

TODO(ossu): Actually describe the changes made in human-words.
TODO(ossu): Maybe the change is still too big. Perhaps divide it into
            chunks?

BUG=webrtc:5801
==========

to

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a
parameter. The CreatePeerConnectionFactory call has not been changed yet, and
provides our builtin AudioDecoderFactory for that parameter (i.e.
CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the
AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a
WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor
parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing
WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which
creates an AudioCodingModule, which creates an AcmReceiver which in turn creates
a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed
with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an
AudioCodingModule it creates and owns itself. At that point, the path we pass
the AudioDecoderFactory along will go straight through AudioReceiveStream,
instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the
AudioDecoderFactory must be done earlier. There is instead a check here, to make
sure the factories being sent down both paths are one and the same.

TODO(ossu): Maybe the change is too big. Perhaps divide it into chunks?

BUG=webrtc:5801
==========

[ossu, 2016-05-16 12:40:05]

ossu@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[ossu, 2016-05-16 12:40:26]

Hej Karl!

[kwiberg-webrtc, 2016-05-17 13:33:48]

Looks good. And yes please, split it up!

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/api/peerconnection...
File webrtc/api/peerconnectionfactory.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/api/peerconnection...
webrtc/api/peerconnectionfactory.cc:91: CreateBuiltinAudioDecoderFactory()),
unique_ptr converts implicitly to shared_ptr, so no need for a manual cast.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/audio/audio_receiv...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/audio/audio_receiv...
webrtc/audio/audio_receive_stream.cc:103: config.decoder_factory ==
channel_proxy_->GetAudioDecoderFactory());
Better to use two separate check calls. That way, the error message will tell
you which condition was violated.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvoiceengine_unittest.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3468: // TODO(ossu): Should
probably use a mock audio decoder factory.
Your heart's in the right place!

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3471:
std::shared_ptr<webrtc::AudioDecoderFactory>());
nullptr

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3488:
std::shared_ptr<webrtc::AudioDecoderFactory>());
nullptr

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3590:
std::shared_ptr<webrtc::AudioDecoderFactory>());
nullptr

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/include/neteq.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:29: class AudioDecoderFactory;
Why not just include the header instead?

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h:25: :
DecoderDatabase(std::shared_ptr<AudioDecoderFactory>()) {}
You shouldn't need this---nullptr converts implicitly to shared_ptr.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq.cc:37:
std::shared_ptr<AudioDecoderFactory> decoder_factory) {
Why not put the decoder factory in the config?

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
File webrtc/voice_engine/include/voe_base.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
webrtc/voice_engine/include/voe_base.h:136:
std::shared_ptr<AudioDecoderFactory>()) = 0;
Ew! Default values for a virtual method...

Also, nullptr will convert implicitly to shared_ptr<T> for any T.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
File webrtc/voice_engine/voe_base_impl.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
webrtc/voice_engine/voe_base_impl.cc:383: decoder_factory_ =
CreateBuiltinAudioDecoderFactory();
?:

[ossu, 2016-05-17 14:25:34]

ossu@webrtc.org changed reviewers:
- solenberg@webrtc.org

[ossu, 2016-05-17 14:25:36]

-solenberg since he's out of office for a while.

I should have the changes neatly separated into steps in my local repo, so
making new, smaller CLs shouldn't be an issue. Would you prefer it if I fixed
the things you commented on here first, then separate (so it's maybe easier to
track) or just include those fixes in the separate CLs (which should be small
enough to make it easy to track anyways?).

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/api/peerconnection...
File webrtc/api/peerconnectionfactory.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/api/peerconnection...
webrtc/api/peerconnectionfactory.cc:91: CreateBuiltinAudioDecoderFactory()),
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> unique_ptr converts implicitly to shared_ptr, so no need for a manual cast.

Hmm, I've utilized that in other places. Must've had a brain-fart. Will fix.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/audio/audio_receiv...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/audio/audio_receiv...
webrtc/audio/audio_receive_stream.cc:103: config.decoder_factory ==
channel_proxy_->GetAudioDecoderFactory());
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> Better to use two separate check calls. That way, the error message will tell
> you which condition was violated.

Good point!

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvoiceengine_unittest.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine_unittest.cc:3471:
std::shared_ptr<webrtc::AudioDecoderFactory>());
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> nullptr

Yeah, I realized that after a while. I've actually got a whole patchset that
changes this, but I decided against pushing it before investigating
scoped_refptr. Will change!

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/include/neteq.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:29: class AudioDecoderFactory;
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> Why not just include the header instead?

I ... I don't know. Either way works, I guess.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq.cc:37:
std::shared_ptr<AudioDecoderFactory> decoder_factory) {
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> Why not put the decoder factory in the config?

My local git log says this:
    Moved injection of AudioDecoderFactory from NetEq::Config to a separate
    argument in NetEq::Create. This makes the direct usage of NetEqImpl in
    tests unambiguous, since it takes a Config and a DecoderDatabase
    directly (among other things). It also makes sure an
    AudioDecoderFactory is always included - since the signature has
    changed, the compiler will scream at you, rather than just silently
    accepting a nullptr in the Config.

This was before the change to using a Dependencies object, but I believe the
logic goes unchanged: Dependencies contains a DecoderDatabase and is sent in
alongside the Config, so if the factory was in the config, it would be in two
places, or worse, there would be two different factories in different places.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
File webrtc/voice_engine/include/voe_base.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
webrtc/voice_engine/include/voe_base.h:136:
std::shared_ptr<AudioDecoderFactory>()) = 0;
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> Ew! Default values for a virtual method...
> 
> Also, nullptr will convert implicitly to shared_ptr<T> for any T.

I was just following orders! (i.e. there were already default values in place
when I started).

nullptr is right, though. I've a fix for all of these but it got lodged when I
started investigating moving to scoped_refptr.

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
File webrtc/voice_engine/voe_base_impl.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
webrtc/voice_engine/voe_base_impl.cc:383: decoder_factory_ =
CreateBuiltinAudioDecoderFactory();
On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> ?:

That is... punctuation!
If it's a question, my answer is that if the voice-engine is default Init'ed (I
believe in some test, maybe?), it should still behave as previously, i.e. with
the builtin set of codecs.

Eventually we'll want to get away from that, since we don't want VoiceEngine to
depend on our builtin codecs module.

[kwiberg-webrtc, 2016-05-18 03:06:35]

> Would you prefer it if I fixed the things you commented on here
> first, then separate (so it's maybe easier to track) or just include
> those fixes in the separate CLs (which should be small enough to
> make it easy to track anyways?).

One of the advantages of small CLs is that they're easy to work with
even if you're not meticulous about separating out updates like this
one. So go ahead and publish the small CLs in their (current) final
form; I'll just read each of them from scratch.

Do you plan to still use shared_ptr? That'll introduce a dependency on
the successful conclusion of our shared_ptr crusade...

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
File webrtc/voice_engine/include/voe_base.h (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/inclu...
webrtc/voice_engine/include/voe_base.h:136:
std::shared_ptr<AudioDecoderFactory>()) = 0;
On 2016/05/17 14:25:35, ossu wrote:
> On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> > Ew! Default values for a virtual method...
> > 
> > Also, nullptr will convert implicitly to shared_ptr<T> for any T.
> 
> I was just following orders! (i.e. there were already default values in place
> when I started).

Yes, I didn't mean to imply you were at fault. It was just a general
observation...

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
File webrtc/voice_engine/voe_base_impl.cc (right):

https://codereview.webrtc.org/1949533002/diff/40001/webrtc/voice_engine/voe_b...
webrtc/voice_engine/voe_base_impl.cc:383: decoder_factory_ =
CreateBuiltinAudioDecoderFactory();
On 2016/05/17 14:25:35, ossu wrote:
> On 2016/05/17 13:33:48, kwiberg-webrtc wrote:
> > ?:
> 
> That is... punctuation!
> If it's a question, my answer is that if the voice-engine is default Init'ed
(I
> believe in some test, maybe?), it should still behave as previously, i.e. with
> the builtin set of codecs.
> 
> Eventually we'll want to get away from that, since we don't want VoiceEngine
to
> depend on our builtin codecs module.

Oh, nothing so subtle! I was jus suggesting

  decoder_factory_ = decoder_factory ? std::move(decoder_factory)
                                     : CreateBuiltinAudioDecoderFactory();

[ossu, 2016-05-18 10:51:48]

On 2016/05/18 03:06:35, kwiberg-webrtc wrote:
> One of the advantages of small CLs is that they're easy to work with
> even if you're not meticulous about separating out updates like this
> one. So go ahead and publish the small CLs in their (current) final
> form; I'll just read each of them from scratch.

Will do!
 
> Do you plan to still use shared_ptr? That'll introduce a dependency on
> the successful conclusion of our shared_ptr crusade...

I gave scoped_refptr another go and it turns out to be not-so-horrible if
AddRef/Release is implemented in AudioDecoderFactory directly.
I think it's better not to have this project depend on us changing how we handle
shared ownership in general in webrtc.
We can initiate a push for shared_ptr and if we're lucky and it gets allowed
quickly, I can change AudioDecoderFactory use in a separate CL.

[ossu, 2016-05-19 13:21:11]

Description was changed from

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a
parameter. The CreatePeerConnectionFactory call has not been changed yet, and
provides our builtin AudioDecoderFactory for that parameter (i.e.
CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the
AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a
WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor
parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing
WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which
creates an AudioCodingModule, which creates an AcmReceiver which in turn creates
a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed
with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an
AudioCodingModule it creates and owns itself. At that point, the path we pass
the AudioDecoderFactory along will go straight through AudioReceiveStream,
instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the
AudioDecoderFactory must be done earlier. There is instead a check here, to make
sure the factories being sent down both paths are one and the same.

TODO(ossu): Maybe the change is too big. Perhaps divide it into chunks?

BUG=webrtc:5801
==========

to

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a
parameter. The CreatePeerConnectionFactory call has not been changed yet, and
provides our builtin AudioDecoderFactory for that parameter (i.e.
CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the
AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a
WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor
parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing
WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which
creates an AudioCodingModule, which creates an AcmReceiver which in turn creates
a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed
with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an
AudioCodingModule it creates and owns itself. At that point, the path we pass
the AudioDecoderFactory along will go straight through AudioReceiveStream,
instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the
AudioDecoderFactory must be done earlier. There is instead a check here, to make
sure the factories being sent down both paths are one and the same.

TODO(ossu): Maybe the change is too big. Perhaps divide it into chunks?

BUG=webrtc:5805
==========

[ossu, 2016-05-20 08:24:10]

I've put up all the changes from this CL, and then some, as separate CLs.
Should I add links to those Cls in the description and close this CL, or should
I just delete it?

[kwiberg-webrtc, 2016-05-20 08:29:11]

On 2016/05/20 08:24:10, ossu wrote:
> I've put up all the changes from this CL, and then some, as separate CLs.
> Should I add links to those Cls in the description and close this CL, or
should
> I just delete it?

The best option is to close it, and post a comment linking to the CLs that
replace it. (Of course, this is also a bit of work, but from my perspective it's
definitely the best option since I'm not the one doing the work.)

[ossu, 2016-05-20 08:33:54]

Description was changed from

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a
parameter. The CreatePeerConnectionFactory call has not been changed yet, and
provides our builtin AudioDecoderFactory for that parameter (i.e.
CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the
AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a
WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor
parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing
WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which
creates an AudioCodingModule, which creates an AcmReceiver which in turn creates
a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed
with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an
AudioCodingModule it creates and owns itself. At that point, the path we pass
the AudioDecoderFactory along will go straight through AudioReceiveStream,
instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the
AudioDecoderFactory must be done earlier. There is instead a check here, to make
sure the factories being sent down both paths are one and the same.

TODO(ossu): Maybe the change is too big. Perhaps divide it into chunks?

BUG=webrtc:5805
==========

to

==========
WIP: Move the creation of AudioCodecFactory into PeerConnectionFactory.

NOTE: This CL has been split up into the following constituent CLs, in order:
https://codereview.webrtc.org/1990803004/
https://codereview.webrtc.org/1992763002/
https://codereview.webrtc.org/1993783002/
https://codereview.webrtc.org/1991233004/

Original description follows:
The interface of CreatePeerConnectionFactory has not changed, since the
current state of the code requires our AudioCodecFactory to implement
exactly that which was previously implemented in the audio coding
module.

Since this change cuts straight through many layers of WebRTC it's put
up early, so that it can be given a long, hard look at.

The flow is roughly as follows:
The PeerConnectionFactory constructor now takes an AudioDecoderFactory as a
parameter. The CreatePeerConnectionFactory call has not been changed yet, and
provides our builtin AudioDecoderFactory for that parameter (i.e.
CreateBuiltinAudioDecoderFactory()).

PeerConnectionFactory creates a WebRtcMediaEngine2, which takes the
AudioDecoderFactory as a constructor parameter. WebRtcMediaEngine2 contains a
WebRtcVoiceEngine, which is passed the AudioDecoderFactory as a constructor
parameter.
WebRtcVoiceEngine holds on to the factory and uses it when constructing
WebRtcVoiceMediaChannel, providing it as a parameter to VoEBase::Init, which
creates an AudioCodingModule, which creates an AcmReceiver which in turn creates
a NetEq instance that has a DecoderDatabase. This DecoderDatabase is constructed
with the AudioDecoderFactory as a parameter.

Eventually, we want to get to a point where  AudioReceiveStream has an
AudioCodingModule it creates and owns itself. At that point, the path we pass
the AudioDecoderFactory along will go straight through AudioReceiveStream,
instead of VoEBase, ChannelManager etc.
That path has been laid out in this CL, but the actual setting of the
AudioDecoderFactory must be done earlier. There is instead a check here, to make
sure the factories being sent down both paths are one and the same.

BUG=webrtc:5805
==========

[ossu, 2016-05-20 08:34:47]

Closed the CL and updated the description with links to constituent CLs.