Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Add ability to mute AudioServer. #63458

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Jul 25, 2022

Adds the option to change the audio driver to the Dummy driver and back at runtime, with a set of MuteState flags - Disabled (user control), Silence (period of silence), Focus Loss (when app is not in focus), and Paused (when app is paused).

Control for the flags is added for the editor in EditorSettings, and for the project in ProjectSettings.

Editor defaults to muted (Dummy driver) when there is no audio output, and automatically switches to active on output. This significantly reduces CPU usage.

Fixes #28039
Fixes #55608
May help with #38154

Summary

  • Can disable audio processing + driver when running the editor unless sound is being played.
  • Will disable audio processing + driver in the project manager.
  • Can mute audio on losing focus, or on pause / resume.
  • Doesn't load the audio driver unless required in project manager / editor, faster startup / shutdown.
  • How the system responds to state flag changes can be changed independently for the Editor (EditorSettings) and the running project (in ProjectSettings)

mute_proj_settings

mute_editor_settings

Notes

  • I had noticed that with the vital_updates_only redraw option, the biggest CPU hog in the editor was now the audio, even when playing no audio it does a significant amount of processing.

  • There have been various issues / PRs to reduce audio processing but they haven't amounted to much so far, so I had a first look through the audio today, and decided to have a wild stab at this. I'm not very versed in the audio, and am thus far from expert in this area, so the audio maintainers will have to decide whether this type of approach makes any sense. There are probably potential race conditions etc to contend with. Even if this doesn't prove fruitful I hope to spur the audio maintainers to offer such an option.

  • On my system there are two problems:

  1. A driver thread which is constantly using CPU
  2. This thread using Pulse audio which also sits at 5% usage.

Although we can optimize the audio processing code, ultimately it is in a thread which will keep running in a loop unless something stalls it, so we need to look at either sleeping (or increasing existing sleep, I noticed there is a 1ms sleep on some drivers) or pausing or removing threads if not necessary.

Here I have explored using the option to stop the thread, by allowing swapping the driver on the fly to and from the DummyAudioDriver.

  • In order to control the muting of the audio, there are a set of MuteFlags, which can be toggled at runtime. It will only play the audio when all are unset. The flags are:
  1. Disabled - A user flag.
  2. Focus Loss - Set and cleared when the app gains or loses focus.
  3. Paused - Set when app is paused and resumed.
  4. Silence - To be set when the audio encounters a section of silence, and automatically restart when sound detected again.

The sensitivity to each of these flags is controlled by a mask and can be either be set in the Editor / ProjectSettings, or altered at runtime (current defaults: editor is sensitive to all flags, but running projects are only sensitive to the disabled and paused flag). The idea is that most users will use the default for games, but people making apps might want to override this sensitivity mask.

  • In order to turn on and off audio processing, instead of initializing the audio driver once at startup, the AudioDriverManager can now be called at runtime to switch between audio drivers, and in this case, the Dummy driver (for muted) and the desired driver (for sound on).
  • The code required for this so far has been pretty minimal. I don't know to what extent the current driver implementations are safe to use by init and finalizing multiple times. It seems to work fine so far with Pulse, and Dummy (I had to fix a nullptr issue in Dummy).

Results

With the switch, and the vital_updates_only redraw option, my CPU usage (percentages of core, rather than whole CPU):

Audio on : Godot 7%, PulseAudio 5%
Audio off (dummy) : Godot 2%, PulseAudio 0%

This is 1/6 of the CPU usage. The dummy driver actually still creates a thread which does some processing, although this processing is minimized. I did try removing the dummy thread, but it prevented playbacks progressing, so I've avoided that for now.

Discussion

It has also occurred to me, that given in most cases users might wish to reduce CPU usage in the editor in all areas, we could potentially combine such a setting with the vital_updates_only redraw option. This would give one less button. But on the other hand this could be confusing / non obvious.

I've also implemented the approach suggested in #45948 (comment) . There is a Silence flag, which is set when 10 seconds of silence is detected, and cleared when playing the next sound.

Now that the silence flag is implemented and is default in the editor, it no longer seems necessary to have a mute button in the editor. (Although godotengine/godot-proposals#1066 suggests having the mute button also mute any projects that are run from the editor, so maybe we could have it do that, but could be a later PR.)

@Calinou has also suggested allowing switching to the mute mode when the editor loses focus, so I've added this.

There is also the possibility to use the is_audio_processing_allowed() to throttle processing in AudioStreamPlayers. I haven't done that here to keep the PR as minimal as possible. This would also be different in 4.x (a number of things are different in audio in 4.x I believe, so that would need an independent implementation).

An alternative instead of swapping driver might be to prevent heavy processing on the audio driver thread, and increase the sleep to a large value (say 100ms), while using the same logic from this PR.

Bugsquad edit: This closes godotengine/godot-proposals#1066 and partially addresses #54183.

servers/audio_server.h Outdated Show resolved Hide resolved
@lawnjelly lawnjelly force-pushed the audio_mute_editor branch 4 times, most recently from 21c9bac to 412a3c5 Compare July 28, 2022 06:55
@lawnjelly lawnjelly marked this pull request as ready for review July 28, 2022 06:58
@lawnjelly lawnjelly requested review from a team as code owners July 28, 2022 06:58
@lawnjelly
Copy link
Member Author

Marking as ready for review to get some eyes on this. I'm fully expecting a lot of bikeshedding / possible changes to the design, and this will require testing on all platforms.

But it seems well worth doing, as well as throttling CPU in the editor / project manager, it also allows running user apps to play nicely with the OS and turn off audio processing when paused, which may be particularly important on mobile and may be necessary to allow some platforms to suspend.

@ellenhp
Copy link
Contributor

ellenhp commented Jul 28, 2022

I like this approach. Just some background: In 4.x the audio server knows the playback state of almost all audio playback streams. The notable exception is that video players don't use the AudioStreamPlayback API yet, and are still stuck on the old way of registering a callback with the audio server. So if a video player is in the scene, the audio server does not directly know the state of that playback, and therefore doesn't have direct knowledge of whether it can sleep. If a video player is not in the scene, it would be able to choose to sleep whenever there are no playback streams currently playing, or after 10s of that being true, etc.

I like the idea of using an indirect way of determining whether sleeping is possible, though. A video player's audio callback may or may not choose to push samples to the AudioServer depending on its pause state, but if it does want to push samples it will need to do so by requesting a pointer to that buffer via thread_get_channel_mix_buffer. It hadn't occurred to me to do this and I think it accounts for every way that audio playback can occur in Godot.

Edit: this should also allow this to be backported potentially to 3.6+ which is kind of cool. I don't see anything in here that's specific to 4.0 after a brief review.

@Calinou
Copy link
Member

Calinou commented Jul 28, 2022

But it seems well worth doing, as well as throttling CPU in the editor / project manager, it also allows running user apps to play nicely with the OS and turn off audio processing when paused, which may be particularly important on mobile and may be necessary to allow some platforms to suspend.

Another upside of doing this is that the editor and project manager will no longer constantly display a "playing audio" icon in the KDE task bar:

image

@lawnjelly
Copy link
Member Author

lawnjelly commented Jul 28, 2022

Edit: this should also allow this to be backported potentially to 3.6+ which is kind of cool. I don't see anything in here that's specific to 4.0 after a brief review.

This is actually currently written on 3.x and I'm happy to forward port once we have tested out / bikeshedded a bit, as you say most of the code is shared.

The main question in my mind is whether the approach of hot swapping drivers is viable on all platforms (especially in terms of e.g. introducing audio delay or something). If not we could alternatively use more sleeps on some platforms, or an option to pause the audio driver if this exists on a platform, or simply turn off the functionality if problematic on a certain platform.

My instincts are that hot swapping the driver should be okay, as it should be the same to the OS as just loading a new app using the audio driver. But I am no expert here. 😁

I suspect we may have to just test as much as possible among the contributors on different platforms, then go for it in a beta / test build and see what happens. Possibly a 3.6 early beta in we have feature freeze on 4.0, but either / both is fine.

@lawnjelly
Copy link
Member Author

lawnjelly commented Feb 8, 2023

Just to note I'm happy to rebase this and port for 4.x.

We did discuss in a PR meeting last year but at the time @reduz wasn't convinced of the use case (or implementation, not sure which was the blocker). On the other hand it is especially relevant given the increased use of Godot editor on mobile where battery saving is important, and the needs of Mac users who want their machines to sleep.

EDIT: Rebased, and I've removed the mute button from the editor, as it no longer seems necessary now that the editor auto switches between dummy and active driver depending whether there is any audio output. I have backed up the branch with the mute button though.

@Calinou
Copy link
Member

Calinou commented Feb 10, 2023

We did discuss in a PR meeting last year but at the time @reduz wasn't convinced of the use case (or implementation, not sure which was the blocker). On the other hand it is especially relevant given the increased use of Godot editor on mobile where battery saving is important, and the needs of Mac users who want their machines to sleep.

This is definitely something we need in the long term, not only for Godot itself but also for non-game applications that only play sound occasionally. (Some games may also do this, such as idle/incremental games.)

Adds the option to change the audio driver to the Dummy driver and back at runtime, with a set of MuteState flags - Disabled (user control), Silence (period of silence), Focus Loss (when app is not in focus), and Paused (when app is paused).

Control for the flags is added for the editor in EditorSettings, and for the project in ProjectSettings.

Editor defaults to muted (Dummy driver) when there is no audio output, and automatically switches to active on output. This significantly reduces CPU usage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants