From 87d519759e94afa1150b0ab8c032273cd6af6ec7 Mon Sep 17 00:00:00 2001 From: hogliux Date: Thu, 20 Oct 2022 10:51:05 +0200 Subject: [PATCH] Synthesiser: Ensured that the voice stealing algorithm does not allocate --- .../mpe/juce_MPESynthesiser.cpp | 33 +++++++++++------ .../mpe/juce_MPESynthesiser.h | 2 + .../synthesisers/juce_Synthesiser.cpp | 37 +++++++++++++------ .../synthesisers/juce_Synthesiser.h | 2 + 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/modules/juce_audio_basics/mpe/juce_MPESynthesiser.cpp b/modules/juce_audio_basics/mpe/juce_MPESynthesiser.cpp index fde51b2933..f89d3ae6a3 100644 --- a/modules/juce_audio_basics/mpe/juce_MPESynthesiser.cpp +++ b/modules/juce_audio_basics/mpe/juce_MPESynthesiser.cpp @@ -184,15 +184,19 @@ MPESynthesiserVoice* MPESynthesiser::findVoiceToSteal (MPENote noteToStealVoiceF MPESynthesiserVoice* low = nullptr; // Lowest sounding note, might be sustained, but NOT in release phase MPESynthesiserVoice* top = nullptr; // Highest sounding note, might be sustained, but NOT in release phase + // All major OSes use double-locking so this will be lock- and wait-free as long as stealLock is not + // contended. This is always the case if you do not call findVoiceToSteal on multiple threads at + // the same time. + const ScopedLock sl (stealLock); + // this is a list of voices we can steal, sorted by how long they've been running - Array usableVoices; - usableVoices.ensureStorageAllocated (voices.size()); + usableVoicesToStealArray.clear(); for (auto* voice : voices) { jassert (voice->isActive()); // We wouldn't be here otherwise - usableVoices.add (voice); + usableVoicesToStealArray.add (voice); // NB: Using a functor rather than a lambda here due to scare-stories about // compilers generating code containing heap allocations.. @@ -201,7 +205,7 @@ MPESynthesiserVoice* MPESynthesiser::findVoiceToSteal (MPENote noteToStealVoiceF bool operator() (const MPESynthesiserVoice* a, const MPESynthesiserVoice* b) const noexcept { return a->noteOnTime < b->noteOnTime; } }; - std::sort (usableVoices.begin(), usableVoices.end(), Sorter()); + std::sort (usableVoicesToStealArray.begin(), usableVoicesToStealArray.end(), Sorter()); if (! voice->isPlayingButReleased()) // Don't protect released notes { @@ -222,24 +226,24 @@ MPESynthesiserVoice* MPESynthesiser::findVoiceToSteal (MPENote noteToStealVoiceF // If we want to re-use the voice to trigger a new note, // then The oldest note that's playing the same note number is ideal. if (noteToStealVoiceFor.isValid()) - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice->getCurrentlyPlayingNote().initialNote == noteToStealVoiceFor.initialNote) return voice; // Oldest voice that has been released (no finger on it and not held by sustain pedal) - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top && voice->isPlayingButReleased()) return voice; // Oldest voice that doesn't have a finger on it: - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top && voice->getCurrentlyPlayingNote().keyState != MPENote::keyDown && voice->getCurrentlyPlayingNote().keyState != MPENote::keyDownAndSustained) return voice; // Oldest voice that isn't protected - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top) return voice; @@ -256,9 +260,16 @@ MPESynthesiserVoice* MPESynthesiser::findVoiceToSteal (MPENote noteToStealVoiceF //============================================================================== void MPESynthesiser::addVoice (MPESynthesiserVoice* const newVoice) { - const ScopedLock sl (voicesLock); - newVoice->setCurrentSampleRate (getSampleRate()); - voices.add (newVoice); + { + const ScopedLock sl (voicesLock); + newVoice->setCurrentSampleRate (getSampleRate()); + voices.add (newVoice); + } + + { + const ScopedLock sl (stealLock); + usableVoicesToStealArray.ensureStorageAllocated (voices.size() + 1); + } } void MPESynthesiser::clearVoices() diff --git a/modules/juce_audio_basics/mpe/juce_MPESynthesiser.h b/modules/juce_audio_basics/mpe/juce_MPESynthesiser.h index 9a443c5f94..bc4c1b4083 100644 --- a/modules/juce_audio_basics/mpe/juce_MPESynthesiser.h +++ b/modules/juce_audio_basics/mpe/juce_MPESynthesiser.h @@ -304,6 +304,8 @@ private: //============================================================================== std::atomic shouldStealVoices { false }; uint32 lastNoteOnCounter = 0; + mutable CriticalSection stealLock; + mutable Array usableVoicesToStealArray; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (MPESynthesiser) }; diff --git a/modules/juce_audio_basics/synthesisers/juce_Synthesiser.cpp b/modules/juce_audio_basics/synthesisers/juce_Synthesiser.cpp index c407911f30..cfeb6a0928 100644 --- a/modules/juce_audio_basics/synthesisers/juce_Synthesiser.cpp +++ b/modules/juce_audio_basics/synthesisers/juce_Synthesiser.cpp @@ -98,9 +98,20 @@ void Synthesiser::clearVoices() SynthesiserVoice* Synthesiser::addVoice (SynthesiserVoice* const newVoice) { - const ScopedLock sl (lock); - newVoice->setCurrentPlaybackSampleRate (sampleRate); - return voices.add (newVoice); + SynthesiserVoice* voice; + + { + const ScopedLock sl (lock); + newVoice->setCurrentPlaybackSampleRate (sampleRate); + voice = voices.add (newVoice); + } + + { + const ScopedLock sl (stealLock); + usableVoicesToStealArray.ensureStorageAllocated (voices.size() + 1); + } + + return voice; } void Synthesiser::removeVoice (const int index) @@ -514,9 +525,13 @@ SynthesiserVoice* Synthesiser::findVoiceToSteal (SynthesiserSound* soundToPlay, SynthesiserVoice* low = nullptr; // Lowest sounding note, might be sustained, but NOT in release phase SynthesiserVoice* top = nullptr; // Highest sounding note, might be sustained, but NOT in release phase + // All major OSes use double-locking so this will be lock- and wait-free as long as the lock is not + // contended. This is always the case if you do not call findVoiceToSteal on multiple threads at + // the same time. + const ScopedLock sl (stealLock); + // this is a list of voices we can steal, sorted by how long they've been running - Array usableVoices; - usableVoices.ensureStorageAllocated (voices.size()); + usableVoicesToStealArray.clear(); for (auto* voice : voices) { @@ -524,7 +539,7 @@ SynthesiserVoice* Synthesiser::findVoiceToSteal (SynthesiserSound* soundToPlay, { jassert (voice->isVoiceActive()); // We wouldn't be here otherwise - usableVoices.add (voice); + usableVoicesToStealArray.add (voice); // NB: Using a functor rather than a lambda here due to scare-stories about // compilers generating code containing heap allocations.. @@ -533,7 +548,7 @@ SynthesiserVoice* Synthesiser::findVoiceToSteal (SynthesiserSound* soundToPlay, bool operator() (const SynthesiserVoice* a, const SynthesiserVoice* b) const noexcept { return a->wasStartedBefore (*b); } }; - std::sort (usableVoices.begin(), usableVoices.end(), Sorter()); + std::sort (usableVoicesToStealArray.begin(), usableVoicesToStealArray.end(), Sorter()); if (! voice->isPlayingButReleased()) // Don't protect released notes { @@ -553,22 +568,22 @@ SynthesiserVoice* Synthesiser::findVoiceToSteal (SynthesiserSound* soundToPlay, top = nullptr; // The oldest note that's playing with the target pitch is ideal.. - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice->getCurrentlyPlayingNote() == midiNoteNumber) return voice; // Oldest voice that has been released (no finger on it and not held by sustain pedal) - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top && voice->isPlayingButReleased()) return voice; // Oldest voice that doesn't have a finger on it: - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top && ! voice->isKeyDown()) return voice; // Oldest voice that isn't protected - for (auto* voice : usableVoices) + for (auto* voice : usableVoicesToStealArray) if (voice != low && voice != top) return voice; diff --git a/modules/juce_audio_basics/synthesisers/juce_Synthesiser.h b/modules/juce_audio_basics/synthesisers/juce_Synthesiser.h index b9db266cc3..5010667cb2 100644 --- a/modules/juce_audio_basics/synthesisers/juce_Synthesiser.h +++ b/modules/juce_audio_basics/synthesisers/juce_Synthesiser.h @@ -627,6 +627,8 @@ private: bool subBlockSubdivisionIsStrict = false; bool shouldStealNotes = true; BigInteger sustainPedalsDown; + mutable CriticalSection stealLock; + mutable Array usableVoicesToStealArray; template void processNextBlock (AudioBuffer&, const MidiBuffer&, int startSample, int numSamples);