Stopped using the NetEqDecoder enum internally in NetEq.

Stopped using the NetEqDecoder enum internally in NetEq.

NetEqDecoder is still used in the external interfaces, but this change
opens up the ability to use SdpAudioFormats directly, once appropriate
interfaces have been added.

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

[ossu, 2016-09-19 16:07:18]

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/delay_manager.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/delay_manager.cc:374: void
DelayManager::LastDecodedWasCngOrDtmf(bool it_was) {
It has to be possible to do this in a neater way, or at least with a better
name!

[kwiberg-webrtc, 2016-09-20 16:05:42]

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

[kwiberg-webrtc, 2016-09-20 16:05:43]

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:40: :
audio_format_(*acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)),
This conversion can still fail, right? Maybe DCHECK that it doesn't?

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:57: // These are not real
decoders.
// These are not the decoders you are looking for.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:76: return IsType("CN");
Or

  RTC_DCHECK_EQ(!!cng_decoder_, IsType("CN"));
  return !!cng_decoder_;

It's faster, and makes use of (and documents and checks) one of the invariants
you establish.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:151: auto opt_db_format =
const

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:153: SdpAudioFormat format
= opt_db_format.value_or({"arbitrary", 0, 0});
const

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.h (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.h:86: std::string name;
Why can't we just use the name field in audio_format_, which is now always
present?

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.h:90: AudioDecoderFactory*
factory_;
Consider making this const; I think you forget to initialize it in one of the
constructors, and making it const would have let the compiler catch that. (Also
because symmetry---everything else that can be const already is.)

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/delay_manager.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/delay_manager.cc:374: void
DelayManager::LastDecodedWasCngOrDtmf(bool it_was) {
On 2016/09/19 16:07:18, ossu wrote:
> It has to be possible to do this in a neater way, or at least with a better
> name!

I think you came up with good names.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/delay_manager.h (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/delay_manager.h:94: virtual void
LastDecodedWasCngOrDtmf(bool it_was);
Please document what this function does.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler.cc:50: // support timestamp
scaling of them.
The comment probably needs updating.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc:272:
rtc::scoped_refptr<AudioDecoderFactory> factory =
It'd be super handy if it was possible to write

  rtc::scoped_refptr<auto> factory = ...

and have the compiler infer the missing part(s) of they type, and complain if
the type wasn't of the correct form.

I guess fixing that will have to wait for a follow-up CL, though?

[kwiberg-webrtc, 2016-09-20 16:06:17]

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

[kwiberg-webrtc, 2016-09-20 16:06:17]

Adding a competent reviewer.

[ossu, 2016-09-21 09:10:56]

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:40: :
audio_format_(*acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)),
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> This conversion can still fail, right? Maybe DCHECK that it doesn't?

Well if it fails, it'll do an RTC_DCHECK in Optional (RTC_DCHECK(has_value_)),
so it's already covered.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:57: // These are not real
decoders.
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> // These are not the decoders you are looking for.

Yeah, I should phrase this differently. It's disparaging to call them "not
real". :(

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:76: return IsType("CN");
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> Or
> 
>   RTC_DCHECK_EQ(!!cng_decoder_, IsType("CN"));
>   return !!cng_decoder_;
> 
> It's faster, and makes use of (and documents and checks) one of the invariants
> you establish.

I guess that's fair. We've already done the string check when setting up
cng_decoder_, so might as well use that information directly.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:151: auto opt_db_format =
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> const

Acknowledged.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:153: SdpAudioFormat format
= opt_db_format.value_or({"arbitrary", 0, 0});
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> const

Acknowledged.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.h (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.h:86: std::string name;
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> Why can't we just use the name field in audio_format_, which is now always
> present?

I tried that, but there were some ambiguities for when to use what name: since
it's possible to supply a name and a format when registering an external
encoder, it's not clear which one should be put in audio_format_.name. It's even
less clear if it actually matters. So I decided to just put that problem off for
a while and have the name tucked away here for now. It'll go away when
RegisterExternalDecoder does at the latest.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.h:90: AudioDecoderFactory*
factory_;
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> Consider making this const; I think you forget to initialize it in one of the
> constructors, and making it const would have let the compiler catch that.
(Also
> because symmetry---everything else that can be const already is.)

Acknowledged.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/delay_manager.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/delay_manager.cc:374: void
DelayManager::LastDecodedWasCngOrDtmf(bool it_was) {
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> On 2016/09/19 16:07:18, ossu wrote:
> > It has to be possible to do this in a neater way, or at least with a better
> > name!
> 
> I think you came up with good names.

\o_o/

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/delay_manager.h (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/delay_manager.h:94: virtual void
LastDecodedWasCngOrDtmf(bool it_was);
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> Please document what this function does.

I'll try to figure it out.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler.cc:50: // support timestamp
scaling of them.
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> The comment probably needs updating.

The check is there to cover the exact same case, but I guess it's less clear
from the comment that that is the case. Will change!

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc:272:
rtc::scoped_refptr<AudioDecoderFactory> factory =
On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> It'd be super handy if it was possible to write
> 
>   rtc::scoped_refptr<auto> factory = ...
> 
> and have the compiler infer the missing part(s) of they type, and complain if
> the type wasn't of the correct form.
> 
> I guess fixing that will have to wait for a follow-up CL, though?

I should be able to put it just as
  auto factory = CreateBuiltinAudioDecoderFactory();
if we want a shorter line.

[kwiberg-webrtc, 2016-09-21 09:21:19]

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:40: :
audio_format_(*acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)),
On 2016/09/21 09:10:55, ossu wrote:
> On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > This conversion can still fail, right? Maybe DCHECK that it doesn't?
> 
> Well if it fails, it'll do an RTC_DCHECK in Optional (RTC_DCHECK(has_value_)),
> so it's already covered.

Yes, but a DCHECK here will make the problem much easier to diagnose. But since
you'll have to introduce a lambda just to have somewhere to put the DCHECK
statement, I guess it might not be worth it.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:57: // These are not real
decoders.
On 2016/09/21 09:10:55, ossu wrote:
> On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > // These are not the decoders you are looking for.
> 
> Yeah, I should phrase this differently. It's disparaging to call them "not
> real". :(

Well, that too. But my suggestion was because we compile with -Werror
-Wmissing-star-wars-reference.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:76: return IsType("CN");
On 2016/09/21 09:10:55, ossu wrote:
> On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > Or
> > 
> >   RTC_DCHECK_EQ(!!cng_decoder_, IsType("CN"));
> >   return !!cng_decoder_;
> > 
> > It's faster, and makes use of (and documents and checks) one of the
invariants
> > you establish.
> 
> I guess that's fair. We've already done the string check when setting up
> cng_decoder_, so might as well use that information directly.

Yes. And by relying on it, we strengthen the invariant.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.h (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.h:86: std::string name;
On 2016/09/21 09:10:56, ossu wrote:
> On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > Why can't we just use the name field in audio_format_, which is now always
> > present?
> 
> I tried that, but there were some ambiguities for when to use what name: since
> it's possible to supply a name and a format when registering an external
> encoder, it's not clear which one should be put in audio_format_.name. It's
even
> less clear if it actually matters. So I decided to just put that problem off
for
> a while and have the name tucked away here for now. It'll go away when
> RegisterExternalDecoder does at the latest.

Acknowledged.

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc:272:
rtc::scoped_refptr<AudioDecoderFactory> factory =
On 2016/09/21 09:10:56, ossu wrote:
> On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > It'd be super handy if it was possible to write
> > 
> >   rtc::scoped_refptr<auto> factory = ...
> > 
> > and have the compiler infer the missing part(s) of they type, and complain
if
> > the type wasn't of the correct form.
> > 
> > I guess fixing that will have to wait for a follow-up CL, though?
> 
> I should be able to put it just as
>   auto factory = CreateBuiltinAudioDecoderFactory();
> if we want a shorter line.

Yes, but that's what you changed *from*, so I assumed that you wanted it to be
easier to see what type it was (and I agreed that it was useful). So by all
means, keep it like this. It's just a shame that there's almost no middle ground
between auto and the full type.

[ossu, 2016-09-21 15:46:30]

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/decoder_database.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/decoder_database.cc:57: // These are not real
decoders.
On 2016/09/21 09:21:19, kwiberg-webrtc wrote:
> On 2016/09/21 09:10:55, ossu wrote:
> > On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > > // These are not the decoders you are looking for.
> > 
> > Yeah, I should phrase this differently. It's disparaging to call them "not
> > real". :(
> 
> Well, that too. But my suggestion was because we compile with -Werror
> -Wmissing-star-wars-reference.

:)

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc (right):

https://codereview.webrtc.org/2355503002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc:272:
rtc::scoped_refptr<AudioDecoderFactory> factory =
On 2016/09/21 09:21:19, kwiberg-webrtc wrote:
> On 2016/09/21 09:10:56, ossu wrote:
> > On 2016/09/20 16:05:43, kwiberg-webrtc wrote:
> > > It'd be super handy if it was possible to write
> > > 
> > >   rtc::scoped_refptr<auto> factory = ...
> > > 
> > > and have the compiler infer the missing part(s) of they type, and complain
> if
> > > the type wasn't of the correct form.
> > > 
> > > I guess fixing that will have to wait for a follow-up CL, though?
> > 
> > I should be able to put it just as
> >   auto factory = CreateBuiltinAudioDecoderFactory();
> > if we want a shorter line.
> 
> Yes, but that's what you changed *from*, so I assumed that you wanted it to be
> easier to see what type it was (and I agreed that it was useful). So by all
> means, keep it like this. It's just a shame that there's almost no middle
ground
> between auto and the full type.

Ah, look at that! :) Well, I think that's probably just an oversight. Every
other factory in this file seems to be auto, so I'll just put it back to that.

[ossu, 2016-09-21 15:46:38]

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

[commit-bot: I haz the power, 2016-09-21 15:46:44]

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, 2016-09-21 17:47:10]

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

[commit-bot: I haz the power, 2016-09-21 17:47:11]

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

[ossu, 2016-09-22 10:55:22]

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

[commit-bot: I haz the power, 2016-09-22 10:55:30]

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

[ossu, 2016-09-22 10:58:14]

Did a rebase to incorporate the changes to DecoderDatabase::GetFormat.

[commit-bot: I haz the power, 2016-09-22 11:53:28]

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

[commit-bot: I haz the power, 2016-09-22 11:53:30]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2016-09-22 12:23:48]

lgtm

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

https://codereview.webrtc.org/2355503002/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/timestamp_scaler.cc:50: // we cannot do any
timestamp scaling.
Excellent.

[hlundin-webrtc, 2016-09-22 12:36:16]

A number of nits and questions. Otherwise, LGTM.

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

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:125: const auto opt_format
=
What does "opt" mean here?

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:152: const auto
opt_db_format =
opt?

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database.h (right):

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:76: // Returns if the
decoder's format is named |name|.
Returns true...

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:78: // Returns if the
decoder's format is named |name|.
Returns true...

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:174: // Returns if
|rtp_payload_type| is registered with a format named |name|.
Returns true...

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:177: // Returns if
|rtp_payload_type| is registered with a format named |name|.
Returns true...

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

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:261: // Returns the decoder
format for the given payload type. Returns null if no
null -> empty?

[ossu, 2016-09-22 14:43:06]

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

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:125: const auto opt_format
=
On 2016/09/22 12:36:16, hlundin-webrtc wrote:
> What does "opt" mean here?

opt is intended to mean that it's an optional that may contain a format, so it
needs to be checked for validity and dereferenced to get at the value. Maybe
it's a bit redundant?
I've used it a bit in code lately to separate the "value or error" from the
value if everything went alright, by having i.e. if (opt_format) { auto& format
= *opt_format; ... }

I've seen smth_opt or opt_smth or even maybe_smth used to signify optional
variables in other code.

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.cc:152: const auto
opt_db_format =
On 2016/09/22 12:36:16, hlundin-webrtc wrote:
> opt?

Same as above. Here we'll collapse the optional to a proper value using
value_or. I'd like to have a name for it before it's a proper value. It could be
done in one fell swoop, either on line 152:
  const SdpAudioFormat format =
acm2::RentACodec::NetEqDecoderToSdpAudioFormat(codec_type).value_or({"arbitrary",
0, 0});

or on line 157:
  DecoderInfo info(format.value_or({"arbitrary", 0, 0});

if that's more agreeable.

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/decoder_database.h (right):

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:174: // Returns if
|rtp_payload_type| is registered with a format named |name|.
On 2016/09/22 12:36:16, hlundin-webrtc wrote:
> Returns true...

Acknowledged.

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/decoder_database.h:177: // Returns if
|rtp_payload_type| is registered with a format named |name|.
On 2016/09/22 12:36:16, hlundin-webrtc wrote:
> Returns true...

Acknowledged.

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

https://codereview.webrtc.org/2355503002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/include/neteq.h:261: // Returns the decoder
format for the given payload type. Returns null if no
On 2016/09/22 12:36:16, hlundin-webrtc wrote:
> null -> empty?

Yes! Good catch!

[ossu, 2016-09-22 14:43:13]

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

[commit-bot: I haz the power, 2016-09-22 14:43:19]

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, 2016-09-22 15:47:59]

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

[commit-bot: I haz the power, 2016-09-22 15:48:01]

Dry run: This issue passed the CQ dry run.

[ossu, 2016-09-23 09:08:33]

The CQ bit was checked by ossu@webrtc.org

[ossu, 2016-09-23 09:08:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2355503002/#ps60001 (title:
"Clarified comments.")

[commit-bot: I haz the power, 2016-09-23 09:08:43]

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, 2016-09-23 09:19:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-23 09:19:51]

Description was changed from

==========
Stopped using the NetEqDecoder enum internally in NetEq.

NetEqDecoder is still used in the external interfaces, but this change
opens up the ability to use SdpAudioFormats directly, once appropriate
interfaces have been added.

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

to

==========
Stopped using the NetEqDecoder enum internally in NetEq.

NetEqDecoder is still used in the external interfaces, but this change
opens up the ability to use SdpAudioFormats directly, once appropriate
interfaces have been added.

BUG=webrtc:5805

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

[commit-bot: I haz the power, 2016-09-23 09:19:52]

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