From a14a8d8acd359d9df13ac89168a09aba9e183b30 Mon Sep 17 00:00:00 2001 From: Andrew Reeman <1423405+andrewreeman@users.noreply.github.com> Date: Thu, 20 Jun 2024 20:30:31 +0100 Subject: [PATCH] 56 fix au validation (#58) * Clearing out vectors holding spectral processes directory instead of by the parent vector * Removed Logger. This is not useful * Perhaps better clearing of spec processes * validating phase lock * If preparing to play then try not to perform other operations on the audio processors * Added SSF to au validation * added spectral gate to au validation * validating all plugins and exiting early if error * fixed more race conditions * Frequency shift passes validation * fixed bin scrambler out of bounds error * initialising wave table if fft size changed before playback started * Using correct variable when detecting races' --- .../Source/FrequencyShiftAudioPlugin.cpp | 8 ++ PhaseLock/Source/PhaseLockAudioPlugin.cpp | 7 ++ .../Source/SSFAudioPlugin.cpp | 7 ++ .../Source/SSFInteractor.cpp | 4 + auValidate.sh | 25 ++++++ shared/FftSwitcher.h | 2 + shared/FftThread.cpp | 1 + shared/SpectralAudioPlugin.cpp | 85 ++++++++++++------- shared/SpectralAudioPlugin.h | 24 +++--- shared/SpectralAudioProcessorInteractor.cpp | 69 +++++++++++++-- shared/SpectralAudioProcessorInteractor.h | 5 ++ 11 files changed, 187 insertions(+), 50 deletions(-) create mode 100755 auValidate.sh diff --git a/FrequencyShift/Source/FrequencyShiftAudioPlugin.cpp b/FrequencyShift/Source/FrequencyShiftAudioPlugin.cpp index 819c29d..44647e6 100644 --- a/FrequencyShift/Source/FrequencyShiftAudioPlugin.cpp +++ b/FrequencyShift/Source/FrequencyShiftAudioPlugin.cpp @@ -34,6 +34,14 @@ class Factory : public SpectralAudioPlugin::DependencyFactory { // This creates new instances of the plugin.. AudioProcessor* JUCE_CALLTYPE createPluginFilter() { + // To debug AUval +//#if DEBUG +// while (!Process::isRunningUnderDebugger()) +// { +// Thread::sleep(250); +// } +//#endif + return new SpectralAudioPlugin( new Factory() ); diff --git a/PhaseLock/Source/PhaseLockAudioPlugin.cpp b/PhaseLock/Source/PhaseLockAudioPlugin.cpp index 84d3a24..eb34fe2 100644 --- a/PhaseLock/Source/PhaseLockAudioPlugin.cpp +++ b/PhaseLock/Source/PhaseLockAudioPlugin.cpp @@ -30,5 +30,12 @@ class Factory : public SpectralAudioPlugin::DependencyFactory { // This creates new instances of the plugin.. AudioProcessor* JUCE_CALLTYPE createPluginFilter() { + // To debug AUval +//#if DEBUG +// while (!Process::isRunningUnderDebugger()) +// { +// Thread::sleep(250); +// } +//#endif return new SpectralAudioPlugin(new Factory()); } diff --git a/SinusoidalShapedFilter/Source/SSFAudioPlugin.cpp b/SinusoidalShapedFilter/Source/SSFAudioPlugin.cpp index 29ef120..15599b8 100644 --- a/SinusoidalShapedFilter/Source/SSFAudioPlugin.cpp +++ b/SinusoidalShapedFilter/Source/SSFAudioPlugin.cpp @@ -30,5 +30,12 @@ class Factory : public SpectralAudioPlugin::DependencyFactory { // This creates new instances of the plugin.. AudioProcessor* JUCE_CALLTYPE createPluginFilter() { + // To debug AUval +//#if DEBUG +// while (!Process::isRunningUnderDebugger()) +// { +// Thread::sleep(250); +// } +//#endif return new SpectralAudioPlugin(new Factory()); } diff --git a/SinusoidalShapedFilter/Source/SSFInteractor.cpp b/SinusoidalShapedFilter/Source/SSFInteractor.cpp index 7760589..192dd10 100644 --- a/SinusoidalShapedFilter/Source/SSFInteractor.cpp +++ b/SinusoidalShapedFilter/Source/SSFInteractor.cpp @@ -31,5 +31,9 @@ std::unique_ptr SSFInteractor::createSpectralProcess( } void SSFInteractor::onFftSizeChanged(){ + if(m_wavetable == nullptr) { + m_wavetable = std::make_shared >(getFftSize() / 2, 1, 1); + } + m_wavetable->resize(getFftSize() / 2); } diff --git a/auValidate.sh b/auValidate.sh new file mode 100755 index 0000000..77734a2 --- /dev/null +++ b/auValidate.sh @@ -0,0 +1,25 @@ +#!/bin/bash + +# Exit early if any command fails +set -e + +# Frequency shifter +auval -strict -v aumf SPFS STPW + +# Bin Scrambler +auval -strict -v aumf SPBS STPW + +# Frequency magnet +auval -strict -v aumf SPFM STPW + +# Morph +auval -strict -v aumf SPMO STPW + +# Phase lock +auval -strict -v aumf SPPL STPW + +# Sinusoidal Sharped Filer +auval -strict -v aumf SPSS STPW + +# Spectral Gate +auval -strict -v aumf SPSG STPW diff --git a/shared/FftSwitcher.h b/shared/FftSwitcher.h index b747a1c..41e3e16 100644 --- a/shared/FftSwitcher.h +++ b/shared/FftSwitcher.h @@ -19,6 +19,8 @@ class FftSwitcherThread: public Thread { /// Thread /// Do not call this directly. Instead call `switchFftSize` or `switchOverlapCount` void run() override { + if (threadShouldExit()) { return; } + if(m_switchFft) { m_switchFft = false; m_fftSwitcher->switchFftSize(); diff --git a/shared/FftThread.cpp b/shared/FftThread.cpp index 1742089..721bf1e 100644 --- a/shared/FftThread.cpp +++ b/shared/FftThread.cpp @@ -1,3 +1,4 @@ +// TODO: check if this class is unused #include "FftThread.h" diff --git a/shared/SpectralAudioPlugin.cpp b/shared/SpectralAudioPlugin.cpp index 1f993f3..1ee3847 100644 --- a/shared/SpectralAudioPlugin.cpp +++ b/shared/SpectralAudioPlugin.cpp @@ -33,28 +33,11 @@ SpectralAudioPlugin::SpectralAudioPlugin( m_versionCheckThread(VersionCode, "https://www.andrewreeman.com/spectral_suite_publish.json"), m_dependencyFactory(dependencies) { - - - //FileLogger* logger = new FileLogger(FileLogger::getSystemLogFileFolder().getChildFile("logs") - // .getChildFile("spectral_suite" + Time::getCurrentTime().formatted("%Y-%m-%d_%H-%M-%S")) - // .withFileExtension(".log") - // .getNonexistentSibling(), - // "Log started", 0); - - File logFile = FileLogger::getSystemLogFileFolder().getChildFile("SpectralSuite").getChildFile("spectral_suite.log"); - m_logger = std::unique_ptr( - new FileLogger( - logFile, - "Log started" - ) - ); - Logger::setCurrentLogger(m_logger.get()); this->initialiseParameters(); } SpectralAudioPlugin::~SpectralAudioPlugin() { - Logger::setCurrentLogger(nullptr); if(this->m_versionCheckThread.isThreadRunning()) { this->m_versionCheckThread.stopThread(20); } @@ -67,6 +50,9 @@ SpectralAudioPlugin::~SpectralAudioPlugin() /* FFT Switcher methods */ void SpectralAudioPlugin::switchFftSize() { + if (isInvalidFftModificationState()) { return; } +// if (m_audioProcessorInteractor->isPreparingToPlay()) { return; } + setFftSize(m_fftSizeChoiceAdapter.fftSize()); if(getActiveEditor() != nullptr) { @@ -76,6 +62,8 @@ void SpectralAudioPlugin::switchFftSize() } void SpectralAudioPlugin::switchFftStyle() { + if (isInvalidFftModificationState()) { return; } + FftStyle style = m_fftStyleChoiceAdapter.fftStyle(); switch(style) { case FftStyle::DEFAULT: @@ -95,24 +83,52 @@ void SpectralAudioPlugin::switchFftStyle() } void SpectralAudioPlugin::switchOverlapCount() { + if (isInvalidFftModificationState()) { return; } + int overlaps = m_fftOverlapsChoiceAdapter.overlapCount(); m_audioProcessorInteractor->setNumOverlaps(overlaps); const int hopSize = m_audioProcessorInteractor->getHopSize(); - for(std::vector& output : m_output) - { - output.resize(hopSize, 0.f); - } - for(std::vector& input : m_input) - { - input.resize(hopSize, 0.f); - } - + + for(std::vector& output : m_output) + { + if (output.size() == hopSize) + { + continue; + } + + if (isInvalidFftModificationState()) { return; } + // Because we switch overlaps on a different thread m_output might be empty when we resize + // Which means we no longer own the output vector + // output vector expected to only be empty if we have been destructed + // TODO: use mutex when modifying fft buffers + if (!m_output.empty() && !output.empty()) + { + output.resize(hopSize, 0.f); + } + } + + for(std::vector& input : m_input) + { + if (input.size() == hopSize) + { + continue; + } + + if (isInvalidFftModificationState()) { return; } + if (!m_input.empty() && !input.empty()) + { + input.resize(hopSize, 0.f); + } + } + setLatencySamples(m_audioProcessorInteractor->getFftSize() + hopSize); } void SpectralAudioPlugin::switchFftWindowType() { + if (isInvalidFftModificationState()) { return; } + auto windowType = m_fftWindowChoiceAdapter.fftWindow(); m_audioProcessorInteractor->setWindowType(windowType); } @@ -184,6 +200,7 @@ void SpectralAudioPlugin::prepareToPlay (double sampleRate, int) { m_output.clear(); m_input.clear(); + for ( int outputChannelCount = getBusesLayout().getMainOutputChannels(); outputChannelCount > 0; @@ -193,10 +210,11 @@ void SpectralAudioPlugin::prepareToPlay (double sampleRate, int) m_output.push_back(std::vector()); m_input.push_back(std::vector()); } - - const int fftSize = m_fftSizeChoiceAdapter.fftSize(); - m_audioProcessorInteractor->prepareToPlay(fftSize, (int)sampleRate, getBusesLayout().getMainOutputChannels()); - setFftSize(fftSize); + + const int fftSize = m_fftSizeChoiceAdapter.fftSize(); + m_audioProcessorInteractor->prepareToPlay(fftSize, (int)sampleRate, getBusesLayout().getMainOutputChannels()); + + setFftSize(fftSize); } void SpectralAudioPlugin::releaseResources() @@ -232,9 +250,9 @@ void SpectralAudioPlugin::emptyOutputs() { void SpectralAudioPlugin::processBlock (AudioBuffer& buffer, MidiBuffer& midiBuffer) { - if (m_fftSwitcher.isThreadRunning()) { + if (m_fftSwitcher.isThreadRunning()) { m_internalBufferReadWriteIndex = 0; - return; + return; } if (m_fftSizeChoiceAdapter.shouldChangeFftSize()) { @@ -329,6 +347,8 @@ void SpectralAudioPlugin::setStateInformation (const void* data, int sizeInBytes } void SpectralAudioPlugin::setFftSize(int size) { + if (isInvalidFftModificationState()) { return; } + m_audioProcessorInteractor->setFftSize(size); const int hopSize = m_audioProcessorInteractor->getHopSize(); @@ -351,7 +371,6 @@ void SpectralAudioPlugin::checkForUpdates(VersionCheckThread::Listener* listener } void SpectralAudioPlugin::initialiseParameters() { - parameters = m_dependencyFactory->createParams(this); m_audioProcessorInteractor = m_dependencyFactory->createProcessor(this); diff --git a/shared/SpectralAudioPlugin.h b/shared/SpectralAudioPlugin.h index c6f8e71..bd2f1bb 100644 --- a/shared/SpectralAudioPlugin.h +++ b/shared/SpectralAudioPlugin.h @@ -12,10 +12,6 @@ #include "ParameterContainerComponentFactory.h" #include "PluginParameters.h" -//============================================================================== -/** -*/ - class SpectralAudioPluginUi; class SpectralAudioPlugin : public AudioProcessor, private FftSwitcherThread::FftSwitcher @@ -30,8 +26,6 @@ class SpectralAudioPlugin : public AudioProcessor, private FftSwitcherThread::Ff virtual ~DependencyFactory(){}; virtual std::shared_ptr createParams(SpectralAudioPlugin* plugin) = 0; -// virtual std::unique_ptr createUi(SpectralAudioPlugin* plugin) = 0; -// virtual std::unique_ptr createUi(SpectralAudioPlugin* plugin) = 0; virtual ParameterContainerComponent* createUi(SpectralAudioPlugin* plugin) = 0; virtual std::unique_ptr createProcessor(SpectralAudioPlugin* plugin) = 0; virtual Array fftSizesToNotInclude() { return Array(); }; @@ -107,10 +101,18 @@ class SpectralAudioPlugin : public AudioProcessor, private FftSwitcherThread::Ff void emptyOutputs(); void setFftSize(int fftSize); void initialiseParameters(); - - std::shared_ptr parameters; -// std::unique_ptr m_parameterUiComponent; - std::unique_ptr m_audioProcessorInteractor; + bool isPreparingToPlay() const { return m_audioProcessorInteractor->isPreparingToPlay(); } + bool isInvalidFftModificationState() const { + return + m_fftSwitcher.threadShouldExit() + || m_audioProcessorInteractor->isPreparingToPlay() + || m_audioProcessorInteractor->isPlaying() + || m_output.empty() + || m_input.empty(); + }; + + std::shared_ptr parameters; + std::unique_ptr m_audioProcessorInteractor; FftSizeChoiceAdapter m_fftSizeChoiceAdapter; FftStyleChoiceAdapter m_fftStyleChoiceAdapter; @@ -118,8 +120,6 @@ class SpectralAudioPlugin : public AudioProcessor, private FftSwitcherThread::Ff FftWindowChoiceAdapter m_fftWindowChoiceAdapter; FftSwitcherThread m_fftSwitcher; - std::unique_ptr m_logger; - // io buffers: TODO n chan int m_internalBufferReadWriteIndex; std::vector> m_input; diff --git a/shared/SpectralAudioProcessorInteractor.cpp b/shared/SpectralAudioProcessorInteractor.cpp index 3d0650f..4b62058 100644 --- a/shared/SpectralAudioProcessorInteractor.cpp +++ b/shared/SpectralAudioProcessorInteractor.cpp @@ -2,7 +2,7 @@ #include "../../shared/PhaseVocoder.h" SpectralAudioProcessorInteractor::SpectralAudioProcessorInteractor(int numOverlaps) -: m_numOverlaps(numOverlaps), m_sampleRate(48000), m_fftHopSize(0), m_numChans(2) +: m_numOverlaps(numOverlaps), m_sampleRate(48000), m_fftHopSize(0), m_numChans(2), m_isPreparingToPlay(false), m_isPlaying(false), m_setOverlapsCallCount(0) { m_phaseBuffer = std::make_shared(m_numChans * m_numOverlaps, 1024); } @@ -14,14 +14,17 @@ int SpectralAudioProcessorInteractor::getFftSize() void SpectralAudioProcessorInteractor::prepareToPlay(int fftSize, int sampleRate, int channelCount) { + m_isPreparingToPlay = true; m_fftSize = fftSize; m_numChans = channelCount; m_sampleRate = sampleRate; setNumOverlaps(m_numOverlaps); + m_isPreparingToPlay = false; } void SpectralAudioProcessorInteractor::process(std::vector>* input, std::vector>* output) { + m_isPlaying = true; //need to take pointer to vector of vector of float jassert(input != nullptr); jassert(output != nullptr); @@ -29,13 +32,24 @@ void SpectralAudioProcessorInteractor::process(std::vector>* jassert(input->at(0).size() > 0); jassert(input->size() == output->size()); jassert(input->size() == m_spectralProcess.size()); - + for (int chan = 0; chan < input->size(); chan++) { + if (input->at(chan).size() == 0) + { + continue; + } + + if (m_spectralProcess.size() <= chan) + { + continue; + } + for (auto& spectralProcessOverlap : m_spectralProcess.at(chan)) { prepareProcess(spectralProcessOverlap.get()); spectralProcessOverlap->process(input->at(chan).data(), output->at(chan).data(), m_fftHopSize); } } + m_isPlaying = false; } void SpectralAudioProcessorInteractor::setFftSize(int fftSize) @@ -74,16 +88,60 @@ void SpectralAudioProcessorInteractor::setFftSize(int fftSize) } void SpectralAudioProcessorInteractor::setNumOverlaps(int newOverlapCount) { - if(newOverlapCount < 1 || newOverlapCount > 8 ){ return; } + int newOverlapCallCount = (m_setOverlapsCallCount + 1) % 100; + m_setOverlapsCallCount = newOverlapCallCount; + + if (newOverlapCount < 1 ||newOverlapCount > 8) { + return; + } + + if (m_spectralProcess.size() == m_numChans && m_numChans > 0) { + if (m_spectralProcess.at(0).size() == newOverlapCount) + { + if (m_numOverlaps != newOverlapCount) + { + m_numOverlaps = newOverlapCount; + } + + return; + } + } + if (m_setOverlapsCallCount != newOverlapCallCount) { /* race detected */ return;} + m_numOverlaps = newOverlapCount; m_fftHopSize = getFftSize() / m_numOverlaps; - m_spectralProcess.clear(); - + + for (auto& processesPerChannel : m_spectralProcess) { + if (!processesPerChannel.empty()) + { + if (m_setOverlapsCallCount != newOverlapCallCount) { /* race detected */ return;} + processesPerChannel.clear(); + } + } + + // TODO: sometimes this is called from the fft switcher thread and we being destroyed while creating new allocations + // If spectral process is empty then we may be destroyed and should not clear the buffer. Using a lock instead may prevent us needing to do these hacky checks + if (!m_spectralProcess.empty()) + { + if (m_setOverlapsCallCount != newOverlapCallCount) { /* race detected */ return;} + m_spectralProcess.clear(); + } + for (int chan = 0; chan < m_numChans; ++chan) { + if (m_setOverlapsCallCount != newOverlapCallCount) { /* race detected */ return;} m_spectralProcess.push_back(std::vector>()); for (int specProcess = 0; specProcess < m_numOverlaps; specProcess++) { + // TODO: sometimes this is called from the fft switcher thread and we being destroyed while creating new allocations + // If spectral process is empty then we should not continue here. Using a lock instead may prevent us needing to do these hacky checks + if (m_spectralProcess.empty()) + { + return; + } + + if (m_setOverlapsCallCount != newOverlapCallCount) { /* race detected */ return;} + m_spectralProcess.at(chan).push_back( createSpectralProcess(specProcess, m_fftSize, m_fftHopSize, m_sampleRate, m_numOverlaps, chan, m_numChans) ); @@ -94,6 +152,7 @@ void SpectralAudioProcessorInteractor::setNumOverlaps(int newOverlapCount) { void SpectralAudioProcessorInteractor::setWindowType(FftWindowType windowType) { for(auto& processes : m_spectralProcess) { for(auto& p : processes) { + if (isPreparingToPlay()) { return; } p->setWindowType(windowType); } } diff --git a/shared/SpectralAudioProcessorInteractor.h b/shared/SpectralAudioProcessorInteractor.h index a0b7bea..0cce2a4 100644 --- a/shared/SpectralAudioProcessorInteractor.h +++ b/shared/SpectralAudioProcessorInteractor.h @@ -32,6 +32,8 @@ class SpectralAudioProcessorInteractor { void usePvoc(bool usePvoc) { m_phaseBuffer->setUsePvoc(usePvoc); }; void setNumOverlaps(int newOverlapCount); void setWindowType(FftWindowType newWindowType); + bool isPreparingToPlay() const { return m_isPreparingToPlay; } + bool isPlaying() const { return m_isPlaying; } protected: std::vector< std::vector> > m_spectralProcess; @@ -42,6 +44,9 @@ class SpectralAudioProcessorInteractor { int m_fftHopSize; int m_numChans; int m_fftSize; + bool m_isPreparingToPlay; + bool m_isPlaying; + int m_setOverlapsCallCount; std::shared_ptr m_phaseBuffer; };