From 6ca0a68042c8affa4f105db0d6b00695418b2fbb Mon Sep 17 00:00:00 2001 From: Sam Edwards Date: Thu, 1 Mar 2018 16:44:46 -0700 Subject: [PATCH] openal: Retry deleting a buffer until success The rationale for this change is Apple's OpenAL implementation, which needs a little time after the `alSourcei(source, AL_BUFFER, 0);` call before any buffers used by that source are free for deletion. The defaults in the config variables are such that the OpenAL manager will attempt to delete a buffer up to 6 times (that is, the original attempt plus 5 reattempts), with delays of 1ms, 2ms, 4ms, 8ms, and 16ms before each reattempt - which means it'll wait a grand total of 31ms for a buffer to be free before assuming that some even greater problem must be happening and giving up. --- panda/src/audiotraits/config_openalAudio.cxx | 17 ++++++++ panda/src/audiotraits/config_openalAudio.h | 2 + panda/src/audiotraits/openalAudioManager.cxx | 41 +++++++++++++++++++- panda/src/audiotraits/openalAudioManager.h | 2 + panda/src/audiotraits/openalAudioSound.cxx | 5 +-- 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/panda/src/audiotraits/config_openalAudio.cxx b/panda/src/audiotraits/config_openalAudio.cxx index 4f9019c013..36803e7d04 100644 --- a/panda/src/audiotraits/config_openalAudio.cxx +++ b/panda/src/audiotraits/config_openalAudio.cxx @@ -30,6 +30,23 @@ ConfigVariableString openal_device PRC_DESC("Specify the OpenAL device string for audio playback (no quotes). If this " "is not specified, the OpenAL default device is used.")); +ConfigVariableInt openal_buffer_delete_reattempts +("openal-buffer-delete-reattempts", 5, + PRC_DESC("If deleting a buffer fails due to still being in use, the OpenAL " + "sound plugin will wait a moment and reattempt deletion, with an " + "exponentially-increasing delay for each attempt. This number " + "specifies how many repeat attempts (not counting the initial attempt) " + "should be made before giving up and raising an error.")); + +ConfigVariableDouble openal_buffer_delete_delay +("openal-buffer-delete-delay", 0.001, + PRC_DESC("If deleting a buffer fails due to still being in use, the OpenAL " + "sound plugin will wait a moment and reattempt deletion, with an " + "exponentially-increasing delay for each attempt. This number " + "specifies how long, in seconds, the OpenAL plugin will wait after " + "its first failed attempt. The second attempt will be double this " + "delay, the third quadruple, and so on.")); + /** * Initializes the library. This must be called at least once before any of diff --git a/panda/src/audiotraits/config_openalAudio.h b/panda/src/audiotraits/config_openalAudio.h index bcd8481acf..96d3429519 100644 --- a/panda/src/audiotraits/config_openalAudio.h +++ b/panda/src/audiotraits/config_openalAudio.h @@ -26,5 +26,7 @@ extern "C" EXPCL_OPENAL_AUDIO void init_libOpenALAudio(); extern "C" EXPCL_OPENAL_AUDIO Create_AudioManager_proc *get_audio_manager_func_openal_audio(); extern ConfigVariableString openal_device; +extern ConfigVariableInt openal_buffer_delete_reattempts; +extern ConfigVariableDouble openal_buffer_delete_delay; #endif // CONFIG_OPENALAUDIO_H diff --git a/panda/src/audiotraits/openalAudioManager.cxx b/panda/src/audiotraits/openalAudioManager.cxx index cbdaadbfa1..bc254d4791 100644 --- a/panda/src/audiotraits/openalAudioManager.cxx +++ b/panda/src/audiotraits/openalAudioManager.cxx @@ -1052,7 +1052,7 @@ OpenALAudioManager::SoundData:: if (_sample != 0) { if (_manager->_is_valid) { _manager->make_current(); - alDeleteBuffers(1,&_sample); + _manager->delete_buffer(_sample); } _sample = 0; } @@ -1128,3 +1128,42 @@ discard_excess_cache(int sample_limit) { delete sd; } } + +/** + * Deletes an OpenAL buffer. This is a special function because some + * implementations of OpenAL (e.g. Apple's) don't unlock the buffers + * immediately, due to needing to coordinate with another thread. If this is + * the case, the alDeleteBuffers call will error back with AL_INVALID_OPERATION + * as if trying to delete an actively-used buffer, which will tell us to wait a + * bit and try again. + */ +void OpenALAudioManager:: +delete_buffer(ALuint buffer) { + ReMutexHolder holder(_lock); + int attempt = 0; + ALuint error; + + // Keep trying until we succeed (or give up). + while (true) { + alDeleteBuffers(1, &buffer); + error = alGetError(); + + if (error == AL_NO_ERROR) { + // Success! This will happen right away 99% of the time. + return; + } else if (error != AL_INVALID_OPERATION) { + // We weren't expecting that. This should be reported. + break; + } else if (attempt >= openal_buffer_delete_reattempts.get_value()) { + // We ran out of reattempts. Give up. + break; + } else { + // Make another attempt after (delay * 2^n) seconds. + Thread::sleep(openal_buffer_delete_delay.get_value() * (1 << attempt)); + attempt++; + } + } + + // If we got here, one of the breaks above happened, indicating an error. + audio_error("failed to delete a buffer: " << alGetString(error) ); +} diff --git a/panda/src/audiotraits/openalAudioManager.h b/panda/src/audiotraits/openalAudioManager.h index 5917900869..1e16e4000b 100644 --- a/panda/src/audiotraits/openalAudioManager.h +++ b/panda/src/audiotraits/openalAudioManager.h @@ -129,6 +129,8 @@ private: void decrement_client_count(SoundData *sd); void discard_excess_cache(int limit); + void delete_buffer(ALuint buffer); + void starting_sound(OpenALAudioSound* audio); void stopping_sound(OpenALAudioSound* audio); diff --git a/panda/src/audiotraits/openalAudioSound.cxx b/panda/src/audiotraits/openalAudioSound.cxx index da04514e8b..86fc6e2bef 100644 --- a/panda/src/audiotraits/openalAudioSound.cxx +++ b/panda/src/audiotraits/openalAudioSound.cxx @@ -208,8 +208,7 @@ stop() { for (int i=0; i<((int)(_stream_queued.size())); i++) { ALuint buffer = _stream_queued[i]._buffer; if (buffer != _sd->_sample) { - alDeleteBuffers(1, &buffer); - al_audio_errcheck("deleting a buffer"); + _manager->delete_buffer(buffer); } } _stream_queued.resize(0); @@ -472,7 +471,7 @@ pull_used_buffers() { correct_calibrated_clock(rtc, al); } if (buffer != _sd->_sample) { - alDeleteBuffers(1,&buffer); + _manager->delete_buffer(buffer); } } } else {