diff --git a/src/detail/os/linux.cpp b/src/detail/os/linux.cpp index 6b26ced9..a056e6ea 100644 --- a/src/detail/os/linux.cpp +++ b/src/detail/os/linux.cpp @@ -230,6 +230,8 @@ void LinuxHelper::run() if (!anyoneWantsTicking()) { LOGDETAIL("clap-wrapper: every attached object has a run loop, pausing the idle thread"); + // Unbounded on purpose: every change that can turn the predicate back to + // "tick me" reaches this thread under _standInLock, so none can be lost. _standInWakeup.wait(guard, [this] { return !_standInRunning || anyoneWantsTicking(); }); continue; } @@ -270,6 +272,10 @@ void LinuxHelper::detach(IPlugObject *plugobject) void LinuxHelper::idleSourceChanged() { + // The lock is not optional: run() tests its predicate and parks in one step, + // so an unlocked notify can be lost, leaving the helper parked forever. Safe + // to block on: lock order is helper-then-plug-object, onIdle() only try_locks. + std::lock_guard guard(_standInLock); _standInWakeup.notify_all(); } diff --git a/src/detail/vst3/parameter.cpp b/src/detail/vst3/parameter.cpp index e21139e0..56f0ccae 100644 --- a/src/detail/vst3/parameter.cpp +++ b/src/detail/vst3/parameter.cpp @@ -198,12 +198,27 @@ Vst3Parameter *Vst3Parameter::createPresetSelector(Steinberg::Vst::ParamID id, i // load rebuilds a plugin's state, which is not something to draw a curve // through. v.flags = Vst::ParameterInfo::kIsProgramChange | Vst::ParameterInfo::kIsList; - v.stepCount = presetCount > 1 ? presetCount - 1 : 0; + v.stepCount = 0; auto *p = new Vst3Parameter(v, 0, 0, 0); p->isMidi = false; p->isPreset = true; p->min_value = 0; - p->max_value = v.stepCount; + p->max_value = 0; + p->resizePresetSelector(presetCount); return p; } + +void Vst3Parameter::resizePresetSelector(int32_t presetCount) +{ + auto &v = getInfo(); + // A host reads stepCount+1 as the program count, so a single preset and an + // empty list are both stepCount 0 - kIsHidden is what tells them apart. + v.stepCount = presetCount > 1 ? presetCount - 1 : 0; + if (presetCount > 0) + v.flags &= ~Vst::ParameterInfo::kIsHidden; + else + v.flags |= Vst::ParameterInfo::kIsHidden; + min_value = 0; + max_value = v.stepCount; +} diff --git a/src/detail/vst3/parameter.h b/src/detail/vst3/parameter.h index c55f14c5..944deb1d 100644 --- a/src/detail/vst3/parameter.h +++ b/src/detail/vst3/parameter.h @@ -72,7 +72,11 @@ class Vst3Parameter : public Steinberg::Vst::Parameter { return floor(clapvalue - min_value) / float(info.stepCount); } - return (clapvalue - min_value) / (max_value - min_value); + // min == max - an empty preset selector, or a CLAP parameter that declares + // it - has exactly one plain value, and dividing would hand the host a NaN. + const auto range = max_value - min_value; + if (range <= 0.0) return 0.0; + return (clapvalue - min_value) / range; } static Vst3Parameter *create(const clap_param_info_t *info, std::function getUnitId); @@ -83,7 +87,21 @@ class Vst3Parameter : public Steinberg::Vst::Parameter // separate kind because the process adapter turns every isMidi program // change into an actual 0xC0 message, which is emphatically not what // selecting a preset should do. + // + // Created even for presetCount 0 (hidden, stepCount 0): the parameter count + // must not change while the component is active - see resizePresetSelector(). static Vst3Parameter *createPresetSelector(Steinberg::Vst::ParamID id, int32_t presetCount); + // Resizes an existing selector in place. The caller must exclude process() + // while the fields are written, and afterwards announce kParamTitlesChanged. + void resizePresetSelector(int32_t presetCount); + // Programs published to the host: 0 while hidden, stepCount+1 otherwise. The + // live index may be larger while a crawl is still running. + int32_t presetCount() const + { + auto &info = this->getInfo(); + if (!isPreset || (info.flags & Steinberg::Vst::ParameterInfo::kIsHidden)) return 0; + return info.stepCount + 1; + } // copies from the clap_param_info_t uint32_t param_index_for_clap_get_info = 0; clap_id id = 0; diff --git a/src/wrapasauv2.cpp b/src/wrapasauv2.cpp index 63f3b8a2..0c715ebc 100644 --- a/src/wrapasauv2.cpp +++ b/src/wrapasauv2.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -1641,6 +1642,27 @@ OSStatus WrapAsAUV2::Render(AudioUnitRenderActionFlags &inFlags, const AudioTime // {} // ); } + else + { + // Nothing rendered this cycle (the CLAP is down while onIdle() cycles it + // for a restart), and AUBase will not silence anything for us: the AU stays + // initialized throughout, so DoRenderBus copies the output element's cache + // into the host's buffer after every noErr Render and returning without + // writing replays the last block. Zero every output element, not just the + // bus being rendered - RenderBus answers the others from their caches - and + // the silence flag on top is only a hint. _renderedSinceIdle is left alone: + // the idle tick's flush still has to make up for this block. + const auto numOutputs = Outputs().GetNumberOfElements(); + for (UInt32 i = 0; i < numOutputs; ++i) + { + AudioBufferList &buffers = Output(i).PrepareBuffer(inFrames); + for (UInt32 j = 0; j < buffers.mNumberBuffers; ++j) + { + std::memset(buffers.mBuffers[j].mData, 0, buffers.mBuffers[j].mDataByteSize); + } + } + inFlags |= kAudioUnitRenderAction_OutputIsSilence; + } return noErr; } @@ -1814,8 +1836,8 @@ void WrapAsAUV2::onIdle() // rebuild. Render holds it for its whole body, so once it is acquired no // render is inside the process adapter; clearing _initialized under it // keeps the ones that follow out while the plugin is torn down and stood - // back up. Those renders return without touching the buffers, exactly as - // they do before the AU is initialized. + // back up. The AU stays initialized throughout, so those renders still + // reach Render(), which zeroes the output (see the else branch there). bool wasInitialized; { ClapWrapper::detail::shared::SpinLockGuard processGuard(_processLock); @@ -1827,8 +1849,8 @@ void WrapAsAUV2::onIdle() deactivateCLAP(); // Cannot fail for the format-pair reason Initialize guards against: // the formats have not changed since the last successful activation. - // If it fails anyway, _initialized stays false and renders return - // silence, the same state as before Initialize. + // If it fails anyway, _initialized stays false and renders are silent + // until request_process() below or the host's next Initialize recovers. if (!activateCLAP()) { LOGINFO("[clap-wrapper] restart: could not reactivate the plugin"); @@ -1848,9 +1870,8 @@ void WrapAsAUV2::onIdle() // activate/start_processing pair, which it drives from AU Initialize() -- // so if the AU is initialized and the CLAP is not running underneath it, // stand it back up. No lock is needed to decide that: activateCLAP() - // publishes _initialized last, and a render that reads it false returns - // without touching the plugin, exactly as it does before the AU is - // initialized at all. + // publishes _initialized last, and a render that reads it false outputs + // silence without touching the plugin. if (IsInitialized() && !_initialized) { activateCLAP(); diff --git a/src/wrapasvst3.cpp b/src/wrapasvst3.cpp index 0baf2881..d71f9f73 100644 --- a/src/wrapasvst3.cpp +++ b/src/wrapasvst3.cpp @@ -325,15 +325,10 @@ tresult PLUGIN_API ClapAsVst3::setState(IBStream *state) // then the better authority rather than the worse one. if (result == kResultOk) { - // Drop a request the audio thread queued from a block that ran before the - // state did. It cannot close the window on its own - process() does not - // take _mainThreadLock, so a request can still be stored after this - but - // the adopt below is what actually decides, and onIdle() re-tests against - // _presetIndexInEffect before it loads anything. - _presetLoadRequest.store(-1); - - // Whatever the host sends next is where its selector stands, not a request - // to go there. \see onRequestPresetLoad(). + // One store drops a request the audio thread queued before the state and + // arms the adopt. It has to be one store: process() takes no lock, so a + // request published between a separate drop and arm would survive both and + // be loaded over the state just restored. \see onRequestPresetLoad(). // // Reading the selector parameter here instead does not work, and it is // worth writing down why, because it looks like it should: at this point @@ -345,7 +340,7 @@ tresult PLUGIN_API ClapAsVst3::setState(IBStream *state) // therefore seeds 0 whatever the project said, which both misses every // preset except index 0 and makes index 0 itself unreachable for the life // of the instance. - _adoptNextPresetValue.store(true, std::memory_order_relaxed); + _presetLoadRequest.store(kAdoptNextPresetValue); } return result; @@ -1345,9 +1340,8 @@ void ClapAsVst3::setupPresets() _presetParamId = Vst::kNoParamId; _presetUnitId = Vst::kRootUnitId; - // setupParameters() can run more than once (param_rescan, and the rebuild in - // onIdle below), and the index is shared and long-lived - so drop any - // listener from a previous pass rather than stacking another onto it. + // setupParameters() can run more than once, and the index is shared and + // long-lived - drop any listener from a previous pass rather than stack one. if (_presetIndex && _presetIndexToken) { _presetIndex->removeCompletionListener(_presetIndexToken); @@ -1371,15 +1365,11 @@ void ClapAsVst3::setupPresets() Vst::ParamID id = 0xc00000; while (parameters.getParameter(id)) ++id; - // The list has to be sized now, because a parameter's stepCount is fixed at - // creation and a host reads stepCount+1 as the program count (the SDK's own - // preset sample sets kNumPrograms-1). So wait briefly for the crawl rather - // than announce a size that is wrong: an embedded container resolves in - // milliseconds, and a folder crawl that outlasts the wait is still covered - // by the rescan onIdle() asks for when it completes. - _presetIndex->waitUntilComplete(1000); - const auto presetCount = _presetIndex->size(); - if (presetCount == 0) return; // nothing to show; the rescan will come back + // Created whatever the crawl has found, and never waited for: the parameter + // count must not change once the component is active (the process adapter + // holds a raw pointer into the container), so onIdle() grows it in place. + // Only a completed crawl is published; a mid-crawl size is a fragment. + const auto presetCount = _presetIndex->isComplete() ? _presetIndex->size() : 0; auto *selector = Vst3Parameter::createPresetSelector(id, (int32_t)presetCount); @@ -1406,6 +1396,12 @@ void ClapAsVst3::setupPresets() _presetIndexToken = _presetIndex->addCompletionListener([this]() { onPresetIndexComplete(); }); } +Vst3Parameter *ClapAsVst3::presetSelector() const +{ + if (_presetParamId == Vst::kNoParamId) return nullptr; + return static_cast(parameters.getParameter(_presetParamId)); +} + void ClapAsVst3::onPresetIndexComplete() { // Called from the index's crawl thread. Nothing that talks to the host may @@ -1435,18 +1431,28 @@ void ClapAsVst3::onRequestPresetLoad(size_t presetIndex) // A second pick loads. That is the lesser of the two - the alternative // reloads the saved preset over every restored project, on every host that // streams, every time. - if (_adoptNextPresetValue.exchange(false, std::memory_order_relaxed)) + // + // Decide and publish in one compare-exchange: setState() can arm at any + // moment here, and a failed exchange re-decides against the armed value. + const auto requested = static_cast(presetIndex); + auto current = _presetLoadRequest.load(); + for (;;) { - _presetIndexInEffect.store(static_cast(presetIndex), std::memory_order_relaxed); - return; - } + if (current == kAdoptNextPresetValue) + { + if (!_presetLoadRequest.compare_exchange_weak(current, kNoPresetRequest)) continue; + _presetIndexInEffect.store(requested, std::memory_order_relaxed); + return; + } - if (static_cast(presetIndex) == _presetIndexInEffect.load(std::memory_order_relaxed)) - { - return; - } + if (requested == _presetIndexInEffect.load(std::memory_order_relaxed)) + { + return; + } - _presetLoadRequest.store(static_cast(presetIndex)); + // `current` is kNoPresetRequest or an earlier request this one coalesces. + if (_presetLoadRequest.compare_exchange_weak(current, requested)) return; + } } void ClapAsVst3::preset_loaded(uint32_t locationKind, const char *location, const char *loadKey) @@ -1461,13 +1467,24 @@ void ClapAsVst3::preset_loaded(uint32_t locationKind, const char *location, cons size_t index = 0; if (!_presetIndex->indexOf(locationKind, location, loadKey, index)) return; - auto *param = (Vst3Parameter *)parameters.getParameter(_presetParamId); + auto *param = presetSelector(); if (!param) return; _presetIndexInEffect.store(static_cast(index), std::memory_order_relaxed); - const auto normalized = param->asVst3Value(static_cast(index)); - if (param->getNormalized() == normalized) + // The index is from the live list, which can be ahead of what the selector + // publishes; an index past stepCount normalizes above 1.0. onIdle() moves the + // selector to _presetIndexInEffect once the list has grown to include it. + if (index >= static_cast(param->presetCount())) return; + + moveSelectorTo(*param, index); +} + +void ClapAsVst3::moveSelectorTo(Vst3Parameter ¶m, size_t index) +{ + // [main-thread] index must be within what the selector publishes. + const auto normalized = param.asVst3Value(static_cast(index)); + if (param.getNormalized() == normalized) { // Already where the host put it, which is the usual case: this is the // confirmation of a load the host itself asked for. Reporting it as an @@ -1475,7 +1492,7 @@ void ClapAsVst3::preset_loaded(uint32_t locationKind, const char *location, cons return; } - param->setNormalized(normalized); + param.setNormalized(normalized); if (componentHandler) { // Bracketed, like any value a plugin originates: an unbracketed @@ -1509,10 +1526,11 @@ Steinberg::tresult PLUGIN_API ClapAsVst3::getProgramListInfo(Steinberg::int32 li if (_presetParamId == Vst::kNoParamId || listIndex != inherited) return Steinberg::kResultFalse; info.id = (Vst::ProgramListID)_presetParamId; - // What the host will show as the number of slots. Reporting the count found - // so far (rather than the parameter's 128 steps) keeps a browser from - // listing empty entries while the crawl is still running. - info.programCount = _presetIndex ? (Steinberg::int32)_presetIndex->size() : 0; + // The slots the host will show: the selector's own stepCount+1, not the live + // index size. If the list were the longer of the two, picking a program past + // stepCount clamps to 1.0 and silently loads the wrong preset. + auto *selector = presetSelector(); + info.programCount = selector ? selector->presetCount() : 0; stringconv::convert(std::string("Presets"), info.name); return Steinberg::kResultOk; } @@ -1523,8 +1541,14 @@ Steinberg::tresult PLUGIN_API ClapAsVst3::getProgramName(Vst::ProgramListID list { if (!isPresetProgramList(listId)) return super::getProgramName(listId, programIndex, name); + // Bounded by what getProgramListInfo() published: mid-crawl the index knows + // names for slots the host has not been told exist. + auto *selector = presetSelector(); + if (!selector || programIndex < 0 || programIndex >= selector->presetCount()) + return Steinberg::kResultFalse; + Clap::PresetEntry entry; - if (!_presetIndex || programIndex < 0 || !_presetIndex->presetAt((size_t)programIndex, entry)) + if (!_presetIndex || !_presetIndex->presetAt((size_t)programIndex, entry)) return Steinberg::kResultFalse; stringconv::convert(entry.displayName(), name); @@ -1564,7 +1588,9 @@ void ClapAsVst3::param_rescan(clap_param_rescan_flags flags) for (decltype(len) i = 0; i < len; ++i) { auto p = static_cast(parameters.getParameterByIndex(i)); - if (p->isMidi) continue; + // Neither names a CLAP parameter: the selector's index is left at 0, so + // get_info would rename the host's program control after parameter 0. + if (p->isMidi || p->isPreset) continue; clap_param_info_t info; if (_plugin->_ext._params->get_info(_plugin->_plugin, p->param_index_for_clap_get_info, &info)) { @@ -1745,9 +1771,9 @@ bool ClapAsVst3::unregister_timer(clap_id timer_id) to.period = 0; to.nexttick = 0; #if LIN - if (to.handler && _iRunLoop) + if (auto *const runLoop = _iRunLoop.load(); to.handler && runLoop) { - _iRunLoop->unregisterTimer(to.handler.get()); + runLoop->unregisterTimer(to.handler.get()); } to.handler.reset(); #endif @@ -1792,13 +1818,21 @@ void ClapAsVst3::onIdle() // A preset the host asked for on the audio thread, and a preset list that // filled in on the crawl thread. Both have to happen here: from_location() // is [main-thread], and so is notifyProgramListChange(). - if (auto requested = _presetLoadRequest.exchange(-1); requested >= 0) + // + // Take only an actual request: a plain exchange would also take the + // kAdoptNextPresetValue setState() left there and disarm the adopt. + auto requested = _presetLoadRequest.load(); + while (requested >= 0 && !_presetLoadRequest.compare_exchange_weak(requested, kNoPresetRequest)) + { + } + if (requested >= 0) { Clap::PresetEntry entry; - // Clamp anyway: stepCount matches the count at creation, but a host may - // still have a stale value from before a rescan. - const auto count = _presetIndex ? _presetIndex->size() : 0; - if (count > 0) + // Clamp against what the selector publishes - the range asClapValue() + // decoded against; a host may still replay a value from an older stepCount. + auto *selector = presetSelector(); + const auto count = selector ? (size_t)selector->presetCount() : 0; + if (count > 0 && _presetIndex) { const auto index = std::min((size_t)requested, count - 1); @@ -1827,21 +1861,40 @@ void ClapAsVst3::onIdle() if (_presetListChanged.exchange(false)) { - if (_presetParamId == Vst::kNoParamId) + // Grow the existing selector in place; never rebuild the parameters here - + // that destroys Parameter objects the process adapter is dereferencing on + // the audio thread. kParamTitlesChanged is the SDK's channel for this. + auto *selector = presetSelector(); + if (selector && _presetIndex && _presetIndex->isComplete() && + (Steinberg::int32)_presetIndex->size() != selector->presetCount()) { - // The crawl finished after setupPresets() had nothing to size the list - // with, so there is no selector parameter at all yet. Only rebuilding - // the parameters can introduce one; restartComponent is what makes the - // host re-read them. - setupParameters(_plugin->_plugin, _plugin->_ext._params); + // The size test is not just an optimisation: with the shared index already + // complete there is nothing to tell the host, and a restart is not free. + const auto presetCount = _presetIndex->size(); + { + // process() decodes the selector against stepCount and min/max_value on + // the audio thread, so exclude it for the field writes and nothing else. + ClapWrapper::detail::shared::SpinLockGuard growLock(_processOrFlushLock); + selector->resizePresetSelector((int32_t)presetCount); + } + + // Both, in this order: the parameter info, then the list it selects from. if (componentHandler) componentHandler->restartComponent(Vst::RestartFlags::kParamTitlesChanged | Vst::RestartFlags::kParamValuesChanged); - } - else if (auto unitHandler = Steinberg::FUnknownPtr(componentHandler)) - { - // -1: every program in the list changed, not one of them. - unitHandler->notifyProgramListChange((Vst::ProgramListID)_presetParamId, -1); + if (auto unitHandler = Steinberg::FUnknownPtr(componentHandler)) + { + // -1: every program in the list changed, not one of them. + unitHandler->notifyProgramListChange((Vst::ProgramListID)_presetParamId, -1); + } + + // Cubase sends the selector's value in every process block, so a preset the + // plug-in loaded mid-crawl must be caught up to or preset 0 wins over it. + const auto inEffect = _presetIndexInEffect.load(std::memory_order_relaxed); + if (inEffect >= 0 && inEffect < static_cast(presetCount)) + { + moveSelectorTo(*selector, static_cast(inEffect)); + } } } @@ -1969,46 +2022,54 @@ void ClapAsVst3::attachTimers(Steinberg::Linux::IRunLoop *r) { if (r) { - _iRunLoop = r; + const auto previous = _iRunLoop.exchange(r); if (_idleHandler) { - _iRunLoop->unregisterTimer(_idleHandler.get()); + r->unregisterTimer(_idleHandler.get()); } else { _idleHandler = Steinberg::owned(new IdleHandler(this)); } - _iRunLoop->registerTimer(_idleHandler.get(), 30); + r->registerTimer(_idleHandler.get(), 30); for (auto &t : _timersObjects) { if (!t.handler) { t.handler = Steinberg::owned(new TimerHandler(this, t.timer_id)); - _iRunLoop->registerTimer(t.handler.get(), t.period); + r->registerTimer(t.handler.get(), t.period); } } - // the host's own main thread drives the idle from here on - os::idleSourceChanged(); + // The host's main thread drives the idle from here on, but announce it + // only on the transition: register_timer() also lands here, and it may run + // from on_main_thread() with _mainThreadLock held, where taking the + // helper's lock would invert the documented order. + if (previous == nullptr) + { + os::idleSourceChanged(); + } } } void ClapAsVst3::detachTimers(Steinberg::Linux::IRunLoop *r) { - if (r && r == _iRunLoop) + // Read once: the helper thread only reads _iRunLoop, never writes it. + auto *const runLoop = _iRunLoop.load(); + if (r && r == runLoop) { if (_idleHandler) { - _iRunLoop->unregisterTimer(_idleHandler.get()); + runLoop->unregisterTimer(_idleHandler.get()); _idleHandler.reset(); } for (auto &t : _timersObjects) { if (t.handler) { - _iRunLoop->unregisterTimer(t.handler.get()); + runLoop->unregisterTimer(t.handler.get()); t.handler.reset(); } } @@ -2049,9 +2110,9 @@ bool ClapAsVst3::unregister_fd(int fd) if (it->fd == fd) { res = true; - if (_iRunLoop && it->handler) + if (auto *const runLoop = _iRunLoop.load(); runLoop && it->handler) { - _iRunLoop->unregisterEventHandler(it->handler.get()); + runLoop->unregisterEventHandler(it->handler.get()); } it->handler.reset(); it = _posixFDObjects.erase(it); @@ -2086,14 +2147,16 @@ void ClapAsVst3::attachPosixFD(Steinberg::Linux::IRunLoop *r) { if (r) { - _iRunLoop = r; + // No idleSourceChanged() here: the frame callback calls attachTimers() + // first, which announces the run loop. \see attachTimers() + _iRunLoop.store(r); for (auto &p : _posixFDObjects) { if (!p.handler) { p.handler = Steinberg::owned(new FDHandler(this, p.fd, p.flags)); - _iRunLoop->registerEventHandler(p.handler.get(), p.fd); + r->registerEventHandler(p.handler.get(), p.fd); } } } @@ -2101,13 +2164,14 @@ void ClapAsVst3::attachPosixFD(Steinberg::Linux::IRunLoop *r) void ClapAsVst3::detachPosixFD(Steinberg::Linux::IRunLoop *r) { - if (r && r == _iRunLoop) + auto *const runLoop = _iRunLoop.load(); + if (r && r == runLoop) { for (auto &p : _posixFDObjects) { if (p.handler) { - _iRunLoop->unregisterEventHandler(p.handler.get()); + runLoop->unregisterEventHandler(p.handler.get()); p.handler.reset(); } } diff --git a/src/wrapasvst3.h b/src/wrapasvst3.h index 5d809c73..606716e4 100644 --- a/src/wrapasvst3.h +++ b/src/wrapasvst3.h @@ -52,6 +52,7 @@ namespace Clap { class ProcessAdapter; } +class Vst3Parameter; class queueEvent { @@ -370,9 +371,11 @@ class ClapAsVst3 : public Steinberg::Vst::SingleComponentEffect, #if LIN // While an editor is open the host's run loop drives onIdle() on the real // main thread and the Linux helper thread stands down. \see attachTimers() + // + // Called on the helper thread, which is why _iRunLoop is atomic. bool hasOwnIdleSource() const override { - return _iRunLoop != nullptr; + return _iRunLoop.load() != nullptr; } #endif @@ -468,9 +471,15 @@ class ClapAsVst3 : public Steinberg::Vst::SingleComponentEffect, // Built in setupParameters() when the plugin implements preset-load. The // index itself is shared per module and crawls on a background thread, so // the list can be empty here and fill in later; onPresetIndexComplete() is - // what tells the host to look again. + // what tells the host to look again. The selector always exists once an index + // does - hidden while the crawl runs - and onIdle() grows it in place. void setupPresets(); void onPresetIndexComplete(); + // The selector parameter, or nullptr. Its presetCount() - never the live size + // of _presetIndex - is what the host has been told about. + Vst3Parameter *presetSelector() const; + // Moves the selector to `index` and tells the host, as a bracketed edit. + void moveSelectorTo(Vst3Parameter ¶m, size_t index); bool isPresetProgramList(Vst::ProgramListID listId) const { return _presetParamId != Vst::kNoParamId && listId == (Vst::ProgramListID)_presetParamId; @@ -486,7 +495,22 @@ class ClapAsVst3 : public Steinberg::Vst::SingleComponentEffect, // Set from the audio thread by onRequestPresetLoad(), drained in onIdle(). // Coalescing is correct: three program changes in one block should load the // last preset, not three. - std::atomic _presetLoadRequest{-1}; + // + // One word carries two facts, on purpose: + // + // >= 0 a preset the host asked for, not yet loaded + // kNoPresetRequest nothing pending + // kAdoptNextPresetValue armed by a successful setState(): the next + // selector value the host sends is where the + // host's selector stands, not a preset to load + // + // One word is what closes the race: setState() arms and drops in a single + // store, the audio thread decides and publishes in a single compare-exchange + // (\see onRequestPresetLoad). Armed by setState() and nowhere else, so a + // fresh instance's first program change is a real one. + static constexpr int64_t kNoPresetRequest = -1; + static constexpr int64_t kAdoptNextPresetValue = -2; + std::atomic _presetLoadRequest{kNoPresetRequest}; // The selector value already in effect: the index onIdle() last acted on, // or the one preset_loaded() resolved. A parameter change is only a request // when it names something else. @@ -502,19 +526,6 @@ class ClapAsVst3 : public Steinberg::Vst::SingleComponentEffect, // Written on the main thread (onIdle, preset_loaded), read on the audio // thread (onRequestPresetLoad). std::atomic _presetIndexInEffect{-1}; - // Armed by a successful setState(): the next selector value the host sends - // is where its selector stands, not a preset to load. A restored project - // hands back the value it was saved with, and obeying it reloads that preset - // over the state that has just been restored. - // - // Armed there and nowhere else. A fresh instance has no state to protect, so - // its first program change is a real one and is obeyed - which is also why - // this cannot be inferred from _presetIndexInEffect being -1, a condition the - // two cases share. - // - // Written on the main thread (setState), cleared on the audio thread - // (onRequestPresetLoad). - std::atomic _adoptNextPresetValue{false}; // Set when the crawl finishes; onIdle() turns it into the host notification, // because notifyProgramListChange() is not for a background thread. std::atomic _presetListChanged{false}; @@ -544,7 +555,9 @@ class ClapAsVst3 : public Steinberg::Vst::SingleComponentEffect, void attachTimers(Steinberg::Linux::IRunLoop *); void detachTimers(Steinberg::Linux::IRunLoop *); - Steinberg::Linux::IRunLoop *_iRunLoop{nullptr}; + // Written on the host's main thread, read from the helper thread; every + // write to null is followed by os::idleSourceChanged() to wake the helper. + std::atomic _iRunLoop{nullptr}; #endif #if LIN