Plugin scanning rework: token/ephemeral-port protocol, shared scanner, cache healing, per-format settings - #2214
Merged
Merged
Conversation
jcelerier
force-pushed
the
plugin-scan-rework
branch
2 times, most recently
from
August 16, 2026 21:23
ace9ebd to
ef99c61
Compare
Consolidate the four plug-in scanner puppets on shared helpers
(score/tools/PuppetJson.hpp + PuppetClient.hpp), fixing in one move:
* vst3puppet emitted "Request":"<id>" as a JSON *string*; the host's
QJsonValue::toInt() therefore always returned 0, so every reply was
attributed to scan slot 0: the first reply killed an unrelated scan,
the in-flight counter leaked, and successfully-scanned plug-ins were
re-added as invalid by the 10s reaper. Request is now a number.
* Plug-in metadata (names, vendors, descriptions) was interpolated into
the reply without JSON escaping: one quote character silently broke
the whole reply. All puppets now escape every string (lv2puppet's
escaper, promoted to the shared header).
* vst3puppet dereferenced the module after a failed Module::create.
* vst3puppet emitted ClassFlags as a double; now an integer, and load
failures produce an explicit {"Error": ...} reply instead of nothing.
* clappuppet produced invalid JSON ("[,{...}") when a factory returned a
null descriptor at index 0.
* All puppets accept `<path> [id] [port] [token]` and echo the token so
a host can reject replies that belong to another score instance's
scan session; the legacy fixed ports remain the argv defaults.
* vst/vst3/clap puppets adopt lv2puppet's hardened exit path (async
send flush delay + _Exit) instead of exit() from asio handlers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The plug-in scan caches were raw QVariant metatype blobs in QSettings: any change to an Info struct's datastream layout made old blobs decode as garbage that canConvert() accepted (the VST2 "vst_invalid_format" global was a workaround for one such migration). Media/AudioPluginCache.hpp frames the blob with magic + format version + count and fails cleanly on mismatch, truncation or corruption, with a one-shot fallback to the legacy QVariant key so existing caches migrate instead of rescanning. deduplicate() / dropShadowedInvalidEntries() / sanitizePluginCache() heal caches that already accumulated damage: cross-instance scan replies used to be appended to whichever instance owned the fixed notification port (observed: every CLAP plug-in duplicated 200+ times), and a reply arriving after the 10s reaper recorded a plug-in both as valid and as an "<Invalid>" marker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One implementation of the puppet-process state machine instead of four
divergent copies, incorporating every fix the LV2 rework had and the
others lacked:
* Ephemeral server port + per-scanner session token, validated before
any reply can touch a plug-in database. The fixed ports (37587..90,
listen() failure ignored) meant that concurrent score-derived
processes delivered their scan results to whichever instance owned
the port: that instance persisted duplicates of every plug-in
(observed: 200+ copies each), while the scanner timed out and marked
everything invalid ("Got invalid VST request ID 0/1/2/3...").
* Event-driven batch refill; timeouts armed per process, so hung
puppets in the tail batch are reaped too (the old poll only checked
timeouts while saturated).
* A reply-vs-process-exit grace period: the WebSocket delivery of a
reply races the puppet's exit, and the close handshake can even make
a successful puppet exit non-zero. Failure is only declared when the
grace period elapses with no reply - one scanFailed per path, no
matter which combination of errorOccurred/finished/timeout fires.
* Accepts legacy string request ids; rescan-while-scanning cancels the
old scan cleanly instead of leaking TU-static in-flight counters.
Tested end-to-end against a scriptable fake puppet
(tests/fixtures/fake_puppet): token isolation between two concurrent
scanners, crash/exit/garbage/hang single-failure semantics, the
reply-then-exit(1) race, and mid-scan rescans.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ings
Replace the four hand-rolled puppet state machines with the shared
Media::PluginScanner. This retires, per backend:
* VST2/VST3: the always-id-0 reply attribution (string "Request" +
QJsonValue::toInt), the first reply killing scan slot 0, leaked
TU-static in-flight counters, the 1s polling loop whose false 10s
timeouts re-recorded successfully-scanned plug-ins as invalid, and
the deferred `m_processes[id] = {}` clobbering unrelated slots after
a rescan. VST3 additionally reads ClassFlags from the right key
(it was parsed from the "Version" string and always came out 0).
* CLAP: unbounded cache growth (nothing was ever pruned or
deduplicated; cross-instance replies multiplied every plug-in by the
number of runs), the macOS bundle-path rewrite running *after* the
known-paths check (one full duplicate set per launch), the
errorOccurred+finished double-invalid, the reply-vs-exit race, and
the 100000 ms (100 s!) reaper typo. Bundle-style directories on
Linux (Cardinal & other DPF plug-ins) no longer produce spurious
invalid entries.
* LV2: ported onto the same machinery (its rework was the blueprint);
spec bundles and sanitizer options move to an environment provider.
All four caches move to the versioned AudioPluginCache blob with a
one-shot migration from the legacy QVariant keys and load-time healing
(dedup + valid-over-invalid), persisted at initialize() so existing
polluted configurations are repaired on first launch.
The settings refactor is completed: Vst3Paths / ClapPaths / Lv2Paths
join VstPaths in Media::Settings::Model (defaults = the previously
hardcoded per-platform lists; VST3_PATH/CLAP_PATH/LV2_PATH stay
additive), each backend rescans on *its own* path setting - VST3 no
longer wipes its database when the VST2 paths change (the old
`//! TODO` coupling) - and every format now has an Effects-page tab
(paths editor + rescan + working/faulty tables) built on one shared
widget, Media::Settings::makePluginSettingsWidget.
The backends' changed-signals become E_SIGNAL: with
-fvisibility-inlines-hidden, W_SIGNAL bodies are per-DSO copies and
cross-library connections to them silently never fire.
End-to-end validated against the system's real plug-in collection:
315 VST2s + full VST3/CLAP/LV2 sets scanned with zero invalid-request
messages, zero dropped replies, and only genuinely-broken plug-ins
marked invalid.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also asserts that a plug-in's signal emission is observable across the shared-library boundary (the E_SIGNAL requirement). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An adversarial review of the branch and a forensic diagnosis of every scan failure on a real plug-in collection surfaced these: Reply-loss hardening (three real plug-ins - Atlas, a yabridged Permut8, DrumSynth - were transiently recorded invalid because the scanner lost their replies while the main thread was stalled at startup): * Grace-period expiry now defers its verdict by one event-loop iteration: after a stall, the expired grace timer and the WebSocket delivering the reply become ready in the same poll iteration, and a timer source is not ordered after a socket source - the reply that is already sitting in the socket must win. * Puppets only echo their JSON to stdout on manual runs: when spawned by the host nobody drains the pipe, and lsp-plugins.clap's ~100KB reply overflows the 64KB pipe buffer. * VST2/VST3/CLAP process timeout 10s -> 30s (cold wineserver starts, 4 formats x 8 puppets sharing the machine); the puppet-side watchdog is now per-format (30s/60s), skips exiting once the reply is in flight (it used to _Exit(1) right between send and flush after a >10s scan, truncating the reply), and matches the host budget. Cache robustness (review findings #1/#2 - both genuine losses of pre-existing behavior): * The versioned blob's per-element status check could never fire: the score datastream operators route reads through an internal stream, so truncation and layout drift decoded as garbage (the failure mode the deleted VST2 vst_invalid_format global used to catch). Elements are now length-prefixed and decode from isolated buffers with exact- consumption checks; covered by a real-type truncation test. * Quitting mid-scan silently discarded the whole batch for VST2/VST3/ CLAP: the 500ms debounce restarts on every reply and thus never fires during a busy scan, and unlike LV2 they had no destructor persist. The debounce no longer restarts (results and settings-tab tables now appear progressively again, as before the rework) and all three got the destructor safety net. * The legacy QVariant cache key is removed even when the versioned cache wins, so an older score run can no longer plant a stale blob that a future format bump would resurrect. Scanner lifecycle: * FailedToStart is resolved through a deferred call: it is emitted synchronously from QProcess::start(), and resolving inline recursed start -> errorOccurred -> refill -> start through the whole queue on one stack when the puppet binary is missing. Regression-tested. * Sockets of puppets that connect but never reply are reaped on disconnect instead of living until scanner destruction. * A malformed puppet request id maps to -1 (dropped) instead of masquerading as scan slot 0. Also: Windows CLAP/LV2 path defaults use %APPDATA%/%LOCALAPPDATA% env vars (evaluated in a static initializer, QStandardPaths::App*Location would bake in whatever application identity exists at that point - and the plain env paths are the lilv/Carla convention anyway), and stale comments about Cancel-rollback and socket teardown were corrected. Diagnosis of the 18 failures from the validation scan: 9 genuine non-plug-ins (Carla helper/interposer/style libraries, the lsp-plugins shared core - its ~200 stubs export VSTPluginMain and scan fine), 3 missing system dependency (ProM needs libprojectM.so.3), 3 fixed CLAP bundle-dir artifacts, 3 reply losses fixed above. A full rescan now loads 318 VST2s with every failure accounted for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLAP song positions are fixed-point: clap_beattime/clap_sectime are int64 scaled by 1<<31 (clap/fixedpoint.h). make_transport cast the raw beat count (and floored away the fractional beat first), so a host-side read of song_pos_beats / CLAP_BEATTIME_FACTOR yielded ~0 on every tick: transport-following plug-ins - the Stochas step sequencer, arpeggiators, synced delays - saw IS_PLAYING but a song position frozen at zero and never advanced. Verified against clap-juce-extensions' playhead implementation, which is exactly how Stochas consumes it. Also compute song_pos_seconds from the musical position and tempo - the previous formula multiplied a flicks date by a flicks-per-sample ratio - and guard the bar computation against a zero signature. The conversion moves to Clap/Transport.hpp as a pure function with unit coverage: fractional beats survive, consecutive ticks yield strictly increasing positions, pause clears IS_PLAYING, 6/8 bar math. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two ways an LV2 UI could take score down, both found with qmidiarp: * The UI chooser took the first suil-supported UI. qmidiarp declares an OpenGL UI whose binary is not shipped by the distribution package, so the whole UI failed on a missing file even though the X11 UI declared right after it exists. The chooser now keeps the best-supported UI whose binary is actually present. * That X11 UI links Qt5, and a UI linked against another Qt major cannot run in this process: Qt keeps identical mangled names across majors for its exported symbols, so the foreign Qt's internal calls resolve against *our* Qt operating on their objects. Dev builds die with a QStringView assert inside Qt5's QFactoryLoader before the UI even finishes creating its QApplication; static-Qt release builds (which export their Qt through ENABLE_EXPORTS for addon loading) corrupt silently. GTK hosts load these UIs fine as no other Qt lives in their process - we must refuse, and fall back to the generated controls. Same root cause as in-process Carla (Ildaeil) loading such UIs; that path is out of our hands. Also guard Window creation against the suil UI host not existing (SCORE_DISABLE_AUDIOPLUGINS/SCORE_DISABLE_LV2: suil dereferences the host unchecked - segfault), and cover the Qt-major detector with tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Extract linuxcheck's ElfInspector into score/tools/ElfInspector.hpp and use it for the LV2 UI Qt-major compatibility check: on Linux the answer now comes from the binary's actual DT_NEEDED entries (any libQt<N>* with N != ours) instead of a byte scan. The shared version returns nullopt on unreadable/corrupt/non-64-bit ELF instead of printing to stderr and, for truncated files, throwing through the caller. Non-ELF platforms (LV2 is also supported on macOS, one day Windows) and binaries the reader cannot parse fall back to the generic scan, which now knows every platform's Qt linkage shapes: lib names (.so/.dylib), macOS framework paths (QtGui.framework/Versions/<N>), and Windows DLL imports (Qt<N>Gui.dll). Covered by tests incl. real ELFs: our own Qt6-linked plug-in accepted through the DT_NEEDED path, an installed libQt5Gui rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jcelerier
force-pushed
the
plugin-scan-rework
branch
from
August 17, 2026 03:52
b467a90 to
a1428dd
Compare
QWebSocket::errorOccurred only exists since Qt 6.5; the Coverage CI container builds the test tree with Qt 6.4, where the signal is the overloaded `error`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reworks VST2/VST3/CLAP/LV2 plug-in scanning around one shared, tested state machine, fixing three long-standing issue clusters:
listen()and no session identity, so any concurrent score-derived process delivered its scan replies into whichever instance owned the port — which appended and persisted them, while the scanning instance timed out and marked everything invalid.QJsonValue::toInt()attributed every reply to scan slot 0 — killing an unrelated scan, leaking the in-flight counter, and re-recording successfully-scanned plug-ins as invalid via false 10s timeouts.PluginSettingsTabinterface existed with only a VST2 implementation; VST3/CLAP/LV2 had no path configuration or scan feedback, and VST3 wiped its whole database whenever the VST2 paths changed (//! TODO).What changed
Media::PluginScanner: ephemeral WebSocket port + per-scanner session token validated before any reply touches a plug-in database; event-driven batch refill; per-process timeouts that also cover the tail batch; a reply-vs-process-exit grace period (onescanFailedper path, no double/false invalids); clean rescan-while-scanning.score/tools/PuppetJson.hpp/PuppetClient.hpp), JSON escaping of all plug-in metadata (one quote used to silently break a whole reply), numericRequest,Tokenecho, explicitErrorreplies, per-format watchdogs that can't truncate a reply that is already in flight.vst_invalid_formatglobal hack), with one-shot migration from the legacy keys and load-time healing — existing configs polluted with duplicates repair themselves on first launch.Vst3Paths/ClapPaths/Lv2PathsjoinVstPathsinMedia::Settings::Model(defaults = the previously hardcoded per-platform lists;VST3_PATH/CLAP_PATH/LV2_PATHstay additive); each backend rescans on its own signal; all four formats get an Effects-page tab (paths editor + rescan + working/faulty tables) built on one shared widget.classFlagswas parsed from the wrong key and always 0; CLAP's macOS bundle resolution ran after the known-paths check (one duplicate set per launch); Linux bundle-style.clapdirectories (Cardinal & other DPF plug-ins) produced spurious invalid entries; backend change-signals are nowE_SIGNAL(with-fvisibility-inlines-hidden, cross-library connections toW_SIGNALbodies silently never fire).Validation
tests/unit/{PuppetJson,AudioPluginCache,PluginScanner,VstScanReply,Vst3ScanReply,ClapScan,PluginPathSettings,LV2Loading}Test.cpp), incl. an end-to-end scriptable fake puppet: token isolation between concurrent scanners, crash/timeout/garbage single-failure semantics, reply-then-exit(1) races, 200×-duplicate healing through a real app boot, legacy-cache migration, per-format path signals.libprojectM.so.3). A second run is a pure cache hit. Bridged plug-ins through yabridge/wine scan correctly.🤖 Generated with Claude Code