From bea7195c656a08c17fb2f6e23c32d094c7fd189a Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 7 Dec 2020 09:41:36 +0100 Subject: [PATCH] Add more thread-safety in VST --- vst/SfizzVstController.cpp | 96 +++++++++++++--------- vst/SfizzVstController.h | 19 +++-- vst/SfizzVstEditor.cpp | 161 +++++++++++++++++++------------------ vst/SfizzVstEditor.h | 18 +++-- 4 files changed, 166 insertions(+), 128 deletions(-) diff --git a/vst/SfizzVstController.cpp b/vst/SfizzVstController.cpp index 089cafd3..2e6854f1 100644 --- a/vst/SfizzVstController.cpp +++ b/vst/SfizzVstController.cpp @@ -140,16 +140,20 @@ IPlugView* PLUGIN_API SfizzVstController::createView(FIDString _name) return nullptr; if (_editor) { - _uiState = _editor->getCurrentUiState(); + withStateLock([this]() { + _uiState = _editor->getCurrentUiState(); + }); _editor.reset(); } SfizzVstEditor* editor = new SfizzVstEditor(this); _editor = Steinberg::owned(editor); - editor->updateState(_state); - editor->updateUiState(_uiState); - editor->updatePlayState(_playState); + withStateLock([this, editor]() { + editor->updateState(_state); + editor->updateUiState(_uiState); + editor->updatePlayState(_playState); + }); editor->remember(); return editor; @@ -204,53 +208,61 @@ tresult PLUGIN_API SfizzVstController::setParamNormalized(Vst::ParamID tag, Vst: } if (slotF32 && *slotF32 != value) { - *slotF32 = value; - if (SfizzVstEditor* editor = _editor) - editor->updateState(_state); + withStateLock([this, slotF32, value]() { + *slotF32 = value; + if (SfizzVstEditor* editor = _editor) + editor->updateState(_state); + }); } else if (slotI32 && *slotI32 != (int32)value) { - *slotI32 = (int32)value; - if (SfizzVstEditor* editor = _editor) - editor->updateState(_state); + withStateLock([this, slotI32, value]() { + *slotI32 = (int32)value; + if (SfizzVstEditor* editor = _editor) + editor->updateState(_state); + }); } return kResultTrue; } -tresult PLUGIN_API SfizzVstController::setState(IBStream* state) +tresult PLUGIN_API SfizzVstController::setState(IBStream* stream) { SfizzUiState s; - tresult r = s.load(state); + tresult r = s.load(stream); if (r != kResultTrue) return r; - _uiState = s; - - if (SfizzVstEditor* editor = _editor) - editor->updateUiState(_uiState); + withStateLock([this, &s]() { + _uiState = s; + if (SfizzVstEditor* editor = _editor) + editor->updateUiState(_uiState); + }); return kResultTrue; } -tresult PLUGIN_API SfizzVstController::getState(IBStream* state) +tresult PLUGIN_API SfizzVstController::getState(IBStream* stream) { - if (_editor) - _uiState = _editor->getCurrentUiState(); + tresult result; - return _uiState.store(state); + withStateLock([this, stream, &result]() { + if (_editor) + _uiState = _editor->getCurrentUiState(); + result = _uiState.store(stream); + }); + + return result; } -tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* state) +tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* stream) { SfizzVstState s; - tresult r = s.load(state); + tresult r = s.load(stream); if (r != kResultTrue) return r; - _state = s; - setParamNormalized(kPidVolume, kParamVolumeRange.normalize(s.volume)); setParamNormalized(kPidNumVoices, kParamNumVoicesRange.normalize(s.numVoices)); setParamNormalized(kPidOversampling, kParamOversamplingRange.normalize(s.oversamplingLog2)); @@ -259,14 +271,19 @@ tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* state) setParamNormalized(kPidTuningFrequency, kParamTuningFrequencyRange.normalize(s.tuningFrequency)); setParamNormalized(kPidStretchedTuning, kParamStretchedTuningRange.normalize(s.stretchedTuning)); - if (SfizzVstEditor* editor = _editor) - editor->updateState(_state); + withStateLock([this, &s]() { + _state = s; + if (SfizzVstEditor* editor = _editor) + editor->updateState(_state); + }); return kResultTrue; } tresult SfizzVstController::notify(Vst::IMessage* message) { + // Note: may be called from any thread (Reaper) + tresult result = SfizzVstControllerNoUi::notify(message); if (result != kResultFalse) return result; @@ -282,10 +299,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message) if (result != kResultTrue) return result; - _state.sfzFile.assign(static_cast(data), size); - - if (SfizzVstEditor* editor = _editor) - editor->updateState(_state); + withStateLock([this, data, size]() { + _state.sfzFile.assign(static_cast(data), size); + if (SfizzVstEditor* editor = _editor) + editor->updateState(_state); + }); } else if (!strcmp(id, "LoadedScala")) { const void* data = nullptr; @@ -295,10 +313,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message) if (result != kResultTrue) return result; - _state.scalaFile.assign(static_cast(data), size); - - if (SfizzVstEditor* editor = _editor) - editor->updateState(_state); + withStateLock([this, data, size]() { + _state.scalaFile.assign(static_cast(data), size); + if (SfizzVstEditor* editor = _editor) + editor->updateState(_state); + }); } else if (!strcmp(id, "NotifiedPlayState")) { const void* data = nullptr; @@ -308,10 +327,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message) if (result != kResultTrue) return result; - _playState = *static_cast(data); - - if (SfizzVstEditor* editor = _editor) - editor->updatePlayState(_playState); + withStateLock([this, data]() { + _playState = *static_cast(data); + if (SfizzVstEditor* editor = _editor) + editor->updatePlayState(_playState); + }); } else if (!strcmp(id, "ReceivedMessage")) { const void* data = nullptr; diff --git a/vst/SfizzVstController.h b/vst/SfizzVstController.h index 69670a55..9e5ba8d4 100644 --- a/vst/SfizzVstController.h +++ b/vst/SfizzVstController.h @@ -10,6 +10,7 @@ #include "public.sdk/source/vst/vstparameters.h" #include "vstgui/plugin-bindings/vst3editor.h" #include +#include class SfizzVstState; class SfizzVstEditor; @@ -43,9 +44,9 @@ public: IPlugView* PLUGIN_API createView(FIDString name) override; tresult PLUGIN_API setParamNormalized(Vst::ParamID tag, Vst::ParamValue value) override; - tresult PLUGIN_API setState(IBStream* state) override; - tresult PLUGIN_API getState(IBStream* state) override; - tresult PLUGIN_API setComponentState(IBStream* state) override; + tresult PLUGIN_API setState(IBStream* stream) override; + tresult PLUGIN_API getState(IBStream* stream) override; + tresult PLUGIN_API setComponentState(IBStream* stream) override; tresult PLUGIN_API notify(Vst::IMessage* message) override; /// @@ -54,8 +55,16 @@ public: static FUID cid; private: - SfizzVstState _state; - SfizzUiState _uiState; + template void withStateLock(F&& fn) const + { + std::lock_guard lock(_stateMutex); + fn(); + } + +private: + mutable std::mutex _stateMutex; // for R/W the state data + SfizzVstState _state {}; + SfizzUiState _uiState {}; // updated on UI open/close/state-request SfizzPlayState _playState {}; Steinberg::IPtr _editor; }; diff --git a/vst/SfizzVstEditor.cpp b/vst/SfizzVstEditor.cpp index 21a0f862..c04b5d1d 100644 --- a/vst/SfizzVstEditor.cpp +++ b/vst/SfizzVstEditor.cpp @@ -25,7 +25,6 @@ SfizzVstEditor::SfizzVstEditor(SfizzVstController* controller) : VSTGUIEditor(controller, &sfizzUiViewRect), oscTemp_(new uint8_t[kOscTempSize]) { - oscQueue_.reserve(kOscQueueSize); } SfizzVstEditor::~SfizzVstEditor() @@ -56,10 +55,14 @@ bool PLUGIN_API SfizzVstEditor::open(void* parent, const VSTGUI::PlatformType& p editor_.reset(editor); } - mustRedisplayState_ = true; - mustRedisplayUiState_ = true; - mustRedisplayPlayState_ = true; - flushOscQueue(); + withStateLock([this]() { + mustRedisplayState_ = true; + mustRedisplayUiState_ = true; + mustRedisplayPlayState_ = true; + OscByteVec* queue = new OscByteVec; + oscQueue_.reset(queue); + queue->reserve(kOscQueueSize); + }); updateStateDisplay(); @@ -86,7 +89,9 @@ void PLUGIN_API SfizzVstEditor::close() this->frame = nullptr; } - flushOscQueue(); + withStateLock([this]() { + oscQueue_.reset(); + }); } /// @@ -121,71 +126,73 @@ CMessageResult SfizzVstEditor::notify(CBaseObject* sender, const char* message) void SfizzVstEditor::updateState(const SfizzVstState& state) { - std::lock_guard lock(stateMutex_); - state_ = state; - mustRedisplayState_ = true; + withStateLock([this, &state]() { + state_ = state; + mustRedisplayState_ = true; + }); } void SfizzVstEditor::updateUiState(const SfizzUiState& uiState) { - std::lock_guard lock(stateMutex_); - uiState_ = uiState; - mustRedisplayUiState_ = true; + withStateLock([this, &uiState]() { + uiState_ = uiState; + mustRedisplayUiState_ = true; + }); } void SfizzVstEditor::updatePlayState(const SfizzPlayState& playState) { - std::lock_guard lock(stateMutex_); - playState_ = playState; - mustRedisplayPlayState_ = true; + withStateLock([this, &playState]() { + playState_ = playState; + mustRedisplayPlayState_ = true; + }); } SfizzUiState SfizzVstEditor::getCurrentUiState() const { - std::lock_guard lock(stateMutex_); - return uiState_; + SfizzUiState uiState; + withStateLock([this, &uiState]() { + uiState = uiState_; + }); + return uiState; } void SfizzVstEditor::receiveMessage(const void* data, uint32_t size) { - if (!frame) { - // only accumulate if message processing is active - return; - } + // Note: may be called from non-UI thread (Reaper) - std::lock_guard lock(stateMutex_); - std::copy( - reinterpret_cast(data), - reinterpret_cast(data) + size, - std::back_inserter(oscQueue_)); + withStateLock([this, data, size]() { + if (OscByteVec* queue = oscQueue_.get()) { + const uint8_t* bytes = reinterpret_cast(data); + std::copy(bytes, bytes + size, std::back_inserter(*queue)); + } + }); } void SfizzVstEditor::processOscQueue() { - std::lock_guard lock(stateMutex_); + withStateLock([this]() { + OscByteVec* queue = oscQueue_.get(); + if (!queue) + return; - const uint8_t* oscData = oscQueue_.data(); - size_t oscSize = oscQueue_.size(); + const uint8_t* oscData = queue->data(); + size_t oscSize = queue->size(); - const char* path; - const char* sig; - const sfizz_arg_t* args; - uint8_t buffer[1024]; + const char* path; + const char* sig; + const sfizz_arg_t* args; + uint8_t buffer[1024]; - uint32_t msgSize; - while ((msgSize = sfizz_extract_message(oscData, oscSize, buffer, sizeof(buffer), &path, &sig, &args)) > 0) { - uiReceiveMessage(path, sig, args); - oscData += msgSize; - oscSize -= msgSize; - } + uint32_t msgSize; + while ((msgSize = sfizz_extract_message(oscData, oscSize, buffer, sizeof(buffer), &path, &sig, &args)) > 0) { + uiReceiveMessage(path, sig, args); + oscData += msgSize; + oscSize -= msgSize; + } - oscQueue_.clear(); -} - -void SfizzVstEditor::flushOscQueue() -{ - std::lock_guard lock(stateMutex_); - oscQueue_.clear(); + queue->clear(); + }); } /// @@ -333,41 +340,37 @@ void SfizzVstEditor::updateStateDisplay() if (!frame) return; - if (!(mustRedisplayState_ || mustRedisplayUiState_ || mustRedisplayPlayState_)) - return; + withStateLock([this]() { + if (mustRedisplayState_) { + uiReceiveValue(EditId::SfzFile, state_.sfzFile); + uiReceiveValue(EditId::Volume, state_.volume); + uiReceiveValue(EditId::Polyphony, state_.numVoices); + uiReceiveValue(EditId::Oversampling, 1u << state_.oversamplingLog2); + uiReceiveValue(EditId::PreloadSize, state_.preloadSize); + uiReceiveValue(EditId::ScalaFile, state_.scalaFile); + uiReceiveValue(EditId::ScalaRootKey, state_.scalaRootKey); + uiReceiveValue(EditId::TuningFrequency, state_.tuningFrequency); + uiReceiveValue(EditId::StretchTuning, state_.stretchedTuning); + mustRedisplayState_ = false; + } - std::lock_guard lock(stateMutex_); + /// + if (mustRedisplayUiState_) { + uiReceiveValue(EditId::UIActivePanel, uiState_.activePanel); + mustRedisplayUiState_ = false; + } - /// - if (mustRedisplayState_) { - uiReceiveValue(EditId::SfzFile, state_.sfzFile); - uiReceiveValue(EditId::Volume, state_.volume); - uiReceiveValue(EditId::Polyphony, state_.numVoices); - uiReceiveValue(EditId::Oversampling, 1u << state_.oversamplingLog2); - uiReceiveValue(EditId::PreloadSize, state_.preloadSize); - uiReceiveValue(EditId::ScalaFile, state_.scalaFile); - uiReceiveValue(EditId::ScalaRootKey, state_.scalaRootKey); - uiReceiveValue(EditId::TuningFrequency, state_.tuningFrequency); - uiReceiveValue(EditId::StretchTuning, state_.stretchedTuning); - mustRedisplayState_ = false; - } - - /// - if (mustRedisplayUiState_) { - uiReceiveValue(EditId::UIActivePanel, uiState_.activePanel); - mustRedisplayUiState_ = false; - } - - /// - if (mustRedisplayPlayState_) { - uiReceiveValue(EditId::UINumCurves, playState_.curves); - uiReceiveValue(EditId::UINumMasters, playState_.masters); - uiReceiveValue(EditId::UINumGroups, playState_.groups); - uiReceiveValue(EditId::UINumRegions, playState_.regions); - uiReceiveValue(EditId::UINumPreloadedSamples, playState_.preloadedSamples); - uiReceiveValue(EditId::UINumActiveVoices, playState_.activeVoices); - mustRedisplayPlayState_ = false; - } + /// + if (mustRedisplayPlayState_) { + uiReceiveValue(EditId::UINumCurves, playState_.curves); + uiReceiveValue(EditId::UINumMasters, playState_.masters); + uiReceiveValue(EditId::UINumGroups, playState_.groups); + uiReceiveValue(EditId::UINumRegions, playState_.regions); + uiReceiveValue(EditId::UINumPreloadedSamples, playState_.preloadedSamples); + uiReceiveValue(EditId::UINumActiveVoices, playState_.activeVoices); + mustRedisplayPlayState_ = false; + } + }); } Vst::ParamID SfizzVstEditor::parameterOfEditId(EditId id) diff --git a/vst/SfizzVstEditor.h b/vst/SfizzVstEditor.h index e6b48478..e5fa0d8d 100644 --- a/vst/SfizzVstEditor.h +++ b/vst/SfizzVstEditor.h @@ -43,7 +43,12 @@ public: private: void processOscQueue(); - void flushOscQueue(); + + template void withStateLock(F&& fn) const + { + std::lock_guard lock(stateMutex_); + fn(); + } protected: // EditorController @@ -72,12 +77,13 @@ private: // editor state // note: might be updated from a non-UI thread - mutable std::recursive_mutex stateMutex_; - SfizzVstState state_; - SfizzUiState uiState_; - SfizzPlayState playState_; + mutable std::recursive_mutex stateMutex_; // for R/W the state data, and OSC queue + SfizzVstState state_ {}; + SfizzUiState uiState_ {}; + SfizzPlayState playState_ {}; volatile bool mustRedisplayState_ = false; volatile bool mustRedisplayUiState_ = false; volatile bool mustRedisplayPlayState_ = false; - std::vector oscQueue_; + typedef std::vector OscByteVec; + std::unique_ptr oscQueue_; };