Removing webrtc::AudioFrame::energy_.

Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315
Committed: https://crrev.com/bacae81a1d91ae7b2a4bde37a66ab70513449989
Cr-Commit-Position: refs/heads/master@{#11372}

[minyue-webrtc, 2016-01-14 14:03:55]

Description was changed from

==========
remove webrtc::AudioFrame::energy_

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

to

==========
Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315
==========

[minyue-webrtc, 2016-01-14 14:03:55]

minyue@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[minyue-webrtc, 2016-01-14 14:05:06]

Hi Henrik,

This CL is upon request in webrtc:3315. It does not change any result of the
audio processing. Would you help review it?

[kjellander_webrtc, 2016-01-14 18:53:05]

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

[kjellander_webrtc, 2016-01-14 18:53:05]

I'm neither know this code or is an owner; you need a root owner approval for
this CL.

[hlundin-webrtc, 2016-01-19 08:25:02]

LGTM with one nit and a question.

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
File webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
(right):

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc:592:
uint32_t energy = CalculateEnergy(**iter);
const

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
File webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc
(right):

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc:42:
uint32_t CalculateEnergy(const AudioFrame& audioFrame)
Would you consider changing this to an int? I know it potentially changes the
code behavior, but I'm thinking it is for the better. WDYT? (If so, you can also
skip the extra #include in the .h file.)

[minyue-webrtc, 2016-01-22 10:03:49]

On 2016/01/19 08:25:02, hlundin-webrtc wrote:
> LGTM with one nit and a question.
> 
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> File
webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
> (right):
> 
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
>
webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc:592:
> uint32_t energy = CalculateEnergy(**iter);
> const
> 
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> File webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc
> (right):
> 
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc:42:
> uint32_t CalculateEnergy(const AudioFrame& audioFrame)
> Would you consider changing this to an int? I know it potentially changes the
> code behavior, but I'm thinking it is for the better. WDYT? (If so, you can
also
> skip the extra #include in the .h file.)

I sometimes get confused. Is int a preferred type to int32_t uint32_t etc? If
so, for uint32_t, do we want to use "unsigned int"?

[hlundin-webrtc, 2016-01-22 14:19:04]

On 2016/01/22 10:03:49, minyue-webrtc wrote:
> On 2016/01/19 08:25:02, hlundin-webrtc wrote:
> > LGTM with one nit and a question.
> > 
> >
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> > File
> webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
> > (right):
> > 
> >
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> >
>
webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc:592:
> > uint32_t energy = CalculateEnergy(**iter);
> > const
> > 
> >
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> > File webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc
> > (right):
> > 
> >
>
https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
> > webrtc/modules/audio_conference_mixer/source/audio_frame_manipulator.cc:42:
> > uint32_t CalculateEnergy(const AudioFrame& audioFrame)
> > Would you consider changing this to an int? I know it potentially changes
the
> > code behavior, but I'm thinking it is for the better. WDYT? (If so, you can
> also
> > skip the extra #include in the .h file.)
> 
> I sometimes get confused. Is int a preferred type to int32_t uint32_t etc? If
> so, for uint32_t, do we want to use "unsigned int"?

See https://google.github.io/styleguide/cppguide.html#Integer_Types.
Don't use "unsigned int". Use uint*_t only when representing a bit pattern
rather than a number, or you need defined overflow modulo 2^N. Do not use
unsigned types to say a number will never be negative.

In general, int is preferred to the stdint.h types, unless there is a valid
reason.

In this case, I change my mind. :) Keep it as is, or change to int64_t to reduce
risk of overflow.

[minyue-webrtc, 2016-01-22 15:05:50]

Thanks for the comments! I prefer to use uint32_t for this to be consistent with
before.

But I have to make an additional change. I found another use of CalculateEnergy
which is dead code. I want to remove it, since 1) it is dead, 2) with the change
in CalculateEnergy it does not do the same.

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
File webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
(right):

https://codereview.webrtc.org/1589953002/diff/1/webrtc/modules/audio_conferen...
webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc:592:
uint32_t energy = CalculateEnergy(**iter);
On 2016/01/19 08:25:02, hlundin-webrtc wrote:
> const

Done.

[minyue-webrtc, 2016-01-25 11:58:10]

Hi,

Is there any concern on removing the dead function, or can I CQ?

[hlundin-webrtc, 2016-01-25 18:38:46]

Still LGTM.

[minyue-webrtc, 2016-01-25 21:10:43]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-01-25 21:10:53]

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

[commit-bot: I haz the power, 2016-01-25 21:20:02]

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

[commit-bot: I haz the power, 2016-01-25 21:20:02]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/3065)

[minyue-webrtc, 2016-01-25 21:52:17]

minyue@webrtc.org changed reviewers:
+ tommi@webrtc.org

[minyue-webrtc, 2016-01-25 21:53:04]

+tommi,

Tommi,

Are you a root owner? I need you to take a look at this.

[tommi, 2016-01-25 22:04:06]

The CQ bit was checked by tommi@webrtc.org

[tommi, 2016-01-25 22:04:07]

lgtm

[commit-bot: I haz the power, 2016-01-25 22:04:11]

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

[tommi, 2016-01-25 22:04:35]

one question btw - does this have an impact on CPU usage?

[minyue-webrtc, 2016-01-25 22:07:37]

On 2016/01/25 22:04:35, tommi-webrtc wrote:
> one question btw - does this have an impact on CPU usage?

It should not. I paid attention and the calculation effort should be the same.

[commit-bot: I haz the power, 2016-01-25 22:10:39]

Description was changed from

==========
Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315
==========

to

==========
Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315
==========

[commit-bot: I haz the power, 2016-01-25 22:10:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-25 22:10:43]

Description was changed from

==========
Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315
==========

to

==========
Remove webrtc::AudioFrame::energy_.

BUG=webrtc:3315

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

[commit-bot: I haz the power, 2016-01-25 22:10:44]

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

[terelius, 2016-01-26 10:29:35]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.webrtc.org/1634143002/ by terelius@webrtc.org.

The reason for reverting is: The change breaks google3 tests for
buzz/mediaengines/sae/audio_mixer..