From 09aae0240d1c6c566547ea62936ccec060f49e4a Mon Sep 17 00:00:00 2001 From: Foaly Date: Thu, 1 May 2014 04:22:53 +0200 Subject: [PATCH 1/3] Fixed calling SoundStream::setPlayingOffset() unpausing a paused SoundStream (#203), guard m_isStreaming by a mutex, fixed calling SoundStream::pause() before the stream thread starts not properly pausing the stream (http://en.sfml-dev.org/forums/index.php?topic=15197.0), minor documentation fix in SoundStream. Signed-off-by: binary1248 --- include/SFML/Audio/SoundStream.hpp | 7 ++- src/SFML/Audio/SoundStream.cpp | 96 ++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/include/SFML/Audio/SoundStream.hpp b/include/SFML/Audio/SoundStream.hpp index 12102d8c..293623e4 100644 --- a/include/SFML/Audio/SoundStream.hpp +++ b/include/SFML/Audio/SoundStream.hpp @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -65,8 +66,8 @@ public : /// \brief Start or resume playing the audio stream /// /// This function starts the stream if it was stopped, resumes - /// it if it was paused, and restarts it from beginning if it - /// was it already playing. + /// it if it was paused, and restarts it from the beginning if + /// it was already playing. /// This function uses its own thread so that it doesn't block /// the rest of the program while the stream is played. /// @@ -282,6 +283,8 @@ private : // Member data //////////////////////////////////////////////////////////// Thread m_thread; ///< Thread running the background tasks + mutable Mutex m_threadMutex; ///< Thread mutex + Status m_threadStartState; ///< State the thread starts in (Playing, Paused, Stopped) bool m_isStreaming; ///< Streaming state (true = playing, false = stopped) unsigned int m_buffers[BufferCount]; ///< Sound buffers used to store temporary audio data unsigned int m_channelCount; ///< Number of channels (1 = mono, 2 = stereo, ...) diff --git a/src/SFML/Audio/SoundStream.cpp b/src/SFML/Audio/SoundStream.cpp index 972bdbf9..d21b3605 100644 --- a/src/SFML/Audio/SoundStream.cpp +++ b/src/SFML/Audio/SoundStream.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #ifdef _MSC_VER #pragma warning(disable : 4355) // 'this' used in base member initializer list @@ -41,6 +42,8 @@ namespace sf //////////////////////////////////////////////////////////// SoundStream::SoundStream() : m_thread (&SoundStream::streamData, this), +m_threadMutex (), +m_threadStartState(Stopped), m_isStreaming (false), m_channelCount (0), m_sampleRate (0), @@ -89,11 +92,17 @@ void SoundStream::play() return; } - // If the sound is already playing (probably paused), just resume it - if (m_isStreaming) { - alCheck(alSourcePlay(m_source)); - return; + Lock lock(m_threadMutex); + + // If the sound is already playing (probably paused), just resume it + if (m_isStreaming) + { + alCheck(alSourcePlay(m_source)); + return; + } + + m_isStreaming = true; } // Move to the beginning @@ -101,7 +110,7 @@ void SoundStream::play() // Start updating the stream in a separate thread to avoid blocking the application m_samplesProcessed = 0; - m_isStreaming = true; + m_threadStartState = Playing; m_thread.launch(); } @@ -109,6 +118,12 @@ void SoundStream::play() //////////////////////////////////////////////////////////// void SoundStream::pause() { + // Handle pause() being called before the thread has started + { + Lock lock(m_threadMutex); + m_threadStartState = Paused; + } + alCheck(alSourcePause(m_source)); } @@ -116,8 +131,13 @@ void SoundStream::pause() //////////////////////////////////////////////////////////// void SoundStream::stop() { + // Request the thread to terminate + { + Lock lock(m_threadMutex); + m_isStreaming = false; + } + // Wait for the thread to terminate - m_isStreaming = false; m_thread.wait(); } @@ -142,8 +162,13 @@ SoundStream::Status SoundStream::getStatus() const Status status = SoundSource::getStatus(); // To compensate for the lag between play() and alSourceplay() - if ((status == Stopped) && m_isStreaming) - status = Playing; + if (status == Stopped) + { + Lock lock(m_threadMutex); + + if (m_isStreaming) + status = m_threadStartState; + } return status; } @@ -152,6 +177,9 @@ SoundStream::Status SoundStream::getStatus() const //////////////////////////////////////////////////////////// void SoundStream::setPlayingOffset(Time timeOffset) { + // Get old playing status + Status oldStatus = getStatus(); + // Stop the stream stop(); @@ -160,7 +188,12 @@ void SoundStream::setPlayingOffset(Time timeOffset) // Restart streaming m_samplesProcessed = static_cast(timeOffset.asSeconds() * m_sampleRate * m_channelCount); + + if (oldStatus == Stopped) + return; + m_isStreaming = true; + m_threadStartState = oldStatus; m_thread.launch(); } @@ -199,19 +232,42 @@ bool SoundStream::getLoop() const //////////////////////////////////////////////////////////// void SoundStream::streamData() { - // Create the buffers - alCheck(alGenBuffers(BufferCount, m_buffers)); - for (int i = 0; i < BufferCount; ++i) - m_endBuffers[i] = false; + bool requestStop = false; - // Fill the queue - bool requestStop = fillQueue(); - - // Play the sound - alCheck(alSourcePlay(m_source)); - - while (m_isStreaming) { + Lock lock(m_threadMutex); + + // Check if the thread was launched Stopped + if (m_threadStartState == Stopped) + { + m_isStreaming = false; + return; + } + + // Create the buffers + alCheck(alGenBuffers(BufferCount, m_buffers)); + for (int i = 0; i < BufferCount; ++i) + m_endBuffers[i] = false; + + // Fill the queue + requestStop = fillQueue(); + + // Play the sound + alCheck(alSourcePlay(m_source)); + + // Check if the thread was launched Paused + if (m_threadStartState == Paused) + alCheck(alSourcePause(m_source)); + } + + for (;;) + { + { + Lock lock(m_threadMutex); + if (!m_isStreaming) + break; + } + // The stream has been interrupted! if (SoundSource::getStatus() == Stopped) { @@ -223,6 +279,7 @@ void SoundStream::streamData() else { // End streaming + Lock lock(m_threadMutex); m_isStreaming = false; } } @@ -266,6 +323,7 @@ void SoundStream::streamData() << "and initialize() has been called correctly" << std::endl; // Abort streaming (exit main loop) + Lock lock(m_threadMutex); m_isStreaming = false; requestStop = true; break; From 74e425a9ed392e537a5b17330ee792311d303c91 Mon Sep 17 00:00:00 2001 From: binary1248 Date: Fri, 4 Jul 2014 22:24:48 +0200 Subject: [PATCH 2/3] Made sure SoundStream adhered to its documented behavior, added a hint to SoundStream and Sound documentation regarding setting the offset while stopped. --- include/SFML/Audio/Sound.hpp | 4 +++- include/SFML/Audio/SoundStream.hpp | 4 +++- src/SFML/Audio/SoundStream.cpp | 37 ++++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/include/SFML/Audio/Sound.hpp b/include/SFML/Audio/Sound.hpp index 74b88269..6c346ba0 100644 --- a/include/SFML/Audio/Sound.hpp +++ b/include/SFML/Audio/Sound.hpp @@ -144,7 +144,9 @@ public : /// \brief Change the current playing position of the sound /// /// The playing position can be changed when the sound is - /// either paused or playing. + /// either paused or playing. Changing the playing position + /// when the sound is stopped has no effect, since playing + /// the sound would reset its position. /// /// \param timeOffset New playing position, from the beginning of the sound /// diff --git a/include/SFML/Audio/SoundStream.hpp b/include/SFML/Audio/SoundStream.hpp index 293623e4..de36f620 100644 --- a/include/SFML/Audio/SoundStream.hpp +++ b/include/SFML/Audio/SoundStream.hpp @@ -132,7 +132,9 @@ public : /// \brief Change the current playing position of the stream /// /// The playing position can be changed when the stream is - /// either paused or playing. + /// either paused or playing. Changing the playing position + /// when the stream is stopped has no effect, since playing + /// the stream would reset its position. /// /// \param timeOffset New playing position, from the beginning of the stream /// diff --git a/src/SFML/Audio/SoundStream.cpp b/src/SFML/Audio/SoundStream.cpp index d21b3605..1c37d3ee 100644 --- a/src/SFML/Audio/SoundStream.cpp +++ b/src/SFML/Audio/SoundStream.cpp @@ -92,17 +92,29 @@ void SoundStream::play() return; } + bool isStreaming = false; + Status threadStartState = Stopped; + { Lock lock(m_threadMutex); - // If the sound is already playing (probably paused), just resume it - if (m_isStreaming) - { - alCheck(alSourcePlay(m_source)); - return; - } + isStreaming = m_isStreaming; + threadStartState = m_threadStartState; + } - m_isStreaming = true; + + if (isStreaming && (threadStartState == Paused)) + { + // If the sound is paused, resume it + Lock lock(m_threadMutex); + m_threadStartState = Playing; + alCheck(alSourcePlay(m_source)); + return; + } + else if (isStreaming && (threadStartState == Playing)) + { + // If the sound is playing, stop it and continue as if it was stopped + stop(); } // Move to the beginning @@ -110,6 +122,7 @@ void SoundStream::play() // Start updating the stream in a separate thread to avoid blocking the application m_samplesProcessed = 0; + m_isStreaming = true; m_threadStartState = Playing; m_thread.launch(); } @@ -121,6 +134,10 @@ void SoundStream::pause() // Handle pause() being called before the thread has started { Lock lock(m_threadMutex); + + if (!m_isStreaming) + return; + m_threadStartState = Paused; } @@ -139,6 +156,12 @@ void SoundStream::stop() // Wait for the thread to terminate m_thread.wait(); + + // Move to the beginning + onSeek(Time::Zero); + + // Reset the playing position + m_samplesProcessed = 0; } From 57941c1696a7e4a6b0491e3b2adcb2a5f6e5f573 Mon Sep 17 00:00:00 2001 From: binary1248 Date: Sat, 5 Jul 2014 16:19:30 +0200 Subject: [PATCH 3/3] Fixed invoking a virtual method (onSeek) in the destructor of SoundStream. --- src/SFML/Audio/SoundStream.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/SFML/Audio/SoundStream.cpp b/src/SFML/Audio/SoundStream.cpp index 1c37d3ee..eb84f25f 100644 --- a/src/SFML/Audio/SoundStream.cpp +++ b/src/SFML/Audio/SoundStream.cpp @@ -59,7 +59,15 @@ m_samplesProcessed(0) SoundStream::~SoundStream() { // Stop the sound if it was playing - stop(); + + // Request the thread to terminate + { + Lock lock(m_threadMutex); + m_isStreaming = false; + } + + // Wait for the thread to terminate + m_thread.wait(); }