New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 649018 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Chrome Dev is holding AudioMix lock causing increasing battery consumption

Project Member Reported by mef@chromium.org, Sep 21 2016

Issue description

Device name: Nexus 5x

From "Settings > About Chrome"
Application version: 54.0.2837.2
OS: N

URLs (if applicable): N/A

Behavior in Android Browser (if applicable):

There is no audio playing, but Chrome is holding AudioMix, and is causing increased battery consumption. See b/31514677 for details. 

Something is wrong with my Chrome Dev, as it is is stuck at 54.0.2837.2 while it should be 55.0.2860.0 or at least 54.0.2840.25.

Please let me know whether I should just uninstall / reinstall or if more info should be collected
 
Components: Blink>Media>Audio
Cc: dalecur...@chromium.org
Components: -Blink>Media>Audio Internals>Media>Audio
Status: Untriaged (was: Unconfirmed)

Comment 4 by mef@chromium.org, Sep 22 2016

Status: WontFix (was: Untriaged)
Chrome got updated to 55.0.2860.0 and the issue is no longer present. 
I'll reopen if I see it again.
If you encounter this again, check out chrome://media-internals and see what's listed under Audio. There shouldn't be any streams w/o a notification these days.

Comment 6 by mef@chromium.org, Sep 22 2016

Status: Unconfirmed (was: WontFix)
@dalecurtis - thanks! I've done that, and I didn't see any streams. 
Then I've opened lenta.ru in a new tab and audio stream got created.

I've closed the tab, but Output Streams still shows 'Stream 0:0' and I see the same behavior of volume controller as before - showing the note.
How long does that stick around? There is an idle timer which should shut that down.

Comment 8 by mef@chromium.org, Sep 22 2016

Should it shutdown only when tab is closed, or also if it goes into background?

I've had phone sleeping for 30 minutes with chrome in background, and that particular tab in background in chrome, but it didn't go away.

After closing the tab it did go away after short while.

Also, there is no visual or audio indication when it is on.
It should have gone away before then; after 10 seconds or so in fact. Do you remember what you were doing on lenta.ru? I can't get a stream to pop open there.

Comment 10 by mef@chromium.org, Sep 22 2016

Just opening main page and waiting a few seconds brings up an overlay ad, which seems to start media playback. Same happens on m.gazeta.ru
Components: -Internals>Media>Audio Blink>WebAudio
Not able to get the ad causing trouble, but can reproduce with a WebAudio demo page, so I think something has broken the pause code we had there. Taking a look.
Cc: -dalecur...@chromium.org grunell@chromium.org olka@chromium.org
Owner: dalecur...@chromium.org
Status: Started (was: Unconfirmed)
Hmm, this is caused by a deadlock in the WebAudio pausing code with the new mixer framework it looks like. Will see about redoing this code since there have been complaints about it in the past.
Labels: -Pri-3 M-54 ReleaseBlock-Stable Pri-1
Labels: Type-Bug-Regression

Comment 15 by mef@chromium.org, Sep 22 2016

Great, thanks for looking into it!
Cc: hongchan@chromium.org rtoy@chromium.org
https://codereview.chromium.org/2365723003 should resolve this. We'll need to merge it back too.
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b770266d26fc09ab8fb746562d008c5d436e4784

commit b770266d26fc09ab8fb746562d008c5d436e4784
Author: dalecurtis <dalecurtis@chromium.org>
Date: Tue Sep 27 04:54:40 2016

Break out WebAudio suspension code into new class. Add tests.

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}

[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/BUILD.gn
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/fake_audio_render_callback.h
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender.cc
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender.h
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender_unittest.cc

[Bulk edit]

This issue is listed as a release block stable for M54 Android.  We'll be cutting our stable candidate in just about two weeks, so time is running out to fix this bug - please prioritize working on it ASAP.

Are you sure this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label.
Unsure if this issue should block the release, or know the issue should block the release but we won't be able to fix it in time?  CC me so that we can discuss.

Thanks!
Labels: Merge-Request-54

Comment 20 by dimu@chromium.org, Sep 28 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 21 by bugdroid1@chromium.org, Sep 28 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/942fee339500a935be00ac81d3a71e4991aa6666

commit 942fee339500a935be00ac81d3a71e4991aa6666
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:30:19 2016

Merge M54: "Break out WebAudio suspension code into new class. Add tests."

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}
(cherry picked from commit b770266d26fc09ab8fb746562d008c5d436e4784)

Review URL: https://codereview.chromium.org/2382473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#568}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/BUILD.gn
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.cc
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 23 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/942fee339500a935be00ac81d3a71e4991aa6666

commit 942fee339500a935be00ac81d3a71e4991aa6666
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:30:19 2016

Merge M54: "Break out WebAudio suspension code into new class. Add tests."

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}
(cherry picked from commit b770266d26fc09ab8fb746562d008c5d436e4784)

Review URL: https://codereview.chromium.org/2382473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#568}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/BUILD.gn
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.cc
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender_unittest.cc

Sign in to add a comment