From ad004b39b1f47138841b738365746915710915eb Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Thu, 15 Jun 2023 20:11:42 +0300 Subject: [PATCH 1/6] write logs related to audio plugins registration to a separate file (to avoid creating a lot of log files) --- src/framework/global/globalmodule.cpp | 26 ++++++++++++++----- src/framework/global/logremover.cpp | 17 ++++++------ .../global/tests/logremover_tests.cpp | 21 +++++++++++---- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/framework/global/globalmodule.cpp b/src/framework/global/globalmodule.cpp index 87a39e02a79c3..fcb0d192f8704 100644 --- a/src/framework/global/globalmodule.cpp +++ b/src/framework/global/globalmodule.cpp @@ -90,13 +90,27 @@ void GlobalModule::onPreInit(const IApplication::RunMode& mode) io::path_t logPath = m_configuration->userAppDataPath() + "/logs"; fileSystem()->makePath(logPath); - //! Remove old logs - LogRemover::removeLogs(logPath, 7, u"MuseScore_yyMMdd_HHmmss.log"); + io::path_t logFilePath = logPath; + String logFileNamePattern; + + if (mode == IApplication::RunMode::AudioPluginRegistration) { + logFileNamePattern = u"audiopluginregistration_yyMMdd.log"; + + //! This creates a file named "data/logs/audiopluginregistration_yyMMdd.log" + logFilePath += "/audiopluginregistration_" + + QDateTime::currentDateTime().toString("yyMMdd") + + ".log"; + } else { + logFileNamePattern = u"MuseScore_yyMMdd_HHmmss.log"; - //! File, this creates a file named "data/logs/MuseScore_yyMMdd_HHmmss.log" - io::path_t logFilePath = logPath + "/MuseScore_" - + QDateTime::currentDateTime().toString("yyMMdd_HHmmss") - + ".log"; + //! This creates a file named "data/logs/MuseScore_yyMMdd_HHmmss.log" + logFilePath += "/MuseScore_" + + QDateTime::currentDateTime().toString("yyMMdd_HHmmss") + + ".log"; + } + + //! Remove old logs + LogRemover::removeLogs(logPath, 7, logFileNamePattern); FileLogDest* logFile = new FileLogDest(logFilePath.toStdString(), LogLayout("${datetime} | ${type|5} | ${thread} | ${tag|10} | ${message}")); diff --git a/src/framework/global/logremover.cpp b/src/framework/global/logremover.cpp index d3f10971e5cfb..005ef1ba86435 100644 --- a/src/framework/global/logremover.cpp +++ b/src/framework/global/logremover.cpp @@ -32,7 +32,7 @@ void LogRemover::removeLogs(const io::path_t& logsDir, int olderThanDays, const { //! NOTE If the pattern changes, //! then we need to change the implementation of `scanDir` and `parseDate` functions. - IF_ASSERT_FAILED(pattern == u"MuseScore_yyMMdd_HHmmss.log") { + IF_ASSERT_FAILED(pattern.endsWith(u"_yyMMdd.log") || pattern.endsWith(u"_yyMMdd_HHmmss.log")) { return; } @@ -43,7 +43,7 @@ void LogRemover::removeLogs(const io::path_t& logsDir, int olderThanDays, const io::paths_t toRemoveFiles; for (const io::path_t& file : files) { - Date date = parseDate(file.toString()); + Date date = parseDate(io::filename(file).toString()); if (date.isNull()) { continue; } @@ -59,17 +59,13 @@ void LogRemover::removeLogs(const io::path_t& logsDir, int olderThanDays, const mu::Date LogRemover::parseDate(const String& fileName) { - size_t endIdx = fileName.lastIndexOf(u'_'); - if (endIdx == mu::nidx) { + size_t dateStartIdx = fileName.indexOf(u'_'); + if (dateStartIdx == mu::nidx) { return Date(); } - size_t startIdx = fileName.lastIndexOf(u'_', endIdx - 1); - if (startIdx == mu::nidx) { - return Date(); - } + String dateStr = fileName.mid(dateStartIdx + 1, fileName.size() - 1); - String dateStr = fileName.mid(startIdx + 1, (endIdx - startIdx - 1)); // "yyMMdd" String yy = dateStr.mid(0, 2); bool ok = false; @@ -77,16 +73,19 @@ mu::Date LogRemover::parseDate(const String& fileName) if (!ok || !(y > 0)) { return Date(); } + String mm = dateStr.mid(2, 2); int m = mm.toInt(&ok); if (!ok || !(m > 0 && m <= 12)) { return Date(); } + String dd = dateStr.mid(4, 2); int d = dd.toInt(&ok); if (!ok || !(d > 0 && d <= 31)) { return Date(); } + Date date(2000 + y, m, d); return date; } diff --git a/src/framework/global/tests/logremover_tests.cpp b/src/framework/global/tests/logremover_tests.cpp index a53d28708ae31..f3b6f7c81136a 100644 --- a/src/framework/global/tests/logremover_tests.cpp +++ b/src/framework/global/tests/logremover_tests.cpp @@ -32,10 +32,21 @@ class Global_LogRemoverTests : public ::testing::Test TEST_F(Global_LogRemoverTests, ParseDate) { - EXPECT_EQ(LogRemover::parseDate(u"path/to/logs/MuseScore_210629_154033.log"), Date(2021, 6, 29)); - EXPECT_EQ(LogRemover::parseDate(u"path/to_to/logs/MuseScore_210709_154033.log"), Date(2021, 7, 9)); - EXPECT_EQ(LogRemover::parseDate(u"path/to/logs/MuseScore_210709__154033.log"), Date()); - EXPECT_EQ(LogRemover::parseDate(u"path/to/logs/MuseScore_210709_154033_.log"), Date()); - EXPECT_EQ(LogRemover::parseDate(u"path/to/logs/MuseScore_210709_154033-.log"), Date(2021, 7, 9)); + // Correct name format: *_yyMMdd* + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_210229_154033.log"), Date(2021, 2, 29)); + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_390910_154033"), Date(2039, 9, 10)); + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_290910-_154033__"), Date(2029, 9, 10)); + EXPECT_EQ(LogRemover::parseDate(u"audiopluginregistration_230615.log"), Date(2023, 6, 15)); + + // Incorrect name format: missing _ or wrong position + EXPECT_EQ(LogRemover::parseDate(u"MuseScore210229154033.log"), Date()); + EXPECT_EQ(LogRemover::parseDate(u"MuseScore210229154033_.log"), Date()); + EXPECT_EQ(LogRemover::parseDate(u"_MuseScore_210229_154033.log"), Date()); + + // Incorrect name format: missing/wrong date + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_"), Date()); + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_00229_154033"), Date()); // wrong year + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_219929_154033.log"), Date()); // wrong month + EXPECT_EQ(LogRemover::parseDate(u"MuseScore_210299_154033.log"), Date()); // wrong day } } From 13bd0e90b00fa7a4c0dc74352f9e0db5ca658d69 Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Fri, 16 Jun 2023 12:07:19 +0300 Subject: [PATCH 2/6] optimization --- src/framework/audio/audiomodule.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/framework/audio/audiomodule.cpp b/src/framework/audio/audiomodule.cpp index dfe378028478c..48ace34c8ce66 100644 --- a/src/framework/audio/audiomodule.cpp +++ b/src/framework/audio/audiomodule.cpp @@ -191,9 +191,14 @@ void AudioModule::onInit(const framework::IApplication::RunMode& mode) // Init configuration m_configuration->init(); - m_soundFontRepository->init(); m_registerAudioPluginsScenario->init(); + if (mode == framework::IApplication::RunMode::AudioPluginRegistration) { + return; + } + + m_soundFontRepository->init(); + m_audioBuffer->init(m_configuration->audioChannelsCount(), m_configuration->renderStep()); From 8e1faa44f98496b25aaf2c1b761833ecd4e976ac Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Fri, 16 Jun 2023 12:50:06 +0300 Subject: [PATCH 3/6] use known_audio_plugins.json to store information about installed plugins (instead of having a separate file for each plugin to avoid possible name conflicts, the user may have the same plugin installed in several different locations) --- .../internal/diagnosticfileswriter.cpp | 4 +- src/framework/audio/audiomodule.cpp | 2 +- src/framework/audio/audiotypes.h | 10 +- src/framework/audio/iaudioconfiguration.h | 2 +- .../audio/internal/audioconfiguration.cpp | 4 +- .../audio/internal/audioconfiguration.h | 2 +- .../plugins/knownaudiopluginsregister.cpp | 129 +++++++++--------- .../plugins/knownaudiopluginsregister.h | 5 +- .../plugins/registeraudiopluginsscenario.cpp | 8 +- .../tests/knownaudiopluginsregistertest.cpp | 116 ++++++++-------- .../tests/mocks/audioconfigurationmock.h | 2 +- .../registeraudiopluginsscenariotest.cpp | 1 + .../vst/internal/vstpluginmetareader.cpp | 4 +- .../audio/audioconfigurationstub.cpp | 2 +- .../framework/audio/audioconfigurationstub.h | 2 +- 15 files changed, 154 insertions(+), 139 deletions(-) diff --git a/src/diagnostics/internal/diagnosticfileswriter.cpp b/src/diagnostics/internal/diagnosticfileswriter.cpp index bae058acc9895..e9944f4240767 100644 --- a/src/diagnostics/internal/diagnosticfileswriter.cpp +++ b/src/diagnostics/internal/diagnosticfileswriter.cpp @@ -34,9 +34,8 @@ mu::Ret DiagnosticFilesWriter::writeDiagnosticFiles(const path_t& destinationPat { TRACEFUNC; - static const std::vector DIRS_TO_WRITE { + const std::vector DIRS_TO_WRITE { "logs", - "audio_plugins", "plugins", "workspaces", }; @@ -61,6 +60,7 @@ mu::Ret DiagnosticFilesWriter::writeDiagnosticFiles(const path_t& destinationPat const std::vector FILES_TO_WRITE { "shortcuts.xml", "midi_mappings.xml", + "known_audio_plugins.json", }; for (const std::string& fileName : FILES_TO_WRITE) { diff --git a/src/framework/audio/audiomodule.cpp b/src/framework/audio/audiomodule.cpp index 48ace34c8ce66..86647118c94cb 100644 --- a/src/framework/audio/audiomodule.cpp +++ b/src/framework/audio/audiomodule.cpp @@ -214,7 +214,7 @@ void AudioModule::onInit(const framework::IApplication::RunMode& mode) for (const io::path_t& p : paths) { pr->reg("soundfonts", p); } - pr->reg("audio_plugins", m_configuration->knownAudioPluginsDir()); + pr->reg("known_audio_plugins", m_configuration->knownAudioPluginsFilePath()); } } diff --git a/src/framework/audio/audiotypes.h b/src/framework/audio/audiotypes.h index fbbdb96ac3ed9..c9eba70fe37f7 100644 --- a/src/framework/audio/audiotypes.h +++ b/src/framework/audio/audiotypes.h @@ -186,15 +186,15 @@ struct AudioPluginInfo { int errorCode = 0; }; -inline AudioPluginType audioPluginTypeFromCategoriesString(const std::string& categoriesStr) +inline AudioPluginType audioPluginTypeFromCategoriesString(const String& categoriesStr) { - static const std::map STRING_TO_PLUGIN_TYPE_MAP = { - { "Fx", AudioPluginType::Fx }, - { "Instrument", AudioPluginType::Instrument }, + static const std::map STRING_TO_PLUGIN_TYPE_MAP = { + { u"Fx", AudioPluginType::Fx }, + { u"Instrument", AudioPluginType::Instrument }, }; for (auto it = STRING_TO_PLUGIN_TYPE_MAP.cbegin(); it != STRING_TO_PLUGIN_TYPE_MAP.cend(); ++it) { - if (categoriesStr.find(it->first) != std::string::npos) { + if (categoriesStr.contains(it->first)) { return it->second; } } diff --git a/src/framework/audio/iaudioconfiguration.h b/src/framework/audio/iaudioconfiguration.h index 7ed2594581d34..52eaa775b8e6e 100644 --- a/src/framework/audio/iaudioconfiguration.h +++ b/src/framework/audio/iaudioconfiguration.h @@ -70,7 +70,7 @@ class IAudioConfiguration : MODULE_EXPORT_INTERFACE virtual async::Notification synthesizerStateChanged() const = 0; virtual async::Notification synthesizerStateGroupChanged(const std::string& groupName) const = 0; - virtual io::path_t knownAudioPluginsDir() const = 0; + virtual io::path_t knownAudioPluginsFilePath() const = 0; }; } diff --git a/src/framework/audio/internal/audioconfiguration.cpp b/src/framework/audio/internal/audioconfiguration.cpp index 1ca4d5e731f4f..0f3cfc9f33be2 100644 --- a/src/framework/audio/internal/audioconfiguration.cpp +++ b/src/framework/audio/internal/audioconfiguration.cpp @@ -260,9 +260,9 @@ async::Notification AudioConfiguration::synthesizerStateGroupChanged(const std:: return m_synthesizerStateGroupChanged[gname]; } -io::path_t AudioConfiguration::knownAudioPluginsDir() const +io::path_t AudioConfiguration::knownAudioPluginsFilePath() const { - return globalConfiguration()->userAppDataPath() + "/audio_plugins"; + return globalConfiguration()->userAppDataPath() + "/known_audio_plugins.json"; } io::path_t AudioConfiguration::stateFilePath() const diff --git a/src/framework/audio/internal/audioconfiguration.h b/src/framework/audio/internal/audioconfiguration.h index 013756a43aa38..6bc69d4667c54 100644 --- a/src/framework/audio/internal/audioconfiguration.h +++ b/src/framework/audio/internal/audioconfiguration.h @@ -70,7 +70,7 @@ class AudioConfiguration : public IAudioConfiguration async::Notification synthesizerStateChanged() const override; async::Notification synthesizerStateGroupChanged(const std::string& groupName) const override; - io::path_t knownAudioPluginsDir() const override; + io::path_t knownAudioPluginsFilePath() const override; private: async::Channel m_soundFontDirsChanged; diff --git a/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp b/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp index 2c75850d80aaf..0d4ec357f1947 100644 --- a/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp +++ b/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp @@ -38,7 +38,7 @@ static JsonObject attributesToJson(const AudioResourceAttributes& attributes) JsonObject result; for (auto it = attributes.cbegin(); it != attributes.cend(); ++it) { - if (it->second == audio::PLAYBACK_SETUP_DATA_ATTRIBUTE) { + if (it->first == audio::PLAYBACK_SETUP_DATA_ATTRIBUTE) { continue; } @@ -54,10 +54,17 @@ static JsonObject metaToJson(const AudioResourceMeta& meta) result.set("id", meta.id); result.set("type", mu::value(RESOURCE_TYPE_TO_STRING_MAP, meta.type, "Undefined")); - result.set("vendor", meta.vendor); - result.set("attributes", attributesToJson(meta.attributes)); result.set("hasNativeEditorSupport", meta.hasNativeEditorSupport); + if (!meta.vendor.empty()) { + result.set("vendor", meta.vendor); + } + + JsonObject attributesJson = attributesToJson(meta.attributes); + if (!attributesJson.empty()) { + result.set("attributes", attributesJson); + } + return result; } @@ -79,9 +86,13 @@ static AudioResourceMeta metaFromJson(const JsonObject& object) result.id = object.value("id").toStdString(); result.type = mu::key(RESOURCE_TYPE_TO_STRING_MAP, object.value("type").toStdString()); result.vendor = object.value("vendor").toStdString(); - result.attributes = attributesFromJson(object.value("attributes").toObject()); result.hasNativeEditorSupport = object.value("hasNativeEditorSupport").toBool(); + JsonValue attributes = object.value("attributes"); + if (attributes.isObject()) { + result.attributes = attributesFromJson(attributes.toObject()); + } + return result; } } @@ -90,50 +101,45 @@ mu::Ret KnownAudioPluginsRegister::load() { TRACEFUNC; - io::path_t knownAudioPluginsDir = configuration()->knownAudioPluginsDir(); + m_loaded = false; + m_pluginInfoMap.clear(); + m_pluginPaths.clear(); - if (!fileSystem()->exists(knownAudioPluginsDir)) { - fileSystem()->makePath(knownAudioPluginsDir); + io::path_t knownAudioPluginsPath = configuration()->knownAudioPluginsFilePath(); + if (!fileSystem()->exists(knownAudioPluginsPath)) { + m_loaded = true; + return make_ok(); } - RetVal paths = fileSystem()->scanFiles(knownAudioPluginsDir, - { "*.json" }, - io::ScanMode::FilesInCurrentDir); - if (!paths.ret) { - return paths.ret; + RetVal file = fileSystem()->readFile(knownAudioPluginsPath); + if (!file.ret) { + return file.ret; } - m_pluginInfoMap.clear(); - m_pluginPaths.clear(); - - for (const io::path_t& infoPath : paths.val) { - RetVal file = fileSystem()->readFile(infoPath); - if (!file.ret) { - LOGE() << file.ret.toString(); - continue; - } + std::string err; + JsonDocument json = JsonDocument::fromJson(file.val, &err); + if (!err.empty()) { + return Ret(static_cast(Ret::Code::UnknownError), err); + } - std::string err; - JsonDocument json = JsonDocument::fromJson(file.val, &err); - if (!err.empty()) { - LOGE() << err; - continue; - } + JsonArray array = json.rootArray(); - JsonObject object = json.rootObject(); + for (size_t i = 0; i < array.size(); ++i) { + JsonObject object = array.at(i).toObject(); AudioPluginInfo info; info.meta = metaFromJson(object.value("meta").toObject()); - info.meta.attributes.insert({ audio::PLAYBACK_SETUP_DATA_ATTRIBUTE, mpe::GENERIC_SETUP_DATA_STRING }); - info.type = audioPluginTypeFromCategoriesString(info.meta.attributeVal(audio::CATEGORIES_ATTRIBUTE).toStdString()); + info.meta.attributes.emplace(audio::PLAYBACK_SETUP_DATA_ATTRIBUTE, mpe::GENERIC_SETUP_DATA_STRING); + info.type = audioPluginTypeFromCategoriesString(info.meta.attributeVal(audio::CATEGORIES_ATTRIBUTE)); info.path = object.value("path").toString(); info.enabled = object.value("enabled").toBool(); info.errorCode = object.value("errorCode").toInt(); - m_pluginInfoMap[info.meta.id] = info; m_pluginPaths.insert(info.path); + m_pluginInfoMap.emplace(info.meta.id, std::move(info)); } + m_loaded = true; return make_ok(); } @@ -177,60 +183,59 @@ bool KnownAudioPluginsRegister::exists(const AudioResourceId& resourceId) const mu::Ret KnownAudioPluginsRegister::registerPlugin(const AudioPluginInfo& info) { - TRACEFUNC; - - JsonObject obj; - obj.set("meta", metaToJson(info.meta)); - obj.set("path", info.path.toStdString()); - obj.set("enabled", info.enabled); - - if (info.errorCode != 0) { - obj.set("errorCode", info.errorCode); - } - - io::path_t path = pluginInfoPath(info.meta.vendor, info.meta.id); - Ret ret = fileSystem()->writeFile(path, JsonDocument(obj).toJson()); - if (!ret) { - return ret; + IF_ASSERT_FAILED(m_loaded) { + return false; } m_pluginInfoMap[info.meta.id] = info; m_pluginPaths.insert(info.path); - return make_ok(); + Ret ret = writePluginsInfo(); + return ret; } mu::Ret KnownAudioPluginsRegister::unregisterPlugin(const AudioResourceId& resourceId) { - TRACEFUNC; + IF_ASSERT_FAILED(m_loaded) { + return false; + } if (!exists(resourceId)) { return make_ok(); } AudioPluginInfo info = m_pluginInfoMap[resourceId]; - io::path_t path = pluginInfoPath(info.meta.vendor, resourceId); - - Ret ret = fileSystem()->remove(path); - if (!ret) { - return ret; - } mu::remove(m_pluginInfoMap, resourceId); mu::remove(m_pluginPaths, info.path); - return make_ok(); + Ret ret = writePluginsInfo(); + return ret; } -mu::io::path_t KnownAudioPluginsRegister::pluginInfoPath(const AudioResourceVendor& vendor, const AudioResourceId& resourceId) const +mu::Ret KnownAudioPluginsRegister::writePluginsInfo() { - io::path_t fileName; + TRACEFUNC; + + JsonArray array; + + for (const auto& pair : m_pluginInfoMap) { + const AudioPluginInfo& info = pair.second; - if (vendor.empty()) { - fileName = io::escapeFileName(resourceId); - } else { - fileName = io::escapeFileName(vendor + "_" + resourceId); + JsonObject obj; + obj.set("meta", metaToJson(info.meta)); + obj.set("path", info.path.toStdString()); + obj.set("enabled", info.enabled); + + if (info.errorCode != 0) { + obj.set("errorCode", info.errorCode); + } + + array << obj; } - return configuration()->knownAudioPluginsDir() + "/" + fileName + ".json"; + io::path_t knownAudioPluginsPath = configuration()->knownAudioPluginsFilePath(); + Ret ret = fileSystem()->writeFile(knownAudioPluginsPath, JsonDocument(array).toJson()); + + return ret; } diff --git a/src/framework/audio/internal/plugins/knownaudiopluginsregister.h b/src/framework/audio/internal/plugins/knownaudiopluginsregister.h index cf69645c3233e..102b8de1f0aeb 100644 --- a/src/framework/audio/internal/plugins/knownaudiopluginsregister.h +++ b/src/framework/audio/internal/plugins/knownaudiopluginsregister.h @@ -48,9 +48,10 @@ class KnownAudioPluginsRegister : public IKnownAudioPluginsRegister Ret unregisterPlugin(const AudioResourceId& resourceId) override; private: - mu::io::path_t pluginInfoPath(const AudioResourceVendor& vendor, const AudioResourceId& resourceId) const; + Ret writePluginsInfo(); - std::unordered_map m_pluginInfoMap; + bool m_loaded = false; + std::map m_pluginInfoMap; std::set m_pluginPaths; }; } diff --git a/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp b/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp index 9237083557f65..72255e5fb6c33 100644 --- a/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp +++ b/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp @@ -129,7 +129,7 @@ mu::Ret RegisterAudioPluginsScenario::registerPlugin(const io::path_t& pluginPat for (const AudioResourceMeta& meta : metaList.val) { AudioPluginInfo info; - info.type = audioPluginTypeFromCategoriesString(meta.attributeVal(audio::CATEGORIES_ATTRIBUTE).toStdString()); + info.type = audioPluginTypeFromCategoriesString(meta.attributeVal(audio::CATEGORIES_ATTRIBUTE)); info.meta = meta; info.path = pluginPath; info.enabled = true; @@ -153,6 +153,12 @@ mu::Ret RegisterAudioPluginsScenario::registerFailedPlugin(const io::path_t& plu AudioPluginInfo info; info.meta.id = io::filename(pluginPath).toStdString(); + + std::string ext = io::suffix(pluginPath); + if (ext.find("vst") != std::string::npos) { + info.meta.type = AudioResourceType::VstPlugin; + } + info.path = pluginPath; info.enabled = false; info.errorCode = failCode; diff --git a/src/framework/audio/tests/knownaudiopluginsregistertest.cpp b/src/framework/audio/tests/knownaudiopluginsregistertest.cpp index dddff08248adf..1c7690919901c 100644 --- a/src/framework/audio/tests/knownaudiopluginsregistertest.cpp +++ b/src/framework/audio/tests/knownaudiopluginsregistertest.cpp @@ -47,39 +47,55 @@ class Audio_KnownAudioPluginsRegisterTest : public ::testing::Test m_knownPlugins->setfileSystem(m_fileSystem); m_knownPlugins->setconfiguration(m_configuration); - m_knownAudioPluginsDir = "/test/some dir/audio_plugins"; - ON_CALL(*m_configuration, knownAudioPluginsDir()) - .WillByDefault(Return(m_knownAudioPluginsDir)); + m_knownAudioPluginsFilePath = "/test/some dir/known_audio_plugins.json"; + ON_CALL(*m_configuration, knownAudioPluginsFilePath()) + .WillByDefault(Return(m_knownAudioPluginsFilePath)); } - ByteArray pluginInfoToJson(const AudioPluginInfo& info) const + ByteArray pluginInfoListToJson(const std::vector& infoList) const { const std::map RESOURCE_TYPE_TO_STR { { AudioResourceType::VstPlugin, "VstPlugin" }, }; - JsonObject attributesObj; - for (auto it = info.meta.attributes.cbegin(); it != info.meta.attributes.cend(); ++it) { - attributesObj.set(it->first.toStdString(), it->second.toStdString()); - } + JsonArray array; + + for (const AudioPluginInfo& info : infoList) { + JsonObject attributesObj; + for (auto it = info.meta.attributes.cbegin(); it != info.meta.attributes.cend(); ++it) { + if (it->first == audio::PLAYBACK_SETUP_DATA_ATTRIBUTE) { + continue; + } + + attributesObj.set(it->first.toStdString(), it->second.toStdString()); + } + + JsonObject metaObj; + metaObj.set("id", info.meta.id); + metaObj.set("type", mu::value(RESOURCE_TYPE_TO_STR, info.meta.type, "Undefined")); + metaObj.set("hasNativeEditorSupport", info.meta.hasNativeEditorSupport); + + if (!info.meta.vendor.empty()) { + metaObj.set("vendor", info.meta.vendor); + } - JsonObject metaObj; - metaObj.set("id", info.meta.id); - metaObj.set("type", mu::value(RESOURCE_TYPE_TO_STR, info.meta.type, "Undefined")); - metaObj.set("vendor", info.meta.vendor); - metaObj.set("attributes", attributesObj); - metaObj.set("hasNativeEditorSupport", info.meta.hasNativeEditorSupport); + if (!attributesObj.empty()) { + metaObj.set("attributes", attributesObj); + } - JsonObject mainObj; - mainObj.set("meta", metaObj); - mainObj.set("path", info.path.toStdString()); - mainObj.set("enabled", info.enabled); + JsonObject mainObj; + mainObj.set("meta", metaObj); + mainObj.set("path", info.path.toStdString()); + mainObj.set("enabled", info.enabled); - if (info.errorCode != 0) { - mainObj.set("errorCode", info.errorCode); + if (info.errorCode != 0) { + mainObj.set("errorCode", info.errorCode); + } + + array << mainObj; } - return JsonDocument(mainObj).toJson(); + return JsonDocument(array).toJson(); } std::vector setupTestData() @@ -119,42 +135,18 @@ class Audio_KnownAudioPluginsRegisterTest : public ::testing::Test disabledPluginInfo.errorCode = -1; plugins.push_back(disabledPluginInfo); - paths_t infoPaths; - for (const AudioPluginInfo& info : plugins) { - infoPaths.push_back(pluginInfoPath(info.meta.vendor, info.meta.id)); - } - - ON_CALL(*m_fileSystem, scanFiles(m_knownAudioPluginsDir, std::vector { "*.json" }, ScanMode::FilesInCurrentDir)) - .WillByDefault(Return(mu::RetVal::make_ok({ infoPaths }))); - - for (size_t i = 0; i < infoPaths.size(); ++i) { - mu::ByteArray data = pluginInfoToJson(plugins[i]); - - ON_CALL(*m_fileSystem, readFile(infoPaths[i])) - .WillByDefault(Return(mu::RetVal::make_ok(data))); - } + mu::ByteArray data = pluginInfoListToJson(plugins); + ON_CALL(*m_fileSystem, readFile(m_knownAudioPluginsFilePath)) + .WillByDefault(Return(mu::RetVal::make_ok(data))); return plugins; } - io::path_t pluginInfoPath(const AudioResourceVendor& vendor, const AudioResourceId& resourceId) const - { - io::path_t fileName; - - if (vendor.empty()) { - fileName = io::escapeFileName(resourceId); - } else { - fileName = io::escapeFileName(vendor + "_" + resourceId); - } - - return m_knownAudioPluginsDir + "/" + fileName + ".json"; - } - std::shared_ptr m_knownPlugins; std::shared_ptr m_fileSystem; std::shared_ptr m_configuration; - path_t m_knownAudioPluginsDir; + path_t m_knownAudioPluginsFilePath; }; inline bool operator==(const AudioPluginInfo& info1, const AudioPluginInfo& info2) @@ -174,8 +166,15 @@ TEST_F(Audio_KnownAudioPluginsRegisterTest, PluginInfoList) // [GIVEN] All known plugins std::vector expectedPluginInfoList = setupTestData(); + // [GIVEN] File exists + ON_CALL(*m_fileSystem, exists(m_knownAudioPluginsFilePath)) + .WillByDefault(Return(mu::make_ok())); + // [WHEN] Load the info - m_knownPlugins->load(); + mu::Ret ret = m_knownPlugins->load(); + + // [THEN] Successfully loaded the info + EXPECT_TRUE(ret); // [WHEN] Request the info std::vector actualPluginInfoList = m_knownPlugins->pluginInfoList(); @@ -196,19 +195,19 @@ TEST_F(Audio_KnownAudioPluginsRegisterTest, PluginInfoList) // [GIVEN] New plugin for registration AudioPluginInfo newPluginInfo; newPluginInfo.type = AudioPluginType::Instrument; - newPluginInfo.meta.id = "new/plugin"; + newPluginInfo.meta.id = "DDD"; newPluginInfo.meta.type = AudioResourceType::VstPlugin; newPluginInfo.path = "/path/to/new/plugin/plugin.vst"; newPluginInfo.enabled = true; expectedPluginInfoList.push_back(newPluginInfo); - // [THEN] The plugin will be written to the corresponding file - mu::ByteArray expectedNewPluginData = pluginInfoToJson(newPluginInfo); - EXPECT_CALL(*m_fileSystem, writeFile(pluginInfoPath(newPluginInfo.meta.vendor, newPluginInfo.meta.id), expectedNewPluginData)) + // [THEN] All the plugins will be written to the file + mu::ByteArray expectedNewPluginsData = pluginInfoListToJson(expectedPluginInfoList); + EXPECT_CALL(*m_fileSystem, writeFile(m_knownAudioPluginsFilePath, expectedNewPluginsData)) .WillOnce(Return(mu::make_ok())); // [WHEN] Register it - mu::Ret ret = m_knownPlugins->registerPlugin(newPluginInfo); + ret = m_knownPlugins->registerPlugin(newPluginInfo); // [THEN] The plugin successfully registered EXPECT_TRUE(ret); @@ -222,12 +221,15 @@ TEST_F(Audio_KnownAudioPluginsRegisterTest, PluginInfoList) EXPECT_EQ(actualInfo.path, m_knownPlugins->pluginPath(actualInfo.meta.id)); } - // [WHEN] Unregister the first plugin in the list + // [GIVEN] We want to unregister the first plugin in the list AudioPluginInfo unregisteredPlugin = mu::takeFirst(expectedPluginInfoList); - EXPECT_CALL(*m_fileSystem, remove(pluginInfoPath(unregisteredPlugin.meta.vendor, unregisteredPlugin.meta.id), false)) + // [THEN] All the plugins will be written to the file (except the unregistered one) + expectedNewPluginsData = pluginInfoListToJson(expectedPluginInfoList); + EXPECT_CALL(*m_fileSystem, writeFile(m_knownAudioPluginsFilePath, expectedNewPluginsData)) .WillOnce(Return(mu::make_ok())); + // [WHEN] Unregister the plugin ret = m_knownPlugins->unregisterPlugin(unregisteredPlugin.meta.id); // [THEN] The plugin successfully unregistered diff --git a/src/framework/audio/tests/mocks/audioconfigurationmock.h b/src/framework/audio/tests/mocks/audioconfigurationmock.h index efad29c030b7d..4b2449411cbcf 100644 --- a/src/framework/audio/tests/mocks/audioconfigurationmock.h +++ b/src/framework/audio/tests/mocks/audioconfigurationmock.h @@ -61,7 +61,7 @@ class AudioConfigurationMock : public IAudioConfiguration MOCK_METHOD(async::Notification, synthesizerStateChanged, (), (const, override)); MOCK_METHOD(async::Notification, synthesizerStateGroupChanged, (const std::string&), (const, override)); - MOCK_METHOD(io::path_t, knownAudioPluginsDir, (), (const, override)); + MOCK_METHOD(io::path_t, knownAudioPluginsFilePath, (), (const, override)); }; } diff --git a/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp b/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp index bfb1abb7dfe93..c9f3c6b68623a 100644 --- a/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp +++ b/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp @@ -289,6 +289,7 @@ TEST_F(Audio_RegisterAudioPluginsScenarioTest, RegisterFailedPlugin) // [THEN] The plugin has been registered AudioPluginInfo expectedPluginInfo; expectedPluginInfo.meta.id = mu::io::filename(pluginPath).toStdString(); + expectedPluginInfo.meta.type = AudioResourceType::VstPlugin; expectedPluginInfo.path = pluginPath; expectedPluginInfo.enabled = false; expectedPluginInfo.errorCode = -42; diff --git a/src/framework/vst/internal/vstpluginmetareader.cpp b/src/framework/vst/internal/vstpluginmetareader.cpp index e80137e4076da..ac9b8321ec295 100644 --- a/src/framework/vst/internal/vstpluginmetareader.cpp +++ b/src/framework/vst/internal/vstpluginmetareader.cpp @@ -66,11 +66,11 @@ mu::RetVal VstPluginMetaReader::readMeta(const io::path_t audio::AudioResourceMeta meta; meta.id = classInfo.name(); meta.type = audio::AudioResourceType::VstPlugin; - meta.attributes.insert({ audio::CATEGORIES_ATTRIBUTE, String::fromStdString(classInfo.subCategoriesString()) }); + meta.attributes.emplace(audio::CATEGORIES_ATTRIBUTE, String::fromStdString(classInfo.subCategoriesString())); meta.vendor = classInfo.vendor(); meta.hasNativeEditorSupport = hasNativeEditorSupport(); - result.push_back(meta); + result.emplace_back(std::move(meta)); } return RetVal::make_ok(result); diff --git a/src/stubs/framework/audio/audioconfigurationstub.cpp b/src/stubs/framework/audio/audioconfigurationstub.cpp index d9ccb969bfaf5..b95910580569d 100644 --- a/src/stubs/framework/audio/audioconfigurationstub.cpp +++ b/src/stubs/framework/audio/audioconfigurationstub.cpp @@ -136,7 +136,7 @@ async::Notification AudioConfigurationStub::synthesizerStateGroupChanged(const s return async::Notification(); } -io::path_t AudioConfigurationStub::knownAudioPluginsDir() const +io::path_t AudioConfigurationStub::knownAudioPluginsFilePath() const { return {}; } diff --git a/src/stubs/framework/audio/audioconfigurationstub.h b/src/stubs/framework/audio/audioconfigurationstub.h index 0138cadd5f344..9ad5f1e7183f3 100644 --- a/src/stubs/framework/audio/audioconfigurationstub.h +++ b/src/stubs/framework/audio/audioconfigurationstub.h @@ -60,7 +60,7 @@ class AudioConfigurationStub : public IAudioConfiguration async::Notification synthesizerStateChanged() const override; async::Notification synthesizerStateGroupChanged(const std::string& groupName) const override; - io::path_t knownAudioPluginsDir() const override; + io::path_t knownAudioPluginsFilePath() const override; }; } From aec481109aa759bbfe6c874012fb29768f8deb58 Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Wed, 21 Jun 2023 12:58:05 +0300 Subject: [PATCH 4/6] make sure we can register two plugins with the same resourceId but with different location (the same plugin is installed in two different places) --- .../plugins/knownaudiopluginsregister.cpp | 18 ++++++++++++++---- .../plugins/knownaudiopluginsregister.h | 2 +- .../tests/knownaudiopluginsregistertest.cpp | 17 ++++++++++++++++- src/framework/global/containers.h | 6 ++++++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp b/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp index 0d4ec357f1947..69878dfba4485 100644 --- a/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp +++ b/src/framework/audio/internal/plugins/knownaudiopluginsregister.cpp @@ -187,7 +187,14 @@ mu::Ret KnownAudioPluginsRegister::registerPlugin(const AudioPluginInfo& info) return false; } - m_pluginInfoMap[info.meta.id] = info; + auto it = m_pluginInfoMap.find(info.meta.id); + if (it != m_pluginInfoMap.end()) { + IF_ASSERT_FAILED(it->second.path != info.path) { + return false; + } + } + + m_pluginInfoMap.emplace(info.meta.id, info); m_pluginPaths.insert(info.path); Ret ret = writePluginsInfo(); @@ -204,10 +211,13 @@ mu::Ret KnownAudioPluginsRegister::unregisterPlugin(const AudioResourceId& resou return make_ok(); } - AudioPluginInfo info = m_pluginInfoMap[resourceId]; + for (const auto& pair : m_pluginInfoMap) { + if (pair.first == resourceId) { + mu::remove(m_pluginPaths, pair.second.path); + } + } - mu::remove(m_pluginInfoMap, resourceId); - mu::remove(m_pluginPaths, info.path); + m_pluginInfoMap.erase(resourceId); Ret ret = writePluginsInfo(); return ret; diff --git a/src/framework/audio/internal/plugins/knownaudiopluginsregister.h b/src/framework/audio/internal/plugins/knownaudiopluginsregister.h index 102b8de1f0aeb..6063d9c7d854a 100644 --- a/src/framework/audio/internal/plugins/knownaudiopluginsregister.h +++ b/src/framework/audio/internal/plugins/knownaudiopluginsregister.h @@ -51,7 +51,7 @@ class KnownAudioPluginsRegister : public IKnownAudioPluginsRegister Ret writePluginsInfo(); bool m_loaded = false; - std::map m_pluginInfoMap; + std::multimap m_pluginInfoMap; std::set m_pluginPaths; }; } diff --git a/src/framework/audio/tests/knownaudiopluginsregistertest.cpp b/src/framework/audio/tests/knownaudiopluginsregistertest.cpp index 1c7690919901c..f202ca7e53f9a 100644 --- a/src/framework/audio/tests/knownaudiopluginsregistertest.cpp +++ b/src/framework/audio/tests/knownaudiopluginsregistertest.cpp @@ -212,13 +212,28 @@ TEST_F(Audio_KnownAudioPluginsRegisterTest, PluginInfoList) // [THEN] The plugin successfully registered EXPECT_TRUE(ret); + // [GIVEN] Same plugin (with the same resourceId) is installed in a different location + AudioPluginInfo duplicatedPluginInfo = newPluginInfo; + duplicatedPluginInfo.path = "/another/path/to/new/plugin/plugin.vst"; + expectedPluginInfoList.push_back(duplicatedPluginInfo); + + // [THEN] All the plugins will be written to the file + expectedNewPluginsData = pluginInfoListToJson(expectedPluginInfoList); + EXPECT_CALL(*m_fileSystem, writeFile(m_knownAudioPluginsFilePath, expectedNewPluginsData)) + .WillOnce(Return(mu::make_ok())); + + // [WHEN] Register it + ret = m_knownPlugins->registerPlugin(duplicatedPluginInfo); + + // [THEN] The duplicated plugin successfully registered + EXPECT_TRUE(ret); + actualPluginInfoList = m_knownPlugins->pluginInfoList(); EXPECT_EQ(expectedPluginInfoList.size(), actualPluginInfoList.size()); for (const AudioPluginInfo& actualInfo : actualPluginInfoList) { EXPECT_TRUE(mu::contains(expectedPluginInfoList, actualInfo)); EXPECT_TRUE(m_knownPlugins->exists(actualInfo.path)); EXPECT_TRUE(m_knownPlugins->exists(actualInfo.meta.id)); - EXPECT_EQ(actualInfo.path, m_knownPlugins->pluginPath(actualInfo.meta.id)); } // [GIVEN] We want to unregister the first plugin in the list diff --git a/src/framework/global/containers.h b/src/framework/global/containers.h index 6493b1acaeff9..7e1675412d0fb 100644 --- a/src/framework/global/containers.h +++ b/src/framework/global/containers.h @@ -216,6 +216,12 @@ inline bool contains(const std::map& m, const K& k) return m.find(k) != m.cend(); } +template +inline bool contains(const std::multimap& m, const K& k) +{ + return m.find(k) != m.cend(); +} + template inline bool contains(const std::unordered_map& m, const K& k) { From a33d4ed327d82943748ce3e622df0ed4d821c30d Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Tue, 4 Jul 2023 16:33:36 +0300 Subject: [PATCH 5/6] use io::completeBasename for generating resourceId as it was before PR #16990 (to maintain backward compatibility with old projects) --- .../audio/internal/plugins/registeraudiopluginsscenario.cpp | 2 +- .../audio/tests/registeraudiopluginsscenariotest.cpp | 2 +- src/framework/vst/internal/vstplugin.cpp | 4 ---- src/framework/vst/internal/vstpluginmetareader.cpp | 3 ++- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp b/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp index 72255e5fb6c33..75ec4174ec8a8 100644 --- a/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp +++ b/src/framework/audio/internal/plugins/registeraudiopluginsscenario.cpp @@ -152,7 +152,7 @@ mu::Ret RegisterAudioPluginsScenario::registerFailedPlugin(const io::path_t& plu } AudioPluginInfo info; - info.meta.id = io::filename(pluginPath).toStdString(); + info.meta.id = io::completeBasename(pluginPath).toStdString(); std::string ext = io::suffix(pluginPath); if (ext.find("vst") != std::string::npos) { diff --git a/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp b/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp index c9f3c6b68623a..9170d683d48b1 100644 --- a/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp +++ b/src/framework/audio/tests/registeraudiopluginsscenariotest.cpp @@ -288,7 +288,7 @@ TEST_F(Audio_RegisterAudioPluginsScenarioTest, RegisterFailedPlugin) // [THEN] The plugin has been registered AudioPluginInfo expectedPluginInfo; - expectedPluginInfo.meta.id = mu::io::filename(pluginPath).toStdString(); + expectedPluginInfo.meta.id = mu::io::completeBasename(pluginPath).toStdString(); expectedPluginInfo.meta.type = AudioResourceType::VstPlugin; expectedPluginInfo.path = pluginPath; expectedPluginInfo.enabled = false; diff --git a/src/framework/vst/internal/vstplugin.cpp b/src/framework/vst/internal/vstplugin.cpp index 5d494880a2beb..865bdc31bcc24 100644 --- a/src/framework/vst/internal/vstplugin.cpp +++ b/src/framework/vst/internal/vstplugin.cpp @@ -108,10 +108,6 @@ void VstPlugin::load() continue; } - if (classInfo.name() != m_resourceId) { - continue; - } - m_pluginProvider = std::make_unique(factory, classInfo); m_classInfo = classInfo; break; diff --git a/src/framework/vst/internal/vstpluginmetareader.cpp b/src/framework/vst/internal/vstpluginmetareader.cpp index ac9b8321ec295..a417054c0c075 100644 --- a/src/framework/vst/internal/vstpluginmetareader.cpp +++ b/src/framework/vst/internal/vstpluginmetareader.cpp @@ -64,13 +64,14 @@ mu::RetVal VstPluginMetaReader::readMeta(const io::path_t } audio::AudioResourceMeta meta; - meta.id = classInfo.name(); + meta.id = io::completeBasename(pluginPath).toStdString(); meta.type = audio::AudioResourceType::VstPlugin; meta.attributes.emplace(audio::CATEGORIES_ATTRIBUTE, String::fromStdString(classInfo.subCategoriesString())); meta.vendor = classInfo.vendor(); meta.hasNativeEditorSupport = hasNativeEditorSupport(); result.emplace_back(std::move(meta)); + break; } return RetVal::make_ok(result); From 4d2816d8bf4c8668b60a467bb60634b6eb5d3aae Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Tue, 4 Jul 2023 16:55:23 +0300 Subject: [PATCH 6/6] register vst3 files without any audio effect inside as failed plugins --- src/framework/vst/internal/vstpluginmetareader.cpp | 4 ++++ src/framework/vst/vsterrors.h | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/framework/vst/internal/vstpluginmetareader.cpp b/src/framework/vst/internal/vstpluginmetareader.cpp index a417054c0c075..9118a041a956e 100644 --- a/src/framework/vst/internal/vstpluginmetareader.cpp +++ b/src/framework/vst/internal/vstpluginmetareader.cpp @@ -74,5 +74,9 @@ mu::RetVal VstPluginMetaReader::readMeta(const io::path_t break; } + if (result.empty()) { + return make_ret(Err::NoAudioEffect); + } + return RetVal::make_ok(result); } diff --git a/src/framework/vst/vsterrors.h b/src/framework/vst/vsterrors.h index fb7815a2bd472..777ac5ad9be97 100644 --- a/src/framework/vst/vsterrors.h +++ b/src/framework/vst/vsterrors.h @@ -35,7 +35,8 @@ enum class Err { NoPluginFactory = 1402, NoPluginProvider = 1403, NoPluginController = 1404, - NoPluginWithId = 1405 + NoPluginWithId = 1405, + NoAudioEffect = 1406, }; inline Ret make_ret(Err e) @@ -50,6 +51,7 @@ inline Ret make_ret(Err e) case Err::NoPluginProvider: return Ret(retCode, "No VST3 audio module class found"); case Err::NoPluginController: return Ret(retCode, "No VST3 editor controller class found"); case Err::NoPluginWithId: return Ret(retCode, "VST3 plugin is not found"); + case Err::NoAudioEffect: return Ret(retCode, "VST3 file contains no audio effect"); } return Ret(retCode);