Expose new video codec factories in the PeerConnectionFactory API

Expose new video codec factories in the PeerConnectionFactory API

This CL exposes the new type of video codec factories that represent all
video codecs in the PeerConnectionFactory API, i.e. no extra internal SW
video codecs will be added. Clients of the new functions will be
responsible for adding all SW video codecs themselves, and also handling
SW fallback and simulcast.

BUG=webrtc:7925
R=deadbeef@webrtc.org

Review-Url: https://codereview.webrtc.org/3004353002 .
Cr-Commit-Position: refs/heads/master@{#19866}
Committed: https://webrtc.googlesource.com/src/+/58b0316f3d19918788b7a484e3e6a7797ad7e2b8

[magjed_webrtc, 2017-09-04 16:48:22]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2017-09-13 14:21:01]

Description was changed from

==========
Expose new video codec factories in the PeerConnectionFactory API

BUG=webrtc:7925
==========

to

==========
Expose new video codec factories in the PeerConnectionFactory API

This CL exposes the new type of video codec factories that represent all
video codecs in the PeerConnectionFactory API, i.e. no extra internal SW
video codecs will be added. Clients of the new functions will be
responsible for adding all SW video codecs themselves, and also handling
SW fallback and simulcast.

BUG=webrtc:7925
==========

[magjed_webrtc, 2017-09-13 14:21:01]

magjed@webrtc.org changed reviewers:
+ deadbeef@webrtc.org

[magjed_webrtc, 2017-09-13 14:21:26]

Taylor - please take a look.

[Taylor Brandstetter, 2017-09-15 00:42:51]

lgtm

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/api/peerconnection...
File webrtc/api/peerconnectioninterface.h (right):

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/api/peerconnection...
webrtc/api/peerconnectioninterface.h:1131: rtc::scoped_refptr<AudioProcessing>
audio_processing);
nit: Maybe it's time to make a struct that contains all these arguments? See
https://bugs.chromium.org/p/webrtc/issues/detail?id=7913

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/pc/createpeerconne...
File webrtc/pc/createpeerconnectionfactory.cc (right):

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/pc/createpeerconne...
webrtc/pc/createpeerconnectionfactory.cc:104: audio_encoder_factory,
audio_decoder_factory,
Something I'm realizing now: the audio_encoder_factory/audio_decoder_factory
arguments here are actually ignored. Was it always this way? Is there a bug for
this?

[magjed_webrtc, 2017-09-15 13:39:17]

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/api/peerconnection...
File webrtc/api/peerconnectioninterface.h (right):

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/api/peerconnection...
webrtc/api/peerconnectioninterface.h:1131: rtc::scoped_refptr<AudioProcessing>
audio_processing);
On 2017/09/15 00:42:51, Taylor Brandstetter wrote:
> nit: Maybe it's time to make a struct that contains all these arguments? See
> https://bugs.chromium.org/p/webrtc/issues/detail?id=7913

I totally agree, these create functions are obviously telescoping and we need
some kind of builder pattern. I'm not sure I'm the right person to fix it though
since I'm not an owner of this part of the code base, and I can imagine people
have a lot of opinions how this should be done. I'll go ahead and land this
first regardless.

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/pc/createpeerconne...
File webrtc/pc/createpeerconnectionfactory.cc (right):

https://codereview.webrtc.org/3004353002/diff/20001/webrtc/pc/createpeerconne...
webrtc/pc/createpeerconnectionfactory.cc:104: audio_encoder_factory,
audio_decoder_factory,
On 2017/09/15 00:42:51, Taylor Brandstetter wrote:
> Something I'm realizing now: the audio_encoder_factory/audio_decoder_factory
> arguments here are actually ignored. Was it always this way? Is there a bug
for
> this?

Yeah, it looks strange but I haven't been working on the audio factories. I
think ossu@ knows this best. I'll file an issue/CL to fix it.

[magjed_webrtc, 2017-09-15 13:39:26]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2017-09-15 13:39:33]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-15 13:43:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-09-15 13:43:03]

Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/25059)

[magjed_webrtc, 2017-09-15 13:53:22]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2017-09-15 13:53:23]

The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3004353002/#ps40001 (title:
"Rebase")

[commit-bot: I haz the power, 2017-09-15 13:53:34]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-15 14:00:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-09-15 14:00:03]

Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/25061)

[magjed_webrtc, 2017-09-15 15:03:10]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2017-09-15 15:03:11]

The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3004353002/#ps60001 (title:
"Add missing includes")

[commit-bot: I haz the power, 2017-09-15 15:03:23]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-15 15:06:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-09-15 15:06:14]

Try jobs failed on following builders:
  linux_memcheck on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/1...)

[magjed_webrtc, 2017-09-15 15:20:08]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-09-15 15:20:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[magjed_webrtc, 2017-09-15 15:21:01]

Patchset #3 (id:60001) has been deleted

[magjed_webrtc, 2017-09-15 17:02:24]

The CQ bit was unchecked by magjed@webrtc.org

[magjed_webrtc, 2017-09-15 17:02:55]

Description was changed from

==========
Expose new video codec factories in the PeerConnectionFactory API

This CL exposes the new type of video codec factories that represent all
video codecs in the PeerConnectionFactory API, i.e. no extra internal SW
video codecs will be added. Clients of the new functions will be
responsible for adding all SW video codecs themselves, and also handling
SW fallback and simulcast.

BUG=webrtc:7925
==========

to

==========
Expose new video codec factories in the PeerConnectionFactory API

This CL exposes the new type of video codec factories that represent all
video codecs in the PeerConnectionFactory API, i.e. no extra internal SW
video codecs will be added. Clients of the new functions will be
responsible for adding all SW video codecs themselves, and also handling
SW fallback and simulcast.

BUG=webrtc:7925
R=deadbeef@webrtc.org

Review-Url: https://codereview.webrtc.org/3004353002 .
Cr-Commit-Position: refs/heads/master@{#19866}
Committed:
https://webrtc.googlesource.com/src/+/58b0316f3d19918788b7a484e3e6a7797ad7e2b8
==========

[magjed_webrtc, 2017-09-15 17:02:58]

Committed patchset #3 (id:80001) manually as
58b0316f3d19918788b7a484e3e6a7797ad7e2b8 (presubmit successful).