Move CNG/RED payload type extraction to Rent-A-Codec

Move CNG/RED payload type extraction to Rent-A-Codec

BUG=webrtc:5028
Committed: https://crrev.com/e1a27d48ad5157a106fe215be1e9fe99d5bed73e
Cr-Commit-Position: refs/heads/master@{#10691}

[kwiberg-webrtc, 2015-11-17 12:53:13]

Patchset #2 (id:20001) has been deleted

[kwiberg-webrtc, 2015-11-17 12:53:26]

Patchset #1 (id:1) has been deleted

[kwiberg-webrtc, 2015-11-17 12:54:27]

Description was changed from

==========
Move CNG/RED payload type extraction to Rent-A-Codec

BUG=webrtc:5028
==========

to

==========
Move CNG/RED payload type extraction to Rent-A-Codec

BUG=webrtc:5028
==========

[kwiberg-webrtc, 2015-11-17 12:54:27]

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

[hlundin-webrtc, 2015-11-18 10:19:18]

LGTM, but depending on what you are planning on doing next, you may want to
consider my suggestion about map ownership.

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/codec_manager.h (right):

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/codec_manager.h:82: std::map<int, int>
cng_payload_types_;
Why don't you make these maps part of Rent-A-Codec?

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/rent_a_codec.h (right):

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/rent_a_codec.h:184: // Parse codec_inst
and extract payload types. If the given CodecInst was for
Parses... extracts...

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/rent_a_codec.h:189: static
RegistrationResult RegisterCngPayloadType(std::map<int, int>* pt_map,
You should probably include what you use: <map>.
Granted, you are just using pointers to maps here, so the
include-what-you-use-nag could be deferred to the .cc file. But that would make
me nag about forward-declaring here instead.

[kwiberg-webrtc, 2015-11-18 14:19:29]

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/codec_manager.h (right):

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/codec_manager.h:82: std::map<int, int>
cng_payload_types_;
On 2015/11/18 10:19:18, hlundin-webrtc wrote:
> Why don't you make these maps part of Rent-A-Codec?

Discussed this offline. The tl;dr is that they, and a bunch of other state from
CodecManager, will end up in an object that contains the information necessary
to construct the RED/CNG stack.

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/rent_a_codec.h (right):

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/rent_a_codec.h:184: // Parse codec_inst
and extract payload types. If the given CodecInst was for
On 2015/11/18 10:19:18, hlundin-webrtc wrote:
> Parses... extracts...

No. It's plural, because the text applies to two methods!

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/rent_a_codec.h:189: static
RegistrationResult RegisterCngPayloadType(std::map<int, int>* pt_map,
On 2015/11/18 10:19:18, hlundin-webrtc wrote:
> You should probably include what you use: <map>.
> Granted, you are just using pointers to maps here, so the
> include-what-you-use-nag could be deferred to the .cc file. But that would
make
> me nag about forward-declaring here instead.

Done.

[kwiberg-webrtc, 2015-11-18 14:20:43]

The CQ bit was checked by kwiberg@webrtc.org

[kwiberg-webrtc, 2015-11-18 14:20:43]

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/1450883002/#ps60001 (title:
"include <map>")

[commit-bot: I haz the power, 2015-11-18 14:20:46]

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

[hlundin-webrtc, 2015-11-18 14:49:37]

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/rent_a_codec.h (right):

https://codereview.webrtc.org/1450883002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/rent_a_codec.h:184: // Parse codec_inst
and extract payload types. If the given CodecInst was for
On 2015/11/18 14:19:29, kwiberg-webrtc wrote:
> On 2015/11/18 10:19:18, hlundin-webrtc wrote:
> > Parses... extracts...
> 
> No. It's plural, because the text applies to two methods!

I stand corrected.

[commit-bot: I haz the power, 2015-11-18 15:32:54]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-11-18 15:33:08]

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