From 422458821f16c5b424d780987f4b18f90fad5ade Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 3 Mar 2023 13:58:52 +0100 Subject: [PATCH 01/14] ListenBrainz: try to match listens using track MBID if present --- .../impl/listenbrainz/ListenTypes.cpp | 2 ++ .../impl/listenbrainz/ListenTypes.hpp | 1 + .../impl/listenbrainz/ListensParser.cpp | 1 + .../impl/listenbrainz/ListensSynchronizer.cpp | 20 +++++++++++++++++-- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.cpp b/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.cpp index 53ff4564..88f7d40d 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.cpp @@ -31,6 +31,8 @@ namespace Scrobbling::ListenBrainz os << ", releaseName = '" << listen.releaseName << "'"; if (listen.trackNumber) os << ", trackNumber = " << *listen.trackNumber; + if (listen.trackMBID) + os << ", trackMBID = '" << listen.trackMBID->getAsString() << "'"; if (listen.recordingMBID) os << ", recordingMBID = '" << listen.recordingMBID->getAsString() << "'"; diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.hpp b/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.hpp index d5a17c9e..10f21477 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.hpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListenTypes.hpp @@ -33,6 +33,7 @@ namespace Scrobbling::ListenBrainz std::string releaseName; std::string artistName; std::optional recordingMBID; + std::optional trackMBID; std::optional releaseMBID; std::optional trackNumber; Wt::WDateTime listenedAt; diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.cpp b/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.cpp index 495ac101..765b5b5c 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.cpp @@ -51,6 +51,7 @@ namespace if (metadata.type("additional_info") == Wt::Json::Type::Object) { const Wt::Json::Object& additionalInfo = metadata.get("additional_info"); + listen.trackMBID = UUID::fromString(additionalInfo.get("track_mbid").orIfNull("")); listen.recordingMBID = UUID::fromString(additionalInfo.get("recording_mbid").orIfNull("")); listen.releaseMBID = UUID::fromString(additionalInfo.get("release_mbid").orIfNull("")); diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp index f1a79df0..caf42117 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp @@ -150,7 +150,23 @@ namespace auto transaction {session.createSharedTransaction()}; - // first try to match using recording MBID, and then fallback on possibly ambiguous info + // first try to match using track MBID, and then fallback on possibly ambiguous info + if (listen.trackMBID) + { + const auto tracks {Track::findByMBID(session, *listen.trackMBID)}; + // if duplicated files, do not record it (let the user correct its database) + if (tracks.size() == 1) + { + LOG(DEBUG) << "Matched listen '" << listen << "' using track MBID"; + return tracks.front()->getId(); + } + else if (tracks.size() > 1) + { + LOG(DEBUG) << "Too many matches for listen '" << listen << "' using track MBID!"; + return {}; + } + } + if (listen.recordingMBID) { const auto tracks {Track::findByRecordingMBID(session, *listen.recordingMBID)}; @@ -282,7 +298,7 @@ namespace Scrobbling::ListenBrainz using namespace Database; Session& session {_db.getTLSSession()}; - auto transaction {session.createUniqueTransaction()}; + auto transaction {session.createUniqueTransaction()}; // TODO: unique only if needed Database::Listen::pointer dbListen {Database::Listen::find(session, listen.userId, listen.trackId, Database::Scrobbler::ListenBrainz, listen.listenedAt)}; if (!dbListen) From 3a9bde0092208de91008b7fa212646af10614aea Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 6 Mar 2023 23:10:31 +0100 Subject: [PATCH 02/14] Moved total_disc from Track to Release --- src/libs/metadata/impl/AvFormatParser.cpp | 42 ++-- src/libs/metadata/impl/TagLibParser.cpp | 212 ++++++++++-------- src/libs/metadata/impl/TagLibParser.hpp | 2 +- src/libs/metadata/impl/Utils.cpp | 4 +- src/libs/metadata/impl/Utils.hpp | 2 +- .../metadata/include/metadata/IParser.hpp | 49 ++-- src/libs/services/database/impl/Migration.cpp | 43 ++++ src/libs/services/database/impl/Migration.hpp | 2 +- src/libs/services/database/impl/Release.cpp | 24 -- src/libs/services/database/impl/Track.cpp | 24 -- .../include/services/database/Release.hpp | 36 +-- .../include/services/database/Track.hpp | 85 ++++--- src/libs/services/database/test/Release.cpp | 29 ++- .../scanner/impl/ScanStepScanFiles.cpp | 70 +++--- src/libs/utils/impl/String.cpp | 4 +- src/libs/utils/include/utils/String.hpp | 2 +- src/lms/ui/PlayQueue.cpp | 1 - src/tools/metadata/LmsMetadata.cpp | 66 +++--- 18 files changed, 377 insertions(+), 320 deletions(-) diff --git a/src/libs/metadata/impl/AvFormatParser.cpp b/src/libs/metadata/impl/AvFormatParser.cpp index 1284ffb7..adba14a4 100644 --- a/src/libs/metadata/impl/AvFormatParser.cpp +++ b/src/libs/metadata/impl/AvFormatParser.cpp @@ -66,23 +66,25 @@ findFirstValueOfAs(const Av::IAudioFile::MetadataMap& metadataMap, std::initiali static -std::optional -getAlbum(const Av::IAudioFile::MetadataMap& metadataMap) +std::optional +getRelease(const Av::IAudioFile::MetadataMap& metadataMap) { - std::optional res; + std::optional res; - auto album {findFirstValueOfAs(metadataMap, {"ALBUM"})}; - if (!album) + std::optional releaseName {findFirstValueOfAs(metadataMap, {"ALBUM"})}; + if (!releaseName) return res; - auto albumMBID {findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"})}; + res.emplace(); + res->name = *releaseName; + res->releaseMBID = findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"}); - return Album{*album, albumMBID}; + return res; } static std::vector -getAlbumArtists(const Av::IAudioFile::MetadataMap& metadataMap) +getReleaseArtists(const Av::IAudioFile::MetadataMap& metadataMap) { std::vector res; @@ -151,6 +153,18 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) const Av::IAudioFile::MetadataMap metadataMap {mediaFile->getMetaData()}; + track.artists = getArtists(metadataMap); + track.release = getRelease(metadataMap); + if (track.release) + track.release->releaseArtists = getReleaseArtists(metadataMap); + + auto getOrCreateDisc = [&]() -> Disc& + { + if (!track.disc) + track.disc.emplace(); + return *track.disc; + }; + for (const auto& [tag, value] : metadataMap) { if (debug) @@ -167,7 +181,7 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) track.trackNumber = StringUtils::readAs(strings[0]); if (strings.size() > 1) - track.totalTrack = StringUtils::readAs(strings[1]); + getOrCreateDisc().totalTrack = StringUtils::readAs(strings[1]); } } else if (tag == "DISC") @@ -178,8 +192,8 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) { track.discNumber = StringUtils::readAs(strings[0]); - if (strings.size() > 1) - track.totalDisc = StringUtils::readAs(strings[1]); + if (strings.size() > 1 && track.release) + track.release->totalDisc = StringUtils::readAs(strings[1]); } } else if (tag == "DATE" @@ -211,7 +225,7 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) || tag == "DISCSUBTITLE" || tag == "SETSUBTITLE") { - track.discSubtitle = value; + getOrCreateDisc().subtitle = value; } else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { @@ -227,10 +241,6 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) } } } - - track.artists = getArtists(metadataMap); - track.album = getAlbum(metadataMap); - track.albumArtists = getAlbumArtists(metadataMap); } catch(Av::Exception& e) { diff --git a/src/libs/metadata/impl/TagLibParser.cpp b/src/libs/metadata/impl/TagLibParser.cpp index 1227c11d..62273854 100644 --- a/src/libs/metadata/impl/TagLibParser.cpp +++ b/src/libs/metadata/impl/TagLibParser.cpp @@ -19,6 +19,8 @@ #include "TagLibParser.hpp" +#include + #include #include #include @@ -43,23 +45,30 @@ namespace MetaData { +// TODO use string_views here for values +using TagMap = std::map>; + template std::vector -getPropertyValuesFirstMatchAs(const TagLib::PropertyMap& properties, const std::vector& keys) +getPropertyValuesFirstMatchAs(const TagMap& tags, const std::vector& keys) { std::vector res; for (std::string_view key : keys) { - const TagLib::StringList& values {properties[std::string {key}]}; - if (values.isEmpty()) + const auto itValues {tags.find(std::string {key})}; + if (itValues == std::cend(tags)) + continue; + + const std::vector& values {itValues->second}; + if (values.empty()) continue; res.reserve(values.size()); for (const auto& value : values) { - auto val {StringUtils::readAs(StringUtils::stringTrim(value.to8Bit(true)))}; + std::optional val {StringUtils::readAs(value)}; if (!val) continue; @@ -74,33 +83,31 @@ getPropertyValuesFirstMatchAs(const TagLib::PropertyMap& properties, const std:: template std::vector -getPropertyValuesAs(const TagLib::PropertyMap& properties, const std::string& key) +getPropertyValuesAs(const TagMap& tags, const std::string& key) { - return getPropertyValuesFirstMatchAs(properties, {std::move(key)}); + return getPropertyValuesFirstMatchAs(tags, {key}); } static -std::vector -splitAndTrimString(const std::string& str, std::string_view delimiters) +std::vector +splitAndTrimString(std::string_view str, std::string_view delimiters) { - std::vector res; - std::vector strings {StringUtils::splitString(str, delimiters)}; - for (std::string_view s : strings) - res.emplace_back(StringUtils::stringTrim(s)); + for (std::string_view& s : strings) + s = StringUtils::stringTrim(s); - return res; + return strings; } static std::vector -getArtists(const TagLib::PropertyMap& properties, +getArtists(const TagMap& tags, const std::vector& artistTagNames, const std::vector& artistSortTagNames, const std::vector& artistMBIDTagNames ) { - const std::vector artistNames {getPropertyValuesFirstMatchAs(properties, artistTagNames)}; + const std::vector artistNames {getPropertyValuesFirstMatchAs(tags, artistTagNames)}; if (artistNames.empty()) return {}; @@ -110,7 +117,7 @@ getArtists(const TagLib::PropertyMap& properties, [&](const std::string& name) { return Artist {name}; }); { - const std::vector artistSortNames {getPropertyValuesFirstMatchAs(properties, artistSortTagNames)}; + const std::vector artistSortNames {getPropertyValuesFirstMatchAs(tags, artistSortTagNames)}; if (artistSortNames.size() == artists.size()) { for (std::size_t i {}; i < artistSortNames.size(); ++i) @@ -119,12 +126,12 @@ getArtists(const TagLib::PropertyMap& properties, } { - const std::vector artistsMBID {getPropertyValuesFirstMatchAs(properties, artistMBIDTagNames)}; + const std::vector artistsMBID {getPropertyValuesFirstMatchAs(tags, artistMBIDTagNames)}; if (artistNames.size() == artistsMBID.size()) { for (std::size_t i {}; i < artistsMBID.size(); ++i) - artists[i].musicBrainzArtistID = artistsMBID[i]; + artists[i].artistMBID = artistsMBID[i]; } } @@ -134,7 +141,7 @@ getArtists(const TagLib::PropertyMap& properties, static PerformerContainer -getPerformerArtists(const TagLib::PropertyMap& properties, +getPerformerArtists(const TagMap& tags, const std::vector& artistTagNames) { PerformerContainer performers; @@ -142,7 +149,7 @@ getPerformerArtists(const TagLib::PropertyMap& properties, // picard stores like this: (see https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#performer) // We may hit both styles for the same track // PERFORMER: artist (role) - if (const std::vector artistNames {getPropertyValuesFirstMatchAs(properties, artistTagNames)}; !artistNames.empty()) + if (const std::vector artistNames {getPropertyValuesFirstMatchAs(tags, artistTagNames)}; !artistNames.empty()) { for (std::string_view entry : artistNames) { @@ -152,11 +159,11 @@ getPerformerArtists(const TagLib::PropertyMap& properties, } } // PERFORMER:role (MP3) - for (const auto& [key, values] : properties) + for (const auto& [key, values] : tags) { - if (key.startsWith("PERFORMER:")) + if (key.find("PERFORMER:") == 0) { - std::string performerStr {key.to8Bit(true)}; + std::string performerStr {key}; std::string role; if (const std::size_t rolePos {performerStr.find(':')}; rolePos != std::string::npos) { @@ -165,7 +172,7 @@ getPerformerArtists(const TagLib::PropertyMap& properties, } for (const auto& value : values) - performers[role].push_back(Artist {value.to8Bit(true)}); + performers[role].push_back(Artist {value}); } } @@ -173,19 +180,23 @@ getPerformerArtists(const TagLib::PropertyMap& properties, } static -std::optional -getAlbum(const TagLib::PropertyMap& properties) +std::optional +getRelease(const TagMap& tags) { - std::vector albumName {getPropertyValuesAs(properties, "ALBUM")}; - if (albumName.empty()) - return std::nullopt; + std::optional release; - const std::vector albumMBID {getPropertyValuesFirstMatchAs(properties, {"MUSICBRAINZ_ALBUMID", "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ/ALBUM ID"})}; + std::vector releaseName {getPropertyValuesAs(tags, "ALBUM")}; + if (releaseName.empty()) + return release; - if (albumMBID.empty()) - return Album {std::move(albumName.front()), {}}; - else - return Album {std::move(albumName.front()), albumMBID.front()}; + const std::vector releaseMBID {getPropertyValuesFirstMatchAs(tags, {"MUSICBRAINZ_ALBUMID", "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ/ALBUM ID"})}; + + release.emplace(); + release->name = std::move(releaseName.front()); + if (!releaseMBID.empty()) + release->releaseMBID = releaseMBID.front(); + + return release; } static @@ -202,27 +213,28 @@ readStyleToTagLibReadStyle(ParserReadStyle readStyle) throw LmsException {"Cannot convert read style"}; } - TagLibParser::TagLibParser(ParserReadStyle readStyle) : _readStyle {readStyleToTagLibReadStyle(readStyle)} { } void -TagLibParser::processTag(Track& track, const std::string& tag, const TagLib::StringList& values, bool debug) +TagLibParser::processTag(Track& track, const std::string& tag, const std::vector& values, bool debug) { if (debug) - { - std::vector strs; - std::transform(values.begin(), values.end(), std::back_inserter(strs), [](const auto& value) { return value.to8Bit(true); }); + std::cout << "[" << tag << "] = " << StringUtils::joinStrings(values, "*SEP*") << std::endl; - std::cout << "[" << tag << "] = " << StringUtils::joinStrings(strs, "*SEP*") << std::endl; - } - - if (tag.empty() || values.isEmpty() || values.front().isEmpty()) + if (tag.empty() || values.empty()) return; - std::string value {StringUtils::stringTrim(values.front().to8Bit(true))}; + auto getOrCreateDisc = [&]() -> Disc& + { + if (!track.disc) + track.disc.emplace(); + return *track.disc; + }; + + std::string_view value {values.front()}; if (tag == "TITLE") track.title = value; @@ -240,29 +252,26 @@ TagLibParser::processTag(Track& track, const std::string& tag, const TagLib::Str track.acoustID = UUID::fromString(value); else if (tag == "TRACKTOTAL") { - auto totalTrack = StringUtils::readAs(value); - if (totalTrack) - track.totalTrack = totalTrack; + getOrCreateDisc().totalTrack = StringUtils::readAs(value); } else if (tag == "TRACKNUMBER") { // Expecting 'Number/Total' - std::vector strings {splitAndTrimString(value, "/")}; + std::vector strings {splitAndTrimString(value, "/")}; if (!strings.empty()) { track.trackNumber = StringUtils::readAs(strings[0]); // Lower priority than TRACKTOTAL - if (strings.size() > 1 && !track.totalTrack) - track.totalTrack = StringUtils::readAs(strings[1]); + if (strings.size() > 1 && !getOrCreateDisc().totalTrack) + getOrCreateDisc().totalTrack = StringUtils::readAs(strings[1]); } } else if (tag == "DISCTOTAL") { - auto totalDisc = StringUtils::readAs(value); - if (totalDisc) - track.totalDisc = totalDisc; + if (track.release) + track.release->totalDisc = StringUtils::readAs(value); } else if (tag == "DISCNUMBER") { @@ -273,8 +282,8 @@ TagLibParser::processTag(Track& track, const std::string& tag, const TagLib::Str track.discNumber = StringUtils::readAs(strings[0]); // Lower priority than DISCTOTAL - if (strings.size() > 1 && !track.totalDisc) - track.totalDisc = StringUtils::readAs(strings[1]); + if (strings.size() > 1 && track.release && !track.release->totalDisc) + track.release->totalDisc = StringUtils::readAs(strings[1]); } } else if (tag == "DATE") @@ -306,20 +315,20 @@ TagLibParser::processTag(Track& track, const std::string& tag, const TagLib::Str else if (tag == "COPYRIGHTURL") track.copyrightURL = value; else if (tag == "REPLAYGAIN_ALBUM_GAIN") - track.albumReplayGain = StringUtils::readAs(value); + getOrCreateDisc().replayGain = StringUtils::readAs(value); else if (tag == "REPLAYGAIN_TRACK_GAIN") - track.trackReplayGain = StringUtils::readAs(value); + track.replayGain = StringUtils::readAs(value); else if (tag == "DISCSUBTITLE" || tag == "SETSUBTITLE") - track.discSubtitle = value; + getOrCreateDisc().subtitle = value; else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { std::set clusterNames; for (const auto& valueList : values) { - const auto splittedValues {splitAndTrimString(valueList.to8Bit(true), "/,;")}; + const std::vector splittedValues {splitAndTrimString(valueList, "/,;")}; - for (const auto& value : splittedValues) - clusterNames.insert(value); + for (std::string_view value : splittedValues) + clusterNames.insert(std::string {value}); } if (!clusterNames.empty()) @@ -327,6 +336,37 @@ TagLibParser::processTag(Track& track, const std::string& tag, const TagLib::Str } } +static +TagMap +constructTagMap(const TagLib::PropertyMap& properties) +{ + TagMap tagMap; + + for (const auto& [propertyName, propertyValues] : properties) + { + std::vector& values {tagMap[propertyName.upper().to8Bit(true)]}; + for (const TagLib::String& propertyValue : propertyValues) + { + std::string trimedValue {StringUtils::stringTrim(propertyValue.to8Bit(true))}; + if (!trimedValue.empty()) + values.emplace_back(std::move(trimedValue)); + } + } + + return tagMap; +} + +static +void +mergeTagMaps(TagMap& dst, TagMap&& src) +{ + for (auto&& [tag, values] : src) + { + if (dst.find(tag) == std::cend(dst)) + dst[tag] = std::move(values); + } +} + std::optional TagLibParser::parse(const std::filesystem::path& p, bool debug) { @@ -357,21 +397,14 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) track.audioStreams = {std::move(audioStream)}; } - TagLib::PropertyMap properties {f.file()->properties()}; + TagMap tags {constructTagMap(f.file()->properties())}; auto getAPETags = [&](const TagLib::APE::Tag* apeTag) { if (!apeTag) return; - for (const auto& [name, values] : apeTag->properties()) - { - if (debug) - std::cout << "APE property: '" << name << "'" << std::endl; - - if (!properties.contains(name)) - properties.insert(name, values); - } + mergeTagMaps(tags, constructTagMap(apeTag->properties())); }; // Not that good embedded pictures handling @@ -387,23 +420,23 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) for (const auto& [name, attributeList] : tag->attributeListMap()) { - if (name.to8Bit().find("WM/") == 0 || properties.contains(name)) + std::string strName {name.to8Bit(true)}; + if (strName.find("WM/") == 0 || tags.find(strName) != std::cend(tags)) continue; - TagLib::StringList stringAttributeList; + std::vector attributes; for (const auto& attribute : attributeList) { if (attribute.type() == TagLib::ASF::Attribute::AttributeTypes::UnicodeType) - stringAttributeList.append(attribute.toString()); + attributes.emplace_back(attribute.toString().to8Bit(true)); } - if (!stringAttributeList.isEmpty()) + if (!attributes.empty()) { if (debug) std::cout << "ASF property: '" << name << "'" << std::endl; - if (!properties.contains(name)) - properties.insert(name, stringAttributeList); + tags[strName] = std::move(attributes); } } } @@ -418,7 +451,7 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) if (!frameListMap["APIC"].isEmpty()) track.hasCover = true; if (!frameListMap["TSST"].isEmpty()) - properties.insert("DISCSUBTITLE", frameListMap["TSST"].front()->toString()); + tags["DISCSUBTITLE"] = {frameListMap["TSST"].front()->toString().to8Bit(true)}; } getAPETags(mp3File->APETag()); @@ -458,19 +491,20 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) track.hasCover = true; } - for (const auto& [tag, values] : properties) - processTag(track, tag.upper().to8Bit(true), values, debug); + track.release = getRelease(tags); + if (track.release) + track.release->releaseArtists = getArtists(tags, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); + track.artists = getArtists(tags, {"ARTISTS", "ARTIST"}, {"ARTISTSORT"}, {"MUSICBRAINZ_ARTISTID", "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ/ARTIST ID"}); + track.conductorArtists = getArtists(tags, {"CONDUCTORS", "CONDUCTOR"}, {"CONDUCTORSSORT", "CONDUCTORSORT"}, {}); + track.composerArtists = getArtists(tags, {"COMPOSERS", "COMPOSER"}, {"COMPOSERSSORT", "COMPOSERSORT"}, {}); + track.lyricistArtists = getArtists(tags, {"LYRICISTS", "LYRICIST"}, {"LYRICISTSSORT", "LYRICISTSORT"}, {}); + track.mixerArtists = getArtists(tags, {"MIXERS", "MIXER"}, {"MIXERSSORT", "MIXERSORT"}, {}); + track.producerArtists = getArtists(tags, {"PRODUCERS", "PRODUCER"}, {"PRODUCERSSORT", "PRODUCERSORT"}, {}); + track.remixerArtists = getArtists(tags, {"REMIXERS", "REMIXER", "ModifiedBy"}, {"REMIXERSSORT", "REMIXERSORT"}, {}); + track.performerArtists = getPerformerArtists(tags, {"PERFORMERS", "PERFORMER"}); - track.album = getAlbum(properties); - track.artists = getArtists(properties, {"ARTISTS", "ARTIST"}, {"ARTISTSORT"}, {"MUSICBRAINZ_ARTISTID", "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ/ARTIST ID"}); - track.albumArtists = getArtists(properties, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); - track.conductorArtists = getArtists(properties, {"CONDUCTORS", "CONDUCTOR"}, {"CONDUCTORSSORT", "CONDUCTORSORT"}, {}); - track.composerArtists = getArtists(properties, {"COMPOSERS", "COMPOSER"}, {"COMPOSERSSORT", "COMPOSERSORT"}, {}); - track.lyricistArtists = getArtists(properties, {"LYRICISTS", "LYRICIST"}, {"LYRICISTSSORT", "LYRICISTSORT"}, {}); - track.mixerArtists = getArtists(properties, {"MIXERS", "MIXER"}, {"MIXERSSORT", "MIXERSORT"}, {}); - track.producerArtists = getArtists(properties, {"PRODUCERS", "PRODUCER"}, {"PRODUCERSSORT", "PRODUCERSORT"}, {}); - track.remixerArtists = getArtists(properties, {"REMIXERS", "REMIXER", "ModifiedBy"}, {"REMIXERSSORT", "REMIXERSORT"}, {}); - track.performerArtists = getPerformerArtists(properties, {"PERFORMERS", "PERFORMER"}); + for (const auto& [tag, values] : tags) + processTag(track, tag, values, debug); return track; } diff --git a/src/libs/metadata/impl/TagLibParser.hpp b/src/libs/metadata/impl/TagLibParser.hpp index 8990fd1c..1fd27202 100644 --- a/src/libs/metadata/impl/TagLibParser.hpp +++ b/src/libs/metadata/impl/TagLibParser.hpp @@ -38,7 +38,7 @@ class TagLibParser : public IParser private: std::optional parse(const std::filesystem::path& p, bool debug = false) override; - void processTag(Track& track, const std::string& tag, const TagLib::StringList& values, bool debug); + void processTag(Track& track, const std::string& tag, const std::vector& values, bool debug); const TagLib::AudioProperties::ReadStyle _readStyle; }; diff --git a/src/libs/metadata/impl/Utils.cpp b/src/libs/metadata/impl/Utils.cpp index 40923964..b8ae6810 100644 --- a/src/libs/metadata/impl/Utils.cpp +++ b/src/libs/metadata/impl/Utils.cpp @@ -28,7 +28,7 @@ namespace MetaData::Utils { Wt::WDate - parseDate(const std::string& dateStr) + parseDate(std::string_view dateStr) { static constexpr const char* formats[] { @@ -39,7 +39,7 @@ namespace MetaData::Utils for (const char* format : formats) { std::tm tm = {}; - std::stringstream ss {dateStr}; + std::istringstream ss {std::string {dateStr}}; // TODO, remove extra copy here ss >> std::get_time(&tm, format); if (ss.fail()) continue; diff --git a/src/libs/metadata/impl/Utils.hpp b/src/libs/metadata/impl/Utils.hpp index 021d7619..1268eb4c 100644 --- a/src/libs/metadata/impl/Utils.hpp +++ b/src/libs/metadata/impl/Utils.hpp @@ -27,7 +27,7 @@ namespace MetaData::Utils { - Wt::WDate parseDate(const std::string& dateStr); + Wt::WDate parseDate(std::string_view dateStr); std::string_view readStyleToString(ParserReadStyle readStyle); struct PerformerArtist diff --git a/src/libs/metadata/include/metadata/IParser.hpp b/src/libs/metadata/include/metadata/IParser.hpp index 4a49cef0..d3adc13c 100644 --- a/src/libs/metadata/include/metadata/IParser.hpp +++ b/src/libs/metadata/include/metadata/IParser.hpp @@ -39,18 +39,27 @@ namespace MetaData { std::string name; std::optional sortName; - std::optional musicBrainzArtistID; + std::optional artistMBID; Artist(std::string_view _name) : name {_name} {} - Artist(std::string_view _name, std::optional _sortName, std::optional _musicBrainzArtistID) : name {_name}, sortName {_sortName}, musicBrainzArtistID {_musicBrainzArtistID} {} + Artist(std::string_view _name, std::optional _sortName, std::optional _artistMBID) : name {_name}, sortName {std::move(_sortName)}, artistMBID {std::move(_artistMBID)} {} }; using PerformerContainer = std::map>; - struct Album + struct Release { - std::string name; - std::optional musicBrainzAlbumID; + std::string name; + std::vector releaseArtists; + std::optional releaseMBID; + std::optional totalDisc; + }; + + struct Disc + { + std::string subtitle; + std::optional replayGain; + std::optional totalTrack; }; struct AudioStream @@ -61,34 +70,30 @@ namespace MetaData struct Track { std::vector artists; - std::vector albumArtists; std::string title; std::optional trackMBID; std::optional recordingMBID; - std::optional album; + std::optional release; + std::optional disc; Clusters clusters; std::chrono::milliseconds duration; std::optional trackNumber; - std::optional totalTrack; std::optional discNumber; - std::optional totalDisc; Wt::WDate date; Wt::WDate originalDate; bool hasCover {}; std::vector audioStreams; - std::optional acoustID; - std::string copyright; - std::string copyrightURL; - std::optional trackReplayGain; - std::optional albumReplayGain; - std::string discSubtitle; - std::vector conductorArtists; - std::vector composerArtists; - std::vector lyricistArtists; - std::vector mixerArtists; - PerformerContainer performerArtists; - std::vector producerArtists; - std::vector remixerArtists; + std::optional acoustID; + std::string copyright; + std::string copyrightURL; + std::optional replayGain; + std::vector conductorArtists; + std::vector composerArtists; + std::vector lyricistArtists; + std::vector mixerArtists; + PerformerContainer performerArtists; + std::vector producerArtists; + std::vector remixerArtists; }; class IParser diff --git a/src/libs/services/database/impl/Migration.cpp b/src/libs/services/database/impl/Migration.cpp index 753dbc19..45561592 100644 --- a/src/libs/services/database/impl/Migration.cpp +++ b/src/libs/services/database/impl/Migration.cpp @@ -611,6 +611,48 @@ CREATE TABLE IF NOT EXISTS "track_artist_link_backup" ( ScanSettings::get(session).modify()->incScanVersion(); } + static + void + migrateFromV38(Session& session) + { + // migrate release-specific tags from Track to Release + session.getDboSession().execute("ALTER TABLE release ADD total_disc INTEGER"); + + session.getDboSession().execute(R"( +CREATE TABLE IF NOT EXISTS "track_backup" ( + "id" integer primary key autoincrement, + "version" integer not null, + "scan_version" integer not null, + "track_number" integer, + "disc_number" integer, + "total_track" integer, + "disc_subtitle" text not null, + "name" text not null, + "duration" integer, + "date" text, + "original_date" text, + "file_path" text not null, + "file_last_write" text, + "file_added" text, + "has_cover" boolean not null, + "mbid" text not null, + "recording_mbid" text not null, + "copyright" text not null, + "copyright_url" text not null, + "track_replay_gain" real, + "release_replay_gain" real, + "release_id" bigint, + constraint "fk_track_release" foreign key ("release_id") references "release" ("id") on delete cascade deferrable initially deferred +); +))"); + session.getDboSession().execute("INSERT INTO track_backup SELECT id, version, scan_version, track_number, disc_number, total_track, disc_subtitle, name, duration, date, original_date, file_path, file_last_write, file_added, has_cover, mbid, recording_mbid, copyright, copyright_url, track_replay_gain, release_replay_gain, release_id FROM track"); + session.getDboSession().execute("DROP TABLE track"); + session.getDboSession().execute("ALTER TABLE track_backup RENAME TO track"); + + // Just increment the scan version of the settings to make the next scheduled scan rescan everything + ScanSettings::get(session).modify()->incScanVersion(); + } + void doDbMigration(Session& session) { @@ -655,6 +697,7 @@ CREATE TABLE IF NOT EXISTS "track_artist_link_backup" ( {35, migrateFromV35}, {36, migrateFromV36}, {37, migrateFromV37}, + {38, migrateFromV38}, }; while (1) diff --git a/src/libs/services/database/impl/Migration.hpp b/src/libs/services/database/impl/Migration.hpp index 682dc197..ed87913b 100644 --- a/src/libs/services/database/impl/Migration.hpp +++ b/src/libs/services/database/impl/Migration.hpp @@ -26,7 +26,7 @@ namespace Database class Session; using Version = std::size_t; - static constexpr Version LMS_DATABASE_VERSION {38}; + static constexpr Version LMS_DATABASE_VERSION {39}; class VersionInfo { public: diff --git a/src/libs/services/database/impl/Release.cpp b/src/libs/services/database/impl/Release.cpp index f6fc2b73..fb09c963 100644 --- a/src/libs/services/database/impl/Release.cpp +++ b/src/libs/services/database/impl/Release.cpp @@ -263,30 +263,6 @@ Release::find(Session& session, const FindParameters& params) return Utils::execQuery(query, params.range); } -std::optional -Release::getTotalTrack() const -{ - assert(session()); - - int res = session()->query("SELECT COALESCE(MAX(total_track),0) FROM track t INNER JOIN release r ON r.id = t.release_id") - .where("r.id = ?") - .bind(getId()); - - return (res > 0) ? std::make_optional(res) : std::nullopt; -} - -std::optional -Release::getTotalDisc() const -{ - assert(session()); - - int res = session()->query("SELECT COALESCE(MAX(total_disc),0) FROM track t INNER JOIN release r ON r.id = t.release_id") - .where("r.id = ?") - .bind(getId()); - - return (res > 0) ? std::make_optional(res) : std::nullopt; -} - std::size_t Release::getDiscCount() const { diff --git a/src/libs/services/database/impl/Track.cpp b/src/libs/services/database/impl/Track.cpp index 89af7b53..a4be79f6 100644 --- a/src/libs/services/database/impl/Track.cpp +++ b/src/libs/services/database/impl/Track.cpp @@ -361,30 +361,6 @@ Track::setClusters(const std::vector>& clusters) _clusters.insert(getDboPtr(cluster)); } -std::optional -Track::getTrackNumber() const -{ - return (_trackNumber > 0) ? std::make_optional(_trackNumber) : std::nullopt; -} - -std::optional -Track::getTotalTrack() const -{ - return (_totalTrack > 0) ? std::make_optional(_totalTrack) : std::nullopt; -} - -std::optional -Track::getDiscNumber() const -{ - return (_discNumber > 0) ? std::make_optional(_discNumber) : std::nullopt; -} - -std::optional -Track::getTotalDisc() const -{ - return (_totalDisc > 0) ? std::make_optional(_totalDisc) : std::nullopt; -} - std::optional Track::getYear() const { diff --git a/src/libs/services/database/include/services/database/Release.hpp b/src/libs/services/database/include/services/database/Release.hpp index 5bb00607..27932db0 100644 --- a/src/libs/services/database/include/services/database/Release.hpp +++ b/src/libs/services/database/include/services/database/Release.hpp @@ -97,34 +97,37 @@ class Release : public Object // size is the max number of cluster per cluster type std::vector>> getClusterGroups(const std::vector>& clusterTypes, std::size_t size) const; - // Utility functions + // Utility functions (if all tracks have the same values, which is legit to not be the case) std::optional getReleaseYear(bool originalDate = false) const; std::optional getCopyright() const; std::optional getCopyrightURL() const; // Accessors - const std::string& getName() const { return _name; } - std::optional getMBID() const { return UUID::fromString(_MBID); } - std::optional getTotalTrack() const; - std::optional getTotalDisc() const; + const std::string& getName() const { return _name; } + std::optional getMBID() const { return UUID::fromString(_MBID); } + std::optional getTotalDisc() const { return _totalDisc; } std::size_t getDiscCount() const; // may not be total disc (if incomplete for example) std::chrono::milliseconds getDuration() const; Wt::WDateTime getLastWritten() const; - // Get the artists of this release - std::vector > getArtists(TrackArtistLinkType type = TrackArtistLinkType::Artist) const; - std::vector > getReleaseArtists() const { return getArtists(TrackArtistLinkType::ReleaseArtist); } - bool hasVariousArtists() const; - std::vector getSimilarReleases(std::optional offset = {}, std::optional count = {}) const; + // Setters + void setName(std::string_view name) { _name = name; } + void setMBID(const std::optional& mbid) { _MBID = mbid ? mbid->getAsString() : ""; } + void setTotalDisc(std::optional totalDisc) { _totalDisc = totalDisc; } + + // Get the artists of this release + std::vector> getArtists(TrackArtistLinkType type = TrackArtistLinkType::Artist) const; + std::vector> getReleaseArtists() const { return getArtists(TrackArtistLinkType::ReleaseArtist); } + bool hasVariousArtists() const; + std::vector getSimilarReleases(std::optional offset = {}, std::optional count = {}) const; - void setName(std::string_view name) { _name = name; } - void setMBID(const std::optional& mbid) { _MBID = mbid ? mbid->getAsString() : ""; } template void persist(Action& a) { - Wt::Dbo::field(a, _name, "name"); - Wt::Dbo::field(a, _MBID, "mbid"); + Wt::Dbo::field(a, _name, "name"); + Wt::Dbo::field(a, _MBID, "mbid"); + Wt::Dbo::field(a, _totalDisc, "total_disc"); Wt::Dbo::hasMany(a, _tracks, Wt::Dbo::ManyToOne, "release"); } @@ -136,8 +139,9 @@ class Release : public Object static constexpr std::size_t _maxNameLength {128}; - std::string _name; - std::string _MBID; + std::string _name; + std::string _MBID; + std::optional _totalDisc {}; Wt::Dbo::collection> _tracks; // Tracks in the release }; diff --git a/src/libs/services/database/include/services/database/Track.hpp b/src/libs/services/database/include/services/database/Track.hpp index dd9b3428..a5495284 100644 --- a/src/libs/services/database/include/services/database/Track.hpp +++ b/src/libs/services/database/include/services/database/Track.hpp @@ -119,60 +119,57 @@ class Track : public Object static RangeResults findWithRecordingMBIDAndMissingFeatures(Session& session, Range range); // Accessors - void setScanVersion(std::size_t version) { _scanVersion = version; } - void setTrackNumber(int num) { _trackNumber = num; } - void setDiscNumber(int num) { _discNumber = num; } - void setTotalTrack(std::optional totalTrack) { _totalTrack = totalTrack ? *totalTrack : 0; } - void setTotalDisc(std::optional totalDisc) { _totalDisc = totalDisc ? *totalDisc : 0; } + void setScanVersion(std::size_t version) { _scanVersion = version; } + void setTrackNumber(std::optional num) { _trackNumber = num; } + void setDiscNumber(std::optional num) { _discNumber = num; } + void setTotalTrack(std::optional totalTrack) { _totalTrack = totalTrack; } void setDiscSubtitle(const std::string& name) { _discSubtitle = name; } - void setName(const std::string& name) { _name = std::string(name, 0, _maxNameLength); } - void setPath(const std::filesystem::path& filePath) { _filePath = filePath; } - void setDuration(std::chrono::milliseconds duration) { _duration = duration; } - void setLastWriteTime(Wt::WDateTime time) { _fileLastWrite = time; } - void setAddedTime(Wt::WDateTime time) { _fileAdded = time; } - void setDate(const Wt::WDate& date) { _date = date; } - void setOriginalDate(const Wt::WDate& date) { _originalDate = date; } - void setHasCover(bool hasCover) { _hasCover = hasCover; } - void setTrackMBID(const std::optional& MBID) { _trackMBID = MBID ? MBID->getAsString() : ""; } - void setRecordingMBID(const std::optional& MBID) { _recordingMBID = MBID ? MBID->getAsString() : ""; } + void setName(const std::string& name) { _name = std::string(name, 0, _maxNameLength); } + void setPath(const std::filesystem::path& filePath) { _filePath = filePath; } + void setDuration(std::chrono::milliseconds duration) { _duration = duration; } + void setLastWriteTime(Wt::WDateTime time) { _fileLastWrite = time; } + void setAddedTime(Wt::WDateTime time) { _fileAdded = time; } + void setDate(const Wt::WDate& date) { _date = date; } + void setOriginalDate(const Wt::WDate& date) { _originalDate = date; } + void setHasCover(bool hasCover) { _hasCover = hasCover; } + void setTrackMBID(const std::optional& MBID) { _trackMBID = MBID ? MBID->getAsString() : ""; } + void setRecordingMBID(const std::optional& MBID) { _recordingMBID = MBID ? MBID->getAsString() : ""; } void setCopyright(const std::string& copyright) { _copyright = std::string(copyright, 0, _maxCopyrightLength); } - void setCopyrightURL(const std::string& copyrightURL) { _copyrightURL = std::string(copyrightURL, 0, _maxCopyrightURLLength); } - void setTrackReplayGain(std::optional replayGain) { _trackReplayGain = replayGain; } - void setReleaseReplayGain(std::optional replayGain) { _releaseReplayGain = replayGain; } + void setCopyrightURL(const std::string& copyrightURL) { _copyrightURL = std::string(copyrightURL, 0, _maxCopyrightURLLength); } + void setTrackReplayGain(std::optional replayGain) { _trackReplayGain = replayGain; } + void setReleaseReplayGain(std::optional replayGain) { _releaseReplayGain = replayGain; } // may be by disc! void clearArtistLinks(); void addArtistLink(const ObjectPtr& artistLink); void setRelease(ObjectPtr release) { _release = getDboPtr(release); } void setClusters(const std::vector>& clusters ); std::size_t getScanVersion() const { return _scanVersion; } - std::optional getTrackNumber() const; - std::optional getTotalTrack() const; - std::optional getDiscNumber() const; + std::optional getTrackNumber() const { return _trackNumber; } + std::optional getTotalTrack() const { return _totalTrack; } + std::optional getDiscNumber() const { return _discNumber; } const std::string& getDiscSubtitle() const { return _discSubtitle; } - std::optional getTotalDisc() const; std::string getName() const { return _name; } std::filesystem::path getPath() const { return _filePath; } std::chrono::milliseconds getDuration() const { return _duration; } const Wt::WDateTime& getLastWritten() const { return _fileLastWrite; } std::optional getYear() const; std::optional getOriginalYear() const; - Wt::WDateTime getLastWriteTime() const { return _fileLastWrite; } - Wt::WDateTime getAddedTime() const { return _fileAdded; } - bool hasCover() const { return _hasCover; } + Wt::WDateTime getLastWriteTime() const { return _fileLastWrite; } + Wt::WDateTime getAddedTime() const { return _fileAdded; } + bool hasCover() const { return _hasCover; } std::optional getTrackMBID() const { return UUID::fromString(_trackMBID); } - std::optional getRecordingMBID() const { return UUID::fromString(_recordingMBID); } + std::optional getRecordingMBID() const { return UUID::fromString(_recordingMBID); } std::optional getCopyright() const; std::optional getCopyrightURL() const; - std::optional getTrackReplayGain() const { return _trackReplayGain; } + std::optional getTrackReplayGain() const { return _trackReplayGain; } std::optional getReleaseReplayGain() const { return _releaseReplayGain; } - // no artistLinkTypes means get all - std::vector> getArtists(EnumSet artistLinkTypes) const; // no type means all - std::vector getArtistIds(EnumSet artistLinkTypes) const; // no type means all + std::vector> getArtists(EnumSet artistLinkTypes) const; // no type means all + std::vector getArtistIds(EnumSet artistLinkTypes) const; // no type means all std::vector> getArtistLinks() const; - ObjectPtr getRelease() const { return _release; } - std::vector> getClusters() const; - std::vector getClusterIds() const; + ObjectPtr getRelease() const { return _release; } + std::vector> getClusters() const; + std::vector getClusterIds() const; std::vector>> getClusterGroups(const std::vector>& clusterTypes, std::size_t size) const; @@ -182,9 +179,8 @@ class Track : public Object Wt::Dbo::field(a, _scanVersion, "scan_version"); Wt::Dbo::field(a, _trackNumber, "track_number"); Wt::Dbo::field(a, _discNumber, "disc_number"); - Wt::Dbo::field(a, _discSubtitle, "disc_subtitle"); - Wt::Dbo::field(a, _totalTrack, "total_track"); - Wt::Dbo::field(a, _totalDisc, "total_disc"); + Wt::Dbo::field(a, _totalTrack, "total_track"); // here in Track since Release does not have concept of "disc" (yet?) + Wt::Dbo::field(a, _discSubtitle, "disc_subtitle"); // here in Track since Release does not have concept of "disc" (yet?) Wt::Dbo::field(a, _name, "name"); Wt::Dbo::field(a, _duration, "duration"); Wt::Dbo::field(a, _date, "date"); @@ -198,7 +194,7 @@ class Track : public Object Wt::Dbo::field(a, _copyright, "copyright"); Wt::Dbo::field(a, _copyrightURL, "copyright_url"); Wt::Dbo::field(a, _trackReplayGain, "track_replay_gain"); - Wt::Dbo::field(a, _releaseReplayGain, "release_replay_gain"); + Wt::Dbo::field(a, _releaseReplayGain, "release_replay_gain"); // here in Track since Release does not have concept of "disc" (yet?) Wt::Dbo::belongsTo(a, _release, "release", Wt::Dbo::OnDeleteCascade); Wt::Dbo::hasMany(a, _trackArtistLinks, Wt::Dbo::ManyToOne, "track"); Wt::Dbo::hasMany(a, _clusters, Wt::Dbo::ManyToMany, "track_cluster", "", Wt::Dbo::OnDeleteCascade); @@ -214,14 +210,11 @@ class Track : public Object static constexpr std::size_t _maxCopyrightURLLength {128}; int _scanVersion {}; - int _trackNumber {}; - int _discNumber {}; + std::optional _trackNumber {}; + std::optional _discNumber {}; + std::optional _totalTrack {}; std::string _discSubtitle; - int _totalTrack {}; - int _totalDisc {}; std::string _name; - std::string _artistName; - std::string _releaseName; std::chrono::duration _duration {}; Wt::WDate _date; Wt::WDate _originalDate; @@ -236,9 +229,9 @@ class Track : public Object std::optional _trackReplayGain; std::optional _releaseReplayGain; - Wt::Dbo::ptr _release; - Wt::Dbo::collection> _trackArtistLinks; - Wt::Dbo::collection> _clusters; + Wt::Dbo::ptr _release; + Wt::Dbo::collection> _trackArtistLinks; + Wt::Dbo::collection> _clusters; }; namespace Debug diff --git a/src/libs/services/database/test/Release.cpp b/src/libs/services/database/test/Release.cpp index 6ba03920..f9697eea 100644 --- a/src/libs/services/database/test/Release.cpp +++ b/src/libs/services/database/test/Release.cpp @@ -187,7 +187,6 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) { auto transaction {session.createSharedTransaction()}; - EXPECT_FALSE(release1->getTotalTrack()); EXPECT_FALSE(release1->getTotalDisc()); } @@ -201,7 +200,6 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) { auto transaction {session.createSharedTransaction()}; - EXPECT_FALSE(release1->getTotalTrack()); EXPECT_FALSE(release1->getTotalDisc()); } @@ -209,14 +207,14 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) auto transaction {session.createUniqueTransaction()}; track1.get().modify()->setTotalTrack(36); - track1.get().modify()->setTotalDisc(6); + release1.get().modify()->setTotalDisc(6); } { auto transaction {session.createSharedTransaction()}; - ASSERT_TRUE(release1->getTotalTrack()); - EXPECT_EQ(*release1->getTotalTrack(), 36); + ASSERT_TRUE(track1->getTotalTrack()); + EXPECT_EQ(*track1->getTotalTrack(), 36); ASSERT_TRUE(release1->getTotalDisc()); EXPECT_EQ(*release1->getTotalDisc(), 6); } @@ -227,14 +225,14 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) track2.get().modify()->setRelease(release1.get()); track2.get().modify()->setTotalTrack(37); - track2.get().modify()->setTotalDisc(67); + release1.get().modify()->setTotalDisc(67); } { auto transaction {session.createSharedTransaction()}; - ASSERT_TRUE(release1->getTotalTrack()); - EXPECT_EQ(*release1->getTotalTrack(), 37); + ASSERT_TRUE(track1->getTotalTrack()); + EXPECT_EQ(*track1->getTotalTrack(), 36); ASSERT_TRUE(release1->getTotalDisc()); EXPECT_EQ(*release1->getTotalDisc(), 67); } @@ -243,7 +241,6 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) { auto transaction {session.createSharedTransaction()}; - EXPECT_FALSE(release2->getTotalTrack()); EXPECT_FALSE(release2->getTotalDisc()); } @@ -253,17 +250,17 @@ TEST_F(DatabaseFixture, MultiTracksSingleReleaseTotalDiscTrack) track3.get().modify()->setRelease(release2.get()); track3.get().modify()->setTotalTrack(7); - track3.get().modify()->setTotalDisc(5); + release2.get().modify()->setTotalDisc(5); } { auto transaction {session.createSharedTransaction()}; - ASSERT_TRUE(release1->getTotalTrack()); - EXPECT_EQ(*release1->getTotalTrack(), 37); + ASSERT_TRUE(track1->getTotalTrack()); + EXPECT_EQ(*track1->getTotalTrack(), 36); ASSERT_TRUE(release1->getTotalDisc()); - EXPECT_EQ(*release1->getTotalDisc(), 67); - ASSERT_TRUE(release2->getTotalTrack()); - EXPECT_EQ(*release2->getTotalTrack(), 7); + EXPECT_EQ(*release2->getTotalDisc(), 5); + ASSERT_TRUE(track3->getTotalTrack()); + EXPECT_EQ(*track3->getTotalTrack(), 7); ASSERT_TRUE(release2->getTotalDisc()); EXPECT_EQ(*release2->getTotalDisc(), 5); } @@ -482,7 +479,7 @@ TEST_F(DatabaseFixture, Release_getDiscCount) } { auto transaction {session.createSharedTransaction()}; - EXPECT_EQ(release.get()->getDiscCount(), 1); + EXPECT_EQ(release.get()->getDiscCount(), 0); } { auto transaction {session.createUniqueTransaction()}; diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index 4df5dd71..d2207494 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -42,8 +42,8 @@ namespace { Artist::pointer artist {session.create(artistInfo.name)}; - if (artistInfo.musicBrainzArtistID) - artist.modify()->setMBID(*artistInfo.musicBrainzArtistID); + if (artistInfo.artistMBID) + artist.modify()->setMBID(*artistInfo.artistMBID); if (artistInfo.sortName) artist.modify()->setSortName(*artistInfo.sortName); @@ -76,9 +76,9 @@ namespace Artist::pointer artist; // First try to get by MBID - if (artistInfo.musicBrainzArtistID) + if (artistInfo.artistMBID) { - artist = Artist::find(session, *artistInfo.musicBrainzArtistID); + artist = Artist::find(session, *artistInfo.artistMBID); if (!artist) artist = createArtist(session, artistInfo); else @@ -115,45 +115,49 @@ namespace return artists; } + void + updateReleaseIfNeeded(Release::pointer release, const MetaData::Release& releaseInfo) + { + if (release->getName() != releaseInfo.name) + release.modify()->setName(releaseInfo.name); + if (release->getTotalDisc() != releaseInfo.totalDisc) + release.modify()->setTotalDisc(releaseInfo.totalDisc); + } + Release::pointer - getOrCreateRelease(Session& session, const MetaData::Album& album) + getOrCreateRelease(Session& session, const MetaData::Release& releaseInfo) { Release::pointer release; // First try to get by MBID - if (album.musicBrainzAlbumID) + if (releaseInfo.releaseMBID) { - release = Release::find(session, *album.musicBrainzAlbumID); + release = Release::find(session, *releaseInfo.releaseMBID); if (!release) - { - release = session.create(album.name, album.musicBrainzAlbumID); - } - else if (release->getName() != album.name) - { - // Name may have been updated - release.modify()->setName(album.name); - } + release = session.create(releaseInfo.name, releaseInfo.releaseMBID); + updateReleaseIfNeeded(release, releaseInfo); return release; } // Fall back on release name (collisions may occur) - if (!album.name.empty()) + if (!releaseInfo.name.empty()) { - for (const Release::pointer& sameNamedRelease : Release::find(session, album.name)) + for (const Release::pointer& sameNamedRelease : Release::find(session, releaseInfo.name)) { // do not fallback on properly tagged releases - if (!sameNamedRelease->getMBID()) - { - release = sameNamedRelease; - break; - } + if (sameNamedRelease->getMBID()) + continue; + + release = sameNamedRelease; + break; } // No release found with the same name and without MBID -> creating if (!release) - release = session.create(album.name); + release = session.create(releaseInfo.name); + updateReleaseIfNeeded(release, releaseInfo); return release; } @@ -389,8 +393,11 @@ namespace Scanner for (const Artist::pointer& artist : getOrCreateArtists(dbSession, trackInfo->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, artist, TrackArtistLinkType::Artist)); - for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->albumArtists, false)) - track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, releaseArtist, TrackArtistLinkType::ReleaseArtist)); + if (trackInfo->release) + { + for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->release->releaseArtists, false)) + track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, releaseArtist, TrackArtistLinkType::ReleaseArtist)); + } // Allow fallbacks on artists with the same name even if they have MBID, since there is no tag to indicate the MBID of these artists // We could ask MusicBrainz to get all the information, but that would heavily slow down the import process @@ -419,10 +426,13 @@ namespace Scanner track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, remixer, TrackArtistLinkType::Remixer)); track.modify()->setScanVersion(_settings.scanVersion); - if (trackInfo->album) - track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->album)); + if (trackInfo->release) + track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->release)); else track.modify()->setRelease({}); + track.modify()->setTotalTrack(trackInfo->disc ? trackInfo->disc->totalTrack : std::nullopt); + track.modify()->setReleaseReplayGain(trackInfo->disc ? trackInfo->disc->replayGain : std::nullopt); + track.modify()->setDiscSubtitle(trackInfo->disc ? trackInfo->disc->subtitle : ""); track.modify()->setClusters(getOrCreateClusters(dbSession, trackInfo->clusters)); track.modify()->setLastWriteTime(lastWriteTime); track.modify()->setName(title); @@ -430,9 +440,6 @@ namespace Scanner track.modify()->setAddedTime(Wt::WDateTime::currentDateTime()); track.modify()->setTrackNumber(trackInfo->trackNumber ? *trackInfo->trackNumber : 0); track.modify()->setDiscNumber(trackInfo->discNumber ? *trackInfo->discNumber : 0); - track.modify()->setTotalTrack(trackInfo->totalTrack); - track.modify()->setTotalDisc(trackInfo->totalDisc); - track.modify()->setDiscSubtitle(trackInfo->discSubtitle); track.modify()->setDate(trackInfo->date); track.modify()->setOriginalDate(trackInfo->originalDate); @@ -447,7 +454,6 @@ namespace Scanner track.modify()->setHasCover(trackInfo->hasCover); track.modify()->setCopyright(trackInfo->copyright); track.modify()->setCopyrightURL(trackInfo->copyrightURL); - track.modify()->setTrackReplayGain(trackInfo->trackReplayGain); - track.modify()->setReleaseReplayGain(trackInfo->albumReplayGain); + track.modify()->setTrackReplayGain(trackInfo->replayGain); } } diff --git a/src/libs/utils/impl/String.cpp b/src/libs/utils/impl/String.cpp index 72005454..344e3eeb 100644 --- a/src/libs/utils/impl/String.cpp +++ b/src/libs/utils/impl/String.cpp @@ -132,10 +132,10 @@ stringTrim(std::string_view str, std::string_view whitespaces) return res; } -std::string +std::string_view stringTrimEnd(std::string_view str, std::string_view whitespaces) { - return std::string {str.substr(0, str.find_last_not_of(whitespaces) + 1)}; + return str.substr(0, str.find_last_not_of(whitespaces) + 1); } std::string diff --git a/src/libs/utils/include/utils/String.hpp b/src/libs/utils/include/utils/String.hpp index 73d11d03..95e97c60 100644 --- a/src/libs/utils/include/utils/String.hpp +++ b/src/libs/utils/include/utils/String.hpp @@ -48,7 +48,7 @@ std::string_view stringTrim(std::string_view str, std::string_view whitespaces = " \t"); [[nodiscard]] -std::string +std::string_view stringTrimEnd(std::string_view str, std::string_view whitespaces = " \t"); [[nodiscard]] diff --git a/src/lms/ui/PlayQueue.cpp b/src/lms/ui/PlayQueue.cpp index 243e6251..e04d7443 100644 --- a/src/lms/ui/PlayQueue.cpp +++ b/src/lms/ui/PlayQueue.cpp @@ -28,7 +28,6 @@ #include #include -#include "services/database/Cluster.hpp" #include "services/database/Release.hpp" #include "services/database/Session.hpp" #include "services/database/Track.hpp" diff --git a/src/tools/metadata/LmsMetadata.cpp b/src/tools/metadata/LmsMetadata.cpp index a9f03e02..628a968a 100644 --- a/src/tools/metadata/LmsMetadata.cpp +++ b/src/tools/metadata/LmsMetadata.cpp @@ -29,12 +29,14 @@ #include "metadata/IParser.hpp" #include "utils/StreamLogger.hpp" -std::ostream& operator<<(std::ostream& os, const MetaData::Artist& artist) +static +std::ostream& +operator<<(std::ostream& os, const MetaData::Artist& artist) { os << artist.name; - if (artist.musicBrainzArtistID) - os << " (" << artist.musicBrainzArtistID->getAsString() << ")"; + if (artist.artistMBID) + os << " (" << artist.artistMBID->getAsString() << ")"; if (artist.sortName) os << " '" << *artist.sortName << "'"; @@ -42,12 +44,36 @@ std::ostream& operator<<(std::ostream& os, const MetaData::Artist& artist) return os; } -std::ostream& operator<<(std::ostream& os, const MetaData::Album& album) +static +std::ostream& +operator<<(std::ostream& os, const MetaData::Release& release) { - os << album.name; + os << release.name; - if (album.musicBrainzAlbumID) - os << " (" << album.musicBrainzAlbumID->getAsString() << ")"; + if (release.releaseMBID) + os << " (" << release.releaseMBID->getAsString() << ")" << std::endl; + + if (release.totalDisc) + std::cout << "\tTotalDisc: " << *release.totalDisc << std::endl; + + for (const MetaData::Artist& artist : release.releaseArtists) + std::cout << "\tAlbum artist: " << artist << std::endl; + + return os; +} + +static +std::ostream& +operator<<(std::ostream& os, const MetaData::Disc& disc) +{ + if (!disc.subtitle.empty()) + os << disc.subtitle << std::endl; + + if (disc.totalTrack) + std::cout << "\tTotalTrack: " << *disc.totalTrack << std::endl; + + if (disc.replayGain) + std::cout << "\tDisc replay gain: " << *disc.replayGain << std::endl; return os; } @@ -74,9 +100,6 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) for (const Artist& artist : track->artists) std::cout << "Artist: " << artist << std::endl; - for (const Artist& artist : track->albumArtists) - std::cout << "Album artist: " << artist << std::endl; - for (const Artist& artist : track->conductorArtists) std::cout << "Conductor: " << artist << std::endl; @@ -105,8 +128,11 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) for (const Artist& artist : track->remixerArtists) std::cout << "Remixer: " << artist << std::endl; - if (track->album) - std::cout << "Album: " << *track->album << std::endl; + if (track->release) + std::cout << "Release: " << *track->release; + + if (track->disc) + std::cout << "Disc: " << *track->disc; std::cout << "Title: " << track->title << std::endl; @@ -130,18 +156,9 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) if (track->trackNumber) std::cout << "Track: " << *track->trackNumber << std::endl; - if (track->totalTrack) - std::cout << "TotalTrack: " << *track->totalTrack << std::endl; - if (track->discNumber) std::cout << "Disc: " << *track->discNumber << std::endl; - if (!track->discSubtitle.empty()) - std::cout << "Disc Subtitle: " << track->discSubtitle << std::endl; - - if (track->totalDisc) - std::cout << "TotalDisc: " << *track->totalDisc << std::endl; - if (track->date.isValid()) std::cout << "Date: " << track->date.toString("yyyy-MM-dd") << std::endl; @@ -153,11 +170,8 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) for (const auto& audioStream : track->audioStreams) std::cout << "Audio stream: " << audioStream.bitRate << " bps" << std::endl; - if (track->trackReplayGain) - std::cout << "Track replay gain: " << *track->trackReplayGain << std::endl; - - if (track->albumReplayGain) - std::cout << "Album replay gain: " << *track->albumReplayGain << std::endl; + if (track->replayGain) + std::cout << "Track replay gain: " << *track->replayGain << std::endl; if (track->acoustID) std::cout << "AcoustID: " << track->acoustID->getAsString() << std::endl; From 59eff32377995cf4ba13ec3d4c3e5d2a0863a378 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 7 Mar 2023 17:11:36 +0100 Subject: [PATCH 03/14] Generalized 'disc' to 'medium' in tag parser --- src/libs/metadata/impl/AvFormatParser.cpp | 32 +++++----- src/libs/metadata/impl/TagLibParser.cpp | 43 ++++++------- .../metadata/include/metadata/IParser.hpp | 33 +++++----- .../scanner/impl/ScanStepScanFiles.cpp | 48 +++++++-------- src/tools/metadata/LmsMetadata.cpp | 61 ++++++++++--------- 5 files changed, 113 insertions(+), 104 deletions(-) diff --git a/src/libs/metadata/impl/AvFormatParser.cpp b/src/libs/metadata/impl/AvFormatParser.cpp index adba14a4..08cea3de 100644 --- a/src/libs/metadata/impl/AvFormatParser.cpp +++ b/src/libs/metadata/impl/AvFormatParser.cpp @@ -77,7 +77,7 @@ getRelease(const Av::IAudioFile::MetadataMap& metadataMap) res.emplace(); res->name = *releaseName; - res->releaseMBID = findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"}); + res->mbid = findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"}); return res; } @@ -149,20 +149,20 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) track.duration = mediaFile->getDuration(); track.hasCover = mediaFile->hasAttachedPictures(); - MetaData::Clusters clusters; + MetaData::Tags tags; const Av::IAudioFile::MetadataMap metadataMap {mediaFile->getMetaData()}; track.artists = getArtists(metadataMap); track.release = getRelease(metadataMap); if (track.release) - track.release->releaseArtists = getReleaseArtists(metadataMap); + track.release->artists = getReleaseArtists(metadataMap); - auto getOrCreateDisc = [&]() -> Disc& + auto getOrCreateMedium = [&]() -> Medium& { - if (!track.disc) - track.disc.emplace(); - return *track.disc; + if (!track.medium) + track.medium.emplace(); + return *track.medium; }; for (const auto& [tag, value] : metadataMap) @@ -178,10 +178,10 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) const std::vector strings {StringUtils::splitString(value, "/") }; if (strings.size() > 0) { - track.trackNumber = StringUtils::readAs(strings[0]); + track.position = StringUtils::readAs(strings[0]); if (strings.size() > 1) - getOrCreateDisc().totalTrack = StringUtils::readAs(strings[1]); + getOrCreateMedium().trackCount = StringUtils::readAs(strings[1]); } } else if (tag == "DISC") @@ -190,10 +190,10 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) const std::vector strings {StringUtils::splitString(value, "/")}; if (strings.size() > 0) { - track.discNumber = StringUtils::readAs(strings[0]); + getOrCreateMedium().position = StringUtils::readAs(strings[0]); if (strings.size() > 1 && track.release) - track.release->totalDisc = StringUtils::readAs(strings[1]); + track.release->mediumCount = StringUtils::readAs(strings[1]); } } else if (tag == "DATE" @@ -214,18 +214,22 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) else if (tag == "MUSICBRAINZ RELEASE TRACK ID" || tag == "MUSICBRAINZ_RELEASETRACKID") { - track.trackMBID = UUID::fromString(value); + track.mbid = UUID::fromString(value); } else if (tag == "MUSICBRAINZ_TRACKID" || tag == "MUSICBRAINZ/TRACK ID") { track.recordingMBID = UUID::fromString(value); } + else if (tag == "MEDIA") + { + getOrCreateMedium().type = value; + } else if (tag == "TSST" || tag == "DISCSUBTITLE" || tag == "SETSUBTITLE") { - getOrCreateDisc().subtitle = value; + getOrCreateMedium().name = value; } else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { @@ -237,7 +241,7 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) std::transform(std::cbegin(clusterNames), std::cend(clusterNames), std::inserter(values, std::begin(values)), [](std::string_view clusterName) { return std::string {clusterName}; }); - track.clusters[tag] = std::move(values); + track.tags[tag] = std::move(values); } } } diff --git a/src/libs/metadata/impl/TagLibParser.cpp b/src/libs/metadata/impl/TagLibParser.cpp index 62273854..a3caa7f3 100644 --- a/src/libs/metadata/impl/TagLibParser.cpp +++ b/src/libs/metadata/impl/TagLibParser.cpp @@ -131,7 +131,7 @@ getArtists(const TagMap& tags, if (artistNames.size() == artistsMBID.size()) { for (std::size_t i {}; i < artistsMBID.size(); ++i) - artists[i].artistMBID = artistsMBID[i]; + artists[i].mbid = artistsMBID[i]; } } @@ -194,7 +194,7 @@ getRelease(const TagMap& tags) release.emplace(); release->name = std::move(releaseName.front()); if (!releaseMBID.empty()) - release->releaseMBID = releaseMBID.front(); + release->mbid = releaseMBID.front(); return release; } @@ -227,11 +227,11 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector if (tag.empty() || values.empty()) return; - auto getOrCreateDisc = [&]() -> Disc& + auto getOrCreateMedium = [&]() -> Medium& { - if (!track.disc) - track.disc.emplace(); - return *track.disc; + if (!track.medium) + track.medium.emplace(); + return *track.medium; }; std::string_view value {values.front()}; @@ -242,7 +242,7 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector || tag == "MUSICBRAINZ RELEASE TRACK ID" || tag == "MUSICBRAINZ/RELEASE TRACK ID") { - track.trackMBID = UUID::fromString(value); + track.mbid = UUID::fromString(value); } else if (tag == "MUSICBRAINZ_TRACKID" || tag == "MUSICBRAINZ TRACK ID" @@ -252,7 +252,7 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector track.acoustID = UUID::fromString(value); else if (tag == "TRACKTOTAL") { - getOrCreateDisc().totalTrack = StringUtils::readAs(value); + getOrCreateMedium().trackCount = StringUtils::readAs(value); } else if (tag == "TRACKNUMBER") { @@ -261,17 +261,17 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector if (!strings.empty()) { - track.trackNumber = StringUtils::readAs(strings[0]); + track.position = StringUtils::readAs(strings[0]); // Lower priority than TRACKTOTAL - if (strings.size() > 1 && !getOrCreateDisc().totalTrack) - getOrCreateDisc().totalTrack = StringUtils::readAs(strings[1]); + if (strings.size() > 1 && !getOrCreateMedium().trackCount) + getOrCreateMedium().trackCount = StringUtils::readAs(strings[1]); } } else if (tag == "DISCTOTAL") { if (track.release) - track.release->totalDisc = StringUtils::readAs(value); + track.release->mediumCount = StringUtils::readAs(value); } else if (tag == "DISCNUMBER") { @@ -279,11 +279,11 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector std::vector strings {StringUtils::splitString(value, "/")}; if (!strings.empty()) { - track.discNumber = StringUtils::readAs(strings[0]); + getOrCreateMedium().position = StringUtils::readAs(strings[0]); // Lower priority than DISCTOTAL - if (strings.size() > 1 && track.release && !track.release->totalDisc) - track.release->totalDisc = StringUtils::readAs(strings[1]); + if (strings.size() > 1 && track.release && !track.release->mediumCount) + track.release->mediumCount = StringUtils::readAs(strings[1]); } } else if (tag == "DATE") @@ -315,24 +315,25 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector else if (tag == "COPYRIGHTURL") track.copyrightURL = value; else if (tag == "REPLAYGAIN_ALBUM_GAIN") - getOrCreateDisc().replayGain = StringUtils::readAs(value); + getOrCreateMedium().replayGain = StringUtils::readAs(value); else if (tag == "REPLAYGAIN_TRACK_GAIN") track.replayGain = StringUtils::readAs(value); else if (tag == "DISCSUBTITLE" || tag == "SETSUBTITLE") - getOrCreateDisc().subtitle = value; + getOrCreateMedium().name = value; + else if (tag == "MEDIA") + getOrCreateMedium().type = value; else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { std::set clusterNames; - for (const auto& valueList : values) + for (std::string_view valueList : values) { const std::vector splittedValues {splitAndTrimString(valueList, "/,;")}; - for (std::string_view value : splittedValues) clusterNames.insert(std::string {value}); } if (!clusterNames.empty()) - track.clusters[tag] = clusterNames; + track.tags[tag] = std::move(clusterNames); } } @@ -493,7 +494,7 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) track.release = getRelease(tags); if (track.release) - track.release->releaseArtists = getArtists(tags, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); + track.release->artists = getArtists(tags, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); track.artists = getArtists(tags, {"ARTISTS", "ARTIST"}, {"ARTISTSORT"}, {"MUSICBRAINZ_ARTISTID", "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ/ARTIST ID"}); track.conductorArtists = getArtists(tags, {"CONDUCTORS", "CONDUCTOR"}, {"CONDUCTORSSORT", "CONDUCTORSORT"}, {}); track.composerArtists = getArtists(tags, {"COMPOSERS", "COMPOSER"}, {"COMPOSERSSORT", "COMPOSERSORT"}, {}); diff --git a/src/libs/metadata/include/metadata/IParser.hpp b/src/libs/metadata/include/metadata/IParser.hpp index d3adc13c..80612bd4 100644 --- a/src/libs/metadata/include/metadata/IParser.hpp +++ b/src/libs/metadata/include/metadata/IParser.hpp @@ -33,16 +33,16 @@ namespace MetaData { - using Clusters = std::map /* names */>; + using Tags = std::map /* names */>; struct Artist { - std::string name; - std::optional sortName; - std::optional artistMBID; + std::string name; + std::optional sortName; + std::optional mbid; Artist(std::string_view _name) : name {_name} {} - Artist(std::string_view _name, std::optional _sortName, std::optional _artistMBID) : name {_name}, sortName {std::move(_sortName)}, artistMBID {std::move(_artistMBID)} {} + Artist(std::string_view _name, std::optional _sortName, std::optional _mbid) : name {_name}, sortName {std::move(_sortName)}, mbid {std::move(_mbid)} {} }; using PerformerContainer = std::map>; @@ -50,16 +50,18 @@ namespace MetaData struct Release { std::string name; - std::vector releaseArtists; - std::optional releaseMBID; - std::optional totalDisc; + std::vector artists; + std::optional mbid; + std::optional mediumCount; }; - struct Disc + struct Medium { - std::string subtitle; + std::optional position; + std::string type; + std::string name; std::optional replayGain; - std::optional totalTrack; + std::optional trackCount; }; struct AudioStream @@ -71,14 +73,13 @@ namespace MetaData { std::vector artists; std::string title; - std::optional trackMBID; + std::optional mbid; std::optional recordingMBID; std::optional release; - std::optional disc; - Clusters clusters; + std::optional medium; + Tags tags; std::chrono::milliseconds duration; - std::optional trackNumber; - std::optional discNumber; + std::optional position; // in medium Wt::WDate date; Wt::WDate originalDate; bool hasCover {}; diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index d2207494..ea567356 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -42,8 +42,8 @@ namespace { Artist::pointer artist {session.create(artistInfo.name)}; - if (artistInfo.artistMBID) - artist.modify()->setMBID(*artistInfo.artistMBID); + if (artistInfo.mbid) + artist.modify()->setMBID(*artistInfo.mbid); if (artistInfo.sortName) artist.modify()->setSortName(*artistInfo.sortName); @@ -76,9 +76,9 @@ namespace Artist::pointer artist; // First try to get by MBID - if (artistInfo.artistMBID) + if (artistInfo.mbid) { - artist = Artist::find(session, *artistInfo.artistMBID); + artist = Artist::find(session, *artistInfo.mbid); if (!artist) artist = createArtist(session, artistInfo); else @@ -120,8 +120,8 @@ namespace { if (release->getName() != releaseInfo.name) release.modify()->setName(releaseInfo.name); - if (release->getTotalDisc() != releaseInfo.totalDisc) - release.modify()->setTotalDisc(releaseInfo.totalDisc); + if (release->getTotalDisc() != releaseInfo.mediumCount) + release.modify()->setTotalDisc(releaseInfo.mediumCount); } Release::pointer @@ -130,11 +130,11 @@ namespace Release::pointer release; // First try to get by MBID - if (releaseInfo.releaseMBID) + if (releaseInfo.mbid) { - release = Release::find(session, *releaseInfo.releaseMBID); + release = Release::find(session, *releaseInfo.mbid); if (!release) - release = session.create(releaseInfo.name, releaseInfo.releaseMBID); + release = session.create(releaseInfo.name, releaseInfo.mbid); updateReleaseIfNeeded(release, releaseInfo); return release; @@ -165,17 +165,17 @@ namespace } std::vector - getOrCreateClusters(Session& session, const MetaData::Clusters& clustersNames) + getOrCreateClusters(Session& session, const MetaData::Tags& tags) { - std::vector< Cluster::pointer > clusters; + std::vector clusters; - for (auto clusterNames : clustersNames) + for (const auto& [tag, values] : tags) { - auto clusterType = ClusterType::find(session, clusterNames.first); + auto clusterType = ClusterType::find(session, tag); if (!clusterType) continue; - for (auto clusterName : clusterNames.second) + for (auto clusterName : values) { auto cluster = clusterType->getCluster(clusterName); if (!cluster) @@ -287,9 +287,9 @@ namespace Scanner Track::pointer track {Track::findByPath(dbSession, file) }; - if (trackInfo->trackMBID && (!track || _settings.skipDuplicateMBID)) + if (trackInfo->mbid && (!track || _settings.skipDuplicateMBID)) { - std::vector duplicateTracks {Track::findByMBID(dbSession, *trackInfo->trackMBID)}; + std::vector duplicateTracks {Track::findByMBID(dbSession, *trackInfo->mbid)}; // find for existing MBIDs as the file may have just been moved if (!track && duplicateTracks.size() == 1) @@ -395,7 +395,7 @@ namespace Scanner if (trackInfo->release) { - for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->release->releaseArtists, false)) + for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->release->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, releaseArtist, TrackArtistLinkType::ReleaseArtist)); } @@ -430,16 +430,16 @@ namespace Scanner track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->release)); else track.modify()->setRelease({}); - track.modify()->setTotalTrack(trackInfo->disc ? trackInfo->disc->totalTrack : std::nullopt); - track.modify()->setReleaseReplayGain(trackInfo->disc ? trackInfo->disc->replayGain : std::nullopt); - track.modify()->setDiscSubtitle(trackInfo->disc ? trackInfo->disc->subtitle : ""); - track.modify()->setClusters(getOrCreateClusters(dbSession, trackInfo->clusters)); + track.modify()->setTotalTrack(trackInfo->medium ? trackInfo->medium->trackCount : std::nullopt); + track.modify()->setReleaseReplayGain(trackInfo->medium ? trackInfo->medium->replayGain : std::nullopt); + track.modify()->setDiscSubtitle(trackInfo->medium ? trackInfo->medium->name : ""); + track.modify()->setClusters(getOrCreateClusters(dbSession, trackInfo->tags)); track.modify()->setLastWriteTime(lastWriteTime); track.modify()->setName(title); track.modify()->setDuration(trackInfo->duration); track.modify()->setAddedTime(Wt::WDateTime::currentDateTime()); - track.modify()->setTrackNumber(trackInfo->trackNumber ? *trackInfo->trackNumber : 0); - track.modify()->setDiscNumber(trackInfo->discNumber ? *trackInfo->discNumber : 0); + track.modify()->setTrackNumber(trackInfo->position); + track.modify()->setDiscNumber(trackInfo->medium ? trackInfo->medium->position : std::nullopt); track.modify()->setDate(trackInfo->date); track.modify()->setOriginalDate(trackInfo->originalDate); @@ -448,7 +448,7 @@ namespace Scanner track.modify()->setDate(trackInfo->originalDate); track.modify()->setRecordingMBID(trackInfo->recordingMBID); - track.modify()->setTrackMBID(trackInfo->trackMBID); + track.modify()->setTrackMBID(trackInfo->mbid); if (auto trackFeatures {TrackFeatures::find(dbSession, track->getId())}) trackFeatures.remove(); // TODO: only if MBID changed? track.modify()->setHasCover(trackInfo->hasCover); diff --git a/src/tools/metadata/LmsMetadata.cpp b/src/tools/metadata/LmsMetadata.cpp index 628a968a..d0f372a9 100644 --- a/src/tools/metadata/LmsMetadata.cpp +++ b/src/tools/metadata/LmsMetadata.cpp @@ -35,8 +35,8 @@ operator<<(std::ostream& os, const MetaData::Artist& artist) { os << artist.name; - if (artist.artistMBID) - os << " (" << artist.artistMBID->getAsString() << ")"; + if (artist.mbid) + os << " (" << artist.mbid->getAsString() << ")"; if (artist.sortName) os << " '" << *artist.sortName << "'"; @@ -50,30 +50,36 @@ operator<<(std::ostream& os, const MetaData::Release& release) { os << release.name; - if (release.releaseMBID) - os << " (" << release.releaseMBID->getAsString() << ")" << std::endl; + if (release.mbid) + os << " (" << release.mbid->getAsString() << ")" << std::endl; - if (release.totalDisc) - std::cout << "\tTotalDisc: " << *release.totalDisc << std::endl; + if (release.mediumCount) + std::cout << "\tMediumCount: " << *release.mediumCount << std::endl; - for (const MetaData::Artist& artist : release.releaseArtists) - std::cout << "\tAlbum artist: " << artist << std::endl; + for (const MetaData::Artist& artist : release.artists) + std::cout << "\tRelease artist: " << artist << std::endl; return os; } static std::ostream& -operator<<(std::ostream& os, const MetaData::Disc& disc) +operator<<(std::ostream& os, const MetaData::Medium& medium) { - if (!disc.subtitle.empty()) - os << disc.subtitle << std::endl; + if (!medium.name.empty()) + os << medium.name << std::endl; - if (disc.totalTrack) - std::cout << "\tTotalTrack: " << *disc.totalTrack << std::endl; + if (medium.position) + os << "\tPosition: " << *medium.position << std::endl; - if (disc.replayGain) - std::cout << "\tDisc replay gain: " << *disc.replayGain << std::endl; + if (!medium.type.empty()) + os << "\tType: " << medium.type << std::endl; + + if (medium.trackCount) + std::cout << "\tTrackCount: " << *medium.trackCount << std::endl; + + if (medium.replayGain) + std::cout << "\tReplay gain: " << *medium.replayGain << std::endl; return os; } @@ -131,33 +137,30 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) if (track->release) std::cout << "Release: " << *track->release; - if (track->disc) - std::cout << "Disc: " << *track->disc; + if (track->medium) + std::cout << "Medium: " << *track->medium; std::cout << "Title: " << track->title << std::endl; - if (track->trackMBID) - std::cout << "track MBID = " << track->trackMBID->getAsString() << std::endl; + if (track->mbid) + std::cout << "Track MBID = " << track->mbid->getAsString() << std::endl; if (track->recordingMBID) - std::cout << "recording MBID = " << track->recordingMBID->getAsString() << std::endl; + std::cout << "Recording MBID = " << track->recordingMBID->getAsString() << std::endl; - for (const auto& cluster : track->clusters) + for (const auto& [tag, values] : track->tags) { - std::cout << "Cluster: " << cluster.first << std::endl; - for (const auto& name : cluster.second) + std::cout << "Tag: " << tag << std::endl; + for (const auto& value : values) { - std::cout << "\t" << name << std::endl; + std::cout << "\t" << value << std::endl; } } std::cout << "Duration: " << std::fixed << std::setprecision(2) << track->duration.count() / 1000. << "s" << std::endl; - if (track->trackNumber) - std::cout << "Track: " << *track->trackNumber << std::endl; - - if (track->discNumber) - std::cout << "Disc: " << *track->discNumber << std::endl; + if (track->position) + std::cout << "Position: " << *track->position << std::endl; if (track->date.isValid()) std::cout << "Date: " << track->date.toString("yyyy-MM-dd") << std::endl; From 87df73990b9d0649aff9703476ebcaff49273c32 Mon Sep 17 00:00:00 2001 From: emeric Date: Wed, 8 Mar 2023 13:38:17 +0100 Subject: [PATCH 04/14] Minor update --- src/libs/metadata/impl/AvFormatParser.cpp | 6 +++--- src/libs/metadata/include/metadata/IParser.hpp | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libs/metadata/impl/AvFormatParser.cpp b/src/libs/metadata/impl/AvFormatParser.cpp index 08cea3de..c959ae97 100644 --- a/src/libs/metadata/impl/AvFormatParser.cpp +++ b/src/libs/metadata/impl/AvFormatParser.cpp @@ -94,7 +94,7 @@ getReleaseArtists(const Av::IAudioFile::MetadataMap& metadataMap) auto mbid {findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"})}; - return {Artist {*name, std::nullopt, mbid} }; + return {Artist {mbid, *name, std::nullopt} }; } static @@ -118,9 +118,9 @@ getArtists(const Av::IAudioFile::MetadataMap& metadataMap) for (std::size_t i {}; i < artistNames.size(); ++i) { if (artistMBIDs && artistNames.size() == artistMBIDs->size()) - artists.emplace_back(Artist {artistNames[i], std::nullopt, (*artistMBIDs)[i]}); + artists.emplace_back(Artist {(*artistMBIDs)[i], artistNames[i], std::nullopt}); else - artists.emplace_back(Artist {artistNames[i], std::nullopt, {}}); + artists.emplace_back(Artist {std::nullopt, artistNames[i], std::nullopt}); } return artists; diff --git a/src/libs/metadata/include/metadata/IParser.hpp b/src/libs/metadata/include/metadata/IParser.hpp index 80612bd4..f23c5573 100644 --- a/src/libs/metadata/include/metadata/IParser.hpp +++ b/src/libs/metadata/include/metadata/IParser.hpp @@ -37,31 +37,31 @@ namespace MetaData struct Artist { + std::optional mbid; std::string name; std::optional sortName; - std::optional mbid; Artist(std::string_view _name) : name {_name} {} - Artist(std::string_view _name, std::optional _sortName, std::optional _mbid) : name {_name}, sortName {std::move(_sortName)}, mbid {std::move(_mbid)} {} + Artist(std::optional _mbid, std::string_view _name, std::optional _sortName) : mbid {std::move(_mbid)}, name {_name}, sortName {std::move(_sortName)} {} }; using PerformerContainer = std::map>; struct Release { + std::optional mbid; std::string name; std::vector artists; - std::optional mbid; std::optional mediumCount; }; struct Medium { - std::optional position; std::string type; std::string name; - std::optional replayGain; + std::optional position; // in release std::optional trackCount; + std::optional replayGain; }; struct AudioStream @@ -71,15 +71,14 @@ namespace MetaData struct Track { - std::vector artists; - std::string title; std::optional mbid; std::optional recordingMBID; - std::optional release; + std::string title; std::optional medium; + std::optional position; // in medium + std::optional release; Tags tags; std::chrono::milliseconds duration; - std::optional position; // in medium Wt::WDate date; Wt::WDate originalDate; bool hasCover {}; @@ -88,6 +87,7 @@ namespace MetaData std::string copyright; std::string copyrightURL; std::optional replayGain; + std::vector artists; std::vector conductorArtists; std::vector composerArtists; std::vector lyricistArtists; From a5de51c9155740049ae8c4a7110dc94eda36f99b Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 20 Mar 2023 21:24:28 +0100 Subject: [PATCH 05/14] Medium now part of a release --- src/libs/metadata/impl/AvFormatParser.cpp | 131 +++++++++-------- src/libs/metadata/impl/TagLibParser.cpp | 135 ++++++++++-------- src/libs/metadata/impl/Utils.hpp | 1 + .../metadata/include/metadata/IParser.hpp | 4 +- .../scanner/impl/ScanStepScanFiles.cpp | 8 +- src/tools/metadata/LmsMetadata.cpp | 11 +- 6 files changed, 165 insertions(+), 125 deletions(-) diff --git a/src/libs/metadata/impl/AvFormatParser.cpp b/src/libs/metadata/impl/AvFormatParser.cpp index c959ae97..8b3938f6 100644 --- a/src/libs/metadata/impl/AvFormatParser.cpp +++ b/src/libs/metadata/impl/AvFormatParser.cpp @@ -64,24 +64,6 @@ findFirstValueOfAs(const Av::IAudioFile::MetadataMap& metadataMap, std::initiali return res; } - -static -std::optional -getRelease(const Av::IAudioFile::MetadataMap& metadataMap) -{ - std::optional res; - - std::optional releaseName {findFirstValueOfAs(metadataMap, {"ALBUM"})}; - if (!releaseName) - return res; - - res.emplace(); - res->name = *releaseName; - res->mbid = findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"}); - - return res; -} - static std::vector getReleaseArtists(const Av::IAudioFile::MetadataMap& metadataMap) @@ -126,6 +108,75 @@ getArtists(const Av::IAudioFile::MetadataMap& metadataMap) return artists; } +static +std::optional +getRelease(const Av::IAudioFile::MetadataMap& metadataMap) +{ + std::optional res; + + std::optional releaseName {findFirstValueOfAs(metadataMap, {"ALBUM", "TALB", "WM/ALBUMTITLE"})}; + if (!releaseName) + return res; + + res.emplace(); + res->name = std::move(*releaseName); + res->mbid = findFirstValueOfAs(metadataMap, {"MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID", "MUSICBRAINZ/ALBUM ID"}); + res->artists = getReleaseArtists(metadataMap); + res->mediumCount = findFirstValueOfAs(metadataMap, {"TOTALDISCS", "DISCTOTAL"}); + if (!res->mediumCount) + { + // mediumCount may be encoded as position/count + if (const auto value {findFirstValueOfAs(metadataMap, {"TPOS", "DISC", "DISK", "DISCNUMBER", "WM/PARTOFSET"})}) + { + // Expecting 'Number/Total' + const std::vector strings {StringUtils::splitString(*value, "/") }; + if (strings.size() == 2) + res->mediumCount = StringUtils::readAs(strings[1]); + } + } + + return res; +} + +static +std::optional +getMedium(const Av::IAudioFile::MetadataMap& metadataMap) +{ + std::optional res; + res.emplace(); + + res->type = findFirstValueOfAs(metadataMap, {"TMED", "MEDIA", "WM/MEDIA"}).value_or(""); + res->name = findFirstValueOfAs(metadataMap, {"TSST", "DISCSUBTITLE", "SETSUBTITLE"}).value_or(""); + res->trackCount = findFirstValueOfAs(metadataMap, {"TOTALTRACKS", "TRACKTOTAL"}); + if (!res->trackCount) + { + // totalTracks may be encoded as "position/count" + if (const auto value {findFirstValueOfAs(metadataMap, {"TRCK", "TRACK", "TRACKNUMBER", "TRKN", "WM/TRACKNUMBER"})}) + { + // Expecting 'Number/Total' + const std::vector strings {StringUtils::splitString(*value, "/") }; + if (strings.size() == 2) + res->trackCount = StringUtils::readAs(strings[1]); + } + } + + // position may be encoded in TPOS/DISC/DISK as "position/count". Expecting 'Number[/Total]' + res->position = findFirstValueOfAs(metadataMap, {"TPOS", "DISC", "DISK", "DISCNUMBER", "WM/PARTOFSET"}); + res->release = getRelease(metadataMap); + + if (res->type.empty() + && res->name.empty() + && !res->trackCount + && !res->position + && !res->release + && !res->replayGain) + { + res.reset(); + } + + return res; +} + std::optional AvFormatParser::parse(const std::filesystem::path& p, bool debug) { @@ -154,16 +205,7 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) const Av::IAudioFile::MetadataMap metadataMap {mediaFile->getMetaData()}; track.artists = getArtists(metadataMap); - track.release = getRelease(metadataMap); - if (track.release) - track.release->artists = getReleaseArtists(metadataMap); - - auto getOrCreateMedium = [&]() -> Medium& - { - if (!track.medium) - track.medium.emplace(); - return *track.medium; - }; + track.medium = getMedium(metadataMap); for (const auto& [tag, value] : metadataMap) { @@ -175,30 +217,11 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) else if (tag == "TRACK") { // Expecting 'Number/Total' - const std::vector strings {StringUtils::splitString(value, "/") }; - if (strings.size() > 0) - { - track.position = StringUtils::readAs(strings[0]); - - if (strings.size() > 1) - getOrCreateMedium().trackCount = StringUtils::readAs(strings[1]); - } - } - else if (tag == "DISC") - { - // Expecting 'Number/Total' - const std::vector strings {StringUtils::splitString(value, "/")}; - if (strings.size() > 0) - { - getOrCreateMedium().position = StringUtils::readAs(strings[0]); - - if (strings.size() > 1 && track.release) - track.release->mediumCount = StringUtils::readAs(strings[1]); - } + track.position = StringUtils::readAs(value); } else if (tag == "DATE" || tag == "YEAR" - || tag == "WM/Year") + || tag == "WM/YEAR") { track.date = Utils::parseDate(value); } @@ -221,16 +244,6 @@ AvFormatParser::parse(const std::filesystem::path& p, bool debug) { track.recordingMBID = UUID::fromString(value); } - else if (tag == "MEDIA") - { - getOrCreateMedium().type = value; - } - else if (tag == "TSST" - || tag == "DISCSUBTITLE" - || tag == "SETSUBTITLE") - { - getOrCreateMedium().name = value; - } else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { const std::vector clusterNames {StringUtils::splitString(value, "/,;")}; diff --git a/src/libs/metadata/impl/TagLibParser.cpp b/src/libs/metadata/impl/TagLibParser.cpp index a3caa7f3..b9144658 100644 --- a/src/libs/metadata/impl/TagLibParser.cpp +++ b/src/libs/metadata/impl/TagLibParser.cpp @@ -81,6 +81,18 @@ getPropertyValuesFirstMatchAs(const TagMap& tags, const std::vector +std::optional +getPropertyValueFirstMatchAs(const TagMap& tags, const std::vector& keys) +{ + std::optional res; + std::vector values {getPropertyValuesFirstMatchAs(tags, keys)}; + if (!values.empty()) + res = std::move(values.front()); + + return res; +} + template std::vector getPropertyValuesAs(const TagMap& tags, const std::string& key) @@ -88,6 +100,13 @@ getPropertyValuesAs(const TagMap& tags, const std::string& key) return getPropertyValuesFirstMatchAs(tags, {key}); } +template +std::optional +getPropertyValueAs(const TagMap& tags, const std::string& key) +{ + return getPropertyValueFirstMatchAs(tags, {key}); +} + static std::vector splitAndTrimString(std::string_view str, std::string_view delimiters) @@ -185,20 +204,70 @@ getRelease(const TagMap& tags) { std::optional release; - std::vector releaseName {getPropertyValuesAs(tags, "ALBUM")}; - if (releaseName.empty()) + auto releaseName {getPropertyValueAs(tags, "ALBUM")}; + if (!releaseName) return release; - const std::vector releaseMBID {getPropertyValuesFirstMatchAs(tags, {"MUSICBRAINZ_ALBUMID", "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ/ALBUM ID"})}; - release.emplace(); - release->name = std::move(releaseName.front()); - if (!releaseMBID.empty()) - release->mbid = releaseMBID.front(); + release->name = std::move(*releaseName); + release->mbid = getPropertyValueFirstMatchAs(tags, {"MUSICBRAINZ_ALBUMID", "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ/ALBUM ID"}); + release->artists = getArtists(tags, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); + release->mediumCount = getPropertyValueAs(tags, "DISCTOTAL"); + if (!release->mediumCount) + { + // mediumCount may be encoded as "position/count" + if (const auto value {getPropertyValueAs(tags, "DISCNUMBER")}) + { + // Expecting 'Number/Total' + const std::vector strings {StringUtils::splitString(*value, "/") }; + if (strings.size() == 2) + release->mediumCount = StringUtils::readAs(strings[1]); + } + } return release; } +static +std::optional +getMedium(const TagMap& tags) +{ + std::optional medium; + medium.emplace(); + + medium->type = getPropertyValueAs(tags, "MEDIA").value_or(""); + medium->name = getPropertyValueFirstMatchAs(tags, {"DISCSUBTITLE", "SETSUBTITLE"}).value_or(""); + medium->trackCount = getPropertyValueAs(tags, "TRACKTOTAL"); + if (!medium->trackCount) + { + // totalTracks may be encoded as "position/count" + if (const auto value {getPropertyValueAs(tags, "TRACKNUMBER")}) + { + // Expecting 'Number/Total' + const std::vector strings {StringUtils::splitString(*value, "/") }; + if (strings.size() == 2) + medium->trackCount = StringUtils::readAs(strings[1]); + } + + } + // Expecting 'Number[/Total]' + medium->position = getPropertyValueAs(tags, "DISCNUMBER"); + medium->release = getRelease(tags); + medium->replayGain = getPropertyValueAs(tags, "REPLAYGAIN_ALBUM_GAIN"); + + if (medium->type.empty() + && medium->name.empty() + && !medium->trackCount + && !medium->position + && !medium->release + && !medium->replayGain) + { + medium.reset(); + } + + return medium; +} + static TagLib::AudioProperties::ReadStyle readStyleToTagLibReadStyle(ParserReadStyle readStyle) @@ -227,13 +296,6 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector if (tag.empty() || values.empty()) return; - auto getOrCreateMedium = [&]() -> Medium& - { - if (!track.medium) - track.medium.emplace(); - return *track.medium; - }; - std::string_view value {values.front()}; if (tag == "TITLE") @@ -250,41 +312,10 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector track.recordingMBID = UUID::fromString(value); else if (tag == "ACOUSTID_ID") track.acoustID = UUID::fromString(value); - else if (tag == "TRACKTOTAL") - { - getOrCreateMedium().trackCount = StringUtils::readAs(value); - } else if (tag == "TRACKNUMBER") { // Expecting 'Number/Total' - std::vector strings {splitAndTrimString(value, "/")}; - - if (!strings.empty()) - { - track.position = StringUtils::readAs(strings[0]); - - // Lower priority than TRACKTOTAL - if (strings.size() > 1 && !getOrCreateMedium().trackCount) - getOrCreateMedium().trackCount = StringUtils::readAs(strings[1]); - } - } - else if (tag == "DISCTOTAL") - { - if (track.release) - track.release->mediumCount = StringUtils::readAs(value); - } - else if (tag == "DISCNUMBER") - { - // Expecting 'Number/Total' - std::vector strings {StringUtils::splitString(value, "/")}; - if (!strings.empty()) - { - getOrCreateMedium().position = StringUtils::readAs(strings[0]); - - // Lower priority than DISCTOTAL - if (strings.size() > 1 && track.release && !track.release->mediumCount) - track.release->mediumCount = StringUtils::readAs(strings[1]); - } + track.position = StringUtils::readAs(value); } else if (tag == "DATE") { @@ -314,14 +345,8 @@ TagLibParser::processTag(Track& track, const std::string& tag, const std::vector track.copyright = value; else if (tag == "COPYRIGHTURL") track.copyrightURL = value; - else if (tag == "REPLAYGAIN_ALBUM_GAIN") - getOrCreateMedium().replayGain = StringUtils::readAs(value); else if (tag == "REPLAYGAIN_TRACK_GAIN") track.replayGain = StringUtils::readAs(value); - else if (tag == "DISCSUBTITLE" || tag == "SETSUBTITLE") - getOrCreateMedium().name = value; - else if (tag == "MEDIA") - getOrCreateMedium().type = value; else if (_clusterTypeNames.find(tag) != _clusterTypeNames.end()) { std::set clusterNames; @@ -395,7 +420,7 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) track.duration = std::chrono::milliseconds {properties->lengthInMilliseconds()}; MetaData::AudioStream audioStream {static_cast(properties->bitrate() * 1000)}; - track.audioStreams = {std::move(audioStream)}; + track.audioStreams = {audioStream}; } TagMap tags {constructTagMap(f.file()->properties())}; @@ -492,9 +517,7 @@ TagLibParser::parse(const std::filesystem::path& p, bool debug) track.hasCover = true; } - track.release = getRelease(tags); - if (track.release) - track.release->artists = getArtists(tags, {"ALBUMARTISTS", "ALBUMARTIST"}, {"ALBUMARTISTSSORT", "ALBUMARTISTSORT"}, {"MUSICBRAINZ_ALBUMARTISTID", "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ/ALBUM ARTIST ID"}); + track.medium = getMedium(tags); track.artists = getArtists(tags, {"ARTISTS", "ARTIST"}, {"ARTISTSORT"}, {"MUSICBRAINZ_ARTISTID", "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ/ARTIST ID"}); track.conductorArtists = getArtists(tags, {"CONDUCTORS", "CONDUCTOR"}, {"CONDUCTORSSORT", "CONDUCTORSORT"}, {}); track.composerArtists = getArtists(tags, {"COMPOSERS", "COMPOSER"}, {"COMPOSERSSORT", "COMPOSERSORT"}, {}); diff --git a/src/libs/metadata/impl/Utils.hpp b/src/libs/metadata/impl/Utils.hpp index 1268eb4c..5d89e2d9 100644 --- a/src/libs/metadata/impl/Utils.hpp +++ b/src/libs/metadata/impl/Utils.hpp @@ -38,5 +38,6 @@ namespace MetaData::Utils // format is "artist name (role)" PerformerArtist extractPerformerAndRole(std::string_view entry); + } diff --git a/src/libs/metadata/include/metadata/IParser.hpp b/src/libs/metadata/include/metadata/IParser.hpp index f23c5573..765d8ac6 100644 --- a/src/libs/metadata/include/metadata/IParser.hpp +++ b/src/libs/metadata/include/metadata/IParser.hpp @@ -35,6 +35,8 @@ namespace MetaData { using Tags = std::map /* names */>; + // Very simplified version of https://musicbrainz.org/doc/MusicBrainz_Database/Schema + struct Artist { std::optional mbid; @@ -59,6 +61,7 @@ namespace MetaData { std::string type; std::string name; + std::optional release; std::optional position; // in release std::optional trackCount; std::optional replayGain; @@ -76,7 +79,6 @@ namespace MetaData std::string title; std::optional medium; std::optional position; // in medium - std::optional release; Tags tags; std::chrono::milliseconds duration; Wt::WDate date; diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index ea567356..011fea52 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -393,9 +393,9 @@ namespace Scanner for (const Artist::pointer& artist : getOrCreateArtists(dbSession, trackInfo->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, artist, TrackArtistLinkType::Artist)); - if (trackInfo->release) + if (trackInfo->medium && trackInfo->medium->release) { - for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->release->artists, false)) + for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackInfo->medium->release->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, releaseArtist, TrackArtistLinkType::ReleaseArtist)); } @@ -426,8 +426,8 @@ namespace Scanner track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, remixer, TrackArtistLinkType::Remixer)); track.modify()->setScanVersion(_settings.scanVersion); - if (trackInfo->release) - track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->release)); + if (trackInfo->medium && trackInfo->medium->release) + track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->medium->release)); else track.modify()->setRelease({}); track.modify()->setTotalTrack(trackInfo->medium ? trackInfo->medium->trackCount : std::nullopt); diff --git a/src/tools/metadata/LmsMetadata.cpp b/src/tools/metadata/LmsMetadata.cpp index d0f372a9..81e77b18 100644 --- a/src/tools/metadata/LmsMetadata.cpp +++ b/src/tools/metadata/LmsMetadata.cpp @@ -67,7 +67,8 @@ std::ostream& operator<<(std::ostream& os, const MetaData::Medium& medium) { if (!medium.name.empty()) - os << medium.name << std::endl; + os << medium.name; + os << std::endl; if (medium.position) os << "\tPosition: " << *medium.position << std::endl; @@ -81,6 +82,9 @@ operator<<(std::ostream& os, const MetaData::Medium& medium) if (medium.replayGain) std::cout << "\tReplay gain: " << *medium.replayGain << std::endl; + if (medium.release) + std::cout << "Release: " << *medium.release << std::endl; + return os; } @@ -101,7 +105,7 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) std::cout << "Parsing time: " << std::fixed << std::setprecision(2) << std::chrono::duration_cast(end - start).count() / 1000. << "ms" << std::endl; - std::cout << "Track metadata:" << std::endl; + std::cout << "Parsed metadata:" << std::endl; for (const Artist& artist : track->artists) std::cout << "Artist: " << artist << std::endl; @@ -134,9 +138,6 @@ void parse(MetaData::IParser& parser, const std::filesystem::path& file) for (const Artist& artist : track->remixerArtists) std::cout << "Remixer: " << artist << std::endl; - if (track->release) - std::cout << "Release: " << *track->release; - if (track->medium) std::cout << "Medium: " << *track->medium; From a62810e47ac2464f89b9e4af41b7058bcdc8c897 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 26 Mar 2023 14:19:26 +0200 Subject: [PATCH 06/14] New layout test --- approot/tracks.xml | 9 ++++----- src/libs/services/database/impl/Track.cpp | 18 ++++++++---------- src/libs/services/database/test/Artist.cpp | 12 ++++++++++++ src/lms/ui/explore/ReleaseView.cpp | 15 ++------------- src/lms/ui/explore/TrackListHelpers.cpp | 11 +++++++++++ 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/approot/tracks.xml b/approot/tracks.xml index 44992ebb..2779b3bc 100644 --- a/approot/tracks.xml +++ b/approot/tracks.xml @@ -44,7 +44,10 @@
${name}
- ${}${release class="text-decoration-none link-success text-truncate"}${} +
+ ${}${artists class="d-none d-sm-inline"}${} + ${}${release class="d-inline text-decoration-none link-success"}${} +
${duration} @@ -64,10 +67,6 @@
- - ${artist class="text-decoration-none link-secondary"} - -