1621684 - audio-playing wakelock isn't being created on Windows

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

The audio-playing wakelock is used by the ProcessPriorityManager to know not to lower the process priority of background tabs that are playing audio.

I just checked builds going back to when this first landed, and it seems as if the audio wakelock never was being set. This means that background audio tabs always get low process priority unless they're playing video (at which time they are set to "below normal").

Strangely, it looks like the audio wakelocks are being set on my macOS device. Not sure why.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I find this really strange, because the bug that originally added support for the audio-playing wakelock in bug 1524193, and that bug was verified.

We should definitely add some regression tests for this.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Any idea why we're not taking an audio-wakelock on Windows? I was able to reproduce the issue on Windows 10 and 7.

I can also work on a regression test for the process priority manager while we figure out how to fix the audio-wakelock.

Comment 3

[Paul Adenot (:padenot)]

No audio wakelock is taken when using the Web Audio API it seems like: https://searchfox.org/mozilla-central/search?q=%22audio-playing%22&path=.

Patching AudioDestinationNode should make this work. The wake lock should be acquired on construction, then dropped in suspend and resume: https://searchfox.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#483-491.

Comment 4

[Paul Adenot (:padenot)]

And yes, we really need a regression test for this, more and more websites are using the Web Audio API for their output component (usually it's fine because Web Audio API is used for interactive things: people don't switch tabs), and it's not going to slow down with AudioWorklet shipping.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1621684 - Take an audio-playing wakelock when using WebAudio. r?padenot!

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I guess I'll take this. Test coming up.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1621684 - Add tests for the ProcessPriorityManager. r?gsvelto!

Comment 8

[Mike Conley (:mconley) (:⚙️)]

ni?ing padenot for https://phabricator.services.mozilla.com/D67544#2063945.

Comment 9

[Paul Adenot (:padenot)]

I'm sorry didn't see this, test looks perfect, I added another it to the C++ (just a line to remove). Thanks!

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Heyo - do you have time these days for a review like this? No worries if you don't - just let me know. :)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac87de0a73e9
Take an audio-playing wakelock when using WebAudio. r=padenot
https://hg.mozilla.org/integration/autoland/rev/00bde57608b3
Add tests for the ProcessPriorityManager. r=gsvelto

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/ac87de0a73e9
https://hg.mozilla.org/mozilla-central/rev/00bde57608b3

Comment 14

[Paul Adenot (:padenot)]

[Tracking Requested - why for this release]:
This is really important for audio quality on loaded and/or low power machine. Not taking this results in frequent audio glitches on websites that use the Web Audio API to output music, such as soundcloud, tidal, and probably ton of others.

Comment 15

[Paul Adenot (:padenot)]

Comment on attachment 9134511 [details]
Bug 1621684 - Take an audio-playing wakelock when using WebAudio. r?padenot!

Beta/Release Uplift Approval Request

• User impact if declined: Frequent audio gliches on Windows when the machine is under load or underpowered, on services that use the Web Audio API to output audio (soundcloud, tidal, others.), when the tab is not in foreground
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: (this has been verified fixed by one of the reporter of this issue)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is not risky because it's exactly the same thing we're doing in HTMLMediaElement (probably to the letter, in fact).
• String changes made/needed: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9134511 [details]
Bug 1621684 - Take an audio-playing wakelock when using WebAudio. r?padenot!

Approving for 75.0rc2 on Julien's behalf.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/fdbdf870e369
https://hg.mozilla.org/releases/mozilla-release/rev/b3173b25e606

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.