Broke out 'level_indicator' of the voice_engine GN target.

Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True

Committed: https://crrev.com/e6ca9ec7b97efbe6574fbe14774a0cb02fa73af8
Cr-Commit-Position: refs/heads/master@{#13709}

[aleloi, 2016-08-09 09:41:20]

aleloi@webrtc.org changed reviewers:
+ henrika@webrtc.org, kjellander@webrtc.org

[aleloi, 2016-08-09 09:41:20]

It's a small change: I broke out a piece of voice_engine. This is in order to
depend on it in the new audio mixer. The alternative would be to depend on the
whole of voice_engine, which would introduce cyclic dependencies later.

[aleloi, 2016-08-09 09:41:33]

Description was changed from

==========
Broke out 'level_indicator' of the voice_engine GN target.

Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.
==========

to

==========
Broke out 'level_indicator' of the voice_engine GN target.

Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

NOTRY=True
==========

[kjellander_webrtc, 2016-08-09 21:25:17]

a few comments. Please also make the GYP changes for now.

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD.gn
File webrtc/voice_engine/BUILD.gn (right):

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
webrtc/voice_engine/BUILD.gn:109: ":level_indicator",
does it need to be public?

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
webrtc/voice_engine/BUILD.gn:118: 
Please add dependency on "../base:rtc_base_approved" since that header is
obviously referenced.
Try make it pass gn gen --check (bonus if the target can be added to
check_targets in
https://chromium.googlesource.com/external/webrtc/+/master/.gn).

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
webrtc/voice_engine/BUILD.gn:118: 
Add a dependency on "../common_audio", as well, since it's used from the .cc.
"webrtc/modules/include/module_common_types.h" is unfortunately not listed in
any GN target yet, so that has to wait.

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
File webrtc/voice_engine/level_indicator.h (right):

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
webrtc/voice_engine/level_indicator.h:16: //#include
"webrtc/voice_engine/voice_engine_defines.h"
Remove instead of commenting.

[aleloi, 2016-08-10 09:08:05]

I seem to have been extra careless this time. I fixed the issues. Now this
target is checked by 'gn check'.

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD.gn
File webrtc/voice_engine/BUILD.gn (right):

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
webrtc/voice_engine/BUILD.gn:109: ":level_indicator",
On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> does it need to be public?

No, it doesn't. I thought it needed to be public because it's used in
TransmitMixer, Channel and OutputMixer. All those targets are in voice_engine,
so level_indicator can stay private.

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
webrtc/voice_engine/BUILD.gn:118: 
On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> Please add dependency on "../base:rtc_base_approved" since that header is
> obviously referenced.
> Try make it pass gn gen --check (bonus if the target can be added to
> check_targets in
> https://chromium.googlesource.com/external/webrtc/+/master/.gn).

Done, and also webrtc_common from webrtc/typedefs.h. It passes the check tests
now.

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
File webrtc/voice_engine/level_indicator.h (right):

https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
webrtc/voice_engine/level_indicator.h:16: //#include
"webrtc/voice_engine/voice_engine_defines.h"
On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> Remove instead of commenting.

Oops! Thanks. Done.

[aleloi, 2016-08-10 11:17:14]

Description was changed from

==========
Broke out 'level_indicator' of the voice_engine GN target.

Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

NOTRY=True
==========

to

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

NOTRY=True
==========

[kjellander_webrtc, 2016-08-10 11:21:55]

On 2016/08/10 09:08:05, aleloi wrote:
> I seem to have been extra careless this time. I fixed the issues. Now this
> target is checked by 'gn check'.

Wohoo, the first target to actually be passing the check in our repo. Yay!

Nice work, lgtm assuming the currently running tryjobs are passing.

> 
>
https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD.gn
> File webrtc/voice_engine/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
> webrtc/voice_engine/BUILD.gn:109: ":level_indicator",
> On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> > does it need to be public?
> 
> No, it doesn't. I thought it needed to be public because it's used in
> TransmitMixer, Channel and OutputMixer. All those targets are in voice_engine,
> so level_indicator can stay private.
> 
>
https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/BUILD...
> webrtc/voice_engine/BUILD.gn:118: 
> On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> > Please add dependency on "../base:rtc_base_approved" since that header is
> > obviously referenced.
> > Try make it pass gn gen --check (bonus if the target can be added to
> > check_targets in
> > https://chromium.googlesource.com/external/webrtc/+/master/.gn).
> 
> Done, and also webrtc_common from webrtc/typedefs.h. It passes the check tests
> now.
> 
>
https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
> File webrtc/voice_engine/level_indicator.h (right):
> 
>
https://codereview.webrtc.org/2221103004/diff/20001/webrtc/voice_engine/level...
> webrtc/voice_engine/level_indicator.h:16: //#include
> "webrtc/voice_engine/voice_engine_defines.h"
> On 2016/08/09 21:25:17, kjellander_webrtc wrote:
> > Remove instead of commenting.
> 
> Oops! Thanks. Done.

[aleloi, 2016-08-10 11:59:19]

Description was changed from

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

NOTRY=True
==========

to

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True
==========

[aleloi, 2016-08-10 12:00:27]

I set henrika@ as TBR, since he's out, it's such a small change and it's
slightly blocking my work on the mixer. I hope it's OK!

[aleloi, 2016-08-10 12:00:32]

The CQ bit was checked by aleloi@webrtc.org

[commit-bot: I haz the power, 2016-08-10 12:00:39]

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-08-10 12:01:54]

Description was changed from

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True
==========

to

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True
==========

[commit-bot: I haz the power, 2016-08-10 12:01:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-10 12:02:06]

Description was changed from

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True
==========

to

==========
Broke out 'level_indicator' of the voice_engine GN target. This is
needed for the new audio_mixer to depend on just
'voice_engine:level_indicator' instead of the whole voice_engine and
avoid a circular dependency.

TBR=henrika@webrtc.org

NOTRY=True

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

[commit-bot: I haz the power, 2016-08-10 12:02:08]

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

[henrika_webrtc, 2016-08-15 14:02:25]

LGTM