From 361f839546b0841860fe42e00a4383f76c005314 Mon Sep 17 00:00:00 2001 From: reuk Date: Tue, 23 Mar 2021 16:19:13 +0000 Subject: [PATCH] VST3 Client: Avoid calling performEdit from the audio thread Previously, all VST3 parameter changes were signalled to the host via performEdit, regardless of the originating thread. This breaks the contract of the IEditController interface, which requires that all calls are made from the UI thread. This change modifies the VST3 wrapper so that it will only call performEdit when a parameter is set on the message thread. If a parameter is set on the audio thread instead, then the parameter change will be signalled to the host using `ProcessData::outputParameterChanges`. If your VST3 plugin uses a background thread to update parameters for some reason, you'll encounter data races. Please don't do that. --- .../VST3/juce_VST3_Wrapper.cpp | 151 +++++++++++++----- 1 file changed, 110 insertions(+), 41 deletions(-) diff --git a/modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp b/modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp index 6a5c0f73a1..528d2e8671 100644 --- a/modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp +++ b/modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp @@ -284,13 +284,13 @@ private: class JuceAudioProcessor : public Vst::IUnitInfo { public: - JuceAudioProcessor (AudioProcessor* source) noexcept - : audioProcessor (source) + explicit JuceAudioProcessor (AudioProcessor* source) noexcept + : audioProcessor (source) { setupParameters(); } - virtual ~JuceAudioProcessor() {} + virtual ~JuceAudioProcessor() = default; AudioProcessor* get() const noexcept { return audioProcessor.get(); } @@ -432,14 +432,29 @@ public: return unitID; } - int getNumParameters() const noexcept { return vstParamIDs.size(); } + const Array& getParamIDs() const noexcept { return vstParamIDs; } + Vst::ParamID getBypassParamID() const noexcept { return bypassParamID; } + Vst::ParamID getProgramParamID() const noexcept { return programParamID; } + bool isBypassRegularParameter() const noexcept { return bypassIsRegularParameter; } + + void setParameterValue (size_t paramIndex, float value) + { + cachedParamValues.set (paramIndex, value); + } + + template + void forAllChangedParameters (Callback&& callback) + { + cachedParamValues.ifSet ([&] (size_t index, float value) + { + callback (cachedParamValues.getParamID (index), value); + }); + } + bool isUsingManagedParameters() const noexcept { return juceParameters.isUsingManagedParameters(); } //============================================================================== static const FUID iid; - Array vstParamIDs; - Vst::ParamID bypassParamID = 0, programParamID = static_cast (paramPreset); - bool bypassIsRegularParameter = false; private: //============================================================================== @@ -537,6 +552,8 @@ private: vstParamIDs.add (programParamID); paramMap.set (static_cast (programParamID), ownedProgramParameter.get()); } + + cachedParamValues = CachedParamValues { { vstParamIDs.begin(), vstParamIDs.end() } }; } Vst::ParamID generateVSTParamIDForParam (AudioProcessorParameter* param) @@ -557,6 +574,12 @@ private: #endif } + //============================================================================== + Array vstParamIDs; + CachedParamValues cachedParamValues; + Vst::ParamID bypassParamID = 0, programParamID = static_cast (paramPreset); + bool bypassIsRegularParameter = false; + //============================================================================== std::atomic refCount { 0 }; std::unique_ptr audioProcessor; @@ -891,12 +914,12 @@ public: if (auto* pluginInstance = getPluginInstance()) { - for (auto vstParamId : audioProcessor->vstParamIDs) + for (auto vstParamId : audioProcessor->getParamIDs()) { auto paramValue = [&] { - if (vstParamId == audioProcessor->programParamID) - return EditController::plainParamToNormalized (audioProcessor->programParamID, + if (vstParamId == audioProcessor->getProgramParamID()) + return EditController::plainParamToNormalized (audioProcessor->getProgramParamID(), pluginInstance->getCurrentProgram()); return (double) audioProcessor->getParamForVSTParamID (vstParamId)->getValue(); @@ -1124,23 +1147,49 @@ public: } //============================================================================== - void paramChanged (Vst::ParamID vstParamId, double newValue) + void beginGesture (Vst::ParamID vstParamId) + { + if (MessageManager::getInstance()->isThisTheMessageThread()) + beginEdit (vstParamId); + } + + void endGesture (Vst::ParamID vstParamId) + { + if (MessageManager::getInstance()->isThisTheMessageThread()) + endEdit (vstParamId); + } + + void paramChanged (int parameterIndex, Vst::ParamID vstParamId, double newValue) { if (inParameterChangedCallback.get()) return; - // NB: Cubase has problems if performEdit is called without setParamNormalized - EditController::setParamNormalized (vstParamId, newValue); - performEdit (vstParamId, newValue); + if (MessageManager::getInstance()->isThisTheMessageThread()) + { + // NB: Cubase has problems if performEdit is called without setParamNormalized + EditController::setParamNormalized (vstParamId, newValue); + performEdit (vstParamId, newValue); + } + else + { + audioProcessor->setParameterValue ((size_t) parameterIndex, (float) newValue); + } } //============================================================================== - void audioProcessorParameterChangeGestureBegin (AudioProcessor*, int index) override { beginEdit (audioProcessor->getVSTParamIDForIndex (index)); } - void audioProcessorParameterChangeGestureEnd (AudioProcessor*, int index) override { endEdit (audioProcessor->getVSTParamIDForIndex (index)); } + void audioProcessorParameterChangeGestureBegin (AudioProcessor*, int index) override + { + beginGesture (audioProcessor->getVSTParamIDForIndex (index)); + } + + void audioProcessorParameterChangeGestureEnd (AudioProcessor*, int index) override + { + endGesture (audioProcessor->getVSTParamIDForIndex (index)); + } void audioProcessorParameterChanged (AudioProcessor*, int index, float newValue) override { - paramChanged (audioProcessor->getVSTParamIDForIndex (index), newValue); + paramChanged (index, audioProcessor->getVSTParamIDForIndex (index), newValue); } void audioProcessorChanged (AudioProcessor*, const ChangeDetails& details) override @@ -1157,20 +1206,26 @@ public: if (auto* pluginInstance = getPluginInstance()) { - if (details.programChanged && audioProcessor->getProgramParameter() != nullptr) + if (details.programChanged) { - auto currentProgram = pluginInstance->getCurrentProgram(); - auto paramValue = roundToInt (EditController::normalizedParamToPlain (audioProcessor->programParamID, - EditController::getParamNormalized (audioProcessor->programParamID))); - - if (currentProgram != paramValue) + if (auto* programParameter = audioProcessor->getProgramParameter()) { - beginEdit (audioProcessor->programParamID); - paramChanged (audioProcessor->programParamID, - EditController::plainParamToNormalized (audioProcessor->programParamID, currentProgram)); - endEdit (audioProcessor->programParamID); + const auto programParameterIndex = programParameter->getParameterIndex(); + const auto programParameterId = audioProcessor->getProgramParamID(); + const auto currentProgram = pluginInstance->getCurrentProgram(); + const auto paramValue = roundToInt (EditController::normalizedParamToPlain (programParameterId, + EditController::getParamNormalized (programParameterId))); - flags |= Vst::kParamValuesChanged; + if (currentProgram != paramValue) + { + beginGesture (programParameterId); + paramChanged (programParameterIndex, + programParameterId, + EditController::plainParamToNormalized (programParameterId, currentProgram)); + endGesture (programParameterId); + + flags |= Vst::kParamValuesChanged; + } } } @@ -1200,6 +1255,7 @@ private: friend class JuceVST3Component; friend struct Param; + //============================================================================== class ComponentRestarter : private AsyncUpdater { public: @@ -1218,7 +1274,7 @@ private: flags = newFlags; - if (Thread::getCurrentThreadId() == messageThreadId) + if (MessageManager::getInstance()->isThisTheMessageThread()) handleAsyncUpdate(); else triggerAsyncUpdate(); @@ -1231,7 +1287,6 @@ private: handler->restartComponent (flags); } - const Thread::ThreadID messageThreadId = MessageManager::getInstance()->getCurrentMessageThread(); JuceVST3EditController& controller; int32 flags = 0; }; @@ -1263,17 +1318,17 @@ private: juceParameter.addListener (this); } - void parameterValueChanged (int, float newValue) override + void parameterValueChanged (int index, float newValue) override { - owner.paramChanged (vstParamID, newValue); + owner.paramChanged (index, vstParamID, newValue); } void parameterGestureChanged (int, bool gestureIsStarting) override { if (gestureIsStarting) - owner.beginEdit (vstParamID); + owner.beginGesture (vstParamID); else - owner.endEdit (vstParamID); + owner.endGesture (vstParamID); } JuceVST3EditController& owner; @@ -1299,20 +1354,20 @@ private: pluginInstance->addListener (this); // as the bypass is not part of the regular parameters we need to listen for it explicitly - if (! audioProcessor->bypassIsRegularParameter) + if (! audioProcessor->isBypassRegularParameter()) ownedParameterListeners.push_back (std::make_unique (*this, *audioProcessor->getBypassParameter(), - audioProcessor->bypassParamID)); + audioProcessor->getBypassParamID())); if (parameters.getParameterCount() <= 0) { - auto n = audioProcessor->getNumParameters(); + auto n = audioProcessor->getParamIDs().size(); for (int i = 0; i < n; ++i) { auto vstParamID = audioProcessor->getVSTParamIDForIndex (i); - if (vstParamID == audioProcessor->programParamID) + if (vstParamID == audioProcessor->getProgramParamID()) continue; auto* juceParam = audioProcessor->getParamForVSTParamID (vstParamID); @@ -1320,16 +1375,16 @@ private: auto unitID = JuceAudioProcessor::getUnitID (parameterGroup); parameters.addParameter (new Param (*this, *juceParam, vstParamID, unitID, - (vstParamID == audioProcessor->bypassParamID))); + (vstParamID == audioProcessor->getBypassParamID()))); } if (auto* programParam = audioProcessor->getProgramParameter()) { ownedParameterListeners.push_back (std::make_unique (*this, *programParam, - audioProcessor->programParamID)); + audioProcessor->getProgramParamID())); - parameters.addParameter (new ProgramChangeParameter (*pluginInstance, audioProcessor->programParamID)); + parameters.addParameter (new ProgramChangeParameter (*pluginInstance, audioProcessor->getProgramParamID())); } } @@ -3133,6 +3188,20 @@ private: jassert (midiBuffer.getNumEvents() <= numMidiEventsComingIn); #endif } + + if (auto* changes = data.outputParameterChanges) + { + comPluginInstance->forAllChangedParameters ([&] (Vst::ParamID paramID, float value) + { + Steinberg::int32 queueIndex = 0; + + if (auto* queue = changes->addParameterData (paramID, queueIndex)) + { + Steinberg::int32 pointIndex = 0; + queue->addPoint (0, value, pointIndex); + } + }); + } } //==============================================================================