From 9cbcd56eb8e721f52d717b7d1de1fcc4d0216810 Mon Sep 17 00:00:00 2001 From: kimci86 Date: Sun, 6 Aug 2023 17:40:12 +0200 Subject: [PATCH] Remove Sound default constructor so that a Sound instance is always bound to a SoundBuffer --- include/SFML/Audio/Sound.hpp | 42 ++++++++++++------------ include/SFML/Audio/SoundBuffer.hpp | 8 ++--- src/SFML/Audio/Sound.cpp | 52 +++++++++++------------------- src/SFML/Audio/SoundBuffer.cpp | 31 ++++++++---------- test/install/Install.cpp | 1 - 5 files changed, 58 insertions(+), 76 deletions(-) diff --git a/include/SFML/Audio/Sound.hpp b/include/SFML/Audio/Sound.hpp index 87d1574c..90e1bb1f 100644 --- a/include/SFML/Audio/Sound.hpp +++ b/include/SFML/Audio/Sound.hpp @@ -46,12 +46,6 @@ class SoundBuffer; class SFML_AUDIO_API Sound : public SoundSource { public: - //////////////////////////////////////////////////////////// - /// \brief Default constructor - /// - //////////////////////////////////////////////////////////// - Sound(); - //////////////////////////////////////////////////////////// /// \brief Construct the sound with a buffer /// @@ -213,18 +207,27 @@ public: //////////////////////////////////////////////////////////// Sound& operator=(const Sound& right); - //////////////////////////////////////////////////////////// - /// \brief Reset the internal buffer of the sound - /// - /// This function is for internal use only, you don't have - /// to use it. It is called by the sf::SoundBuffer that - /// this sound uses, when it is destroyed in order to prevent - /// the sound from using a dead buffer. - /// - //////////////////////////////////////////////////////////// - void resetBuffer(); - private: + friend class SoundBuffer; + + //////////////////////////////////////////////////////////// + /// \brief Detach sound from its internal buffer + /// + /// This allows the sound buffer to temporarily detach the + /// sounds that use it when the sound buffer gets updated. + /// + //////////////////////////////////////////////////////////// + void detachBuffer(); + + //////////////////////////////////////////////////////////// + /// \brief Re-attach sound to its internal buffer + /// + /// This allows the sound buffer to attach back the sounds + /// that use it after the sound buffer has been updated. + /// + //////////////////////////////////////////////////////////// + void reattachBuffer(); + //////////////////////////////////////////////////////////// // Member data //////////////////////////////////////////////////////////// @@ -252,7 +255,7 @@ private: /// /// In order to work, a sound must be given a buffer of audio /// data to play. Audio data (samples) is stored in sf::SoundBuffer, -/// and attached to a sound with the setBuffer() function. +/// and attached to a sound when it is created or with the setBuffer() function. /// The buffer object attached to a sound must remain alive /// as long as the sound uses it. Note that multiple sounds /// can use the same sound buffer at the same time. @@ -265,8 +268,7 @@ private: /// // Handle error... /// } /// -/// sf::Sound sound; -/// sound.setBuffer(buffer); +/// sf::Sound sound(buffer); /// sound.play(); /// \endcode /// diff --git a/include/SFML/Audio/SoundBuffer.hpp b/include/SFML/Audio/SoundBuffer.hpp index db0e77a9..b0326be6 100644 --- a/include/SFML/Audio/SoundBuffer.hpp +++ b/include/SFML/Audio/SoundBuffer.hpp @@ -331,16 +331,14 @@ private: /// // error... /// } /// -/// // Create a sound source and bind it to the buffer -/// sf::Sound sound1; -/// sound1.setBuffer(buffer); +/// // Create a sound source bound to the buffer +/// sf::Sound sound1(buffer); /// /// // Play the sound /// sound1.play(); /// /// // Create another sound source bound to the same buffer -/// sf::Sound sound2; -/// sound2.setBuffer(buffer); +/// sf::Sound sound2(buffer); /// /// // Play it with a higher pitch -- the first sound remains unchanged /// sound2.setPitch(2); diff --git a/src/SFML/Audio/Sound.cpp b/src/SFML/Audio/Sound.cpp index c6789b70..4cd79413 100644 --- a/src/SFML/Audio/Sound.cpp +++ b/src/SFML/Audio/Sound.cpp @@ -36,21 +36,18 @@ namespace sf { //////////////////////////////////////////////////////////// -Sound::Sound() = default; - - -//////////////////////////////////////////////////////////// -Sound::Sound(const SoundBuffer& buffer) +Sound::Sound(const SoundBuffer& buffer) : m_buffer(&buffer) { - setBuffer(buffer); + m_buffer->attachSound(this); + alCheck(alSourcei(m_source, AL_BUFFER, static_cast(m_buffer->m_buffer))); } //////////////////////////////////////////////////////////// -Sound::Sound(const Sound& copy) : SoundSource(copy) +Sound::Sound(const Sound& copy) : SoundSource(copy), m_buffer(copy.m_buffer) { - if (copy.m_buffer) - setBuffer(*copy.m_buffer); + m_buffer->attachSound(this); + alCheck(alSourcei(m_source, AL_BUFFER, static_cast(m_buffer->m_buffer))); setLoop(copy.getLoop()); } @@ -59,8 +56,7 @@ Sound::Sound(const Sound& copy) : SoundSource(copy) Sound::~Sound() { stop(); - if (m_buffer) - m_buffer->detachSound(this); + m_buffer->detachSound(this); } @@ -89,11 +85,8 @@ void Sound::stop() void Sound::setBuffer(const SoundBuffer& buffer) { // First detach from the previous buffer - if (m_buffer) - { - stop(); - m_buffer->detachSound(this); - } + stop(); + m_buffer->detachSound(this); // Assign and use the new buffer m_buffer = &buffer; @@ -164,17 +157,8 @@ Sound& Sound::operator=(const Sound& right) // Delegate to base class, which copies all the sound attributes SoundSource::operator=(right); - // Detach the sound instance from the previous buffer (if any) - if (m_buffer) - { - stop(); - m_buffer->detachSound(this); - m_buffer = nullptr; - } - // Copy the remaining sound attributes - if (right.m_buffer) - setBuffer(*right.m_buffer); + setBuffer(*right.m_buffer); setLoop(right.getLoop()); return *this; @@ -182,18 +166,20 @@ Sound& Sound::operator=(const Sound& right) //////////////////////////////////////////////////////////// -void Sound::resetBuffer() +void Sound::detachBuffer() { // First stop the sound in case it is playing stop(); // Detach the buffer - if (m_buffer) - { - alCheck(alSourcei(m_source, AL_BUFFER, 0)); - m_buffer->detachSound(this); - m_buffer = nullptr; - } + alCheck(alSourcei(m_source, AL_BUFFER, 0)); +} + + +//////////////////////////////////////////////////////////// +void Sound::reattachBuffer() +{ + alCheck(alSourcei(m_source, AL_BUFFER, static_cast(m_buffer->m_buffer))); } } // namespace sf diff --git a/src/SFML/Audio/SoundBuffer.cpp b/src/SFML/Audio/SoundBuffer.cpp index a2c87ab1..51bce993 100644 --- a/src/SFML/Audio/SoundBuffer.cpp +++ b/src/SFML/Audio/SoundBuffer.cpp @@ -38,6 +38,8 @@ #include #include +#include + #if defined(__APPLE__) #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif @@ -68,15 +70,7 @@ SoundBuffer::SoundBuffer(const SoundBuffer& copy) : m_samples(copy.m_samples), m //////////////////////////////////////////////////////////// SoundBuffer::~SoundBuffer() { - // To prevent the iterator from becoming invalid, move the entire buffer to another - // container. Otherwise calling resetBuffer would result in detachSound being - // called which removes the sound from the internal list. - SoundList sounds; - sounds.swap(m_sounds); - - // Detach the buffer from the sounds that use it (to avoid OpenAL errors) - for (Sound* soundPtr : sounds) - soundPtr->resetBuffer(); + assert(m_sounds.empty() && "sf::SoundBuffer must not be destructed while it is used by a sf::Sound"); // Destroy the buffer if (m_buffer) @@ -213,7 +207,13 @@ SoundBuffer& SoundBuffer::operator=(const SoundBuffer& right) std::swap(m_samples, temp.m_samples); std::swap(m_buffer, temp.m_buffer); std::swap(m_duration, temp.m_duration); - std::swap(m_sounds, temp.m_sounds); // swap sounds too, so that they are detached when temp is destroyed + + // Reattach sounds that use this buffer so they get bound to the new OpenAL buffer + for (Sound* soundPtr : m_sounds) + { + soundPtr->stop(); + soundPtr->reattachBuffer(); + } return *this; } @@ -258,12 +258,9 @@ bool SoundBuffer::update(unsigned int channelCount, unsigned int sampleRate) return false; } - // First make a copy of the list of sounds so we can reattach later - const SoundList sounds(m_sounds); - // Detach the buffer from the sounds that use it (to avoid OpenAL errors) - for (Sound* soundPtr : sounds) - soundPtr->resetBuffer(); + for (Sound* soundPtr : m_sounds) + soundPtr->detachBuffer(); // Fill the buffer const auto size = static_cast(m_samples.size() * sizeof(std::int16_t)); @@ -274,8 +271,8 @@ bool SoundBuffer::update(unsigned int channelCount, unsigned int sampleRate) static_cast(m_samples.size()) / static_cast(sampleRate) / static_cast(channelCount)); // Now reattach the buffer to the sounds that use it - for (Sound* soundPtr : sounds) - soundPtr->setBuffer(*this); + for (Sound* soundPtr : m_sounds) + soundPtr->reattachBuffer(); return true; } diff --git a/test/install/Install.cpp b/test/install/Install.cpp index 7de4e249..22141d58 100644 --- a/test/install/Install.cpp +++ b/test/install/Install.cpp @@ -11,7 +11,6 @@ int main() [[maybe_unused]] const sf::InputSoundFile inputSoundFile; [[maybe_unused]] const sf::SoundBufferRecorder soundBufferRecorder; [[maybe_unused]] const sf::Music music; - [[maybe_unused]] const sf::Sound sound; // Graphics [[maybe_unused]] const sf::Color color;