Stop enqueueing data to output audio device if consecutive empty buffers are received

Stop enqueueing data to output audio device if consecutive empty buffers are received

The current android implementation of WebAudio is not power friendly.
It holds the AudioMix wakelock, and causes a lot of battery consumption even if tab is backgrounded.
When an AudioContext is created, WebAudio will start enqueueing data to the output device.
This happens even when no data is decoded or no audio buffer is appended.
This CL let the OpenSLES player enter an idle mode when consecutive empty buffers are received.
In the idle mode, the player will no longer enqueue data to the output device.
It regularly check the received data and exits the idle mode if non-empty data are encountered.
The intervals to check the data is calculated by the consumption time of the last received audio.
This may introduce some drifting issues, but that's not as serious as the power consumption issue.

BUG=470153

[qinmin, 2015-05-28 23:55:17]

qinmin@chromium.org changed reviewers:
+ dalecurtis@chromium.org, henrika@chromium.org

[qinmin, 2015-05-28 23:55:18]

PTAL

[DaleCurtis, 2015-05-29 00:26:16]

I don't think this is the right layer to fix this at; it adds a fair bit of
complexity and hidden behavior for all clients.

Two empty buffers is really too small; that may be < 10ms. I don't think we want
something like this kicking in unless tens of seconds of silence have elapsed.

I'd prefer if this was fixed at the WebAudio layer, or if there's enough
justification for this, instead added at the AudioOutputResampler layer so that
all platforms could benefit from such behavior; we already have a
FakeAudioOutputStream for handling this.

[qinmin, 2015-05-29 04:28:38]

On 2015/05/29 00:26:16, DaleCurtis wrote:
> I don't think this is the right layer to fix this at; it adds a fair bit of
> complexity and hidden behavior for all clients.
> 
> Two empty buffers is really too small; that may be < 10ms. I don't think we
want
> something like this kicking in unless tens of seconds of silence have elapsed.
> 
> I'd prefer if this was fixed at the WebAudio layer, or if there's enough
> justification for this, instead added at the AudioOutputResampler layer so
that
> all platforms could benefit from such behavior; we already have a
> FakeAudioOutputStream for handling this.

I am wondering whether a general approach will be good for all platforms.
A proper way to handle this would be to call Stop() once we received a number of
empty audio chunks, and switch to the FakeAudioOutputStream. When audible data
arrives, switching back to low latency output stream and call start().
However, the different platforms might have different initialization cost when
calling start(). 
And the drifting requirement is different on desktop as compared to mobile. On
desktop, a tab can live for a really long time so drifting will accumulate. On
mobile, a tab can easily get killed.

[henrika (OOO until Aug 14), 2015-05-29 07:40:03]

I must agree with Dale here. If the goal is to come up with a scheme that fixes
a bad design in WebAudio where it keeps audio streams open for too long, then
this level can not be the right place. IMHO, it should be done at a higher level
and with a much more clear interface which shows that it is designed to save
power for WebAudio. Preferably, it should be solved as part of the WebAudio API.

My view is that, if this type of solution is required, it should be exposed for
all platforms and possibly only implemented for Android initially. As is, it can
kick in rather randomly based on a constant and it might affect other clients
(<audio>, WebRTC etc.). If added, a much, much larger value should be used to
ensure that it is only activated when it is absolutely needed.

Also, what are the usage numbers of WebAudio in Chrome on Android and what type
of gains are we talking about by limiting the stream-is-on time?

[qinmin, 2015-05-29 15:03:55]

On 2015/05/29 07:40:03, henrika wrote:
> I must agree with Dale here. If the goal is to come up with a scheme that
fixes
> a bad design in WebAudio where it keeps audio streams open for too long, then
> this level can not be the right place. IMHO, it should be done at a higher
level
> and with a much more clear interface which shows that it is designed to save
> power for WebAudio. Preferably, it should be solved as part of the WebAudio
API.
> 
> My view is that, if this type of solution is required, it should be exposed
for
> all platforms and possibly only implemented for Android initially. As is, it
can
> kick in rather randomly based on a constant and it might affect other clients
> (<audio>, WebRTC etc.). If added, a much, much larger value should be used to
> ensure that it is only activated when it is absolutely needed.
> 
> Also, what are the usage numbers of WebAudio in Chrome on Android and what
type
> of gains are we talking about by limiting the stream-is-on time?

UMA page doesn't have WebAudio usage in Chrome. Even if it has, the problem can
happen as long as the page has a AudioContext object created, with no subsequent
WebAudio Elements and calls.
The problem of constantly writing data to OpenSLES is that chrome will hold the
AudioMix wakelock. That makes the device unable to sleep and drain battery much
much faster than it should.

[henrika (OOO until Aug 14), 2015-05-29 15:21:00]

I would then argue that the WebAudio API is broken on Android and should be
fixed properly in the actual API instead.
Simply creating a context should not start audio rendering IMHO.
I assume the same type of issue exists on iOS then as well.

Also, if UMA stats for WebAudio usage does not exist; why not start by adding
that to learn about the usage first.

[Raymond Toy, 2015-05-29 15:29:46]

On 2015/05/29 15:21:00, henrika wrote:
> I would then argue that the WebAudio API is broken on Android and should be
> fixed properly in the actual API instead.
> Simply creating a context should not start audio rendering IMHO.

What would be the appropriate mechanism to start?  And as I understand it,
starting the 
audio hardware does not happen instantly, so there is an issue that sound is
started
but nothing happens for perhaps 100 ms.  This is not really acceptable.

I think WebAudio could autopause without too much problem.  It's the restarting
that
is difficult.

Plus, there is now an api, suspend(), to pause the audio HW as needed.

> I assume the same type of issue exists on iOS then as well.
> 
> Also, if UMA stats for WebAudio usage does not exist; why not start by adding
> that to learn about the usage first.

https://www.chromestatus.com/metrics/feature/timeline/popularity/652 has some
information.

[henrika (OOO until Aug 14), 2015-05-29 15:48:45]

My point is that it feels as if this CL attacks the issue from the "wrong
direction". It feels less disruptive to add proper support for start/stop from
the API side instead of injecting some sort of detection mechanism for one
platform where it is not clear if other APIs are affected as well. Adding a
latency of x ms at start/stop does not seem as severe are draining the battery.

rtoy@: where do you get the 100ms figure from?

I still advocate some sort of UMA stat to further illustrate the usage of
WebAudio on Android.

[qinmin, 2015-05-29 18:11:29]

On 2015/05/29 15:48:45, henrika wrote:
> My point is that it feels as if this CL attacks the issue from the "wrong
> direction". It feels less disruptive to add proper support for start/stop from
> the API side instead of injecting some sort of detection mechanism for one
> platform where it is not clear if other APIs are affected as well. Adding a
> latency of x ms at start/stop does not seem as severe are draining the
battery.
> 
> rtoy@: where do you get the 100ms figure from?
> 
> I still advocate some sort of UMA stat to further illustrate the usage of
> WebAudio on Android.

A Fix on the API would take much longer time to address the problem. And this is
definitely a P1 issue, if not P0. 
Most web developers would have no idea that a single AudioContext object can
have such a big impact on battery life.
And it pretty common for an ordinary user to come across a page that has an
AudioContext object on it.

[qinmin, 2015-05-29 18:15:42]

On 2015/05/29 18:11:29, qinmin(OOO_till_6-5) wrote:
> On 2015/05/29 15:48:45, henrika wrote:
> > My point is that it feels as if this CL attacks the issue from the "wrong
> > direction". It feels less disruptive to add proper support for start/stop
from
> > the API side instead of injecting some sort of detection mechanism for one
> > platform where it is not clear if other APIs are affected as well. Adding a
> > latency of x ms at start/stop does not seem as severe are draining the
> battery.
> > 
> > rtoy@: where do you get the 100ms figure from?
> > 
> > I still advocate some sort of UMA stat to further illustrate the usage of
> > WebAudio on Android.
> 
> A Fix on the API would take much longer time to address the problem. And this
is
> definitely a P1 issue, if not P0. 
> Most web developers would have no idea that a single AudioContext object can
> have such a big impact on battery life.
> And it pretty common for an ordinary user to come across a page that has an
> AudioContext object on it.

When I mentioned API, i mean spec. And this is definitely an issue we should
address ASAP.

[DaleCurtis, 2015-05-29 18:16:58]

Even p0 this is the wrong place, we can put a block in WebAudio under OS_ANDROID
which forces suspend/resume.  The right location for that is here:

https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

[henrika (OOO until Aug 14), 2015-05-31 08:19:15]

Agree with Dale. Adding this functionality to the native layer is not the right
approach. It hides the actual problem and will make it difficult to maintain and
resolve issues related to audio playout on Android for other APIs.

In addition, adding more complexity to the callback handler is not a good idea
since callback handlers are called from internal non-application thread(s) which
are not attached to the Dalvik virtual machine. And because these internal
threads are critical to the integrity of the OpenSL ES implementation, a
callback handler should not block or perform excessive work.

[qinmin, 2015-05-31 19:10:49]

On 2015/05/31 08:19:15, henrika wrote:
> Agree with Dale. Adding this functionality to the native layer is not the
right
> approach. It hides the actual problem and will make it difficult to maintain
and
> resolve issues related to audio playout on Android for other APIs.
> 
> In addition, adding more complexity to the callback handler is not a good idea
> since callback handlers are called from internal non-application thread(s)
which
> are not attached to the Dalvik virtual machine. And because these internal
> threads are critical to the integrity of the OpenSL ES implementation, a
> callback handler should not block or perform excessive work.

Unlike desktop platforms, I don't this starvation will have any adverse impact
on the integrity of OpenSL ES implementations.
And this is documented in the OpenSL ES document. The starvation state is
expected by OpenSL ES.
"When the player is in the SL_PLAYSTATE_PLAYING state, which is controlled by
the SLPlayItf interface [see section 8.32], adding buffers will implicitly start
playback. In the case of starvation due to insufficient buffers in the queue,
the playing of audio data stops. The player remains in the SL_PLAYSTATE_PLAYING
state. Upon queuing of additional buffers, the playing of audio data resumes.
Note that starvation of queued buffers causes audible gaps in the audio data
stream. In the case where the player is not in the playing state, addition of
buffers does not start audio playback."