From b2644e5c3fcf99e769a7b04118eaad1a3bd7b264 Mon Sep 17 00:00:00 2001 From: Vittorio Romeo Date: Tue, 15 Feb 2022 16:05:49 +0000 Subject: [PATCH] Add move semantics to 'SoundBuffer' --- include/SFML/Audio/SoundBuffer.hpp | 94 +++++++++++++++++++-- src/SFML/Audio/SoundBuffer.cpp | 127 +++++++++++++++++++++++------ 2 files changed, 192 insertions(+), 29 deletions(-) diff --git a/include/SFML/Audio/SoundBuffer.hpp b/include/SFML/Audio/SoundBuffer.hpp index 14c068820..ca9e4242e 100644 --- a/include/SFML/Audio/SoundBuffer.hpp +++ b/include/SFML/Audio/SoundBuffer.hpp @@ -35,6 +35,70 @@ #include #include +namespace sf::priv +{ +//////////////////////////////////////////////////////////// +/// \brief Represents unique ownership of an OpenAL buffer id +/// +//////////////////////////////////////////////////////////// +class SFML_AUDIO_API UniqueOpenALBufferId +{ +public: + //////////////////////////////////////////////////////////// + /// \brief Destructor + /// + //////////////////////////////////////////////////////////// + ~UniqueOpenALBufferId(); + + //////////////////////////////////////////////////////////// + /// \brief Move constructor + /// + //////////////////////////////////////////////////////////// + UniqueOpenALBufferId(UniqueOpenALBufferId&& other) noexcept; + + //////////////////////////////////////////////////////////// + /// \brief Move assignment + /// + //////////////////////////////////////////////////////////// + UniqueOpenALBufferId& operator=(UniqueOpenALBufferId&& other) noexcept; + + //////////////////////////////////////////////////////////// + /// \brief Get the internal buffer id + /// + //////////////////////////////////////////////////////////// + [[nodiscard]] unsigned int get() const; + + //////////////////////////////////////////////////////////// + /// \brief Generate a new buffer + /// + //////////////////////////////////////////////////////////// + [[nodiscard]] static UniqueOpenALBufferId generate(); + + //////////////////////////////////////////////////////////// + /// \brief Return the null buffer id + /// + //////////////////////////////////////////////////////////// + [[nodiscard]] static UniqueOpenALBufferId null(); + +private: + //////////////////////////////////////////////////////////// + // Static member data + //////////////////////////////////////////////////////////// + static constexpr unsigned int NullBufferId = 0u; + + //////////////////////////////////////////////////////////// + /// \brief Construct from an existing buffer identifier + /// + //////////////////////////////////////////////////////////// + explicit UniqueOpenALBufferId(unsigned int bufferId); + + //////////////////////////////////////////////////////////// + // Member data + //////////////////////////////////////////////////////////// + unsigned int m_bufferId; //!< OpenAL buffer identifier +}; + +} namespace sf { @@ -64,6 +128,14 @@ public: //////////////////////////////////////////////////////////// SoundBuffer(const SoundBuffer& copy); + //////////////////////////////////////////////////////////// + /// \brief Move constructor + /// + /// \param other Instance to move + /// + //////////////////////////////////////////////////////////// + SoundBuffer(SoundBuffer&& move) noexcept; + //////////////////////////////////////////////////////////// /// \brief Destructor /// @@ -214,7 +286,7 @@ public: Time getDuration() const; //////////////////////////////////////////////////////////// - /// \brief Overload of assignment operator + /// \brief Overload of copy assignment operator /// /// \param right Instance to assign /// @@ -223,6 +295,16 @@ public: //////////////////////////////////////////////////////////// SoundBuffer& operator =(const SoundBuffer& right); + //////////////////////////////////////////////////////////// + /// \brief Overload of move assignment operator + /// + /// \param right Instance to assign + /// + /// \return Reference to self + /// + //////////////////////////////////////////////////////////// + SoundBuffer& operator =(SoundBuffer&& right) noexcept; + private: friend class Sound; @@ -267,15 +349,15 @@ private: //////////////////////////////////////////////////////////// // Types //////////////////////////////////////////////////////////// - using SoundList = std::unordered_set; //!< Set of unique sound instances + using SoundList = std::unordered_set; //!< Set of unique sound instances //////////////////////////////////////////////////////////// // Member data //////////////////////////////////////////////////////////// - unsigned int m_buffer; //!< OpenAL buffer identifier - std::vector m_samples; //!< Samples buffer - Time m_duration; //!< Sound duration - mutable SoundList m_sounds; //!< List of sounds that are using this buffer + priv::UniqueOpenALBufferId m_uniqueBufferId; //!< OpenAL buffer identifier + std::vector m_samples; //!< Samples buffer + Time m_duration; //!< Sound duration + mutable SoundList m_sounds; //!< List of sounds that are using this buffer }; } // namespace sf diff --git a/src/SFML/Audio/SoundBuffer.cpp b/src/SFML/Audio/SoundBuffer.cpp index ecc58320a..5fd5c8d02 100644 --- a/src/SFML/Audio/SoundBuffer.cpp +++ b/src/SFML/Audio/SoundBuffer.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #if defined(__APPLE__) #if defined(__clang__) @@ -43,34 +44,96 @@ #endif #endif +namespace sf::priv +{ +//////////////////////////////////////////////////////////// +UniqueOpenALBufferId::UniqueOpenALBufferId(unsigned int bufferId) : +m_bufferId(bufferId) +{ +} + + +//////////////////////////////////////////////////////////// +UniqueOpenALBufferId::~UniqueOpenALBufferId() +{ + // Destroy the buffer + if (m_bufferId != NullBufferId) + alCheck(alDeleteBuffers(1, &m_bufferId)); +} + + +//////////////////////////////////////////////////////////// +UniqueOpenALBufferId::UniqueOpenALBufferId(UniqueOpenALBufferId&& other) noexcept : +m_bufferId(std::exchange(other.m_bufferId, NullBufferId)) +{ +} + + +//////////////////////////////////////////////////////////// +UniqueOpenALBufferId& UniqueOpenALBufferId::operator=(UniqueOpenALBufferId&& other) noexcept +{ + m_bufferId = std::exchange(other.m_bufferId, NullBufferId); + return *this; +} + + +//////////////////////////////////////////////////////////// +[[nodiscard]] unsigned int UniqueOpenALBufferId::get() const +{ + return m_bufferId; +} + + +//////////////////////////////////////////////////////////// +[[nodiscard]] UniqueOpenALBufferId UniqueOpenALBufferId::generate() +{ + // Create the buffer + unsigned int bufferId; + alCheck(alGenBuffers(1, &bufferId)); + + return UniqueOpenALBufferId(bufferId); +} + + +//////////////////////////////////////////////////////////// +[[nodiscard]] UniqueOpenALBufferId UniqueOpenALBufferId::null() +{ + return UniqueOpenALBufferId(NullBufferId); +} + +} + namespace sf { //////////////////////////////////////////////////////////// SoundBuffer::SoundBuffer() : -m_buffer (0), -m_duration() +AlResource (), +m_uniqueBufferId(priv::UniqueOpenALBufferId::generate()), +m_samples (), +m_duration (), +m_sounds () { - // Create the buffer - alCheck(alGenBuffers(1, &m_buffer)); } //////////////////////////////////////////////////////////// SoundBuffer::SoundBuffer(const SoundBuffer& copy) : -m_buffer (0), -m_samples (copy.m_samples), -m_duration(copy.m_duration), -m_sounds () // don't copy the attached sounds +AlResource (copy), +m_uniqueBufferId(priv::UniqueOpenALBufferId::generate()), +m_samples (copy.m_samples), +m_duration (copy.m_duration), +m_sounds () // don't copy the attached sounds { - // Create the buffer - alCheck(alGenBuffers(1, &m_buffer)); - // Update the internal buffer with the new samples if (!update(copy.getChannelCount(), copy.getSampleRate())) err() << "Failed to update copy-constructed sound buffer" << std::endl; } +//////////////////////////////////////////////////////////// +SoundBuffer::SoundBuffer(SoundBuffer&& move) noexcept = default; + + //////////////////////////////////////////////////////////// SoundBuffer::~SoundBuffer() { @@ -83,10 +146,6 @@ SoundBuffer::~SoundBuffer() // Detach the buffer from the sounds that use it (to avoid OpenAL errors) for (Sound* soundPtr : sounds) soundPtr->resetBuffer(); - - // Destroy the buffer - if (m_buffer) - alCheck(alDeleteBuffers(1, &m_buffer)); } @@ -186,7 +245,7 @@ Uint64 SoundBuffer::getSampleCount() const unsigned int SoundBuffer::getSampleRate() const { ALint sampleRate; - alCheck(alGetBufferi(m_buffer, AL_FREQUENCY, &sampleRate)); + alCheck(alGetBufferi(m_uniqueBufferId.get(), AL_FREQUENCY, &sampleRate)); return static_cast(sampleRate); } @@ -196,7 +255,7 @@ unsigned int SoundBuffer::getSampleRate() const unsigned int SoundBuffer::getChannelCount() const { ALint channelCount; - alCheck(alGetBufferi(m_buffer, AL_CHANNELS, &channelCount)); + alCheck(alGetBufferi(m_uniqueBufferId.get(), AL_CHANNELS, &channelCount)); return static_cast(channelCount); } @@ -212,12 +271,34 @@ Time SoundBuffer::getDuration() const //////////////////////////////////////////////////////////// SoundBuffer& SoundBuffer::operator =(const SoundBuffer& right) { + if (&right == this) + return *this; + + AlResource::operator=(right); + SoundBuffer temp(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 + std::swap(m_samples, temp.m_samples); + std::swap(m_uniqueBufferId, temp.m_uniqueBufferId); + 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 + + return *this; +} + + +//////////////////////////////////////////////////////////// +SoundBuffer& SoundBuffer::operator =(SoundBuffer&& right) noexcept +{ + if (&right == this) + return *this; + + AlResource::operator=(std::move(right)); + + m_uniqueBufferId = std::move(right.m_uniqueBufferId); + m_samples = std::move(right.m_samples); + m_duration = std::move(right.m_duration); + m_sounds = std::move(right.m_sounds); return *this; } @@ -270,8 +351,8 @@ bool SoundBuffer::update(unsigned int channelCount, unsigned int sampleRate) soundPtr->resetBuffer(); // Fill the buffer - auto size = static_cast(m_samples.size() * sizeof(Int16)); - alCheck(alBufferData(m_buffer, format, m_samples.data(), size, static_cast(sampleRate))); + const auto size = static_cast(m_samples.size() * sizeof(Int16)); + alCheck(alBufferData(m_uniqueBufferId.get(), format, m_samples.data(), size, static_cast(sampleRate))); // Compute the duration m_duration = seconds(static_cast(m_samples.size()) / static_cast(sampleRate) / static_cast(channelCount));