From 5abf3558be70ffeba89087b75f46d9208542b4b1 Mon Sep 17 00:00:00 2001 From: Dmitry Marakasov Date: Tue, 16 Dec 2014 05:51:50 +0300 Subject: [PATCH 1/3] Move audio callback from AudioSpec into AudioDevice Though this is not 100% compatible with SDL2, this makes API much more consistent and less error prone. For example, you don't need to store AudioSpec along with AudioDevice just to have your callback lambda around, you don't need to copy AudioSpec from Wav file just to fill in the callback (see wav demo), you are no more obliged to take care of locking AudioDevice while replacing the callback. --- SDL2pp/Audio.hh | 31 ++++++++++++++----------------- SDL2pp/AudioDevice.cc | 33 ++++++++++++++++++++++++++++----- SDL2pp/AudioSpec.cc | 23 +---------------------- demos/audio_sine.cc | 8 ++++---- demos/audio_wav.cc | 13 +++---------- 5 files changed, 50 insertions(+), 58 deletions(-) diff --git a/SDL2pp/Audio.hh b/SDL2pp/Audio.hh index 73c316a..ffbd554 100644 --- a/SDL2pp/Audio.hh +++ b/SDL2pp/Audio.hh @@ -32,19 +32,10 @@ namespace SDL2pp { class AudioSpec : public SDL_AudioSpec { -public: - typedef std::function AudioCallback; - -private: - AudioCallback callback_; - -private: - static void SDLCallback(void *userdata, Uint8* stream, int len); public: AudioSpec(); - AudioSpec(int freq, SDL_AudioFormat format, Uint8 channels, Uint16 samples, AudioCallback&& callback = AudioCallback()); - AudioSpec(const AudioSpec& other, AudioCallback&& callback = AudioCallback()); + AudioSpec(int freq, SDL_AudioFormat format, Uint8 channels, Uint16 samples); ~AudioSpec(); AudioSpec(AudioSpec&& other); @@ -52,8 +43,6 @@ public: AudioSpec(const AudioSpec& other) = delete; AudioSpec& operator=(const AudioSpec& other) = delete; - void ChangeCallback(AudioCallback&& callback); // should be called with audio device using this spec locked! - void MergeChanges(const SDL_AudioSpec& obtained); const SDL_AudioSpec* Get() const; @@ -61,9 +50,6 @@ public: }; class AudioDevice { -private: - SDL_AudioDeviceID device_id_; - public: class LockHandle { friend class AudioDevice; @@ -83,9 +69,18 @@ public: LockHandle& operator=(const LockHandle& other) = delete; }; + typedef std::function AudioCallback; + +private: + SDL_AudioDeviceID device_id_; + AudioCallback callback_; + +private: + static void SDLCallback(void *userdata, Uint8* stream, int len); + public: - AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec); - AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes); + AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec, AudioCallback&& callback = AudioCallback()); + AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes, AudioCallback&& callback = AudioCallback()); virtual ~AudioDevice(); AudioDevice(const AudioDevice& other) = delete; @@ -98,6 +93,8 @@ public: void Pause(bool pause_on); SDL_AudioStatus GetStatus() const; + void ChangeCallback(AudioCallback&& callback); + LockHandle Lock(); #ifdef SDL2PP_NEW_2_0_4 diff --git a/SDL2pp/AudioDevice.cc b/SDL2pp/AudioDevice.cc index 43da4cd..319b7d0 100644 --- a/SDL2pp/AudioDevice.cc +++ b/SDL2pp/AudioDevice.cc @@ -25,17 +25,32 @@ namespace SDL2pp { -AudioDevice::AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec) { +void AudioDevice::SDLCallback(void *userdata, Uint8* stream, int len) { + AudioDevice* audiodevice = static_cast(userdata); + audiodevice->callback_(stream, len); +} + +AudioDevice::AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec, AudioDevice::AudioCallback&& callback) : callback_(std::move(callback)) { + SDL_AudioSpec spec_with_callback = *spec.Get(); + if (callback_) { + spec_with_callback.callback = SDLCallback; + spec_with_callback.userdata = static_cast(this); + } SDL_AudioSpec obtained; - if ((device_id_ = SDL_OpenAudioDevice(device.empty() ? nullptr : device.c_str(), iscapture ? 1 : 0, &spec, &obtained, 0)) == 0) + if ((device_id_ = SDL_OpenAudioDevice(device.empty() ? nullptr : device.c_str(), iscapture ? 1 : 0, &spec_with_callback, &obtained, 0)) == 0) throw Exception("SDL_OpenAudioDevice failed"); } -AudioDevice::AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes) { +AudioDevice::AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes, AudioDevice::AudioCallback&& callback) : callback_(std::move(callback)) { + SDL_AudioSpec spec_with_callback = *spec.Get(); + if (callback_) { + spec_with_callback.callback = SDLCallback; + spec_with_callback.userdata = static_cast(this); + } SDL_AudioSpec obtained; - if ((device_id_ = SDL_OpenAudioDevice(device.empty() ? nullptr : device.c_str(), iscapture ? 1 : 0, &spec, &obtained, allowed_changes)) == 0) + if ((device_id_ = SDL_OpenAudioDevice(device.empty() ? nullptr : device.c_str(), iscapture ? 1 : 0, &spec_with_callback, &obtained, allowed_changes)) == 0) throw Exception("SDL_OpenAudioDevice failed"); spec.MergeChanges(obtained); @@ -46,7 +61,7 @@ AudioDevice::~AudioDevice() { SDL_CloseAudioDevice(device_id_); } -AudioDevice::AudioDevice(AudioDevice&& other) noexcept : device_id_(other.device_id_) { +AudioDevice::AudioDevice(AudioDevice&& other) noexcept : device_id_(other.device_id_), callback_(std::move(other.callback_)) { other.device_id_ = 0; } @@ -58,6 +73,7 @@ AudioDevice& AudioDevice::operator=(AudioDevice&& other) noexcept { SDL_CloseAudioDevice(device_id_); device_id_ = other.device_id_; + callback_ = std::move(other.callback_); other.device_id_ = 0; return *this; @@ -75,6 +91,13 @@ SDL_AudioStatus AudioDevice::GetStatus() const { return SDL_GetAudioDeviceStatus(device_id_); } +void AudioDevice::ChangeCallback(AudioDevice::AudioCallback&& callback) { + // make sure callback is not called while it's being replaced + LockHandle lock = Lock(); + + callback_ = std::move(callback); +} + AudioDevice::LockHandle AudioDevice::Lock() { return LockHandle(this); } diff --git a/SDL2pp/AudioSpec.cc b/SDL2pp/AudioSpec.cc index 455d146..54de246 100644 --- a/SDL2pp/AudioSpec.cc +++ b/SDL2pp/AudioSpec.cc @@ -25,33 +25,16 @@ namespace SDL2pp { -void AudioSpec::SDLCallback(void *userdata, Uint8* stream, int len) { - AudioSpec* audiospec = static_cast(userdata); - audiospec->callback_(stream, len); -} - AudioSpec::AudioSpec() { std::fill((char*)this, (char*)this + sizeof(SDL_AudioSpec), 0); } -AudioSpec::AudioSpec(int freq, SDL_AudioFormat format, Uint8 channels, Uint16 samples, AudioSpec::AudioCallback&& callback) - : callback_(std::move(callback)) { +AudioSpec::AudioSpec(int freq, SDL_AudioFormat format, Uint8 channels, Uint16 samples) { std::fill((char*)this, (char*)this + sizeof(SDL_AudioSpec), 0); SDL_AudioSpec::freq = freq; SDL_AudioSpec::format = format; SDL_AudioSpec::channels = channels; SDL_AudioSpec::samples = samples; - if (callback) { - SDL_AudioSpec::callback = SDLCallback; - SDL_AudioSpec::userdata = static_cast(this); - } -} - -AudioSpec::AudioSpec(const AudioSpec& other, AudioSpec::AudioCallback&& callback) : SDL_AudioSpec(*other.Get()), callback_(std::move(callback)) { - if (callback) { - SDL_AudioSpec::callback = SDLCallback; - SDL_AudioSpec::userdata = static_cast(this); - } } AudioSpec::~AudioSpec() { @@ -60,10 +43,6 @@ AudioSpec::~AudioSpec() { AudioSpec::AudioSpec(AudioSpec&&) = default; AudioSpec& AudioSpec::operator=(AudioSpec&&) = default; -void AudioSpec::ChangeCallback(AudioCallback&& callback) { - callback_ = callback; -} - void AudioSpec::MergeChanges(const SDL_AudioSpec& obtained) { freq = obtained.freq; format = obtained.format; diff --git a/demos/audio_sine.cc b/demos/audio_sine.cc index 9106494..fdf6564 100644 --- a/demos/audio_sine.cc +++ b/demos/audio_sine.cc @@ -35,16 +35,16 @@ int Run() { int64_t nsample = 0; // Setup audio device, and provide callback which plays sine wave with specified frequency - AudioSpec spec(samplerate, AUDIO_S16SYS, 1, 4096, [&nsample, frequency, samplerate](Uint8* stream, int len) { + AudioSpec spec(samplerate, AUDIO_S16SYS, 1, 4096); + + // Open audio device + AudioDevice dev("", 0, spec, [&nsample, frequency, samplerate](Uint8* stream, int len) { // fill provided buffer with sine wave for (Uint8* ptr = stream; ptr < stream + len; ptr += 2) *(Uint16*)ptr = (Uint16)(32766.0f * sin(nsample++ / (float)samplerate * frequency)); } ); - // Open audio device - AudioDevice dev("", 0, spec); - // Sound plays after this call dev.Pause(false); diff --git a/demos/audio_wav.cc b/demos/audio_wav.cc index 90e87f9..1581838 100644 --- a/demos/audio_wav.cc +++ b/demos/audio_wav.cc @@ -32,10 +32,10 @@ int Run() { SDL sdl(SDL_INIT_AUDIO); Wav wav(TESTDATA_DIR "/test.wav"); - - // Setup audio device, and provide callback which plays looped wave sound Uint8* wav_pos = wav.GetBuffer(); - AudioSpec spec(wav.GetSpec(), [&wav, &wav_pos](Uint8* stream, int len) { + + // Open audio device + AudioDevice dev("", 0, wav.GetSpec(), [&wav, &wav_pos](Uint8* stream, int len) { // Fill provided buffer with wave contents Uint8* stream_pos = stream; Uint8* stream_end = stream + len; @@ -54,13 +54,6 @@ int Run() { } ); - // Open audio device - AudioDevice dev("", 0, spec); - - // Ensure SDL has set up format conversion for us - if (!spec.IsSameFormat(wav.GetSpec())) - throw std::runtime_error("WAV format is not the same as output format"); - // Sound plays after this call dev.Pause(false); From 70fafab8dd2ef5e9ce6533867c74d13f9f3e3076 Mon Sep 17 00:00:00 2001 From: Dmitry Marakasov Date: Tue, 16 Dec 2014 07:06:20 +0300 Subject: [PATCH 2/3] Exception safety Suck callback in only after everything was created to not needlessly destroy it if exception is thrown in the constructor --- SDL2pp/AudioDevice.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/SDL2pp/AudioDevice.cc b/SDL2pp/AudioDevice.cc index 319b7d0..b124f80 100644 --- a/SDL2pp/AudioDevice.cc +++ b/SDL2pp/AudioDevice.cc @@ -30,9 +30,9 @@ void AudioDevice::SDLCallback(void *userdata, Uint8* stream, int len) { audiodevice->callback_(stream, len); } -AudioDevice::AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec, AudioDevice::AudioCallback&& callback) : callback_(std::move(callback)) { +AudioDevice::AudioDevice(const std::string& device, bool iscapture, const AudioSpec& spec, AudioDevice::AudioCallback&& callback) { SDL_AudioSpec spec_with_callback = *spec.Get(); - if (callback_) { + if (callback) { spec_with_callback.callback = SDLCallback; spec_with_callback.userdata = static_cast(this); } @@ -40,11 +40,13 @@ AudioDevice::AudioDevice(const std::string& device, bool iscapture, const AudioS if ((device_id_ = SDL_OpenAudioDevice(device.empty() ? nullptr : device.c_str(), iscapture ? 1 : 0, &spec_with_callback, &obtained, 0)) == 0) throw Exception("SDL_OpenAudioDevice failed"); + + callback_ = std::move(callback); } -AudioDevice::AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes, AudioDevice::AudioCallback&& callback) : callback_(std::move(callback)) { +AudioDevice::AudioDevice(const std::string& device, bool iscapture, AudioSpec& spec, int allowed_changes, AudioDevice::AudioCallback&& callback) { SDL_AudioSpec spec_with_callback = *spec.Get(); - if (callback_) { + if (callback) { spec_with_callback.callback = SDLCallback; spec_with_callback.userdata = static_cast(this); } @@ -54,6 +56,8 @@ AudioDevice::AudioDevice(const std::string& device, bool iscapture, AudioSpec& s throw Exception("SDL_OpenAudioDevice failed"); spec.MergeChanges(obtained); + + callback_ = std::move(callback); } AudioDevice::~AudioDevice() { From 16b9399bd123c83ddc473b2fdc8032081203a18d Mon Sep 17 00:00:00 2001 From: Dmitry Marakasov Date: Tue, 16 Dec 2014 23:19:36 +0300 Subject: [PATCH 3/3] Implement empty constructor for AudioDevice::LockHandle Useful if lock must be initialized after it was created --- SDL2pp/Audio.hh | 1 + SDL2pp/AudioLock.cc | 3 +++ 2 files changed, 4 insertions(+) diff --git a/SDL2pp/Audio.hh b/SDL2pp/Audio.hh index ffbd554..baee880 100644 --- a/SDL2pp/Audio.hh +++ b/SDL2pp/Audio.hh @@ -60,6 +60,7 @@ public: LockHandle(AudioDevice* device); public: + LockHandle(); ~LockHandle(); LockHandle(LockHandle&& other) noexcept; diff --git a/SDL2pp/AudioLock.cc b/SDL2pp/AudioLock.cc index d426856..4193a98 100644 --- a/SDL2pp/AudioLock.cc +++ b/SDL2pp/AudioLock.cc @@ -23,6 +23,9 @@ namespace SDL2pp { +AudioDevice::LockHandle::LockHandle() : device_(nullptr) { +} + AudioDevice::LockHandle::LockHandle(AudioDevice* device) : device_(device) { SDL_LockAudioDevice(device_->device_id_); }