Remove the type parameter to NetEq::GetAudio

Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
Committed: https://crrev.com/55480f5efa62bd09441f4fbabedf12a000461371
Cr-Commit-Position: refs/heads/master@{#11903}

[hlundin-webrtc, 2016-03-07 12:31:29]

Description was changed from

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
==========

to

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
==========

[hlundin-webrtc, 2016-03-07 12:31:29]

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

[hlundin-webrtc, 2016-03-07 12:36:42]

Ivo,

Please, take a look at this CL.

Thanks!

[ivoc, 2016-03-07 16:26:22]

Looks like a nice improvement! See my two questions below.

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

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/neteq_impl.h:61: kOutputNormal,
Do you think it would make sense to change these to match the naming that is
used in the AudioFrame enum (or change the naming in AudioFrame to match this)?

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc (left):

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:1064:
EXPECT_EQ(kOutputNormal, type);
Is this change caused by the refactoring in this CL or is it just a general
improvement of the test?

[hlundin-webrtc, 2016-03-08 08:05:00]

After review

[hlundin-webrtc, 2016-03-08 08:05:33]

PTAL again.

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

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/neteq_impl.h:61: kOutputNormal,
On 2016/03/07 16:26:22, ivoc wrote:
> Do you think it would make sense to change these to match the naming that is
> used in the AudioFrame enum (or change the naming in AudioFrame to match
this)?

Done, as far as possible, since this enum does not have a 1:1 mapping to
AudioFrame::SpeechType.

I also made it an enum class now, to avoid mismatches now that the names are the
same across two enums.

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc (left):

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:1064:
EXPECT_EQ(kOutputNormal, type);
On 2016/03/07 16:26:22, ivoc wrote:
> Is this change caused by the refactoring in this CL or is it just a general
> improvement of the test?

A bit of both. In the old code, the output type was not set when the GetAudio
call failed; the type variable remained the same it was before the call (which
happened to be kOutputNormal in this case). The new code also does not guarantee
that all fields in the AudioFrame are set when an error happens (I specified
that in the function comment), so the correct thing to do in the test is to not
look at the type at all; it can be anything and that is by design.

[hlundin-webrtc, 2016-03-08 08:05:49]

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

[commit-bot: I haz the power, 2016-03-08 08:05:51]

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

[ivoc, 2016-03-08 08:26:29]

LGTM

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc (left):

https://codereview.webrtc.org/1769883002/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:1064:
EXPECT_EQ(kOutputNormal, type);
On 2016/03/08 08:05:33, hlundin-webrtc wrote:
> On 2016/03/07 16:26:22, ivoc wrote:
> > Is this change caused by the refactoring in this CL or is it just a general
> > improvement of the test?
> 
> A bit of both. In the old code, the output type was not set when the GetAudio
> call failed; the type variable remained the same it was before the call (which
> happened to be kOutputNormal in this case). The new code also does not
guarantee
> that all fields in the AudioFrame are set when an error happens (I specified
> that in the function comment), so the correct thing to do in the test is to
not
> look at the type at all; it can be anything and that is by design.

Ok, that makes sense.

[commit-bot: I haz the power, 2016-03-08 09:03:21]

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

[commit-bot: I haz the power, 2016-03-08 09:03:22]

Dry run: This issue passed the CQ dry run.

[hlundin-webrtc, 2016-03-08 09:14:10]

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

[commit-bot: I haz the power, 2016-03-08 09:14:20]

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

[commit-bot: I haz the power, 2016-03-08 09:14:21]

This CL has an open dependency (Issue 1772583002 Patch 1). Please resolve the
dependency and try again.

[hlundin-webrtc, 2016-03-08 10:36:36]

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

[commit-bot: I haz the power, 2016-03-08 10:36:40]

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

[commit-bot: I haz the power, 2016-03-08 10:38:00]

Description was changed from

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
==========

to

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
==========

[commit-bot: I haz the power, 2016-03-08 10:38:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-08 10:38:09]

Description was changed from

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607
==========

to

==========
Remove the type parameter to NetEq::GetAudio

The type is included in the AudioFrame output parameter.

Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.

BUG=webrtc:5607

Committed: https://crrev.com/55480f5efa62bd09441f4fbabedf12a000461371
Cr-Commit-Position: refs/heads/master@{#11903}
==========

[commit-bot: I haz the power, 2016-03-08 10:38:10]

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