From 1ee6d1dbc639fbf961d3b68ce038edd15f142fcb Mon Sep 17 00:00:00 2001 From: Maximilian Wagenbach Date: Fri, 2 Oct 2015 14:49:00 +0200 Subject: [PATCH] Make sure the recording thread in sf::SoundRecorder is stopped before sf::SoundBufferRecorder is destroyed. Fixes a "pure virtual method called" crash. Also updated the documentation and the VoIP example. --- examples/voip/Client.cpp | 20 ++++++++++++++++---- include/SFML/Audio/SoundBufferRecorder.hpp | 6 ++++++ include/SFML/Audio/SoundRecorder.hpp | 9 +++++++++ src/SFML/Audio/SoundBufferRecorder.cpp | 8 ++++++++ src/SFML/Audio/SoundRecorder.cpp | 21 +++++++++++++++------ 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/examples/voip/Client.cpp b/examples/voip/Client.cpp index 595d6247..d66e0e23 100644 --- a/examples/voip/Client.cpp +++ b/examples/voip/Client.cpp @@ -32,10 +32,22 @@ public: { } + //////////////////////////////////////////////////////////// + /// Destructor + /// + /// \see SoundRecorder::~SoundRecorder() + /// + //////////////////////////////////////////////////////////// + ~NetworkRecorder() + { + // Make sure to stop the recording thread + stop(); + } + private: //////////////////////////////////////////////////////////// - /// /see SoundRecorder::OnStart + /// \see SoundRecorder::onStart /// //////////////////////////////////////////////////////////// virtual bool onStart() @@ -52,7 +64,7 @@ private: } //////////////////////////////////////////////////////////// - /// /see SoundRecorder::ProcessSamples + /// \see SoundRecorder::onProcessSamples /// //////////////////////////////////////////////////////////// virtual bool onProcessSamples(const sf::Int16* samples, std::size_t sampleCount) @@ -67,7 +79,7 @@ private: } //////////////////////////////////////////////////////////// - /// /see SoundRecorder::OnStop + /// \see SoundRecorder::onStop /// //////////////////////////////////////////////////////////// virtual void onStop() @@ -98,7 +110,7 @@ private: void doClient(unsigned short port) { // Check that the device can capture audio - if (sf::SoundRecorder::isAvailable() == false) + if (!sf::SoundRecorder::isAvailable()) { std::cout << "Sorry, audio capture is not supported by your system" << std::endl; return; diff --git a/include/SFML/Audio/SoundBufferRecorder.hpp b/include/SFML/Audio/SoundBufferRecorder.hpp index 7715ac58..28b7596e 100644 --- a/include/SFML/Audio/SoundBufferRecorder.hpp +++ b/include/SFML/Audio/SoundBufferRecorder.hpp @@ -45,6 +45,12 @@ class SFML_AUDIO_API SoundBufferRecorder : public SoundRecorder { public: + //////////////////////////////////////////////////////////// + /// \brief destructor + /// + //////////////////////////////////////////////////////////// + ~SoundBufferRecorder(); + //////////////////////////////////////////////////////////// /// \brief Get the sound buffer containing the captured audio data /// diff --git a/include/SFML/Audio/SoundRecorder.hpp b/include/SFML/Audio/SoundRecorder.hpp index 964e4f3f..f05b4a59 100644 --- a/include/SFML/Audio/SoundRecorder.hpp +++ b/include/SFML/Audio/SoundRecorder.hpp @@ -316,11 +316,20 @@ private: /// from this separate thread. It is important to keep this in /// mind, because you may have to take care of synchronization /// issues if you share data between threads. +/// Another thing to bear in mind is that you must call stop() +/// in the destructor of your derived class, so that the recording +/// thread finishes before your object is destroyed. /// /// Usage example: /// \code /// class CustomRecorder : public sf::SoundRecorder /// { +/// ~CustomRecorder() +/// { +/// // Make sure to stop the recording thread +/// stop(); +/// } +/// /// virtual bool onStart() // optional /// { /// // Initialize whatever has to be done before the capture starts diff --git a/src/SFML/Audio/SoundBufferRecorder.cpp b/src/SFML/Audio/SoundBufferRecorder.cpp index 23f5deb5..e09c4ffc 100644 --- a/src/SFML/Audio/SoundBufferRecorder.cpp +++ b/src/SFML/Audio/SoundBufferRecorder.cpp @@ -32,6 +32,14 @@ namespace sf { +//////////////////////////////////////////////////////////// +SoundBufferRecorder::~SoundBufferRecorder() +{ + // Make sure to stop the recording thread + stop(); +} + + //////////////////////////////////////////////////////////// bool SoundBufferRecorder::onStart() { diff --git a/src/SFML/Audio/SoundRecorder.cpp b/src/SFML/Audio/SoundRecorder.cpp index 4d796e4a..510495b0 100644 --- a/src/SFML/Audio/SoundRecorder.cpp +++ b/src/SFML/Audio/SoundRecorder.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #ifdef _MSC_VER #pragma warning(disable: 4355) // 'this' used in base member initializer list @@ -59,7 +60,12 @@ m_deviceName (getDefaultDevice()) //////////////////////////////////////////////////////////// SoundRecorder::~SoundRecorder() { - // Nothing to do + // This assertion is triggered if the recording is still running while + // the object is destroyed. It ensures that stop() is called in the + // destructor of the derived class, which makes sure that the recording + // thread finishes before the derived object is destroyed. Otherwise a + // "pure virtual method called" exception is triggered. + assert(!m_isCapturing && "You must call stop() in the destructor of your derived class, so that the recording thread finishes before your object is destroyed."); } @@ -114,12 +120,15 @@ bool SoundRecorder::start(unsigned int sampleRate) //////////////////////////////////////////////////////////// void SoundRecorder::stop() { - // Stop the capturing thread - m_isCapturing = false; - m_thread.wait(); + // Stop the capturing thread if there is one + if (m_isCapturing) + { + m_isCapturing = false; + m_thread.wait(); - // Notify derived class - onStop(); + // Notify derived class + onStop(); + } }