Skip to content

Fix audio plugins scanner #67

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

Merged
RomanPudashkin merged 4 commits into from
May 29, 2026
Merged

Fix audio plugins scanner #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6caca25
Ensure that scanPlugins() doesn't return duplicate paths
RomanPudashkin May 27, 2026
8bcfc67
Fix logic in scanPlugins() when plugins have SAME path but DIFFERENT IDs
RomanPudashkin May 28, 2026
dce9ea2
Improve logic in registerPlugins / unregisterPlugins + more logs
RomanPudashkin May 28, 2026
c6d7806
Avoid unnecessary load() call and improve tests
RomanPudashkin May 28, 2026
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
25 changes: 19 additions & 6 deletions framework/audioplugins/internal/knownaudiopluginsregister.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,20 +207,27 @@ Ret KnownAudioPluginsRegister::registerPlugins(const AudioPluginInfoList& list)
return make_ok();
}

bool changed = false;

for (const AudioPluginInfo& info : list) {
auto it = m_pluginInfoMap.find(info.meta.id);
if (it != m_pluginInfoMap.end()) {
IF_ASSERT_FAILED(it->second.path != info.path) {
return false;
if (it->second.path == info.path) {
LOGW() << "Plugin is already registered: " << info.path << ", ID: " << info.meta.id;
continue;
}
}

m_pluginInfoMap.emplace(info.meta.id, info);
m_pluginPaths.insert(info.path);
changed = true;
}

Ret ret = writePluginsInfo();
return ret;
if (changed) {
return writePluginsInfo();
}

return make_ok();
}

Ret KnownAudioPluginsRegister::unregisterPlugins(const AudioResourceIdList& resourceIds)
Expand All @@ -233,6 +240,8 @@ Ret KnownAudioPluginsRegister::unregisterPlugins(const AudioResourceIdList& reso
return make_ok();
}

bool changed = false;

for (const AudioResourceId& resourceId : resourceIds) {
if (!exists(resourceId)) {
continue;
Expand All @@ -245,10 +254,14 @@ Ret KnownAudioPluginsRegister::unregisterPlugins(const AudioResourceIdList& reso
}

m_pluginInfoMap.erase(resourceId);
changed = true;
}

Ret ret = writePluginsInfo();
return ret;
if (changed) {
return writePluginsInfo();
}

return make_ok();
}

Ret KnownAudioPluginsRegister::writePluginsInfo()
Expand Down
59 changes: 28 additions & 31 deletions framework/audioplugins/internal/registeraudiopluginsscenario.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "registeraudiopluginsscenario.h"

#include <QCoreApplication>
#include <map>
#include <set>

#include "global/translation.h"

Expand Down Expand Up @@ -55,51 +55,54 @@ PluginScanResult RegisterAudioPluginsScenario::scanPlugins(Progress* progress) c
TRACEFUNC;

PluginScanResult result;
std::set<io::path_t> scannedPaths;

std::map<io::path_t, audio::AudioResourceId> registered;
for (const auto& info : knownPluginsRegister()->pluginInfoList()) {
registered[info.path] = info.meta.id;
}

for (const auto& scanner : scannerRegister()->scanners()) {
for (const auto& path : scanner->scanPlugins(progress)) {
if (auto it = registered.find(path); it != registered.end()) {
registered.erase(it);
} else {
for (const IAudioPluginsScannerPtr& scanner : scannerRegister()->scanners()) {
for (const io::path_t& path : scanner->scanPlugins(progress)) {
if (!knownPluginsRegister()->exists(path)) {
result.newPluginPaths.push_back(path);
}

scannedPaths.insert(path);
}
}
Comment on lines +60 to 68

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

newPluginPaths can still contain duplicates, defeating the PR's dedup goal.

scannedPaths deduplicates, but the decision to append to result.newPluginPaths is gated only on knownPluginsRegister()->exists(path). If the same not-yet-known path is returned by more than one scanner (or twice by one), it is pushed multiple times, so processPluginsRegistration() spawns a subprocess for it more than once. This contradicts the stated objective of ensuring scanPlugins() does not return duplicate paths.

🐛 Gate on first insertion into the dedup set 
     for (const IAudioPluginsScannerPtr& scanner : scannerRegister()->scanners()) {
         for (const io::path_t& path : scanner->scanPlugins(progress)) {
-            if (!knownPluginsRegister()->exists(path)) {
-                result.newPluginPaths.push_back(path);
-            }
-
-            scannedPaths.insert(path);
+            if (!scannedPaths.insert(path).second) {
+                continue;
+            }
+
+            if (!knownPluginsRegister()->exists(path)) {
+                result.newPluginPaths.push_back(path);
+            }
         }
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/audioplugins/internal/registeraudiopluginsscenario.cpp` around
lines 60 - 68, The loop currently pushes path onto result.newPluginPaths
whenever knownPluginsRegister()->exists(path) is false, which allows duplicates;
change the logic to first attempt to insert each scanned path into scannedPaths
and only if that insertion reports it was newly inserted (i.e., first time seen)
and the path is not already known, then append to result.newPluginPaths; update
the loop around scannerRegister()->scanners() / scanner->scanPlugins(progress)
to use the insert result (or an explicit contains check on scannedPaths) before
pushing to result.newPluginPaths so each unknown path is added exactly once and
processPluginsRegistration() won't be invoked multiple times for the same path.


for (const auto& [path, id] : registered) {
result.missingPluginIds.push_back(id);
for (const AudioPluginInfo& info : knownPluginsRegister()->pluginInfoList()) {
if (!muse::contains(scannedPaths, info.path)) {
result.missingPluginIds.push_back(info.meta.id);
}
}

return result;
}

Ret RegisterAudioPluginsScenario::updatePluginsRegistry()
void RegisterAudioPluginsScenario::updatePluginsRegistry()
{
TRACEFUNC;

PluginScanResult result = scanPlugins();
const PluginScanResult result = scanPlugins();

unregisterRemovedPlugins(result.missingPluginIds);
registerNewPlugins(result.newPluginPaths);
Ret ret = unregisterRemovedPlugins(result.missingPluginIds);
if (!ret) {
LOGE() << "Failed to unregister plugins: " << ret.toString();
}

return knownPluginsRegister()->load();
ret = registerNewPlugins(result.newPluginPaths);
if (!ret) {
LOGE() << "Failed to register plugins: " << ret.toString();
}
}

void RegisterAudioPluginsScenario::registerNewPlugins(const io::paths_t& pluginPaths)
Ret RegisterAudioPluginsScenario::registerNewPlugins(const io::paths_t& pluginPaths)
{
TRACEFUNC;

if (pluginPaths.empty()) {
return;
return make_ok();
}

processPluginsRegistration(pluginPaths);
knownPluginsRegister()->load();
return knownPluginsRegister()->load();
}

Ret RegisterAudioPluginsScenario::unregisterRemovedPlugins(const audio::AudioResourceIdList& pluginIds)
Expand All @@ -110,12 +113,7 @@ Ret RegisterAudioPluginsScenario::unregisterRemovedPlugins(const audio::AudioRes
return make_ok();
}

Ret ret = knownPluginsRegister()->unregisterPlugins(pluginIds);
if (!ret) {
LOGE() << "Failed to unregister removed plugins: " << ret.toString();
}

return ret;
return knownPluginsRegister()->unregisterPlugins(pluginIds);
}

void RegisterAudioPluginsScenario::processPluginsRegistration(const io::paths_t& pluginPaths)
Expand All @@ -125,21 +123,20 @@ void RegisterAudioPluginsScenario::processPluginsRegistration(const io::paths_t&
m_aborted = false;
m_progress.start();

std::string appPath = globalConfiguration()->appBinPath().toStdString();
int64_t pluginCount = static_cast<int64_t>(pluginPaths.size());
const std::string appPath = globalConfiguration()->appBinPath().toStdString();
const int64_t pluginCount = static_cast<int64_t>(pluginPaths.size());

for (int64_t i = 0; i < pluginCount; ++i) {
if (m_aborted) {
return;
}

const io::path_t& pluginPath = pluginPaths[i];
std::string pluginPathStr = pluginPath.toStdString();
const std::string pluginPathStr = pluginPath.toStdString();

m_progress.progress(i, pluginCount, io::filename(pluginPath).toStdString());
qApp->processEvents();

LOGD() << "--register-audio-plugin " << pluginPathStr;
int code = process()->execute(appPath, { "--register-audio-plugin", pluginPathStr });
if (code != 0) {
code = process()->execute(appPath, { "--register-failed-audio-plugin", pluginPathStr, "--", std::to_string(code) });
Expand Down
5 changes: 3 additions & 2 deletions framework/audioplugins/internal/registeraudiopluginsscenario.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ class RegisterAudioPluginsScenario : public IRegisterAudioPluginsScenario, publi

PluginScanResult scanPlugins(Progress* progress = nullptr) const override;

Ret updatePluginsRegistry() override;
void registerNewPlugins(const io::paths_t& pluginPaths) override;
void updatePluginsRegistry() override;

Ret registerNewPlugins(const io::paths_t& pluginPaths) override;
Ret unregisterRemovedPlugins(const audio::AudioResourceIdList& pluginIds) override;

Ret registerPlugin(const io::path_t& pluginPath) override;
Expand Down
5 changes: 3 additions & 2 deletions framework/audioplugins/iregisteraudiopluginsscenario.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ class IRegisterAudioPluginsScenario : MODULE_CONTEXT_INTERFACE

virtual PluginScanResult scanPlugins(Progress* progress = nullptr) const = 0;

virtual Ret updatePluginsRegistry() = 0;
virtual void registerNewPlugins(const io::paths_t& pluginPaths) = 0;
virtual void updatePluginsRegistry() = 0;

virtual Ret registerNewPlugins(const io::paths_t& pluginPaths) = 0;
virtual Ret unregisterRemovedPlugins(const audio::AudioResourceIdList& pluginIds) = 0;

virtual Ret registerPlugin(const io::path_t& pluginPath) = 0;
Expand Down
Loading