Don't use VoE legacy APIs in force_mic_volume_max tool.

Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
Committed: https://crrev.com/ae9f2bdcef7f3a338ece6f57744c8c8f74d15483
Committed: https://crrev.com/fc433e65463e3a2e3cc43b3f8cb68849b00f381f
Cr-Original-Commit-Position: refs/heads/master@{#14135}
Cr-Commit-Position: refs/heads/master@{#14169}

[the sun, 2016-08-25 17:45:15]

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

[the sun, 2016-08-25 17:45:16]

I'm wondering if it would make sense to pull out the ADM init stuff in a utility
function... Henrik A, WDYT?

[henrika_webrtc, 2016-08-25 18:58:54]

I don't know the background of this tool. Why is it needed at all? Can't we just
remove it?

[the sun, 2016-08-25 19:17:50]

On 2016/08/25 18:58:54, henrika_webrtc wrote:
> I don't know the background of this tool. Why is it needed at all? Can't we
just
> remove it?

According to kjellander@ it is used on bots to reset the volume after AGC tests.
One could argue though that 
1) either the tests fiddling with the level should set it back afterwards, or
2) whatever tests are hit by the changed level should set up the mic gain to
their liking before they commence...

kjellander@, any intel on when and with which tests this has actually hit us?

[henrika_webrtc, 2016-08-26 07:05:57]

IMO it feels odd to have tests that requires usage of special tools that uses
the same software the test is testing.

Or, how can we have a test that ends up in state where it is not possible to
revert what the test did?

In either case, I hope we can avoid "fancy work" just to maintain this tool.

[kjellander_webrtc, 2016-08-26 07:52:49]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
==========

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
==========

[kjellander_webrtc, 2016-08-26 07:52:49]

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

[kjellander_webrtc, 2016-08-26 07:54:01]

On 2016/08/25 19:17:50, the sun wrote:
> On 2016/08/25 18:58:54, henrika_webrtc wrote:
> > I don't know the background of this tool. Why is it needed at all? Can't we
> just
> > remove it?
> 
> According to kjellander@ it is used on bots to reset the volume after AGC
tests.
> One could argue though that 
> 1) either the tests fiddling with the level should set it back afterwards, or

If the test crashes there's no guarantee that this will happen always.

> 2) whatever tests are hit by the changed level should set up the mic gain to
> their liking before they commence...
> 
> kjellander@, any intel on when and with which tests this has actually hit us?

It's only one browser test that tests audio quality:
https://cs.chromium.org/chromium/src/chrome/browser/media/webrtc_audio_qualit...
Patrik wrote this, so if you want to know more details I suggest tracking down
his CLs with git blame.

[henrika_webrtc, 2016-08-26 08:03:37]

I don't want to block work that fixes tests that crashes today.
Seems like a good idea to create an issue and assign to Patrik so he can modify
the tests and avoid this extra tool.

[henrika_webrtc, 2016-08-26 08:04:53]

LGTM

[phoglund, 2016-09-05 07:24:31]

On 2016/08/26 07:05:57, henrika_webrtc wrote:
> IMO it feels odd to have tests that requires usage of special tools that uses
> the same software the test is testing.
> 
> Or, how can we have a test that ends up in state where it is not possible to
> revert what the test did?
> 
> In either case, I hope we can avoid "fancy work" just to maintain this tool.

This was the easiest way I could force the mic volume to 100%, at least at the
time.
This utility just uses the mic level parts of VoE whereas the test tests pretty
much
the whole VoE.

Yeah, we could certainly try to find at better solution and attempt to set the
volume
in the test itself, instead. Maybe Chrome has some nice helper class that can do
this?
Feel free to file a bug and assign to me.

[the sun, 2016-09-05 07:30:14]

On 2016/09/05 07:24:31, phoglund wrote:
> On 2016/08/26 07:05:57, henrika_webrtc wrote:
> > IMO it feels odd to have tests that requires usage of special tools that
uses
> > the same software the test is testing.
> > 
> > Or, how can we have a test that ends up in state where it is not possible to
> > revert what the test did?
> > 
> > In either case, I hope we can avoid "fancy work" just to maintain this tool.
> 
> This was the easiest way I could force the mic volume to 100%, at least at the
> time.
> This utility just uses the mic level parts of VoE whereas the test tests
pretty
> much
> the whole VoE.
> 
> Yeah, we could certainly try to find at better solution and attempt to set the
> volume
> in the test itself, instead. Maybe Chrome has some nice helper class that can
do
> this?
> Feel free to file a bug and assign to me.

Right, in the meantime, are you ok if we update the tool as suggested here, so
as to stop using the legacy VoE API?

[phoglund, 2016-09-05 12:29:52]

On 2016/09/05 07:30:14, the sun wrote:
> On 2016/09/05 07:24:31, phoglund wrote:
> > On 2016/08/26 07:05:57, henrika_webrtc wrote:
> > > IMO it feels odd to have tests that requires usage of special tools that
> uses
> > > the same software the test is testing.
> > > 
> > > Or, how can we have a test that ends up in state where it is not possible
to
> > > revert what the test did?
> > > 
> > > In either case, I hope we can avoid "fancy work" just to maintain this
tool.
> > 
> > This was the easiest way I could force the mic volume to 100%, at least at
the
> > time.
> > This utility just uses the mic level parts of VoE whereas the test tests
> pretty
> > much
> > the whole VoE.
> > 
> > Yeah, we could certainly try to find at better solution and attempt to set
the
> > volume
> > in the test itself, instead. Maybe Chrome has some nice helper class that
can
> do
> > this?
> > Feel free to file a bug and assign to me.
> 
> Right, in the meantime, are you ok if we update the tool as suggested here, so
> as to stop using the legacy VoE API?

Absolutely, go for it.

[phoglund, 2016-09-06 07:23:12]

In case you need my lgtm: lgtm

[the sun, 2016-09-07 08:48:09]

The CQ bit was checked by solenberg@webrtc.org

[the sun, 2016-09-07 08:48:09]

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

[commit-bot: I haz the power, 2016-09-07 08:48:13]

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-07 09:16:10]

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

[commit-bot: I haz the power, 2016-09-07 09:16:14]

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

[the sun, 2016-09-08 08:43:24]

The CQ bit was checked by solenberg@webrtc.org

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

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

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

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

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

[the sun, 2016-09-08 13:35:43]

The CQ bit was checked by solenberg@webrtc.org

[commit-bot: I haz the power, 2016-09-08 13:35:48]

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

[the sun, 2016-09-08 13:36:19]

solenberg@webrtc.org changed reviewers:
- kjellander@webrtc.org

[commit-bot: I haz the power, 2016-09-08 13:44:25]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
==========

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
==========

[commit-bot: I haz the power, 2016-09-08 13:44:27]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-08 13:44:38]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
==========

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

[commit-bot: I haz the power, 2016-09-08 13:44:39]

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

[the sun, 2016-09-08 14:30:42]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.webrtc.org/2320113002/ by solenberg@webrtc.org.

The reason for reverting is: Breaks Chromium FYI bots because of link issue.

[the sun, 2016-09-08 19:53:08]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

[the sun, 2016-09-09 07:09:18]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:bot1,bot2;tryserver.chromium.mac:bot3

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

[the sun, 2016-09-09 07:11:17]

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

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

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-09 07:11:30]

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

[commit-bot: I haz the power, 2016-09-09 07:11:31]

Dry run: Your CL was about to rely on recently removed CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is no longer supported:
     tryserver.chromium.linux
     tryserver.chromium.mac
   For more details, see http://crbug.com/617627.

[the sun, 2016-09-09 07:12:32]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:bot1,bot2;tryserver.chromium.mac:bot3

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

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

[the sun, 2016-09-09 07:52:09]

On 2016/09/08 14:30:42, the sun wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.webrtc.org/2320113002/ by mailto:solenberg@webrtc.org.
> 
> The reason for reverting is: Breaks Chromium FYI bots because of link issue.

phoglund@: Are you sure this tool runs as expected on the Chromium bots? It
looks to me like it would always fail (with the error "Failed to initialize
voice engine base.") because the rtc_include_internal_audio_device build flag is
unset in Chromium builds, causing VoEBaseImpl::Init() to return -1.

On which bots is it supposed to run on?

[phoglund, 2016-09-09 08:45:20]

On 2016/09/09 07:52:09, the sun wrote:
> On 2016/09/08 14:30:42, the sun wrote:
> > A revert of this CL (patchset #5 id:80001) has been created in
> > https://codereview.webrtc.org/2320113002/ by mailto:solenberg@webrtc.org.
> > 
> > The reason for reverting is: Breaks Chromium FYI bots because of link issue.
> 
> phoglund@: Are you sure this tool runs as expected on the Chromium bots? It
> looks to me like it would always fail (with the error "Failed to initialize
> voice engine base.") because the rtc_include_internal_audio_device build flag
is
> unset in Chromium builds, causing VoEBaseImpl::Init() to return -1.
> 
> On which bots is it supposed to run on?

It doesn't build from source on the Chromium bots. We build it in the WebRTC
repo and check in the binary into a gs bucket (one per platform), which is
downloaded when the test runs. I don't remember why I did it that way. The
version in the buckets is probably several years old.

[the sun, 2016-09-09 11:47:40]

On 2016/09/09 08:45:20, phoglund wrote:
> On 2016/09/09 07:52:09, the sun wrote:
> > On 2016/09/08 14:30:42, the sun wrote:
> > > A revert of this CL (patchset #5 id:80001) has been created in
> > > https://codereview.webrtc.org/2320113002/ by mailto:solenberg@webrtc.org.
> > > 
> > > The reason for reverting is: Breaks Chromium FYI bots because of link
issue.
> > 
> > phoglund@: Are you sure this tool runs as expected on the Chromium bots? It
> > looks to me like it would always fail (with the error "Failed to initialize
> > voice engine base.") because the rtc_include_internal_audio_device build
flag
> is
> > unset in Chromium builds, causing VoEBaseImpl::Init() to return -1.
> > 
> > On which bots is it supposed to run on?
> 
> It doesn't build from source on the Chromium bots. We build it in the WebRTC
> repo and check in the binary into a gs bucket (one per platform), which is
> downloaded when the test runs. I don't remember why I did it that way. The
> version in the buckets is probably several years old.

Ah, that explains things. Then the reason it did build in the Chromium bots
before this change, was because in audio_device.gypi, WEBRTC_DUMMY_AUDIO_BUILD
was set when include_internal_audio_device==0. I think it is better to minimize
the number of compiler flags so I'll just remove the force_mic_volume_max target
when rtc_include_internal_audio_device isn't defined. SGTY?

[phoglund, 2016-09-09 11:58:03]

On 2016/09/09 11:47:40, the sun wrote:
> On 2016/09/09 08:45:20, phoglund wrote:
> > On 2016/09/09 07:52:09, the sun wrote:
> > > On 2016/09/08 14:30:42, the sun wrote:
> > > > A revert of this CL (patchset #5 id:80001) has been created in
> > > > https://codereview.webrtc.org/2320113002/ by
mailto:solenberg@webrtc.org.
> > > > 
> > > > The reason for reverting is: Breaks Chromium FYI bots because of link
> issue.
> > > 
> > > phoglund@: Are you sure this tool runs as expected on the Chromium bots?
It
> > > looks to me like it would always fail (with the error "Failed to
initialize
> > > voice engine base.") because the rtc_include_internal_audio_device build
> flag
> > is
> > > unset in Chromium builds, causing VoEBaseImpl::Init() to return -1.
> > > 
> > > On which bots is it supposed to run on?
> > 
> > It doesn't build from source on the Chromium bots. We build it in the WebRTC
> > repo and check in the binary into a gs bucket (one per platform), which is
> > downloaded when the test runs. I don't remember why I did it that way. The
> > version in the buckets is probably several years old.
> 
> Ah, that explains things. Then the reason it did build in the Chromium bots
> before this change, was because in audio_device.gypi, WEBRTC_DUMMY_AUDIO_BUILD
> was set when include_internal_audio_device==0. I think it is better to
minimize
> the number of compiler flags so I'll just remove the force_mic_volume_max
target
> when rtc_include_internal_audio_device isn't defined. SGTY?

SGTM.

[the sun, 2016-09-09 12:41:31]

The CQ bit was checked by solenberg@webrtc.org

[the sun, 2016-09-09 12:41:32]

The patchset sent to the CQ was uploaded after l-g-t-m from phoglund@webrtc.org,
henrika@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2268183007/#ps120001 (title:
"Exclude force_mic_volume_max target when rtc_include_internal_audio_device not
set.")

[commit-bot: I haz the power, 2016-09-09 12:41:40]

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-09 14:42:17]

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

[commit-bot: I haz the power, 2016-09-09 14:42:18]

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

[the sun, 2016-09-09 17:56:49]

The CQ bit was checked by solenberg@webrtc.org

[commit-bot: I haz the power, 2016-09-09 17:56:53]

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-09 18:04:57]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

[commit-bot: I haz the power, 2016-09-09 18:04:59]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-09 18:05:05]

Description was changed from

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

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

to

==========
Don't use VoE legacy APIs in force_mic_volume_max tool.

BUG=webrtc:4690

Committed: https://crrev.com/ae9f2bdcef7f3a338ece6f57744c8c8f74d15483
Committed: https://crrev.com/fc433e65463e3a2e3cc43b3f8cb68849b00f381f
Cr-Original-Commit-Position: refs/heads/master@{#14135}
Cr-Commit-Position: refs/heads/master@{#14169}
==========

[commit-bot: I haz the power, 2016-09-09 18:05:06]

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