From 218a8149af1eec33ee180ad4a9507a837318990c Mon Sep 17 00:00:00 2001 From: jules Date: Tue, 22 Dec 2015 15:33:00 +0000 Subject: [PATCH] Fixed several bugs in the AudioDeviceManager::playSound() mechanism that could cause feedback and other problems --- .../audio_io/juce_AudioDeviceManager.cpp | 237 ++++++------------ .../format/juce_AudioFormatManager.h | 4 +- 2 files changed, 72 insertions(+), 169 deletions(-) diff --git a/modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.cpp b/modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.cpp index 2959efb126..931c71c8ec 100755 --- a/modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.cpp +++ b/modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.cpp @@ -928,141 +928,74 @@ void AudioDeviceManager::setDefaultMidiOutput (const String& deviceName) //============================================================================== // This is an AudioTransportSource which will own it's assigned source -class AudioSourceOwningTransportSource : public AudioTransportSource +struct AudioSourceOwningTransportSource : public AudioTransportSource { -public: - AudioSourceOwningTransportSource() {} - ~AudioSourceOwningTransportSource() { setSource (nullptr); } - - void setSource (PositionableAudioSource* newSource) + AudioSourceOwningTransportSource (PositionableAudioSource* s) : source (s) { - if (src != newSource) - { - ScopedPointer oldSourceDeleter (src); - src = newSource; + AudioTransportSource::setSource (s); + } - // tell the base class about the new source before deleting the old one - AudioTransportSource::setSource (newSource); - } + ~AudioSourceOwningTransportSource() + { + setSource (nullptr); } private: - ScopedPointer src; + ScopedPointer source; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AudioSourceOwningTransportSource) }; //============================================================================== -// An Audio player which will remove itself from the AudioDeviceManager's +// An AudioSourcePlayer which will remove itself from the AudioDeviceManager's // callback list once it finishes playing its source -class AutoRemovingSourcePlayer : public AudioSourcePlayer, - private ChangeListener +struct AutoRemovingSourcePlayer : public AudioSourcePlayer, + private Timer { -public: - struct DeleteOnMessageThread : public CallbackMessage - { - DeleteOnMessageThread (AutoRemovingSourcePlayer* p) : parent (p) {} - void messageCallback() override { delete parent; } - - AutoRemovingSourcePlayer* parent; - }; - - //============================================================================== - AutoRemovingSourcePlayer (AudioDeviceManager& deviceManager, bool ownSource) - : manager (deviceManager), - deleteWhenDone (ownSource), - hasAddedCallback (false), - recursiveEntry (false) + AutoRemovingSourcePlayer (AudioDeviceManager& dm, AudioTransportSource* ts, bool ownSource) + : manager (dm), transportSource (ts, ownSource) { + jassert (ts != nullptr); + manager.addAudioCallback (this); + AudioSourcePlayer::setSource (transportSource); + startTimerHz (10); } - void changeListenerCallback (ChangeBroadcaster* newSource) override + ~AutoRemovingSourcePlayer() { - if (AudioTransportSource* currentTransport - = dynamic_cast (getCurrentSource())) - { - ignoreUnused (newSource); - jassert (newSource == currentTransport); + setSource (nullptr); + manager.removeAudioCallback (this); + } - if (! currentTransport->isPlaying()) - { - // this will call audioDeviceStopped! - manager.removeAudioCallback (this); - } - else if (! hasAddedCallback) - { - hasAddedCallback = true; - manager.addAudioCallback (this); - } - } + void timerCallback() override + { + if (! transportSource->isPlaying()) + delete this; } void audioDeviceStopped() override { - if (! recursiveEntry) - { - ScopedValueSetter s (recursiveEntry, true, false); - - manager.removeAudioCallback (this); - AudioSourcePlayer::audioDeviceStopped(); - - if (MessageManager* mm = MessageManager::getInstanceWithoutCreating()) - { - if (mm->isThisTheMessageThread()) - delete this; - else - (new DeleteOnMessageThread (this))->post(); - } - } - } - - void setSource (AudioTransportSource* newSource) - { - AudioSource* oldSource = getCurrentSource(); - - if (AudioTransportSource* oldTransport = dynamic_cast (oldSource)) - oldTransport->removeChangeListener (this); - - if (newSource != nullptr) - newSource->addChangeListener (this); - - AudioSourcePlayer::setSource (newSource); - - if (deleteWhenDone) - delete oldSource; - } - -private: - // only allow myself to be deleted when my audio callback has been removed - ~AutoRemovingSourcePlayer() - { + AudioSourcePlayer::audioDeviceStopped(); setSource (nullptr); } +private: AudioDeviceManager& manager; - bool deleteWhenDone, hasAddedCallback, recursiveEntry; + OptionalScopedPointer transportSource; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AutoRemovingSourcePlayer) }; //============================================================================== // An AudioSource which simply outputs a buffer -class AudioSampleBufferSource : public PositionableAudioSource +class AudioSampleBufferSource : public PositionableAudioSource { public: - AudioSampleBufferSource (AudioSampleBuffer* audioBuffer, bool shouldLoop, bool ownBuffer) - : position (0), - buffer (audioBuffer), - looping (shouldLoop), - deleteWhenDone (ownBuffer) + AudioSampleBufferSource (AudioSampleBuffer* audioBuffer, bool ownBuffer) + : buffer (audioBuffer, ownBuffer), + position (0), looping (false) {} - ~AudioSampleBufferSource() - { - if (deleteWhenDone) - delete buffer; - } - //============================================================================== void setNextReadPosition (int64 newPosition) override { @@ -1074,69 +1007,45 @@ public: position = jmin (buffer->getNumSamples(), static_cast (newPosition)); } - int64 getNextReadPosition() const override - { - return static_cast (position); - } + int64 getNextReadPosition() const override { return static_cast (position); } + int64 getTotalLength() const override { return static_cast (buffer->getNumSamples()); } - int64 getTotalLength() const override - { - return static_cast (buffer->getNumSamples()); - } - - bool isLooping() const override - { - return looping; - } - - void setLooping (bool shouldLoop) override - { - looping = shouldLoop; - } + bool isLooping() const override { return looping; } + void setLooping (bool shouldLoop) override { looping = shouldLoop; } //============================================================================== - void prepareToPlay (int samplesPerBlockExpected, double sampleRate) override - { - ignoreUnused (samplesPerBlockExpected, sampleRate); - } - - void releaseResources() override - {} + void prepareToPlay (int, double) override {} + void releaseResources() override {} void getNextAudioBlock (const AudioSourceChannelInfo& bufferToFill) override { - int max = jmin (buffer->getNumSamples() - position, bufferToFill.numSamples); + bufferToFill.clearActiveBufferRegion(); - jassert (max >= 0); + const int bufferSize = buffer->getNumSamples(); + const int samplesNeeded = bufferToFill.numSamples; + const int samplesToCopy = jmin (bufferSize - position, samplesNeeded); + + if (samplesToCopy > 0) { - int ch; - int maxInChannels = buffer->getNumChannels(); - int maxOutChannels = jmin (bufferToFill.buffer->getNumChannels(), - jmax (maxInChannels, 2)); + const int maxInChannels = buffer->getNumChannels(); + const int maxOutChannels = jmin (bufferToFill.buffer->getNumChannels(), jmax (maxInChannels, 2)); - for (ch = 0; ch < maxOutChannels; ch++) - { - int inChannel = ch % maxInChannels; - - if (max > 0) - bufferToFill.buffer->copyFrom (ch, bufferToFill.startSample, *buffer, inChannel, position, max); - } - - for (; ch < bufferToFill.buffer->getNumChannels(); ++ch) - bufferToFill.buffer->clear (ch, bufferToFill.startSample, bufferToFill.numSamples); + for (int i = 0; i < maxOutChannels; ++i) + bufferToFill.buffer->copyFrom (i, bufferToFill.startSample, *buffer, + i % maxInChannels, position, samplesToCopy); } - position += max; + position += samplesNeeded; if (looping) - position = position % buffer->getNumSamples(); + position %= bufferSize; } private: //============================================================================== + OptionalScopedPointer buffer; int position; - AudioSampleBuffer* buffer; - bool looping, deleteWhenDone; + bool looping; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AudioSampleBufferSource) }; @@ -1165,38 +1074,37 @@ void AudioDeviceManager::playSound (const void* resourceData, size_t resourceSiz void AudioDeviceManager::playSound (AudioFormatReader* reader, bool deleteWhenFinished) { - playSound (new AudioFormatReaderSource (reader, deleteWhenFinished), true); + if (reader != nullptr) + playSound (new AudioFormatReaderSource (reader, deleteWhenFinished), true); +} + +void AudioDeviceManager::playSound (AudioSampleBuffer* buffer, bool deleteWhenFinished) +{ + if (buffer != nullptr) + playSound (new AudioSampleBufferSource (buffer, deleteWhenFinished), true); } void AudioDeviceManager::playSound (PositionableAudioSource* audioSource, bool deleteWhenFinished) { if (audioSource != nullptr && currentAudioDevice != nullptr) { - if (AudioTransportSource* transport = dynamic_cast (audioSource)) - { - AutoRemovingSourcePlayer* player = new AutoRemovingSourcePlayer (*this, deleteWhenFinished); - player->setSource (transport); - } - else - { - AudioTransportSource* transportSource; + AudioTransportSource* transport = dynamic_cast (audioSource); + if (transport == nullptr) + { if (deleteWhenFinished) { - AudioSourceOwningTransportSource* owningTransportSource = new AudioSourceOwningTransportSource(); - owningTransportSource->setSource (audioSource); - transportSource = owningTransportSource; + transport = new AudioSourceOwningTransportSource (audioSource); } else { - transportSource = new AudioTransportSource; - transportSource->setSource (audioSource); + transport = new AudioTransportSource(); + transport->setSource (audioSource); } - - // recursively call myself - playSound (transportSource, true); - transportSource->start(); } + + transport->start(); + new AutoRemovingSourcePlayer (*this, transport, deleteWhenFinished); } else { @@ -1205,11 +1113,6 @@ void AudioDeviceManager::playSound (PositionableAudioSource* audioSource, bool d } } -void AudioDeviceManager::playSound (AudioSampleBuffer* buffer, bool deleteWhenFinished) -{ - playSound (new AudioSampleBufferSource (buffer, false, deleteWhenFinished), true); -} - void AudioDeviceManager::playTestSound() { const double sampleRate = currentAudioDevice->getCurrentSampleRate(); diff --git a/modules/juce_audio_formats/format/juce_AudioFormatManager.h b/modules/juce_audio_formats/format/juce_AudioFormatManager.h index 3ad92131a3..2d30bd63f4 100644 --- a/modules/juce_audio_formats/format/juce_AudioFormatManager.h +++ b/modules/juce_audio_formats/format/juce_AudioFormatManager.h @@ -126,8 +126,8 @@ public: The stream that is passed-in must be capable of being repositioned so that all the formats can have a go at opening it. - If none of the registered formats can open the stream, it'll return 0. If it - returns a reader, it's the caller's responsibility to delete the reader. + If none of the registered formats can open the stream, it'll return nullptr. + If it returns a reader, it's the caller's responsibility to delete the reader. */ AudioFormatReader* createReaderFor (InputStream* audioFileStream);