Adding Opus to RTPencode.

Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus RTP payloads.

BUG=webrtc:3987, webrtc:2692
Committed: https://crrev.com/cb23c0d9840a0f48921426cd777c2359bf917300
Cr-Commit-Position: refs/heads/master@{#10987}

[minyue-webrtc, 2015-12-10 18:10:55]

Patchset #1 (id:1) has been deleted

[minyue-webrtc, 2015-12-10 18:13:25]

Description was changed from

==========
adding Opus to RTPencode

BUG=
==========

to

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987
==========

[minyue-webrtc, 2015-12-10 18:13:25]

minyue@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[minyue-webrtc, 2015-12-10 18:15:02]

Hi Henrik,

Would you take a look at this? I have tested the generated rtp file by
rtp_analysis and neteq_unittest(with some change there to allow Opus, which will
come in a following CL)

[hlundin-webrtc, 2015-12-11 08:16:23]

Description was changed from

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987
==========

to

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987, webrtc:2692
==========

[hlundin-webrtc, 2015-12-11 08:30:29]

LGTM with two nits.
Sorry you had to deal with this bad old code... :(

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/test/PayloadTypes.h (right):

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/test/PayloadTypes.h:60: #define
NETEQ_CODEC_OPUS_PT             92
Please, use PT 111 to match the default in neteq_rtpplay.cc. You can remove line
42 above, where 111 is bound to the SC3 codec. (This list has a lot of codecs
that we no longer support.)

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/test/RTPencode.cc (right):

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/test/RTPencode.cc:1730: assert(cdlen > 0);
Nit: CHECK_GT(cdlen, 0);
I think CHECK is better than DCHECK.
Also include checks.h.

[minyue-webrtc, 2015-12-11 09:03:55]

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/test/PayloadTypes.h (right):

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/test/PayloadTypes.h:60: #define
NETEQ_CODEC_OPUS_PT             92
On 2015/12/11 08:30:29, hlundin-webrtc wrote:
> Please, use PT 111 to match the default in neteq_rtpplay.cc. You can remove
line
> 42 above, where 111 is bound to the SC3 codec. (This list has a lot of codecs
> that we no longer support.)

Thank you! I was struggling with this.

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/test/RTPencode.cc (right):

https://codereview.webrtc.org/1516653003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/test/RTPencode.cc:1730: assert(cdlen > 0);
On 2015/12/11 08:30:29, hlundin-webrtc wrote:
> Nit: CHECK_GT(cdlen, 0);
> I think CHECK is better than DCHECK.
> Also include checks.h.

Done.

[minyue-webrtc, 2015-12-11 09:04:00]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2015-12-11 09:04:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1516653003/#ps40001 (title:
"fixing some nits")

[commit-bot: I haz the power, 2015-12-11 09:04:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1516653003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1516653003/40001

[commit-bot: I haz the power, 2015-12-11 09:58:31]

Description was changed from

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987, webrtc:2692
==========

to

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987, webrtc:2692
==========

[commit-bot: I haz the power, 2015-12-11 09:58:32]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-12-11 09:58:34]

Description was changed from

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987, webrtc:2692
==========

to

==========
Adding Opus to RTPencode.

As a step toward fixing webrtc:3987, here we update the RTPencode to allow Opus
RTP payloads.

BUG=webrtc:3987, webrtc:2692

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

[commit-bot: I haz the power, 2015-12-11 09:58:36]

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