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.

Sign up for GitHub

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

Sleep the coreaudio driver in-editor after 15s of silence #45948

Closed
wants to merge 3 commits into from
Closed

Sleep the coreaudio driver in-editor after 15s of silence #45948

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3187f3
Sleep the coreaudio driver in-editor after 15s of silence
ellenhp Feb 13, 2021
02b3631
Fix a comment and refactor coreaudio driver start/stop
ellenhp Feb 13, 2021
d60e691
Manage playing_sources_count with atomics instead of mutexes.
ellenhp Feb 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions drivers/coreaudio/audio_driver_coreaudio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,25 +243,47 @@ OSStatus AudioDriverCoreAudio::input_callback(void *inRefCon,
}

void AudioDriverCoreAudio::start() {
if (!active) {
if (!active && !sleeping) {
ellenhp marked this conversation as resolved.
Show resolved Hide resolved
OSStatus result = AudioOutputUnitStart(audio_unit);
if (result != noErr) {
ERR_PRINT("AudioOutputUnitStart failed, code: " + itos(result));
} else {
active = true;
}
} else if (!active && sleeping) {
active = true;
}
};

void AudioDriverCoreAudio::stop() {
if (active) {
if (active && !sleeping) {
OSStatus result = AudioOutputUnitStop(audio_unit);
if (result != noErr) {
ERR_PRINT("AudioOutputUnitStop failed, code: " + itos(result));
} else {
active = false;
}
} else if (active && sleeping) {
active = false;
}
}

void AudioDriverCoreAudio::set_sleep_state(bool p_sleeping) {
if (active && !sleeping && p_sleeping) {
OSStatus result = AudioOutputUnitStop(audio_unit);
if (result != noErr) {
ERR_PRINT("AudioOutputUnitStop failed, code: " + itos(result));
return;
}
}
if (active && sleeping && !p_sleeping) {
OSStatus result = AudioOutputUnitStart(audio_unit);
if (result != noErr) {
ERR_PRINT("AudioOutputUnitStart failed, code: " + itos(result));
return;
}
}
sleeping = p_sleeping;
}

int AudioDriverCoreAudio::get_mix_rate() const {
Expand Down
2 changes: 2 additions & 0 deletions drivers/coreaudio/audio_driver_coreaudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class AudioDriverCoreAudio : public AudioDriver {
AudioComponentInstance input_unit = nullptr;

bool active = false;
bool sleeping = false;
Mutex mutex;

String device_name = "Default";
Expand Down Expand Up @@ -93,6 +94,7 @@ class AudioDriverCoreAudio : public AudioDriver {

virtual Error init();
virtual void start();
virtual void set_sleep_state(bool p_sleeping);
virtual int get_mix_rate() const;
virtual SpeakerMode get_speaker_mode() const;

Expand Down
22 changes: 22 additions & 0 deletions scene/2d/audio_stream_player_2d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
#include "scene/main/window.h"

void AudioStreamPlayer2D::_mix_audio() {
if (!is_playing() && has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
if (!stream_playback.is_valid() || !active ||
(stream_paused && !stream_paused_fade_out)) {
return;
Expand Down Expand Up @@ -152,6 +156,11 @@ void AudioStreamPlayer2D::_notification(int p_what) {

if (p_what == NOTIFICATION_EXIT_TREE) {
AudioServer::get_singleton()->remove_callback(_mix_audios, this);
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (p_what == NOTIFICATION_PAUSED) {
Expand Down Expand Up @@ -305,6 +314,14 @@ float AudioStreamPlayer2D::get_pitch_scale() const {
}

void AudioStreamPlayer2D::play(float p_from_pos) {
if (!is_playing()) {
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (!is_playing()) {
// Reset the prev_output_count if the stream is stopped
prev_output_count = 0;
Expand All @@ -315,6 +332,11 @@ void AudioStreamPlayer2D::play(float p_from_pos) {
setplay = p_from_pos;
output_ready = false;
set_physics_process_internal(true);
MutexLock lock(playback_lock_mutex);
if (!has_playback_lock) {
AudioServer::get_singleton()->notify_source_playing();
has_playback_lock = true;
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions scene/2d/audio_stream_player_2d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ class AudioStreamPlayer2D : public Node2D {
float max_distance = 2000.0;
float attenuation = 1.0;

bool has_playback_lock = false;
Mutex playback_lock_mutex;
RandomShaper marked this conversation as resolved.
Show resolved Hide resolved

protected:
void _validate_property(PropertyInfo &property) const override;
void _notification(int p_what);
Expand Down
18 changes: 18 additions & 0 deletions scene/3d/audio_stream_player_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ void AudioStreamPlayer3D::_calc_output_vol(const Vector3 &source_dir, real_t tig
}

void AudioStreamPlayer3D::_mix_audio() {
if (!is_playing()) {
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (!stream_playback.is_valid() || !active ||
(stream_paused && !stream_paused_fade_out)) {
return;
Expand Down Expand Up @@ -338,6 +346,11 @@ void AudioStreamPlayer3D::_notification(int p_what) {

if (p_what == NOTIFICATION_EXIT_TREE) {
AudioServer::get_singleton()->remove_callback(_mix_audios, this);
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (p_what == NOTIFICATION_PAUSED) {
Expand Down Expand Up @@ -687,6 +700,11 @@ void AudioStreamPlayer3D::play(float p_from_pos) {
setplay = p_from_pos;
output_ready = false;
set_physics_process_internal(true);
MutexLock lock(playback_lock_mutex);
if (!has_playback_lock) {
AudioServer::get_singleton()->notify_source_playing();
has_playback_lock = true;
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions scene/3d/audio_stream_player_3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ class AudioStreamPlayer3D : public Node3D {

float max_distance = 0.0;

bool has_playback_lock = false;
Mutex playback_lock_mutex;

Ref<VelocityTracker3D> velocity_tracker;

DopplerTracking doppler_tracking = DOPPLER_TRACKING_DISABLED;
Expand Down
18 changes: 18 additions & 0 deletions scene/audio/audio_stream_player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ void AudioStreamPlayer::_mix_internal(bool p_fadeout) {
}

void AudioStreamPlayer::_mix_audio() {
if (!is_playing()) {
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (use_fadeout) {
_mix_to_bus(fadeout_buffer.ptr(), fadeout_buffer.size());
use_fadeout = false;
Expand Down Expand Up @@ -152,6 +160,11 @@ void AudioStreamPlayer::_notification(int p_what) {

if (p_what == NOTIFICATION_EXIT_TREE) {
AudioServer::get_singleton()->remove_callback(_mix_audios, this);
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}

if (p_what == NOTIFICATION_PAUSED) {
Expand Down Expand Up @@ -241,6 +254,11 @@ void AudioStreamPlayer::play(float p_from_pos) {
stop_has_priority = false;
active = true;
set_process_internal(true);
MutexLock lock(playback_lock_mutex);
if (!has_playback_lock) {
AudioServer::get_singleton()->notify_source_playing();
has_playback_lock = true;
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions scene/audio/audio_stream_player.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class AudioStreamPlayer : public Node {

MixTarget mix_target = MIX_TARGET_STEREO;

bool has_playback_lock = false;
Mutex playback_lock_mutex;

void _mix_internal(bool p_fadeout);
void _mix_audio();
static void _mix_audios(void *self) { reinterpret_cast<AudioStreamPlayer *>(self)->_mix_audio(); }
Expand Down
18 changes: 17 additions & 1 deletion scene/gui/video_player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ void VideoPlayer::_mix_audios(void *p_self) {

// Called from audio thread
void VideoPlayer::_mix_audio() {
if (!is_playing()) {
MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
}
if (!stream.is_valid()) {
return;
}
Expand Down Expand Up @@ -139,7 +146,11 @@ void VideoPlayer::_notification(int p_notification) {

case NOTIFICATION_EXIT_TREE: {
AudioServer::get_singleton()->remove_callback(_mix_audios, this);

MutexLock lock(playback_lock_mutex);
if (has_playback_lock) {
AudioServer::get_singleton()->notify_source_stopped_playing();
has_playback_lock = false;
}
} break;

case NOTIFICATION_INTERNAL_PROCESS: {
Expand Down Expand Up @@ -261,6 +272,11 @@ void VideoPlayer::play() {
// AudioServer::get_singleton()->stream_set_active(stream_rid,true);
// AudioServer::get_singleton()->stream_set_volume_scale(stream_rid,volume);
last_audio_time = 0;
MutexLock lock(playback_lock_mutex);
if (!has_playback_lock) {
AudioServer::get_singleton()->notify_source_playing();
has_playback_lock = true;
}
};

void VideoPlayer::stop() {
Expand Down
3 changes: 3 additions & 0 deletions scene/gui/video_player.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class VideoPlayer : public Control {

StringName bus;

bool has_playback_lock = false;
Mutex playback_lock_mutex;

void _mix_audio();
static int _audio_mix_callback(void *p_udata, const float *p_data, int p_frames);
static void _mix_audios(void *p_self);
Expand Down
26 changes: 26 additions & 0 deletions servers/audio_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,22 @@ float AudioServer::get_global_rate_scale() const {
return global_rate_scale;
}

void AudioServer::notify_source_playing() {
MutexLock lock(playing_sources_mutex);
if (playing_sources_count == 0) {
RandomShaper marked this conversation as resolved.
Show resolved Hide resolved
AudioDriver::get_singleton()->set_sleep_state(false);
}
playing_sources_count++;
}

void AudioServer::notify_source_stopped_playing() {
MutexLock lock(playing_sources_mutex);
playing_sources_count--;
if (playing_sources_count == 0) {
last_playback_time_msec = OS::get_singleton()->get_ticks_msec();
}
}

void AudioServer::init_channels_and_buffers() {
channel_count = get_channel_count();
temp_buffer.resize(channel_count);
Expand Down Expand Up @@ -1032,6 +1048,14 @@ void AudioServer::update() {
for (Set<CallbackItem>::Element *E = update_callbacks.front(); E; E = E->next()) {
E->get().callback(E->get().userdata);
}

if (Engine::get_singleton()->is_editor_hint()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this change should only work for editor, maybe #ifdef TOOLS_ENABLED should also be added, so it's not included in templates builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason I see to do that is export binary size and potentially (slightly) reducing lock contention, which makes me wonder if I should do it elsewhere too. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't yet checked the whole PR, but I can already say if you want something to happen only in the editor the current check is better, because it is false both on tools builds when running a project (vs. running the editor) and on non-tools builds.
Furthermore, in non-tools builds, given the check is known to fail at compile time, I'm quite certain that compilers can optimize out the body.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, some places seem to use only Engine::get_singleton()->is_editor_hint() while others use both Engine::get_singleton()->is_editor_hint() and TOOLS_ENABLED checks. Maybe I'm missing something, but if using only Engine::get_singleton()->is_editor_hint() is preferred it's good to know :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no clear policy, but yes we sometimes put both checks in place to ensure that the editor-specific code (often expensive debug code) is compiled out in the runtime. If @RandomShaper is correct that compiler would do it just fine with is_editor_hint() only, then we shouldn't need to do that indeed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to have access to this feature enabled for any other engine mode in project settings, then I guess TOOLS_ENABLED isn't needed too.

// Allow the audio driver to sleep in the engine after 15 seconds of inactivity.
MutexLock lock(playing_sources_mutex);
if (playing_sources_count == 0 && last_playback_time_msec < OS::get_singleton()->get_ticks_msec() - 15000) {
AudioDriver::get_singleton()->set_sleep_state(true);
}
}
}

void AudioServer::load_default_bus_layout() {
Expand Down Expand Up @@ -1319,6 +1343,8 @@ AudioServer::AudioServer() {
mix_time = 0;
mix_size = 0;
global_rate_scale = 1;
playing_sources_count = 0;
last_playback_time_msec = 0;
}

AudioServer::~AudioServer() {
Expand Down
9 changes: 9 additions & 0 deletions servers/audio_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class AudioDriver {
unsigned int get_input_position() { return input_position; }
unsigned int get_input_size() { return input_size; }

virtual void set_sleep_state(bool p_sleep) {}

#ifdef DEBUG_ENABLED
uint64_t get_profiling_time() const { return prof_time; }
void reset_profiling_time() { prof_time = 0; }
Expand Down Expand Up @@ -243,6 +245,10 @@ class AudioServer : public Object {
Set<CallbackItem> callbacks;
Set<CallbackItem> update_callbacks;

Mutex playing_sources_mutex;
size_t playing_sources_count;
uint64_t last_playback_time_msec;

friend class AudioDriver;
void _driver_process(int p_frames, int32_t *p_buffer);

Expand Down Expand Up @@ -319,6 +325,9 @@ class AudioServer : public Object {
void set_global_rate_scale(float p_scale);
float get_global_rate_scale() const;

void notify_source_playing();
void notify_source_stopped_playing();

virtual void init();
virtual void finish();
virtual void update();
Expand Down