From add6422e6b95ca3ca997a2495d97d9d0fa308876 Mon Sep 17 00:00:00 2001 From: Chris Thrasher Date: Sat, 18 May 2024 15:37:06 -0600 Subject: [PATCH] Remove default empty state of `sf::SoundBuffer` --- examples/sound/Sound.cpp | 4 +- examples/tennis/Tennis.cpp | 6 +-- include/SFML/Audio/Sound.hpp | 7 +-- include/SFML/Audio/SoundBuffer.hpp | 50 ++++++++++------------ include/SFML/Audio/SoundBufferRecorder.hpp | 4 +- src/SFML/Audio/SoundBuffer.cpp | 50 ++++++++++++++-------- src/SFML/Audio/SoundBufferRecorder.cpp | 12 ++++-- test/Audio/Sound.test.cpp | 9 ++-- test/Audio/SoundBuffer.test.cpp | 45 ++++++------------- 9 files changed, 87 insertions(+), 100 deletions(-) diff --git a/examples/sound/Sound.cpp b/examples/sound/Sound.cpp index 9b806c1bc..9b6c77940 100644 --- a/examples/sound/Sound.cpp +++ b/examples/sound/Sound.cpp @@ -13,9 +13,7 @@ void playSound() { // Load a sound buffer from a wav file - sf::SoundBuffer buffer; - if (!buffer.loadFromFile("resources/killdeer.wav")) - return; + const auto buffer = sf::SoundBuffer::loadFromFile("resources/killdeer.wav").value(); // Display sound information std::cout << "killdeer.wav:" << '\n' diff --git a/examples/tennis/Tennis.cpp b/examples/tennis/Tennis.cpp index 41b05b8ef..5d84dc13c 100644 --- a/examples/tennis/Tennis.cpp +++ b/examples/tennis/Tennis.cpp @@ -49,10 +49,8 @@ int main() window.setVerticalSyncEnabled(true); // Load the sounds used in the game - sf::SoundBuffer ballSoundBuffer; - if (!ballSoundBuffer.loadFromFile(resourcesDir() / "ball.wav")) - return EXIT_FAILURE; - sf::Sound ballSound(ballSoundBuffer); + const auto ballSoundBuffer = sf::SoundBuffer::loadFromFile(resourcesDir() / "ball.wav").value(); + sf::Sound ballSound(ballSoundBuffer); // Create the SFML logo texture: sf::Texture sfmlLogoTexture; diff --git a/include/SFML/Audio/Sound.hpp b/include/SFML/Audio/Sound.hpp index ae9e3591f..a3b4c199e 100644 --- a/include/SFML/Audio/Sound.hpp +++ b/include/SFML/Audio/Sound.hpp @@ -274,12 +274,7 @@ private: /// /// Usage example: /// \code -/// sf::SoundBuffer buffer; -/// if (!buffer.loadFromFile("sound.wav")) -/// { -/// // Handle error... -/// } -/// +/// const auto buffer = sf::SoundBuffer::loadFromFile("sound.wav").value(); /// sf::Sound sound(buffer); /// sound.play(); /// \endcode diff --git a/include/SFML/Audio/SoundBuffer.hpp b/include/SFML/Audio/SoundBuffer.hpp index bc1dc8afb..33283d01d 100644 --- a/include/SFML/Audio/SoundBuffer.hpp +++ b/include/SFML/Audio/SoundBuffer.hpp @@ -34,6 +34,7 @@ #include #include +#include #include #include @@ -54,12 +55,6 @@ class InputStream; class SFML_AUDIO_API SoundBuffer { public: - //////////////////////////////////////////////////////////// - /// \brief Default constructor - /// - //////////////////////////////////////////////////////////// - SoundBuffer() = default; - //////////////////////////////////////////////////////////// /// \brief Copy constructor /// @@ -82,12 +77,12 @@ public: /// /// \param filename Path of the sound file to load /// - /// \return True if loading succeeded, false if it failed + /// \return Sound buffer if loading succeeded, `std::nullopt` if it failed /// /// \see loadFromMemory, loadFromStream, loadFromSamples, saveToFile /// //////////////////////////////////////////////////////////// - [[nodiscard]] bool loadFromFile(const std::filesystem::path& filename); + [[nodiscard]] static std::optional loadFromFile(const std::filesystem::path& filename); //////////////////////////////////////////////////////////// /// \brief Load the sound buffer from a file in memory @@ -98,12 +93,12 @@ public: /// \param data Pointer to the file data in memory /// \param sizeInBytes Size of the data to load, in bytes /// - /// \return True if loading succeeded, false if it failed + /// \return Sound buffer if loading succeeded, `std::nullopt` if it failed /// /// \see loadFromFile, loadFromStream, loadFromSamples /// //////////////////////////////////////////////////////////// - [[nodiscard]] bool loadFromMemory(const void* data, std::size_t sizeInBytes); + [[nodiscard]] static std::optional loadFromMemory(const void* data, std::size_t sizeInBytes); //////////////////////////////////////////////////////////// /// \brief Load the sound buffer from a custom stream @@ -113,12 +108,12 @@ public: /// /// \param stream Source stream to read from /// - /// \return True if loading succeeded, false if it failed + /// \return Sound buffer if loading succeeded, `std::nullopt` if it failed /// /// \see loadFromFile, loadFromMemory, loadFromSamples /// //////////////////////////////////////////////////////////// - [[nodiscard]] bool loadFromStream(InputStream& stream); + [[nodiscard]] static std::optional loadFromStream(InputStream& stream); //////////////////////////////////////////////////////////// /// \brief Load the sound buffer from an array of audio samples @@ -131,16 +126,17 @@ public: /// \param sampleRate Sample rate (number of samples to play per second) /// \param channelMap Map of position in sample frame to sound channel /// - /// \return True if loading succeeded, false if it failed + /// \return Sound buffer if loading succeeded, `std::nullopt` if it failed /// /// \see loadFromFile, loadFromMemory, saveToFile /// //////////////////////////////////////////////////////////// - [[nodiscard]] bool loadFromSamples(const std::int16_t* samples, - std::uint64_t sampleCount, - unsigned int channelCount, - unsigned int sampleRate, - const std::vector& channelMap); + [[nodiscard]] static std::optional loadFromSamples( + const std::int16_t* samples, + std::uint64_t sampleCount, + unsigned int channelCount, + unsigned int sampleRate, + const std::vector& channelMap); //////////////////////////////////////////////////////////// /// \brief Save the sound buffer to an audio file @@ -247,6 +243,12 @@ public: private: friend class Sound; + //////////////////////////////////////////////////////////// + /// \brief Construct from vector of samples + /// + //////////////////////////////////////////////////////////// + explicit SoundBuffer(std::vector&& samples); + //////////////////////////////////////////////////////////// /// \brief Initialize the internal state after loading a new sound /// @@ -255,7 +257,7 @@ private: /// \return True on successful initialization, false on failure /// //////////////////////////////////////////////////////////// - [[nodiscard]] bool initialize(InputSoundFile& file); + [[nodiscard]] static std::optional initialize(InputSoundFile& file); //////////////////////////////////////////////////////////// /// \brief Update the internal buffer with the cached audio samples @@ -341,14 +343,8 @@ private: /// /// Usage example: /// \code -/// // Declare a new sound buffer -/// sf::SoundBuffer buffer; -/// -/// // Load it from a file -/// if (!buffer.loadFromFile("sound.wav")) -/// { -/// // error... -/// } +/// // Load a new sound buffer from a file +/// const auto buffer = sf::SoundBuffer::loadFromFile("sound.wav").value(); /// /// // Create a sound source bound to the buffer /// sf::Sound sound1(buffer); diff --git a/include/SFML/Audio/SoundBufferRecorder.hpp b/include/SFML/Audio/SoundBufferRecorder.hpp index 1706bdf64..c88f34c6b 100644 --- a/include/SFML/Audio/SoundBufferRecorder.hpp +++ b/include/SFML/Audio/SoundBufferRecorder.hpp @@ -97,8 +97,8 @@ private: //////////////////////////////////////////////////////////// // Member data //////////////////////////////////////////////////////////// - std::vector m_samples; //!< Temporary sample buffer to hold the recorded data - SoundBuffer m_buffer; //!< Sound buffer that will contain the recorded data + std::vector m_samples; //!< Temporary sample buffer to hold the recorded data + std::optional m_buffer; //!< Sound buffer that will contain the recorded data }; } // namespace sf diff --git a/src/SFML/Audio/SoundBuffer.cpp b/src/SFML/Audio/SoundBuffer.cpp index e9176dc77..073257773 100644 --- a/src/SFML/Audio/SoundBuffer.cpp +++ b/src/SFML/Audio/SoundBuffer.cpp @@ -68,52 +68,55 @@ SoundBuffer::~SoundBuffer() //////////////////////////////////////////////////////////// -bool SoundBuffer::loadFromFile(const std::filesystem::path& filename) +std::optional SoundBuffer::loadFromFile(const std::filesystem::path& filename) { InputSoundFile file; if (file.openFromFile(filename)) return initialize(file); else - return false; + return std::nullopt; } //////////////////////////////////////////////////////////// -bool SoundBuffer::loadFromMemory(const void* data, std::size_t sizeInBytes) +std::optional SoundBuffer::loadFromMemory(const void* data, std::size_t sizeInBytes) { InputSoundFile file; if (file.openFromMemory(data, sizeInBytes)) return initialize(file); else - return false; + return std::nullopt; } //////////////////////////////////////////////////////////// -bool SoundBuffer::loadFromStream(InputStream& stream) +std::optional SoundBuffer::loadFromStream(InputStream& stream) { InputSoundFile file; if (file.openFromStream(stream)) return initialize(file); else - return false; + return std::nullopt; } //////////////////////////////////////////////////////////// -bool SoundBuffer::loadFromSamples(const std::int16_t* samples, - std::uint64_t sampleCount, - unsigned int channelCount, - unsigned int sampleRate, - const std::vector& channelMap) +std::optional SoundBuffer::loadFromSamples( + const std::int16_t* samples, + std::uint64_t sampleCount, + unsigned int channelCount, + unsigned int sampleRate, + const std::vector& channelMap) { if (samples && sampleCount && channelCount && sampleRate && !channelMap.empty()) { // Copy the new audio samples - m_samples.assign(samples, samples + sampleCount); + SoundBuffer soundBuffer(std::vector(samples, samples + sampleCount)); // Update the internal buffer with the new samples - return update(channelCount, sampleRate, channelMap); + if (!soundBuffer.update(channelCount, sampleRate, channelMap)) + return std::nullopt; + return soundBuffer; } else { @@ -124,7 +127,7 @@ bool SoundBuffer::loadFromSamples(const std::int16_t* samples, << "channels: " << channelCount << ", " << "samplerate: " << sampleRate << ")" << std::endl; - return false; + return std::nullopt; } } @@ -206,21 +209,30 @@ SoundBuffer& SoundBuffer::operator=(const SoundBuffer& right) //////////////////////////////////////////////////////////// -bool SoundBuffer::initialize(InputSoundFile& file) +SoundBuffer::SoundBuffer(std::vector&& samples) : m_samples(std::move(samples)) +{ +} + + +//////////////////////////////////////////////////////////// +std::optional SoundBuffer::initialize(InputSoundFile& file) { // Retrieve the sound parameters const std::uint64_t sampleCount = file.getSampleCount(); // Read the samples from the provided file - m_samples.resize(static_cast(sampleCount)); - if (file.read(m_samples.data(), sampleCount) == sampleCount) + std::vector samples(static_cast(sampleCount)); + if (file.read(samples.data(), sampleCount) == sampleCount) { // Update the internal buffer with the new samples - return update(file.getChannelCount(), file.getSampleRate(), file.getChannelMap()); + SoundBuffer soundBuffer(std::move(samples)); + if (!soundBuffer.update(file.getChannelCount(), file.getSampleRate(), file.getChannelMap())) + return std::nullopt; + return soundBuffer; } else { - return false; + return std::nullopt; } } diff --git a/src/SFML/Audio/SoundBufferRecorder.cpp b/src/SFML/Audio/SoundBufferRecorder.cpp index dbe02cbd8..1b49377c7 100644 --- a/src/SFML/Audio/SoundBufferRecorder.cpp +++ b/src/SFML/Audio/SoundBufferRecorder.cpp @@ -48,7 +48,7 @@ SoundBufferRecorder::~SoundBufferRecorder() bool SoundBufferRecorder::onStart() { m_samples.clear(); - m_buffer = SoundBuffer(); + m_buffer.reset(); return true; } @@ -69,7 +69,12 @@ void SoundBufferRecorder::onStop() if (m_samples.empty()) return; - if (!m_buffer.loadFromSamples(m_samples.data(), m_samples.size(), getChannelCount(), getSampleRate(), getChannelMap())) + m_buffer = sf::SoundBuffer::loadFromSamples(m_samples.data(), + m_samples.size(), + getChannelCount(), + getSampleRate(), + getChannelMap()); + if (!m_buffer) err() << "Failed to stop capturing audio data" << std::endl; } @@ -77,7 +82,8 @@ void SoundBufferRecorder::onStop() //////////////////////////////////////////////////////////// const SoundBuffer& SoundBufferRecorder::getBuffer() const { - return m_buffer; + assert(m_buffer && "SoundBufferRecorder::getBuffer() Cannot return reference to null buffer"); + return *m_buffer; } } // namespace sf diff --git a/test/Audio/Sound.test.cpp b/test/Audio/Sound.test.cpp index c650f1f16..0cfc10296 100644 --- a/test/Audio/Sound.test.cpp +++ b/test/Audio/Sound.test.cpp @@ -25,8 +25,7 @@ TEST_CASE("[Audio] sf::Sound", runAudioDeviceTests()) STATIC_CHECK(std::has_virtual_destructor_v); } - sf::SoundBuffer soundBuffer; - REQUIRE(soundBuffer.loadFromFile("Audio/ding.flac")); + const auto soundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); SECTION("Construction") { @@ -52,8 +51,8 @@ TEST_CASE("[Audio] sf::Sound", runAudioDeviceTests()) SECTION("Assignment") { - const sf::SoundBuffer emptySoundBuffer; - sf::Sound soundCopy(emptySoundBuffer); + const sf::SoundBuffer otherSoundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); + sf::Sound soundCopy(otherSoundBuffer); soundCopy = sound; CHECK(&soundCopy.getBuffer() == &soundBuffer); CHECK(!soundCopy.getLoop()); @@ -64,7 +63,7 @@ TEST_CASE("[Audio] sf::Sound", runAudioDeviceTests()) SECTION("Set/get buffer") { - const sf::SoundBuffer otherSoundBuffer; + const sf::SoundBuffer otherSoundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); sf::Sound sound(soundBuffer); sound.setBuffer(otherSoundBuffer); CHECK(&sound.getBuffer() == &otherSoundBuffer); diff --git a/test/Audio/SoundBuffer.test.cpp b/test/Audio/SoundBuffer.test.cpp index a907f2470..45af2a3ff 100644 --- a/test/Audio/SoundBuffer.test.cpp +++ b/test/Audio/SoundBuffer.test.cpp @@ -14,6 +14,7 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) { SECTION("Type traits") { + STATIC_CHECK(!std::is_default_constructible_v); STATIC_CHECK(std::is_copy_constructible_v); STATIC_CHECK(std::is_copy_assignable_v); STATIC_CHECK(std::is_move_constructible_v); @@ -22,20 +23,9 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) STATIC_CHECK(!std::is_nothrow_move_assignable_v); } - SECTION("Construction") - { - const sf::SoundBuffer soundBuffer; - CHECK(soundBuffer.getSamples() == nullptr); - CHECK(soundBuffer.getSampleCount() == 0); - CHECK(soundBuffer.getSampleRate() == 44100); - CHECK(soundBuffer.getChannelCount() == 1); - CHECK(soundBuffer.getDuration() == sf::Time::Zero); - } - SECTION("Copy semantics") { - sf::SoundBuffer soundBuffer; - REQUIRE(soundBuffer.loadFromFile("Audio/ding.flac")); + const auto soundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); SECTION("Construction") { @@ -49,8 +39,8 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) SECTION("Assignment") { - sf::SoundBuffer soundBufferCopy; - soundBufferCopy = soundBuffer; + sf::SoundBuffer soundBufferCopy = sf::SoundBuffer::loadFromFile("Audio/doodle_pop.ogg").value(); + soundBufferCopy = soundBuffer; CHECK(soundBufferCopy.getSamples() != nullptr); CHECK(soundBufferCopy.getSampleCount() == 87798); CHECK(soundBufferCopy.getSampleRate() == 44100); @@ -61,16 +51,14 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) SECTION("loadFromFile()") { - sf::SoundBuffer soundBuffer; - SECTION("Invalid filename") { - CHECK(!soundBuffer.loadFromFile("does/not/exist.wav")); + CHECK(!sf::SoundBuffer::loadFromFile("does/not/exist.wav")); } SECTION("Valid file") { - REQUIRE(soundBuffer.loadFromFile("Audio/ding.flac")); + const auto soundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); CHECK(soundBuffer.getSamples() != nullptr); CHECK(soundBuffer.getSampleCount() == 87798); CHECK(soundBuffer.getSampleRate() == 44100); @@ -81,19 +69,17 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) SECTION("loadFromMemory()") { - sf::SoundBuffer soundBuffer; - SECTION("Invalid memory") { - CHECK(!soundBuffer.loadFromMemory(nullptr, 0)); + CHECK(!sf::SoundBuffer::loadFromMemory(nullptr, 0)); constexpr std::array memory{}; - CHECK(!soundBuffer.loadFromMemory(memory.data(), memory.size())); + CHECK(!sf::SoundBuffer::loadFromMemory(memory.data(), memory.size())); } SECTION("Valid memory") { - const auto memory = loadIntoMemory("Audio/ding.flac"); - REQUIRE(soundBuffer.loadFromMemory(memory.data(), memory.size())); + const auto memory = loadIntoMemory("Audio/ding.flac"); + const auto soundBuffer = sf::SoundBuffer::loadFromMemory(memory.data(), memory.size()).value(); CHECK(soundBuffer.getSamples() != nullptr); CHECK(soundBuffer.getSampleCount() == 87798); CHECK(soundBuffer.getSampleRate() == 44100); @@ -105,17 +91,16 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) SECTION("loadFromStream()") { sf::FileInputStream stream; - sf::SoundBuffer soundBuffer; SECTION("Invalid stream") { - CHECK(!soundBuffer.loadFromStream(stream)); + CHECK(!sf::SoundBuffer::loadFromStream(stream)); } SECTION("Valid stream") { REQUIRE(stream.open("Audio/ding.flac")); - REQUIRE(soundBuffer.loadFromStream(stream)); + const auto soundBuffer = sf::SoundBuffer::loadFromStream(stream).value(); CHECK(soundBuffer.getSamples() != nullptr); CHECK(soundBuffer.getSampleCount() == 87798); CHECK(soundBuffer.getSampleRate() == 44100); @@ -129,13 +114,11 @@ TEST_CASE("[Audio] sf::SoundBuffer", runAudioDeviceTests()) const auto filename = std::filesystem::temp_directory_path() / "ding.flac"; { - sf::SoundBuffer soundBuffer; - REQUIRE(soundBuffer.loadFromFile("Audio/ding.flac")); + const auto soundBuffer = sf::SoundBuffer::loadFromFile("Audio/ding.flac").value(); REQUIRE(soundBuffer.saveToFile(filename)); } - sf::SoundBuffer soundBuffer; - REQUIRE(soundBuffer.loadFromFile(filename)); + const auto soundBuffer = sf::SoundBuffer::loadFromFile(filename).value(); CHECK(soundBuffer.getSamples() != nullptr); CHECK(soundBuffer.getSampleCount() == 87798); CHECK(soundBuffer.getSampleRate() == 44100);