Optimize the repeated calls to AudioEffect.queryEffects() on Android

Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when configuring the audio device. Each of these calls was taking 5-10 milliseconds on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling the underlying operating system API; this eliminated about half of these calls. The other half happened inside static methods such as NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching through the list of effects. These calls have been replaced with searching through the cached list of effects, reducing the time to configure audio processing effects from 60-80 ms to 5-10. This results in a similar improvement in call setup time.

BUG=

Committed: https://crrev.com/880ffeb6c00ab5898d06c7ff93af3fe3cf3f790c
Cr-Commit-Position: refs/heads/master@{#13115}

[skvlad, 2016-06-09 22:59:49]

skvlad@webrtc.org changed reviewers:
+ henrika@webrtc.org, henrikg@webrtc.org, pthatcher@webrtc.org

[skvlad, 2016-06-09 22:59:49]

Please take a look, this is a very small change that makes a noticeable
improvement in call setup time.

[skvlad, 2016-06-09 23:11:38]

Patchset #1 (id:1) has been deleted

[skvlad, 2016-06-09 23:11:42]

Patchset #1 (id:20001) has been deleted

[henrika_webrtc, 2016-06-10 07:33:20]

Nice work Vladimir. Like how you figured out that isAvailable() was a wrapper
that looped over all effect types ;-)

[henrika_webrtc, 2016-06-10 07:39:17]

LGTM with some comments.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java
(right):

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:47:
// slow (~10ms) OS call multiple times.
I think this figure can vary a lot between devices and OS versions. Slow seems
enough IMO.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:69:
&& isAcousticEchoCancelerEffectAvailable();
Perhaps add a not on why we don't use the isAvailable() method which does seem
like the most natural approach.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:158:
// Returns true if the OS supports Acoustic Echo Cancellation (AEC).
It is actually not the OS as such that supports it but the device. I would say
"if the device"

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:406:
// queries the operating system for the list of effects.
are you OK with device here as well?

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:411:
// The caching is best effort only - when this method is called from several
Have you seen cases where this method is called from different threads?

[henrika_webrtc, 2016-06-10 07:39:45]

Description was changed from

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

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

to

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

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

[henrika_webrtc, 2016-06-10 07:39:46]

henrika@webrtc.org changed reviewers:
- henrikg@webrtc.org

[henrika_webrtc, 2016-06-10 07:40:31]

Moved grunell to CC list since he does not work in this area.

[skvlad, 2016-06-10 22:13:02]

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java
(right):

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:47:
// slow (~10ms) OS call multiple times.
On 2016/06/10 07:39:17, henrika_webrtc wrote:
> I think this figure can vary a lot between devices and OS versions. Slow seems
> enough IMO.

Done.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:69:
&& isAcousticEchoCancelerEffectAvailable();
On 2016/06/10 07:39:17, henrika_webrtc wrote:
> Perhaps add a not on why we don't use the isAvailable() method which does seem
> like the most natural approach.

Done.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:158:
// Returns true if the OS supports Acoustic Echo Cancellation (AEC).
On 2016/06/10 07:39:16, henrika_webrtc wrote:
> It is actually not the OS as such that supports it but the device. I would say
> "if the device"

Done.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:406:
// queries the operating system for the list of effects.
On 2016/06/10 07:39:16, henrika_webrtc wrote:
> are you OK with device here as well?

I think here "operating system" is the correct word - we're doing an OS API call
to retrieve the list of effects, as opposed to using a cached copy.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:411:
// The caching is best effort only - when this method is called from several
On 2016/06/10 07:39:16, henrika_webrtc wrote:
> Have you seen cases where this method is called from different threads?

It's normally only called from one thread - that's why I didn't try to optimize
for multiple threads. It will still work correctly though.

[henrika_webrtc, 2016-06-13 07:42:07]

I think you can land this change as is without further review of pthatcher@.

[henrika_webrtc, 2016-06-13 07:42:18]

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java
(right):

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:406:
// queries the operating system for the list of effects.
On 2016/06/10 22:13:01, skvlad wrote:
> On 2016/06/10 07:39:16, henrika_webrtc wrote:
> > are you OK with device here as well?
> 
> I think here "operating system" is the correct word - we're doing an OS API
call
> to retrieve the list of effects, as opposed to using a cached copy.

Acknowledged.

https://codereview.webrtc.org/2051323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:411:
// The caching is best effort only - when this method is called from several
On 2016/06/10 22:13:01, skvlad wrote:
> On 2016/06/10 07:39:16, henrika_webrtc wrote:
> > Have you seen cases where this method is called from different threads?
> 
> It's normally only called from one thread - that's why I didn't try to
optimize
> for multiple threads. It will still work correctly though.

Acknowledged.

[skvlad, 2016-06-13 18:02:40]

The CQ bit was checked by skvlad@webrtc.org

[skvlad, 2016-06-13 18:02:40]

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/2051323002/#ps60001 (title:
"Code review feedback")

[commit-bot: I haz the power, 2016-06-13 18:02:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2051323002/60001

[commit-bot: I haz the power, 2016-06-13 19:05:30]

Description was changed from

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

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

to

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

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

[commit-bot: I haz the power, 2016-06-13 19:05:31]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-06-13 19:05:38]

Description was changed from

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

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

to

==========
Optimize the repeated calls to AudioEffect.queryEffects() on Android

This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when
configuring the audio device. Each of these calls was taking 5-10 milliseconds
on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device
involved around 10 of these calls.

This change adds a method that checks the cached list of effects before calling
the underlying operating system API; this eliminated about half of these calls.
The other half happened inside static methods such as
NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching
through the list of effects. These calls have been replaced with searching
through the cached list of effects, reducing the time to configure audio
processing effects from 60-80 ms to 5-10. This results in a similar improvement
in call setup time.

BUG=

Committed: https://crrev.com/880ffeb6c00ab5898d06c7ff93af3fe3cf3f790c
Cr-Commit-Position: refs/heads/master@{#13115}
==========

[commit-bot: I haz the power, 2016-06-13 19:05:39]

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