Support for two audio codec lists down into WebRtcVoiceEngine.

Support for two audio codec lists down into WebRtcVoiceEngine.

Added the plumbing necessary to get two different lists of codecs from
WebRtcVoiceEngine up to MediaSessionDescriptionFactory.

This should be the last step in this set of CLs. Once
https://codereview.webrtc.org/1991233004/ has landed, it's possible to
implement the ReceiveCodecs getter with the info from the
AudioDecoderFactory. The factory needs to be updated to actually
produce the correct list, as well.

BUG=webrtc:5805
Committed: https://crrev.com/dedfd28a52224edba5d5d4d15c020f9a267c18ae
Cr-Commit-Position: refs/heads/master@{#13131}

[ossu, 2016-05-26 14:18:49]

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

[ossu, 2016-05-26 14:18:50]

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine.h
File webrtc/media/base/mediaengine.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine...
webrtc/media/base/mediaengine.h:159: virtual const std::vector<AudioCodec>&
audio_send_codecs() {
Is this an interface change that needs backwards compatibility?

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h
File webrtc/pc/channelmanager.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h#ne...
webrtc/pc/channelmanager.h:64: void
GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
Is this an interface change that needs backwards compatibility?

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc
File webrtc/pc/mediasession.cc (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:2117: if (sc.clockrate == rc.clockrate && sc.bitrate
== rc.bitrate &&
This got clang-formatted. I'll put it back to one comparison per line, to make
it easier to read.

[ossu, 2016-05-30 12:39:44]

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

[ossu, 2016-05-30 12:39:45]

Removing Karl as reviewer, adding pthatcher since being an owner. Feel free to
add another set of eyes if necessary!

[pthatcher1, 2016-05-31 18:26:48]

lgtm, with a few name changes

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine.h
File webrtc/media/base/mediaengine.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine...
webrtc/media/base/mediaengine.h:159: virtual const std::vector<AudioCodec>&
audio_send_codecs() {
On 2016/05/26 14:18:50, ossu wrote:
> Is this an interface change that needs backwards compatibility?

It's only called by ChannelManager, so as long as you change ChannelManager
(which you did), we should be fine.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h
File webrtc/pc/channelmanager.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h#ne...
webrtc/pc/channelmanager.h:64: void
GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
On 2016/05/26 14:18:50, ossu wrote:
> Is this an interface change that needs backwards compatibility?

This is only used by MediaSessionDescriptionFactory, so as long as you change
MediaSessionDescriptionFactory, which you do, then we should be fine.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc
File webrtc/pc/mediasession.cc (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:1271: const AudioCodecs&
MediaSessionDescriptionFactory::audio_codecs() const {
I don't think this is called by anything except one test in
mediasession_unittest.cc.  A better name for it would be audio_sendrev_codecs().
 Can you rename it that?

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:2102: std::vector<AudioCodec> MergeAudioCodecLists(
Can you call this MergeSendRecvCodecs?

[ossu, 2016-06-01 08:34:12]

Addressed comments.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine.h
File webrtc/media/base/mediaengine.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/media/base/mediaengine...
webrtc/media/base/mediaengine.h:159: virtual const std::vector<AudioCodec>&
audio_send_codecs() {
On 2016/05/31 18:26:48, pthatcher1 wrote:
> On 2016/05/26 14:18:50, ossu wrote:
> > Is this an interface change that needs backwards compatibility?
> 
> It's only called by ChannelManager, so as long as you change ChannelManager
> (which you did), we should be fine.

Acknowledged.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h
File webrtc/pc/channelmanager.h (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/channelmanager.h#ne...
webrtc/pc/channelmanager.h:64: void
GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
On 2016/05/31 18:26:48, pthatcher1 wrote:
> On 2016/05/26 14:18:50, ossu wrote:
> > Is this an interface change that needs backwards compatibility?
> 
> This is only used by MediaSessionDescriptionFactory, so as long as you change
> MediaSessionDescriptionFactory, which you do, then we should be fine.

Acknowledged.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc
File webrtc/pc/mediasession.cc (right):

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:1271: const AudioCodecs&
MediaSessionDescriptionFactory::audio_codecs() const {
On 2016/05/31 18:26:48, pthatcher1 wrote:
> I don't think this is called by anything except one test in
> mediasession_unittest.cc.  A better name for it would be
audio_sendrev_codecs().
>  Can you rename it that?

Good call! Will do that.

https://codereview.webrtc.org/2013053002/diff/1/webrtc/pc/mediasession.cc#new...
webrtc/pc/mediasession.cc:2102: std::vector<AudioCodec> MergeAudioCodecLists(
On 2016/05/31 18:26:48, pthatcher1 wrote:
> Can you call this MergeSendRecvCodecs?

Sure thing!

[pthatcher1, 2016-06-01 15:32:17]

lgtm

[ossu, 2016-06-14 08:46:41]

Patchset #3 (id:40001) has been deleted

[ossu, 2016-06-14 12:57:04]

The CQ bit was checked by ossu@webrtc.org

[ossu, 2016-06-14 12:57:05]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2013053002/#ps80001 (title:
"Replaced MergeSendRecvCodecs with NegotiateCodecs.")

[commit-bot: I haz the power, 2016-06-14 12:57:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2013053002/80001

[commit-bot: I haz the power, 2016-06-14 14:12:44]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-06-14 14:12:47]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-14 14:12:49]

Description was changed from

==========
Support for two audio codec lists down into WebRtcVoiceEngine.

Added the plumbing necessary to get two different lists of codecs from
WebRtcVoiceEngine up to MediaSessionDescriptionFactory.

This should be the last step in this set of CLs. Once
https://codereview.webrtc.org/1991233004/ has landed, it's possible to
implement the ReceiveCodecs getter with the info from the
AudioDecoderFactory. The factory needs to be updated to actually
produce the correct list, as well.

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

to

==========
Support for two audio codec lists down into WebRtcVoiceEngine.

Added the plumbing necessary to get two different lists of codecs from
WebRtcVoiceEngine up to MediaSessionDescriptionFactory.

This should be the last step in this set of CLs. Once
https://codereview.webrtc.org/1991233004/ has landed, it's possible to
implement the ReceiveCodecs getter with the info from the
AudioDecoderFactory. The factory needs to be updated to actually
produce the correct list, as well.

BUG=webrtc:5805

Committed: https://crrev.com/dedfd28a52224edba5d5d4d15c020f9a267c18ae
Cr-Commit-Position: refs/heads/master@{#13131}
==========

[commit-bot: I haz the power, 2016-06-14 14:12:51]

Patchset 4 (id:??) landed as
https://crrev.com/dedfd28a52224edba5d5d4d15c020f9a267c18ae
Cr-Commit-Position: refs/heads/master@{#13131}