NetEq: Add codec name and RTP timestamp rate to DecoderInfo

NetEq: Add codec name and RTP timestamp rate to DecoderInfo

The new fields are default-populated for built-in decoders, but for
external decoders, the name can now be given when registering the
decoder.

BUG=webrtc:3520
Committed: https://crrev.com/4cf61dd116288e9f119209c59e07f1d9439d8d05
Cr-Commit-Position: refs/heads/master@{#10952}

[hlundin-webrtc, 2015-12-01 15:47:07]

Patchset #1 (id:1) has been deleted

[hlundin-webrtc, 2015-12-01 15:47:53]

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

[hlundin-webrtc, 2015-12-01 15:47:53]

Karl,

Please, take a look at this change. Later CLs will build upon this new
functionality.

[hlundin-webrtc, 2015-12-01 15:48:12]

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

[commit-bot: I haz the power, 2015-12-01 15:48:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1484343003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1484343003/20001

[commit-bot: I haz the power, 2015-12-01 17:48:29]

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

[commit-bot: I haz the power, 2015-12-01 17:48:30]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[kwiberg-webrtc, 2015-12-02 09:50:42]

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:515: db[i].plfreq,
nullptr, "") < 0) {
Why is the name empty here?

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:570: "");
Why is the name empty here?

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/include/audio_coding_module.h (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/include/audio_coding_module.h:479: const
std::string& name) = 0;
You don't explain what the name is used for. (This matches the other parameters
and the function as a whole, but it isn't a good example to follow.)

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:78: }  // namespace
Should we standardize on either lower- or uppercase names? We currently do
case-insensitive comparisons everywhere, and it'd be nice not to.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/include/neteq.h (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:186: // on failure.
You don't explain what the name is used for. Will that be in a future CL?

[kwiberg-webrtc, 2015-12-02 13:41:37]

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc:115: const
std::string kCodecName = "Robert\'); DROP TABLE Students;";
Note that it's allowed but not necessary to escape the single quote.

[hlundin-webrtc, 2015-12-02 16:12:43]

Always provide a decoder name to AcmReceiver and NetEq

[hlundin-webrtc, 2015-12-02 16:17:56]

Adding another comment

[hlundin-webrtc, 2015-12-02 16:19:24]

Thanks.

I restructured the CL a bit. Now, NetEq and AcmReceiver takes a decoder name
parameter when registering internal decoders too.

PTAL again.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:515: db[i].plfreq,
nullptr, "") < 0) {
On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> Why is the name empty here?

Done.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:570: "");
On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> Why is the name empty here?

Done.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/include/audio_coding_module.h (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/include/audio_coding_module.h:479: const
std::string& name) = 0;
On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> You don't explain what the name is used for. (This matches the other
parameters
> and the function as a whole, but it isn't a good example to follow.)

Done.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:78: }  // namespace
On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> Should we standardize on either lower- or uppercase names? We currently do
> case-insensitive comparisons everywhere, and it'd be nice not to.

Yes, we should. But no longer relevant for this CL.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc:115: const
std::string kCodecName = "Robert\'); DROP TABLE Students;";
On 2015/12/02 13:41:37, kwiberg-webrtc wrote:
> Note that it's allowed but not necessary to escape the single quote.

Acknowledged.

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/include/neteq.h (right):

https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:186: // on failure.
On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> You don't explain what the name is used for. Will that be in a future CL?

Done.

[hlundin-webrtc, 2015-12-09 08:38:54]

On 2015/12/02 16:19:24, hlundin-webrtc wrote:
> Thanks.
> 
> I restructured the CL a bit. Now, NetEq and AcmReceiver takes a decoder name
> parameter when registering internal decoders too.
> 
> PTAL again.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> File webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc (right):
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:515:
db[i].plfreq,
> nullptr, "") < 0) {
> On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> > Why is the name empty here?
> 
> Done.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc:570: "");
> On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> > Why is the name empty here?
> 
> Done.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> File webrtc/modules/audio_coding/include/audio_coding_module.h (right):
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/include/audio_coding_module.h:479: const
> std::string& name) = 0;
> On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> > You don't explain what the name is used for. (This matches the other
> parameters
> > and the function as a whole, but it isn't a good example to follow.)
> 
> Done.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/neteq/decoder_database.cc:78: }  // namespace
> On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> > Should we standardize on either lower- or uppercase names? We currently do
> > case-insensitive comparisons everywhere, and it'd be nice not to.
> 
> Yes, we should. But no longer relevant for this CL.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> File webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc (right):
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc:115: const
> std::string kCodecName = "Robert\'); DROP TABLE Students;";
> On 2015/12/02 13:41:37, kwiberg-webrtc wrote:
> > Note that it's allowed but not necessary to escape the single quote.
> 
> Acknowledged.
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> File webrtc/modules/audio_coding/neteq/include/neteq.h (right):
> 
>
https://codereview.webrtc.org/1484343003/diff/20001/webrtc/modules/audio_codi...
> webrtc/modules/audio_coding/neteq/include/neteq.h:186: // on failure.
> On 2015/12/02 09:50:42, kwiberg-webrtc wrote:
> > You don't explain what the name is used for. Will that be in a future CL?
> 
> Done.

Ping!

[kwiberg-webrtc, 2015-12-09 09:22:43]

lgtm

[hlundin-webrtc, 2015-12-09 12:58:50]

The CQ bit was checked by henrik.lundin@webrtc.org

[hlundin-webrtc, 2015-12-09 12:58:50]

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

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

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

[commit-bot: I haz the power, 2015-12-09 14:21:02]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-12-09 14:21:12]

Description was changed from

==========
NetEq: Add codec name and RTP timestamp rate to DecoderInfo

The new fields are default-populated for built-in decoders, but for
external decoders, the name can now be given when registering the
decoder.

BUG=webrtc:3520
==========

to

==========
NetEq: Add codec name and RTP timestamp rate to DecoderInfo

The new fields are default-populated for built-in decoders, but for
external decoders, the name can now be given when registering the
decoder.

BUG=webrtc:3520

Committed: https://crrev.com/4cf61dd116288e9f119209c59e07f1d9439d8d05
Cr-Commit-Position: refs/heads/master@{#10952}
==========

[commit-bot: I haz the power, 2015-12-09 14:21:13]

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