Add the function ToRtpParameters.

Add the function ToRtpParameters.

The function converts the cricket::Codec, cricket::RtpHeaderExtensions
and cricket::StreamParamsVec to webrtc::RtpParameters.

BUG=webrtc:7311
Review-Url: https://codereview.webrtc.org/2735853004
Cr-Commit-Position: refs/heads/master@{#17124}
Committed: https://chromium.googlesource.com/external/webrtc/+/243663965a9a3a76f01c306191b2b223f687711d

[Zhi Huang, 2017-03-08 18:49:08]

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

[commit-bot: I haz the power, 2017-03-08 18:49:20]

Dry run: 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-03-08 19:14:15]

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

[commit-bot: I haz the power, 2017-03-08 19:14:16]

Dry run: This issue passed the CQ dry run.

[Taylor Brandstetter, 2017-03-08 22:35:48]

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

[Taylor Brandstetter, 2017-03-08 22:35:49]

Looks good; just some minor comments.

Also, here's a bug you can link:
https://bugs.chromium.org/p/webrtc/issues/detail?id=7311

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
File webrtc/ortc/rtpparametersconversion.cc (right):

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion.cc:283: if (stream_params.size() > 0) {
What if there are more than 1 StreamParams? The simplest thing would just be to
change this to a "for each" loop.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion.cc:286: rtp_encoding.ssrc =
rtc::Optional<uint32_t>(stream_param.first_ssrc());
nit: Instead of "= rtc::Optional...", you can do ".emplace"

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
File webrtc/ortc/rtpparametersconversion_unittest.cc (right):

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion_unittest.cc:460: // RtpEncodingParameters
and vice versa.
Since there are already independent tests for ToCricketStreamParamsVec, I think
this test should test ToRtpEncodings independently.

It also would be good to test some corner cases such as an empty list, or list
with 2+ entries.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion_unittest.cc:484: cricket::VideoCodec
cricket_codec;
Should be AudioCodec; it looks like this is a copy of the below test.

[Zhi Huang, 2017-03-08 23:49:39]

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

[commit-bot: I haz the power, 2017-03-08 23:49:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Zhi Huang, 2017-03-08 23:54:51]

Patchset #3 (id:40001) has been deleted

[Zhi Huang, 2017-03-08 23:56:49]

Please take another look. Thanks.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
File webrtc/ortc/rtpparametersconversion.cc (right):

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion.cc:283: if (stream_params.size() > 0) {
On 2017/03/08 22:35:49, Taylor Brandstetter wrote:
> What if there are more than 1 StreamParams? The simplest thing would just be
to
> change this to a "for each" loop.

Oh, I was wrong. I thought that since ORTC api doesn't support more than
RtpEncodingsParameters, StreamParamsVec wouldn't contain more than one.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion.cc:286: rtp_encoding.ssrc =
rtc::Optional<uint32_t>(stream_param.first_ssrc());
On 2017/03/08 22:35:49, Taylor Brandstetter wrote:
> nit: Instead of "= rtc::Optional...", you can do ".emplace"

Done.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
File webrtc/ortc/rtpparametersconversion_unittest.cc (right):

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion_unittest.cc:460: // RtpEncodingParameters
and vice versa.
On 2017/03/08 22:35:49, Taylor Brandstetter wrote:
> Since there are already independent tests for ToCricketStreamParamsVec, I
think
> this test should test ToRtpEncodings independently.
> 
> It also would be good to test some corner cases such as an empty list, or list
> with 2+ entries.

Done.

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion_unittest.cc:484: cricket::VideoCodec
cricket_codec;
On 2017/03/08 22:35:49, Taylor Brandstetter wrote:
> Should be AudioCodec; it looks like this is a copy of the below test.

Oh, I must forgot to update this for some reason.

[Zhi Huang, 2017-03-09 00:06:28]

Description was changed from

==========
Add the function ToRtpParameters.

The function converts the cricket::Codec, cricket::RtpHeaderExtensions
and cricket::StreamParamsVec to webrtc::RtpParameters.

BUG=None
==========

to

==========
Add the function ToRtpParameters.

The function converts the cricket::Codec, cricket::RtpHeaderExtensions
and cricket::StreamParamsVec to webrtc::RtpParameters.

BUG=webrtc:7311
==========

[Taylor Brandstetter, 2017-03-09 00:41:43]

lgtm

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
File webrtc/ortc/rtpparametersconversion.cc (right):

https://codereview.webrtc.org/2735853004/diff/20001/webrtc/ortc/rtpparameters...
webrtc/ortc/rtpparametersconversion.cc:283: if (stream_params.size() > 0) {
On 2017/03/08 23:56:49, Zhi Huang wrote:
> On 2017/03/08 22:35:49, Taylor Brandstetter wrote:
> > What if there are more than 1 StreamParams? The simplest thing would just be
> to
> > change this to a "for each" loop.
> 
> Oh, I was wrong. I thought that since ORTC api doesn't support more than
> RtpEncodingsParameters, StreamParamsVec wouldn't contain more than one.

You're right that the ORTC RtpSender/RtpReceiver doesn't support multiple
encodings (currently). But assuming this is used for the SDP parsing API, it
will be possible to pass in SDP with multiple SSRCs/SSRC groups. The application
might then, for example, pass one of the RtpEncodingParameters into one
RtpSender, and the other into another RtpSender, giving them identical
codec/header extension/etc. parameters. This is basically what "Plan B" SDP
does.

[Zhi Huang, 2017-03-09 00:47:05]

The CQ bit was checked by zhihuang@webrtc.org

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

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-03-09 01:15:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489020425637800,
"parent_rev": "eaa160639b2711218a6b73862be66f55cbd193b7", "commit_rev":
"243663965a9a3a76f01c306191b2b223f687711d"}

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

Description was changed from

==========
Add the function ToRtpParameters.

The function converts the cricket::Codec, cricket::RtpHeaderExtensions
and cricket::StreamParamsVec to webrtc::RtpParameters.

BUG=webrtc:7311
==========

to

==========
Add the function ToRtpParameters.

The function converts the cricket::Codec, cricket::RtpHeaderExtensions
and cricket::StreamParamsVec to webrtc::RtpParameters.

BUG=webrtc:7311

Review-Url: https://codereview.webrtc.org/2735853004
Cr-Commit-Position: refs/heads/master@{#17124}
Committed:
https://chromium.googlesource.com/external/webrtc/+/243663965a9a3a76f01c30619...
==========

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

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/243663965a9a3a76f01c30619...