diff --git a/approot/messages_es.xml b/approot/messages_es.xml index bf9f2e0c..7f47b917 100644 --- a/approot/messages_es.xml +++ b/approot/messages_es.xml @@ -73,9 +73,9 @@ Ruta raíz -Permitir la fusión de artistas sin MBID con aquellos que tienen uno +Permitir la fusión de artistas sin MBID con aquellos que sí lo tienen Delimitadores usados para separar las etiquetas de los artistas -Artistas que no se deben dividir usando los delimitadores (un artista por línea) +Artistas que no se deben separar usando los delimitadores (un artista por línea) Diariamente Delimitadores usados para separar otras etiquetas Etiquetas adicionales a escanear diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index 71640710..a0318ed8 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -35,7 +35,7 @@ namespace lms::db { namespace { - static constexpr Version LMS_DATABASE_VERSION{ 88 }; + static constexpr Version LMS_DATABASE_VERSION{ 90 }; } VersionInfo::VersionInfo() @@ -1164,7 +1164,7 @@ FROM tracklist)"); dropIndexes(session); // Artist merging feature - utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN allow_mbid_artist_merge BOLLEAN DEFAULT(false)"); + utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN allow_mbid_artist_merge BOOLEAN DEFAULT(false)"); utils::executeCommand(*session.getDboSession(), "ALTER TABLE track_artist_link ADD COLUMN artist_name TEXT NULL DEFAULT('')"); utils::executeCommand(*session.getDboSession(), "ALTER TABLE track_artist_link ADD COLUMN artist_sort_name TEXT NULL DEFAULT('')"); @@ -1180,13 +1180,30 @@ FROM tracklist)"); void migrateFromV86(Session& session) { - utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN name TEXT NON NULL DEFAULT('')"); + utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN name TEXT NOT NULL DEFAULT('')"); utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings RENAME COLUMN scan_version TO audio_scan_version"); } void migrateFromV87(Session& session) { - utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN artists_to_not_split TEXT NON NULL DEFAULT('')"); + utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN artists_to_not_split TEXT NOT NULL DEFAULT('')"); + } + + void migrateFromV88(Session& session) + { + // Badly populated mbid_matched fields for tracks and artist info: need to rescan everything + // Just increment the scan version of the settings to make the next scan rescan everything + utils::executeCommand(*session.getDboSession(), "UPDATE scan_settings SET audio_scan_version = audio_scan_version + 1"); + } + + void migrateFromV89(Session& session) + { + // ArtistInfo need to be force rescanned: introduced a field for this + utils::executeCommand(*session.getDboSession(), "ALTER TABLE scan_settings ADD COLUMN artist_info_scan_version INTEGER NOT NULL DEFAULT(0)"); + utils::executeCommand(*session.getDboSession(), "ALTER TABLE artist_info ADD COLUMN scan_version INTEGER NOT NULL DEFAULT(0)"); + + // Just increment the scan version of the settings to make the next scan rescan everything + utils::executeCommand(*session.getDboSession(), "UPDATE scan_settings SET artist_info_scan_version = artist_info_scan_version + 1"); } bool doDbMigration(Session& session) @@ -1253,6 +1270,8 @@ FROM tracklist)"); { 85, migrateFromV85 }, { 86, migrateFromV86 }, { 87, migrateFromV87 }, + { 88, migrateFromV88 }, + { 89, migrateFromV89 }, }; bool migrationPerformed{}; diff --git a/src/libs/database/include/database/ArtistInfo.hpp b/src/libs/database/include/database/ArtistInfo.hpp index 9842e6b6..381fbca2 100644 --- a/src/libs/database/include/database/ArtistInfo.hpp +++ b/src/libs/database/include/database/ArtistInfo.hpp @@ -55,6 +55,7 @@ namespace lms::db static void findWithArtistNameAmbiguity(Session& session, std::optional range, bool allowArtistMBIDFallback, const std::function& func); // getters + std::size_t getScanVersion() const { return _scanVersion; } const std::filesystem::path& getAbsoluteFilePath() const { return _absoluteFilePath; } const Wt::WDateTime& getLastWriteTime() const { return _fileLastWrite; } ObjectPtr getDirectory() const; @@ -69,6 +70,7 @@ namespace lms::db bool isMBIDMatched() const { return _MBIDMatched; } // setters + void setScanVersion(std::size_t version) { _scanVersion = version; } void setAbsoluteFilePath(const std::filesystem::path& filePath); void setLastWriteTime(Wt::WDateTime time) { _fileLastWrite = time; } void setDirectory(ObjectPtr directory); @@ -84,6 +86,7 @@ namespace lms::db template void persist(Action& a) { + Wt::Dbo::field(a, _scanVersion, "scan_version"); Wt::Dbo::field(a, _absoluteFilePath, "absolute_file_path"); Wt::Dbo::field(a, _fileLastWrite, "file_last_write"); @@ -104,6 +107,8 @@ namespace lms::db friend class Session; static pointer create(Session& session); + int _scanVersion{}; + // Set when coming from artist info file std::filesystem::path _absoluteFilePath; std::string _fileStem; diff --git a/src/libs/database/include/database/ScanSettings.hpp b/src/libs/database/include/database/ScanSettings.hpp index 4e54ad2f..53af481f 100644 --- a/src/libs/database/include/database/ScanSettings.hpp +++ b/src/libs/database/include/database/ScanSettings.hpp @@ -64,6 +64,7 @@ namespace lms::db // Getters std::size_t getAudioScanVersion() const { return _audioScanVersion; } + std::size_t getArtistInfoScanVersion() const { return _artistInfoScanVersion; } Wt::WTime getUpdateStartTime() const { return _startTime; } UpdatePeriod getUpdatePeriod() const { return _updatePeriod; } std::vector getExtraTagsToScan() const; @@ -90,6 +91,7 @@ namespace lms::db { Wt::Dbo::field(a, _name, "name"); Wt::Dbo::field(a, _audioScanVersion, "audio_scan_version"); + Wt::Dbo::field(a, _artistInfoScanVersion, "artist_info_scan_version"); Wt::Dbo::field(a, _startTime, "start_time"); Wt::Dbo::field(a, _updatePeriod, "update_period"); Wt::Dbo::field(a, _similarityEngineType, "similarity_engine_type"); @@ -111,6 +113,7 @@ namespace lms::db std::string _name; int _audioScanVersion{}; + int _artistInfoScanVersion{}; Wt::WTime _startTime = Wt::WTime{ 0, 0, 0 }; UpdatePeriod _updatePeriod{ UpdatePeriod::Never }; SimilarityEngineType _similarityEngineType{ SimilarityEngineType::Clusters }; diff --git a/src/libs/metadata/bench/CMakeLists.txt b/src/libs/metadata/bench/CMakeLists.txt index aa093de4..4e3a392d 100644 --- a/src/libs/metadata/bench/CMakeLists.txt +++ b/src/libs/metadata/bench/CMakeLists.txt @@ -1,6 +1,12 @@ add_executable(bench-metadata LyricsBench.cpp + Metadata.cpp + ) + +target_include_directories(bench-metadata PRIVATE + ../impl + ../test ) target_link_libraries(bench-metadata PRIVATE diff --git a/src/libs/metadata/bench/LyricsBench.cpp b/src/libs/metadata/bench/LyricsBench.cpp index caa59ed5..2e1a213e 100644 --- a/src/libs/metadata/bench/LyricsBench.cpp +++ b/src/libs/metadata/bench/LyricsBench.cpp @@ -90,5 +90,3 @@ namespace lms::metadata::benchmarks BENCHMARK(BM_Lyrics); } // namespace lms::metadata::benchmarks - -BENCHMARK_MAIN(); \ No newline at end of file diff --git a/src/libs/metadata/bench/Metadata.cpp b/src/libs/metadata/bench/Metadata.cpp new file mode 100644 index 00000000..306788ca --- /dev/null +++ b/src/libs/metadata/bench/Metadata.cpp @@ -0,0 +1,149 @@ +/* + * Copyright (C) 2024 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include + +#include "AudioFileParser.hpp" +#include "TestTagReader.hpp" +#include "core/String.hpp" +#include "metadata/Types.hpp" + +namespace lms::metadata::benchmarks +{ + class TestAudioFileParser : public AudioFileParser + { + public: + using AudioFileParser::AudioFileParser; + using AudioFileParser::parseMetaData; + }; + + static void BM_Metadata_parse(benchmark::State& state) + { + AudioFileParserParameters params; + params.userExtraTags = { "MY_AWESOME_TAG_A", "MY_AWESOME_TAG_B", "MY_AWESOME_MISSING_TAG" }; + + std::unique_ptr testTags{ tests::createDefaultPopulatedTestTagReader() }; + const TestAudioFileParser parser{ params }; + for (auto _ : state) + { + std::unique_ptr track{ parser.parseMetaData(*testTags) }; + } + } + + static void BM_Metadata_parseArtists(benchmark::State& state) + { + const tests::TestTagReader testTags{ + { + { TagType::Artist, { "AC/DC; MyArtist" } }, + } + }; + + const AudioFileParserParameters params; + const TestAudioFileParser parser{ params }; + + for (auto _ : state) + { + std::unique_ptr track{ parser.parseMetaData(testTags) }; + } + } + + static void BM_Metadata_parseArtists_WithWhitelist(benchmark::State& state) + { + const tests::TestTagReader testTags{ + { + { TagType::Artist, { "AC/DC; MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/", ";" }; + // The list itself is not important, the idea is to have some volume + params.artistsToNotSplit = { "AC/DC", + "+/-", + R"(A/N【eɪ-ɛn)", + "Akron/Family", + "AM/FM", + "Ashes/Dust", + "B/B/S/", + "BLCK/MRKT/RGNS", + "Body/Gate/Head", + "Body/Head", + "Born/Dead", + "Burger/Ink", + "case/lang/veirs", + "Chicago / London Underground", + "Dakota/Dakota", + "Dark/Light", + "Decades/Failures", + "The Denison/Kimball Trio", + "D-W/L-SS", + "F/i", + "Friend / Enemy", + "GZA/Genius", + "I/O", + "I/O3", + "In/Humanity", + "Love/Lust", + "Mirror/Dash", + "Model/Actress", + "N/N", + "Neither/Neither World", + "P1/E", + "Sick/Tired", + "t/e/u/", + "tide/edit", + "V/Vm", + "White/Lichens", + "White/Light", + "Yamantaka // Sonic Titan" }; + + const TestAudioFileParser parser{ params }; + for (auto _ : state) + { + std::unique_ptr track{ parser.parseMetaData(testTags) }; + } + } + + static void BM_Metadata_parseArtists_WithoutWhitelist(benchmark::State& state) + { + const tests::TestTagReader testTags{ + { + { TagType::Artist, { "AC/DC; MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/", ";" }; + + const TestAudioFileParser parser{ params }; + + for (auto _ : state) + { + std::unique_ptr track{ parser.parseMetaData(testTags) }; + } + } + + BENCHMARK(BM_Metadata_parse); + BENCHMARK(BM_Metadata_parseArtists); + BENCHMARK(BM_Metadata_parseArtists_WithWhitelist); + BENCHMARK(BM_Metadata_parseArtists_WithoutWhitelist); + +} // namespace lms::metadata::benchmarks + +BENCHMARK_MAIN(); \ No newline at end of file diff --git a/src/libs/metadata/impl/AudioFileParser.cpp b/src/libs/metadata/impl/AudioFileParser.cpp index 02e99ccd..a75d7c03 100644 --- a/src/libs/metadata/impl/AudioFileParser.cpp +++ b/src/libs/metadata/impl/AudioFileParser.cpp @@ -20,6 +20,8 @@ #include "AudioFileParser.hpp" #include +#include +#include #include "core/ILogger.hpp" #include "core/PartialDateTime.hpp" @@ -64,7 +66,6 @@ namespace lms::metadata template void addTagIfNonEmpty(std::vector& res, std::string_view tag) { - tag = core::stringUtils::stringTrim(tag); if (tag.empty()) return; @@ -81,22 +82,62 @@ namespace lms::metadata { tagReader.visitTagValues(tagType, [&](std::string_view value) { value = core::stringUtils::stringTrim(value); - if (!whitelist || !whitelist->contains(value)) - { - for (std::string_view tagDelimiter : tagDelimiters) - { - if (value.find(tagDelimiter) != std::string_view::npos) - { - for (std::string_view splitTag : core::stringUtils::splitString(value, tagDelimiters)) - addTagIfNonEmpty(res, splitTag); - return; - } + // short path: no custom delimiter + if (tagDelimiters.empty()) + { + addTagIfNonEmpty(res, value); + return; + } + + // Algo: + // 1. replace whitelist entries by placeholders + // 2. apply delimiters + // 3. replace whitelist entries back + + constexpr std::string_view substitutionPrefix{ "__LMS_ENTRY__" }; + std::unordered_map substitutionMap; + std::string strToSplit{ value }; + if (whitelist) + { + std::size_t counter{}; + + for (std::string_view whiteListEntry : *whitelist) + { + whiteListEntry = core::stringUtils::stringTrim(whiteListEntry); + + const std::string::size_type pos{ strToSplit.find(whiteListEntry) }; + if (pos == std::string::npos) + continue; + + std::string substitutionStr{ std::string{ substitutionPrefix } + std::to_string(counter++) }; + strToSplit.replace(pos, whiteListEntry.size(), substitutionStr); + substitutionMap.emplace(std::move(substitutionStr), whiteListEntry); } } - // no delimiter found, or no delimiter to be used - addTagIfNonEmpty(res, value); + for (std::string_view strSplit : core::stringUtils::splitString(strToSplit, tagDelimiters)) + { + std::string str{ core::stringUtils::stringTrim(strSplit) }; + + while (true) + { + std::string::size_type prefixPos{ str.find(substitutionPrefix) }; + if (prefixPos == std::string::npos) + break; + + std::string::size_type counterEnd{ prefixPos + substitutionPrefix.size() }; + while (std::isdigit(str[counterEnd])) + counterEnd++; + + std::string substitutionStr{ str.substr(prefixPos, counterEnd - prefixPos) }; + auto it{ substitutionMap.find(substitutionStr) }; + if (it != std::cend(substitutionMap)) + str.replace(prefixPos, counterEnd - prefixPos, it->second); + } + + addTagIfNonEmpty(res, str); + } }); if (!res.empty()) @@ -321,7 +362,7 @@ namespace lms::metadata return fileExtensions; } - std::unique_ptr AudioFileParser::parseMetaData(const std::filesystem::path& p) + std::unique_ptr AudioFileParser::parseMetaData(const std::filesystem::path& p) const { try { @@ -348,7 +389,7 @@ namespace lms::metadata } } - void AudioFileParser::parseImages(const std::filesystem::path& p, ImageVisitor visitor) + void AudioFileParser::parseImages(const std::filesystem::path& p, ImageVisitor visitor) const { try { @@ -375,7 +416,7 @@ namespace lms::metadata } } - std::unique_ptr AudioFileParser::parseMetaData(const ITagReader& tagReader) + std::unique_ptr AudioFileParser::parseMetaData(const ITagReader& tagReader) const { auto track{ std::make_unique() }; @@ -385,7 +426,7 @@ namespace lms::metadata return track; } - void AudioFileParser::processTags(const ITagReader& tagReader, Track& track) + void AudioFileParser::processTags(const ITagReader& tagReader, Track& track) const { track.title = getTagValueAs(tagReader, TagType::TrackTitle).value_or(""); track.mbid = getTagValueAs(tagReader, TagType::MusicBrainzTrackID); @@ -452,7 +493,7 @@ namespace lms::metadata track.originalYear = track.originalDate.getYear(); } - std::optional AudioFileParser::getMedium(const ITagReader& tagReader) + std::optional AudioFileParser::getMedium(const ITagReader& tagReader) const { std::optional medium; medium.emplace(); @@ -482,7 +523,7 @@ namespace lms::metadata return medium; } - std::optional AudioFileParser::getRelease(const ITagReader& tagReader) + std::optional AudioFileParser::getRelease(const ITagReader& tagReader) const { std::optional release; diff --git a/src/libs/metadata/impl/AudioFileParser.hpp b/src/libs/metadata/impl/AudioFileParser.hpp index e7f8edf5..48176296 100644 --- a/src/libs/metadata/impl/AudioFileParser.hpp +++ b/src/libs/metadata/impl/AudioFileParser.hpp @@ -37,18 +37,18 @@ namespace lms::metadata AudioFileParser& operator=(const AudioFileParser&) = delete; protected: - std::unique_ptr parseMetaData(const std::filesystem::path& p) override; - std::unique_ptr parseMetaData(const ITagReader& reader); + std::unique_ptr parseMetaData(const std::filesystem::path& p) const override; + std::unique_ptr parseMetaData(const ITagReader& reader) const; static void parseImages(const IImageReader& reader, ImageVisitor visitor); private: - void parseImages(const std::filesystem::path& p, ImageVisitor visitor) override; + void parseImages(const std::filesystem::path& p, ImageVisitor visitor) const override; std::span getSupportedExtensions() const override; - void processTags(const ITagReader& reader, Track& track); + void processTags(const ITagReader& reader, Track& track) const; - std::optional getMedium(const ITagReader& tagReader); - std::optional getRelease(const ITagReader& tagReader); + std::optional getMedium(const ITagReader& tagReader) const; + std::optional getRelease(const ITagReader& tagReader) const; const AudioFileParserParameters _params; }; diff --git a/src/libs/metadata/include/metadata/IAudioFileParser.hpp b/src/libs/metadata/include/metadata/IAudioFileParser.hpp index 0d375556..38eb6694 100644 --- a/src/libs/metadata/include/metadata/IAudioFileParser.hpp +++ b/src/libs/metadata/include/metadata/IAudioFileParser.hpp @@ -33,10 +33,10 @@ namespace lms::metadata public: virtual ~IAudioFileParser() = default; - virtual std::unique_ptr parseMetaData(const std::filesystem::path& p) = 0; + virtual std::unique_ptr parseMetaData(const std::filesystem::path& p) const = 0; using ImageVisitor = std::function; - virtual void parseImages(const std::filesystem::path& p, ImageVisitor visitor) = 0; + virtual void parseImages(const std::filesystem::path& p, ImageVisitor visitor) const = 0; virtual std::span getSupportedExtensions() const = 0; }; diff --git a/src/libs/metadata/include/metadata/Types.hpp b/src/libs/metadata/include/metadata/Types.hpp index ebc03c52..873d71f5 100644 --- a/src/libs/metadata/include/metadata/Types.hpp +++ b/src/libs/metadata/include/metadata/Types.hpp @@ -22,10 +22,10 @@ #include #include #include +#include #include #include #include -#include #include #include "core/PartialDateTime.hpp" @@ -214,22 +214,17 @@ namespace lms::metadata Accurate, }; - struct WhiteListHash : std::hash, std::hash + struct SortByLengthDesc { - using is_transparent = void; - - [[nodiscard]] size_t operator()(std::string_view str) const + bool operator()(const std::string& a, const std::string& b) const { - return std::hash{}(str); - } - - [[nodiscard]] size_t operator()(const std::string& str) const - { - return std::hash{}(str); + if (a.length() != b.length()) + return a.length() > b.length(); + return a < b; // Break ties using lexicographical order } }; - using WhiteList = std::unordered_set>; + using WhiteList = std::set; struct AudioFileParserParameters { ParserBackend backend{ ParserBackend::TagLib }; diff --git a/src/libs/metadata/test/AudioFileParser.cpp b/src/libs/metadata/test/AudioFileParser.cpp index b3926e8c..3e90e5b6 100644 --- a/src/libs/metadata/test/AudioFileParser.cpp +++ b/src/libs/metadata/test/AudioFileParser.cpp @@ -25,9 +25,8 @@ #include "AudioFileParser.hpp" #include "TestTagReader.hpp" -#include "metadata/Types.hpp" -namespace lms::metadata +namespace lms::metadata::tests { class TestAudioFileParser : public AudioFileParser { @@ -42,68 +41,13 @@ namespace lms::metadata params.userExtraTags = { "MY_AWESOME_TAG_A", "MY_AWESOME_TAG_B", "MY_AWESOME_MISSING_TAG" }; TestAudioFileParser parser{ params }; - TestTagReader testTags{ - { - { TagType::AcoustID, { "e987a441-e134-4960-8019-274eddacc418" } }, - { TagType::Advisory, { "2" } }, - { TagType::Album, { "MyAlbum" } }, - { TagType::AlbumSortOrder, { "MyAlbumSortName" } }, - { TagType::Artist, { "MyArtist1 & MyArtist2" } }, - { TagType::Artists, { "MyArtist1", "MyArtist2" } }, - { TagType::ArtistSortOrder, { "MyArtist1SortName", "MyArtist2SortName" } }, - { TagType::AlbumArtist, { "MyAlbumArtist1 & MyAlbumArtist2" } }, - { TagType::AlbumArtists, { "MyAlbumArtist1", "MyAlbumArtist2" } }, - { TagType::AlbumArtistsSortOrder, { "MyAlbumArtist1SortName", "MyAlbumArtist2SortName" } }, - { TagType::AlbumComment, { "MyAlbumComment" } }, - { TagType::Barcode, { "MyBarcode" } }, - { TagType::Comment, { "Comment1", "Comment2" } }, - { TagType::Compilation, { "1" } }, - { TagType::Composer, { "MyComposer1", "MyComposer2" } }, - { TagType::ComposerSortOrder, { "MyComposerSortOrder1", "MyComposerSortOrder2" } }, - { TagType::Conductor, { "MyConductor1", "MyConductor2" } }, - { TagType::Copyright, { "MyCopyright" } }, - { TagType::CopyrightURL, { "MyCopyrightURL" } }, - { TagType::Date, { "2020/03/04" } }, - { TagType::DiscNumber, { "2" } }, - { TagType::DiscSubtitle, { "MySubtitle" } }, - { TagType::Genre, { "Genre1", "Genre2" } }, - { TagType::Grouping, { "Grouping1", "Grouping2" } }, - { TagType::Media, { "CD" } }, - { TagType::Mixer, { "MyMixer1", "MyMixer2" } }, - { TagType::Mood, { "Mood1", "Mood2" } }, - { TagType::MusicBrainzArtistID, { "9d2e0c8c-8c5e-4372-a061-590955eaeaae", "5e2cf87f-c8d7-4504-8a86-954dc0840229" } }, - { TagType::MusicBrainzTrackID, { "0afb190a-6735-46df-a16d-199f48206e4a" } }, - { TagType::MusicBrainzReleaseArtistID, { "6fbf097c-1487-43e8-874b-50dd074398a7", "5ed3d6b3-2aed-4a03-828c-3c4d4f7406e1" } }, - { TagType::MusicBrainzReleaseID, { "3fa39992-b786-4585-a70e-85d5cc15ef69" } }, - { TagType::MusicBrainzReleaseGroupID, { "5b1a5a44-8420-4426-9b86-d25dc8d04838" } }, - { TagType::MusicBrainzRecordingID, { "bd3fc666-89de-4ac8-93f6-2dbf028ad8d5" } }, - { TagType::Producer, { "MyProducer1", "MyProducer2" } }, - { TagType::Remixer, { "MyRemixer1", "MyRemixer2" } }, - { TagType::RecordLabel, { "Label1", "Label2" } }, - { TagType::ReleaseCountry, { "MyCountry1", "MyCountry2" } }, - { TagType::Language, { "Language1", "Language2" } }, - { TagType::Lyricist, { "MyLyricist1", "MyLyricist2" } }, - { TagType::OriginalReleaseDate, { "2019/02/03" } }, - { TagType::ReleaseType, { "Album", "Compilation" } }, - { TagType::ReplayGainTrackGain, { "-0.33" } }, - { TagType::ReplayGainAlbumGain, { "-0.5" } }, - { TagType::TrackTitle, { "MyTitle" } }, - { TagType::TrackNumber, { "7" } }, - { TagType::TotalTracks, { "12" } }, - { TagType::TotalDiscs, { "3" } }, - } - }; - testTags.setExtraUserTags({ { "MY_AWESOME_TAG_A", { "MyTagValue1ForTagA", "MyTagValue2ForTagA" } }, - { "MY_AWESOME_TAG_B", { "MyTagValue1ForTagB", "MyTagValue2ForTagB" } } }); - testTags.setPerformersTags({ { "RoleA", { "MyPerformer1ForRoleA", "MyPerformer2ForRoleA" } }, - { "RoleB", { "MyPerformer1ForRoleB", "MyPerformer2ForRoleB" } } }); - testTags.setLyricsTags({ { "eng", "[00:00.00]First line\n[00:01.00]Second line" } }); + std::unique_ptr testTags{ createDefaultPopulatedTestTagReader() }; - const std::unique_ptr track{ parser.parseMetaData(testTags) }; + const std::unique_ptr track{ parser.parseMetaData(*testTags) }; // Audio properties { - const AudioProperties& audioProperties{ testTags.getAudioProperties() }; + const AudioProperties& audioProperties{ testTags->getAudioProperties() }; EXPECT_EQ(track->audioProperties.bitrate, audioProperties.bitrate); EXPECT_EQ(track->audioProperties.bitsPerSample, audioProperties.bitsPerSample); EXPECT_EQ(track->audioProperties.channelCount, audioProperties.channelCount); @@ -328,7 +272,7 @@ namespace lms::metadata EXPECT_EQ(track->medium->release->artistDisplayName, "AC/DC"); } - TEST(AudioFileParser, customArtistDelimiters_whitelist_multi) + TEST(AudioFileParser, customArtistDelimiters_whitelist_multi_artists) { const TestTagReader testTags{ { @@ -339,6 +283,26 @@ namespace lms::metadata AudioFileParserParameters params; params.artistTagDelimiters = { "/" }; + params.artistsToNotSplit = { " AC/DC " }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "AC/DC"); + EXPECT_EQ(track->artists[1].name, "MyArtist"); + EXPECT_EQ(track->artistDisplayName, "AC/DC, MyArtist"); // Reconstructed since this use case is not handled + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_multi_separators_first) + { + const TestTagReader testTags{ + { + { TagType::Artist, { "AC/DC;MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/", ";" }; params.artistsToNotSplit = { "AC/DC" }; TestAudioFileParser parser{ params }; std::unique_ptr track{ parser.parseMetaData(testTags) }; @@ -349,6 +313,124 @@ namespace lms::metadata EXPECT_EQ(track->artistDisplayName, "AC/DC, MyArtist"); // Reconstructed since this use case is not handled } + TEST(AudioFileParser, customArtistDelimiters_whitelist_multi_separators_middle) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " MyArtist1; AC/DC ; MyArtist2 " } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/", ";" }; + params.artistsToNotSplit = { "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 3); + EXPECT_EQ(track->artists[0].name, "MyArtist1"); + EXPECT_EQ(track->artists[1].name, "AC/DC"); + EXPECT_EQ(track->artists[2].name, "MyArtist2"); + EXPECT_EQ(track->artistDisplayName, "MyArtist1, AC/DC, MyArtist2"); // Reconstructed since this use case is not handled + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_multi_separators_last) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " AC/DC; MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { ";", "/" }; + params.artistsToNotSplit = { "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "AC/DC"); + EXPECT_EQ(track->artists[1].name, "MyArtist"); + EXPECT_EQ(track->artistDisplayName, "AC/DC, MyArtist"); // Reconstructed since this use case is not handled + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_longest_first) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " AC/DC; MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { ";", "/" }; + params.artistsToNotSplit = { "AC", "DC", "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "AC/DC"); + EXPECT_EQ(track->artists[1].name, "MyArtist"); + EXPECT_EQ(track->artistDisplayName, "AC/DC, MyArtist"); // Reconstructed since this use case is not handled + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_partial_begin) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " AC/DC; MyArtist" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/" }; + params.artistsToNotSplit = { "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 1); + EXPECT_EQ(track->artists[0].name, "AC/DC; MyArtist"); + EXPECT_EQ(track->artistDisplayName, "AC/DC; MyArtist"); + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_partial_middle) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " MyArtist1; AC/DC ; MyArtist2" } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/" }; + params.artistsToNotSplit = { "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 1); + EXPECT_EQ(track->artists[0].name, "MyArtist1; AC/DC ; MyArtist2"); + EXPECT_EQ(track->artistDisplayName, "MyArtist1; AC/DC ; MyArtist2"); + } + + TEST(AudioFileParser, customArtistDelimiters_whitelist_partial_end) + { + const TestTagReader testTags{ + { + { TagType::Artist, { " MyArtist; AC/DC " } }, + } + }; + + AudioFileParserParameters params; + params.artistTagDelimiters = { "/" }; + params.artistsToNotSplit = { "AC/DC" }; + TestAudioFileParser parser{ params }; + std::unique_ptr track{ parser.parseMetaData(testTags) }; + + ASSERT_EQ(track->artists.size(), 1); + EXPECT_EQ(track->artists[0].name, "MyArtist; AC/DC"); + EXPECT_EQ(track->artistDisplayName, "MyArtist; AC/DC"); + } + TEST(AudioFileParser, customDelimiters_foundInArtist) { const TestTagReader testTags{ @@ -740,4 +822,4 @@ namespace lms::metadata doTest("2020/01", core::PartialDateTime{ 2020, 1 }); doTest("2020", core::PartialDateTime{ 2020 }); } -} // namespace lms::metadata +} // namespace lms::metadata::tests diff --git a/src/libs/metadata/test/TestTagReader.hpp b/src/libs/metadata/test/TestTagReader.hpp index 4977df63..35040052 100644 --- a/src/libs/metadata/test/TestTagReader.hpp +++ b/src/libs/metadata/test/TestTagReader.hpp @@ -18,13 +18,12 @@ */ #include +#include #include -#include - #include "ITagReader.hpp" -namespace lms::metadata +namespace lms::metadata::tests { class TestTagReader : public ITagReader { @@ -45,6 +44,9 @@ namespace lms::metadata : _tags{ std::move(tags) } { } + ~TestTagReader() override = default; + TestTagReader(const TestTagReader&) = delete; + TestTagReader& operator=(const TestTagReader&) = delete; void setPerformersTags(Performers&& performers) { @@ -103,4 +105,65 @@ namespace lms::metadata ExtraUserTags _extraUserTags; LyricsTags _lyricsTags; }; -} // namespace lms::metadata \ No newline at end of file + + inline std::unique_ptr createDefaultPopulatedTestTagReader() + { + std::unique_ptr testTags{ std::make_unique( + TestTagReader::Tags{ + { TagType::AcoustID, { "e987a441-e134-4960-8019-274eddacc418" } }, + { TagType::Advisory, { "2" } }, + { TagType::Album, { "MyAlbum" } }, + { TagType::AlbumSortOrder, { "MyAlbumSortName" } }, + { TagType::Artist, { "MyArtist1 & MyArtist2" } }, + { TagType::Artists, { "MyArtist1", "MyArtist2" } }, + { TagType::ArtistSortOrder, { "MyArtist1SortName", "MyArtist2SortName" } }, + { TagType::AlbumArtist, { "MyAlbumArtist1 & MyAlbumArtist2" } }, + { TagType::AlbumArtists, { "MyAlbumArtist1", "MyAlbumArtist2" } }, + { TagType::AlbumArtistsSortOrder, { "MyAlbumArtist1SortName", "MyAlbumArtist2SortName" } }, + { TagType::AlbumComment, { "MyAlbumComment" } }, + { TagType::Barcode, { "MyBarcode" } }, + { TagType::Comment, { "Comment1", "Comment2" } }, + { TagType::Compilation, { "1" } }, + { TagType::Composer, { "MyComposer1", "MyComposer2" } }, + { TagType::ComposerSortOrder, { "MyComposerSortOrder1", "MyComposerSortOrder2" } }, + { TagType::Conductor, { "MyConductor1", "MyConductor2" } }, + { TagType::Copyright, { "MyCopyright" } }, + { TagType::CopyrightURL, { "MyCopyrightURL" } }, + { TagType::Date, { "2020/03/04" } }, + { TagType::DiscNumber, { "2" } }, + { TagType::DiscSubtitle, { "MySubtitle" } }, + { TagType::Genre, { "Genre1", "Genre2" } }, + { TagType::Grouping, { "Grouping1", "Grouping2" } }, + { TagType::Media, { "CD" } }, + { TagType::Mixer, { "MyMixer1", "MyMixer2" } }, + { TagType::Mood, { "Mood1", "Mood2" } }, + { TagType::MusicBrainzArtistID, { "9d2e0c8c-8c5e-4372-a061-590955eaeaae", "5e2cf87f-c8d7-4504-8a86-954dc0840229" } }, + { TagType::MusicBrainzTrackID, { "0afb190a-6735-46df-a16d-199f48206e4a" } }, + { TagType::MusicBrainzReleaseArtistID, { "6fbf097c-1487-43e8-874b-50dd074398a7", "5ed3d6b3-2aed-4a03-828c-3c4d4f7406e1" } }, + { TagType::MusicBrainzReleaseID, { "3fa39992-b786-4585-a70e-85d5cc15ef69" } }, + { TagType::MusicBrainzReleaseGroupID, { "5b1a5a44-8420-4426-9b86-d25dc8d04838" } }, + { TagType::MusicBrainzRecordingID, { "bd3fc666-89de-4ac8-93f6-2dbf028ad8d5" } }, + { TagType::Producer, { "MyProducer1", "MyProducer2" } }, + { TagType::Remixer, { "MyRemixer1", "MyRemixer2" } }, + { TagType::RecordLabel, { "Label1", "Label2" } }, + { TagType::ReleaseCountry, { "MyCountry1", "MyCountry2" } }, + { TagType::Language, { "Language1", "Language2" } }, + { TagType::Lyricist, { "MyLyricist1", "MyLyricist2" } }, + { TagType::OriginalReleaseDate, { "2019/02/03" } }, + { TagType::ReleaseType, { "Album", "Compilation" } }, + { TagType::ReplayGainTrackGain, { "-0.33" } }, + { TagType::ReplayGainAlbumGain, { "-0.5" } }, + { TagType::TrackTitle, { "MyTitle" } }, + { TagType::TrackNumber, { "7" } }, + { TagType::TotalTracks, { "12" } }, + { TagType::TotalDiscs, { "3" } }, + }) }; + testTags->setExtraUserTags({ { "MY_AWESOME_TAG_A", { "MyTagValue1ForTagA", "MyTagValue2ForTagA" } }, + { "MY_AWESOME_TAG_B", { "MyTagValue1ForTagB", "MyTagValue2ForTagB" } } }); + testTags->setPerformersTags({ { "RoleA", { "MyPerformer1ForRoleA", "MyPerformer2ForRoleA" } }, + { "RoleB", { "MyPerformer1ForRoleB", "MyPerformer2ForRoleB" } } }); + testTags->setLyricsTags({ { "eng", "[00:00.00]First line\n[00:01.00]Second line" } }); + + return testTags; + } +} // namespace lms::metadata::tests \ No newline at end of file diff --git a/src/libs/services/scanner/impl/ScannerService.cpp b/src/libs/services/scanner/impl/ScannerService.cpp index b524eada..8b47c6fb 100644 --- a/src/libs/services/scanner/impl/ScannerService.cpp +++ b/src/libs/services/scanner/impl/ScannerService.cpp @@ -92,6 +92,7 @@ namespace lms::scanner settings.emplace(); settings->audioScanVersion = scanSettings->getAudioScanVersion(); + settings->artistInfoScanVersion = scanSettings->getArtistInfoScanVersion(); settings->startTime = scanSettings->getUpdateStartTime(); settings->updatePeriod = scanSettings->getUpdatePeriod(); @@ -420,7 +421,8 @@ namespace lms::scanner return; LMS_LOG(DBUPDATER, DEBUG, "Scanner settings updated"); - LMS_LOG(DBUPDATER, DEBUG, "Using audio scan settings version " << newSettings->audioScanVersion); + LMS_LOG(DBUPDATER, DEBUG, "Using audio scan version " << newSettings->audioScanVersion); + LMS_LOG(DBUPDATER, DEBUG, "Using artist info scan version " << newSettings->artistInfoScanVersion); _settings = std::move(*newSettings); if (!_lastScanSettings) diff --git a/src/libs/services/scanner/impl/ScannerSettings.hpp b/src/libs/services/scanner/impl/ScannerSettings.hpp index 733921ac..39a9ec90 100644 --- a/src/libs/services/scanner/impl/ScannerSettings.hpp +++ b/src/libs/services/scanner/impl/ScannerSettings.hpp @@ -36,6 +36,7 @@ namespace lms::scanner struct ScannerSettings { std::size_t audioScanVersion{}; + std::size_t artistInfoScanVersion{}; Wt::WTime startTime; db::ScanSettings::UpdatePeriod updatePeriod{ db::ScanSettings::UpdatePeriod::Never }; bool skipDuplicateTrackMBID{}; diff --git a/src/libs/services/scanner/impl/scanners/ArtistInfoFileScanner.cpp b/src/libs/services/scanner/impl/scanners/ArtistInfoFileScanner.cpp index bd8d0434..51382c26 100644 --- a/src/libs/services/scanner/impl/scanners/ArtistInfoFileScanner.cpp +++ b/src/libs/services/scanner/impl/scanners/ArtistInfoFileScanner.cpp @@ -127,6 +127,7 @@ namespace lms::scanner artistInfo.modify()->setAbsoluteFilePath(_file); } + artistInfo.modify()->setScanVersion(_settings.artistInfoScanVersion); artistInfo.modify()->setName(_parsedArtistInfo->name); artistInfo.modify()->setSortName(_parsedArtistInfo->sortName); artistInfo.modify()->setLastWriteTime(fileInfo->lastWriteTime); @@ -141,6 +142,7 @@ namespace lms::scanner const metadata::Artist artistMetadata{ _parsedArtistInfo->mbid, _parsedArtistInfo->name, _parsedArtistInfo->sortName.empty() ? std::nullopt : std::make_optional(_parsedArtistInfo->sortName) }; db::Artist::pointer artist{ helpers::getOrCreateArtist(dbSession, artistMetadata, helpers::AllowFallbackOnMBIDEntry{ _settings.allowArtistMBIDFallback }) }; artistInfo.modify()->setArtist(artist); + artistInfo.modify()->setMBIDMatched(_parsedArtistInfo->mbid.has_value() && _parsedArtistInfo->mbid == artist->getMBID()); if (added) { @@ -149,7 +151,7 @@ namespace lms::scanner } else { - LMS_LOG(DBUPDATER, DEBUG, "Updated artist info file '" << _file); + LMS_LOG(DBUPDATER, DEBUG, "Updated artist info file " << _file); stats.updates++; } } @@ -163,7 +165,7 @@ namespace lms::scanner core::LiteralString ArtistInfoFileScanner::getName() const { - return "Artist info scanner "; + return "Artist info scanner"; } std::span ArtistInfoFileScanner::getSupportedExtensions() const @@ -192,7 +194,9 @@ namespace lms::scanner db::Session& dbSession{ _db.getTLSSession() }; auto transaction{ dbSession.createReadTransaction() }; db::ArtistInfo::pointer artistInfo{ db::ArtistInfo::find(dbSession, file.file) }; - if (artistInfo && artistInfo->getLastWriteTime() == lastWriteTime) + if (artistInfo + && artistInfo->getLastWriteTime() == lastWriteTime + && artistInfo->getScanVersion() == _settings.artistInfoScanVersion) { context.stats.skips++; return false; diff --git a/src/libs/services/scanner/impl/scanners/AudioFileScanOperation.cpp b/src/libs/services/scanner/impl/scanners/AudioFileScanOperation.cpp index 8911e780..8ca0c14e 100644 --- a/src/libs/services/scanner/impl/scanners/AudioFileScanOperation.cpp +++ b/src/libs/services/scanner/impl/scanners/AudioFileScanOperation.cpp @@ -58,7 +58,7 @@ namespace lms::scanner { db::Artist::pointer artist{ helpers::getOrCreateArtist(session, artistInfo, allowArtistMBIDFallback) }; - const bool matchedUsingMbid{ artist->getMBID() == artistInfo.mbid }; + const bool matchedUsingMbid{ artistInfo.mbid.has_value() && artist->getMBID() == artistInfo.mbid }; db::TrackArtistLink::pointer link{ session.create(track, artist, linkType, role, matchedUsingMbid) }; link.modify()->setArtistName(artistInfo.name); if (artistInfo.sortName) diff --git a/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp b/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp index 29177e36..8882649b 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp @@ -41,7 +41,7 @@ namespace lms::scanner { std::ostream& operator<<(std::ostream& os, const db::Artist::pointer& artist) { - os << artist->getName(); + os << "'" << artist->getName() << "'"; if (const auto mbid{ artist->getMBID() }) os << " [" << mbid->getAsString() << "]"; @@ -55,7 +55,7 @@ namespace lms::scanner metadata::Artist artistInfo{ std::nullopt, link->getArtistName(), link->getArtistSortName().empty() ? std::nullopt : std::make_optional(link->getArtistSortName()) }; db::Artist::pointer newArtist{ helpers::getOrCreateArtistByName(session, artistInfo, helpers::AllowFallbackOnMBIDEntry{ allowArtistMBIDFallback }) }; - LMS_LOG(DB, INFO, "Reconcile artist link for track " << link->getTrack()->getAbsoluteFilePath() << ", type " << static_cast(link->getType()) << " from " << link->getArtist() << " to " << newArtist); + LMS_LOG(DB, DEBUG, "Reconcile artist link for track " << link->getTrack()->getAbsoluteFilePath() << ", type " << static_cast(link->getType()) << " from " << link->getArtist() << " to " << newArtist); assert(newArtist != link->getArtist()); link.modify()->setArtist(newArtist); @@ -67,7 +67,7 @@ namespace lms::scanner const metadata::Artist artistMetadata{ std::nullopt, artistInfo->getName(), artistInfo->getSortName().empty() ? std::nullopt : std::make_optional(artistInfo->getSortName()) }; db::Artist::pointer newArtist{ helpers::getOrCreateArtistByName(session, artistMetadata, helpers::AllowFallbackOnMBIDEntry{ allowArtistMBIDFallback }) }; - LMS_LOG(DB, INFO, "Reconcile artist link for artist info " << artistInfo->getAbsoluteFilePath() << " from " << artistInfo->getArtist() << " to " << newArtist); + LMS_LOG(DB, DEBUG, "Reconcile artist link for artist info " << artistInfo->getAbsoluteFilePath() << " from " << artistInfo->getArtist() << " to " << newArtist); assert(newArtist != artistInfo->getArtist()); artistInfo.modify()->setArtist(newArtist); diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index c041406d..5fc5ad54 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -87,27 +87,31 @@ namespace lms::api::subsonic std::string parameterMapToDebugString(const Wt::Http::ParameterMap& parameterMap) { - auto censorValue = [](const std::string& type, const std::string& value) -> std::string { - if (type == "p" || type == "password") - return "*REDACTED*"; + constexpr std::string_view redactedStr{ "*REDACTED*" }; + auto redactValueIfNeeded = [redactedStr](const std::string& type, const std::string& value) -> std::string_view { + if (type == "p" || type == "password" || type == "apiKey") + return redactedStr; return value; }; std::string res; - for (const auto& params : parameterMap) + for (const auto& [type, values] : parameterMap) { - res += "{" + params.first + "="; - if (params.second.size() == 1) + res += "{" + type + "="; + if (values.size() == 1) { - res += censorValue(params.first, params.second.front()); + res += redactValueIfNeeded(type, values.front()); } else { res += "{"; - for (const std::string& param : params.second) - res += censorValue(params.first, param) + ","; + for (const std::string& value : values) + { + res += redactValueIfNeeded(type, value); + res += ','; + } res += "}"; } res += "}, "; diff --git a/src/lms/ui/resource/ArtworkResource.cpp b/src/lms/ui/resource/ArtworkResource.cpp index 1e5951b2..b190b819 100644 --- a/src/lms/ui/resource/ArtworkResource.cpp +++ b/src/lms/ui/resource/ArtworkResource.cpp @@ -206,7 +206,7 @@ namespace lms::ui } } - if (!image) + if (!image && typeStr) { if (*typeStr == "release") image = core::Service::get()->getDefaultReleaseCover();