Skip to content

allow loading multi-component vst3 effects #32908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kryksyh wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

allow loading multi-component vst3 effects #32908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/framework/audioplugins/internal/registeraudiopluginsscenario.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,27 @@ PluginScanResult RegisterAudioPluginsScenario::scanPlugins() const

PluginScanResult result;

std::map<io::path_t, audio::AudioResourceId> registered;
//! NOTE: A single plugin path can map to multiple resource IDs (multi-component bundles),
//! so we track all IDs per path.
std::map<io::path_t, std::vector<audio::AudioResourceId> > pathToIds;
for (const auto& info : knownPluginsRegister()->pluginInfoList()) {
registered[info.path] = info.meta.id;
pathToIds[info.path].push_back(info.meta.id);
}

for (const auto& scanner : scannerRegister()->scanners()) {
for (const auto& path : scanner->scanPlugins()) {
if (auto it = registered.find(path); it != registered.end()) {
registered.erase(it);
if (auto it = pathToIds.find(path); it != pathToIds.end()) {
pathToIds.erase(it);
} else {
result.newPluginPaths.push_back(path);
}
}
Comment on lines 66 to 73
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Rescans can't detect component churn inside an existing bundle.

Line 68 removes the whole entry as soon as the bundle path is seen, but IAudioPluginsScanner::scanPlugins() and src/framework/vst/internal/vstpluginsscanner.cpp only report paths. If a registered bundle at the same path changes from [A, B] to [A, C], this code treats it as unchanged, so B stays registered and C is never added. Matched paths need a metadata refresh/diff instead of being considered fully reconciled by path presence alone.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/framework/audioplugins/internal/registeraudiopluginsscenario.cpp` around
lines 66 - 73, The current loop in registeraudiopluginsscenario.cpp incorrectly
treats any seen bundle path as fully reconciled and erases pathToIds entry,
which misses component-level churn; instead, when a path from
scannerRegister()->scanners()->scanPlugins() is encountered, fetch the current
plugin list/metadata for that path via the scanner
(IAudioPluginsScanner::scanPlugins()/vstpluginsscanner.cpp), diff it against the
existing entry in pathToIds (do not immediately erase the map entry), add any
newly discovered plugin IDs to result.newPluginPaths or result.newPluginIds and
record removed IDs into result.removedPluginIds, and update the stored metadata
in pathToIds to reflect the new per-plugin state; keep the pathToIds entry and
only remove it if the bundle is truly gone.

}

for (const auto& [path, id] : registered) {
result.missingPluginIds.push_back(id);
for (const auto& [path, ids] : pathToIds) {
for (const auto& id : ids) {
result.missingPluginIds.push_back(id);
}
}

return result;
Expand Down
10 changes: 7 additions & 3 deletions src/framework/vst/internal/vstplugininstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,18 @@ void VstPluginInstance::load()

const auto& factory = m_module->getFactory();

//! NOTE: For multi-component bundles, the resource ID is the component name or UID.
//! Match by name or UID against the factory's audio effect components.
for (const ClassInfo& classInfo : factory.classInfos()) {
if (classInfo.category() != kVstAudioEffectClass) {
continue;
}

m_pluginProvider = std::make_unique<VstPluginProvider>(factory, classInfo);
m_classInfo = classInfo;
break;
if (classInfo.name() == m_resourceId || classInfo.ID().toString() == m_resourceId) {
m_pluginProvider = std::make_unique<VstPluginProvider>(factory, classInfo);
m_classInfo = classInfo;
break;
}
Comment on lines +112 to +123
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

This selector no longer matches every ID that readMeta() can emit.

src/framework/vst/internal/vstpluginmetareader.cpp still produces meta.id = baseName for single-component bundles and baseName + " N" when a multi-component class has no name. Lines 119-122 only match classInfo.name() or classInfo.ID().toString(), so those resources can never create a provider and load() falls through to "Unable to load vst plugin provider".

🧰 Tools
🪛 Cppcheck (2.20.0)

[style] 121-121: The function 'createModule' is never used.

(unusedFunction)

}

if (!m_pluginProvider) {
Expand Down
41 changes: 32 additions & 9 deletions src/framework/vst/internal/vstpluginmetareader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,49 @@ RetVal<AudioResourceMetaList> VstPluginMetaReader::readMeta(const io::path_t& pl
}

const auto& factory = module->getFactory();
AudioResourceMetaList result;

std::vector<ClassInfo> audioEffects;
for (const ClassInfo& classInfo : factory.classInfos()) {
if (classInfo.category() != kVstAudioEffectClass) {
continue;
if (classInfo.category() == kVstAudioEffectClass) {
audioEffects.push_back(classInfo);
}
}

if (audioEffects.empty()) {
return make_ret(Err::NoAudioEffect);
}

std::string baseName = io::completeBasename(pluginPath).toStdString();
bool multiComponent = audioEffects.size() > 1;

AudioResourceMetaList result;
std::set<std::string> usedIds;

for (size_t i = 0; i < audioEffects.size(); ++i) {
const ClassInfo& classInfo = audioEffects[i];

std::string id;
if (multiComponent) {
id = classInfo.name();
if (id.empty()) {
id = baseName + " " + std::to_string(i + 1);
}
if (muse::contains(usedIds, id)) {
id = classInfo.ID().toString();
}
usedIds.insert(id);
} else {
id = baseName;
}
Comment on lines +73 to 85
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

classInfo.name() is too weak to be the persisted AudioResourceId.

For multi-component bundles, Line 75 turns the lookup key into the bare class name unless there is an intra-bundle duplicate. Two different VST bundles can both expose IDs like Compressor or Chorus Stereo, but the rest of the pipeline resolves plugins by resourceId alone (VstPluginInstance(const AudioResourceId&) → modulesRepo()->pluginModule(m_resourceId)). That makes cross-bundle collisions ambiguous. Please keep a bundle-scoped/stable token in the stored ID, or persist the class key separately from the display label.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/framework/vst/internal/vstpluginmetareader.cpp` around lines 73 - 85, The
code currently persists a weak display label via classInfo.name() into id for
multiComponent bundles, which can collide across bundles; instead persist a
stable bundle-scoped token (or the class key) separate from the human label.
Change the assignment in the multiComponent branch to use a stable identifier
such as classInfo.ID().toString() (or combine a bundle identifier +
classInfo.name()) as the persisted AudioResourceId, and keep classInfo.name()
only for a displayLabel variable; ensure usedIds still checks uniqueness against
the persisted id and update downstream consumers (e.g., VstPluginInstance(const
AudioResourceId&) / modulesRepo()->pluginModule(m_resourceId)) to expect the
bundle-scoped token rather than the bare class name.


muse::audio::AudioResourceMeta meta;
meta.id = io::completeBasename(pluginPath).toStdString();
meta.id = std::move(id);
meta.type = muse::audio::AudioResourceType::VstPlugin;
meta.attributes.emplace(muse::audio::CATEGORIES_ATTRIBUTE, String::fromStdString(classInfo.subCategoriesString()));
meta.vendor = classInfo.vendor();
meta.hasNativeEditorSupport = true;

result.emplace_back(std::move(meta));
break;
}

if (result.empty()) {
return make_ret(Err::NoAudioEffect);
}

return RetVal<AudioResourceMetaList>::make_ok(result);
Expand Down
Loading