From cd0e88d28d693ec335716369e7571e619bbc4428 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 20 Aug 2024 19:03:28 +0200 Subject: [PATCH] Added support for comment tag, fixes #510 --- SUBSONIC.md | 35 ++++++++++--------- approot/tracks.xml | 4 +++ src/libs/database/impl/Migration.cpp | 12 ++++++- src/libs/database/include/database/Track.hpp | 6 ++++ src/libs/database/test/Track.cpp | 20 +++++++++++ src/libs/metadata/impl/Parser.cpp | 1 + src/libs/metadata/include/metadata/Types.hpp | 1 + src/libs/metadata/test/Parser.cpp | 4 +++ .../scanner/impl/ScanStepScanFiles.cpp | 1 + src/libs/subsonic/impl/responses/Song.cpp | 1 + src/lms/ui/explore/TrackListHelpers.cpp | 10 ++++-- src/tools/metadata/LmsMetadata.cpp | 3 ++ 12 files changed, 78 insertions(+), 20 deletions(-) diff --git a/SUBSONIC.md b/SUBSONIC.md index 25f246cd1..b23195660 100644 --- a/SUBSONIC.md +++ b/SUBSONIC.md @@ -6,7 +6,7 @@ Given the API limitations of folder navigation commands, it is recommended to pl The Subsonic API is enabled by default. -__Note__: since _LMS_ may store hashed and salted passwords or may forward authentication requests to external services, it cannot handle the __token authentication__ method. You may need to check your client to make sure to use the __password__ authentication method. Since logins/passwords are passed in plain text through URLs, it is highly recommended to use a unique password when using the Subsonic API. Note that this may affect the use of authentication via PAM. In any case, ensure that read access to the web server logs (and to the proxy, if applicable) is well protected. +__Note__: since _LMS_ may store hashed and salted passwords or may forward authentication requests to external services, it cannot handle the __token authentication__ method. You may need to check your client to make sure to use the __password__ authentication method. Since logins/passwords are passed in plain text through URLs, it is highly recommended to use a unique password when using the Subsonic API. Note that this may affect the use of authentication via PAM. In any case, ensure the web server logs (and proxy logs, if applicable) are properly secured. # OpenSubsonic API OpenSubsonic is an initiative to patch and extend the legacy Subsonic API. You'll find more details in the [official documentation](https://opensubsonic.netlify.app/) @@ -14,32 +14,33 @@ OpenSubsonic is an initiative to patch and extend the legacy Subsonic API. You'l ## Extra fields The following extra fields are implemented: * `Album` response: - * `mediaType` - * `played` - * `musicBrainzId` - * `genres` * `artists` + * `discTitles`: discs with no subtitle are omitted * `displayArtist` - * `releaseTypes` + * `genres` + * `isCompilation` + * `played` + * `mediaType` * `moods` + * `musicBrainzId` * `originalReleaseDate` - * `isCompilation` - * `discTitles`: discs with no subtitle are omitted + * `releaseTypes` * `Child` response: + * `albumArtists` + * `artists` * `bitDepth` - * `samplingRate` * `channelCount` - * `mediaType` - * `played` - * `musicBrainzId`: note this is actually the recording MBID when this response refers to a song - * `genres` - * `artists` - * `displayArtist` - * `albumArtists` - * `displayAlbumArtist` + * `comment` * `contributors` + * `displayAlbumArtist` + * `displayArtist` + * `genres` + * `mediaType` * `moods` + * `musicBrainzId`: note this is actually the recording MBID when this response refers to a song + * `played` * `replayGain` + * `samplingRate` * `Artist` response: * `mediaType` * `musicBrainzId` diff --git a/approot/tracks.xml b/approot/tracks.xml index 23bc4d735..25a5984c6 100644 --- a/approot/tracks.xml +++ b/approot/tracks.xml @@ -125,6 +125,10 @@ ${playcount} + ${} +
+ 
${comment}
+ ${}
diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index d1d5de1ef..6f11ba0e6 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -35,7 +35,7 @@ namespace lms::db { namespace { - static constexpr Version LMS_DATABASE_VERSION{ 62 }; + static constexpr Version LMS_DATABASE_VERSION{ 63 }; } VersionInfo::VersionInfo() @@ -664,6 +664,15 @@ SELECT session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1"); } + void migrateFromV62(Session& session) + { + // Add a new column comment + session.getDboSession()->execute("ALTER TABLE track ADD comment TEXT NOT NULL DEFAULT ''"); + + // Just increment the scan version of the settings to make the next scheduled scan rescan everything + session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1"); + } + bool doDbMigration(Session& session) { static const std::string outdatedMsg{ "Outdated database, please rebuild it (delete the .db file and restart)" }; @@ -702,6 +711,7 @@ SELECT { 59, migrateFromV59 }, { 60, migrateFromV60 }, { 61, migrateFromV61 }, + { 62, migrateFromV62 }, }; bool migrationPerformed{}; diff --git a/src/libs/database/include/database/Track.hpp b/src/libs/database/include/database/Track.hpp index cfe43e2bf..50060e64a 100644 --- a/src/libs/database/include/database/Track.hpp +++ b/src/libs/database/include/database/Track.hpp @@ -228,6 +228,7 @@ namespace lms::db void setTrackReplayGain(std::optional replayGain) { _trackReplayGain = replayGain; } void setReleaseReplayGain(std::optional replayGain) { _releaseReplayGain = replayGain; } // may be by disc! void setArtistDisplayName(std::string_view name) { _artistDisplayName = name; } + void setComment(std::string_view comment) { _comment = comment; } void clearArtistLinks(); void addArtistLink(const ObjectPtr& artistLink); void setRelease(ObjectPtr release) { _release = getDboPtr(release); } @@ -264,6 +265,8 @@ namespace lms::db std::optional getTrackReplayGain() const { return _trackReplayGain; } std::optional getReleaseReplayGain() const { return _releaseReplayGain; } std::string_view getArtistDisplayName() const { return _artistDisplayName; } + std::string_view getComment() const { return _comment; } + // no artistLinkTypes means get all std::vector> getArtists(core::EnumSet artistLinkTypes) const; // no type means all std::vector getArtistIds(core::EnumSet artistLinkTypes) const; // no type means all @@ -307,6 +310,8 @@ namespace lms::db Wt::Dbo::field(a, _trackReplayGain, "track_replay_gain"); Wt::Dbo::field(a, _releaseReplayGain, "release_replay_gain"); // here in Track since Release does not have concept of "disc" (yet?) Wt::Dbo::field(a, _artistDisplayName, "artist_display_name"); + Wt::Dbo::field(a, _comment, "comment"); + Wt::Dbo::belongsTo(a, _release, "release", Wt::Dbo::OnDeleteCascade); Wt::Dbo::belongsTo(a, _mediaLibrary, "media_library", Wt::Dbo::OnDeleteSetNull); // don't delete track on media library removal, we want to wait for the next scan to have a chance to migrate files Wt::Dbo::belongsTo(a, _directory, "directory", Wt::Dbo::OnDeleteCascade); @@ -350,6 +355,7 @@ namespace lms::db std::optional _trackReplayGain; std::optional _releaseReplayGain; std::string _artistDisplayName; + std::string _comment; Wt::Dbo::ptr _release; Wt::Dbo::ptr _mediaLibrary; diff --git a/src/libs/database/test/Track.cpp b/src/libs/database/test/Track.cpp index 28bc730a4..bd863bfbf 100644 --- a/src/libs/database/test/Track.cpp +++ b/src/libs/database/test/Track.cpp @@ -363,4 +363,24 @@ namespace lms::db::tests EXPECT_EQ(track->getSampleRate(), 44100); } } + + TEST_F(DatabaseFixture, Track_comment) + { + ScopedTrack track{ session }; + + { + auto transaction{ session.createReadTransaction() }; + EXPECT_EQ(track->getComment(), ""); + } + + { + auto transaction{ session.createWriteTransaction() }; + track.get().modify()->setComment("MyComment"); + } + + { + auto transaction{ session.createReadTransaction() }; + EXPECT_EQ(track->getComment(), "MyComment"); + } + } } // namespace lms::db::tests \ No newline at end of file diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp index c33b39b58..9f8d5d4b2 100644 --- a/src/libs/metadata/impl/Parser.cpp +++ b/src/libs/metadata/impl/Parser.cpp @@ -273,6 +273,7 @@ namespace lms::metadata track.originalYear = utils::parseYear(*dateStr); } + track.comments = getTagValuesAs(tagReader, TagType::Comment, {} /* no custom delimiter on comments */); track.copyright = getTagValueAs(tagReader, TagType::Copyright).value_or(""); track.copyrightURL = getTagValueAs(tagReader, TagType::CopyrightURL).value_or(""); track.replayGain = getTagValueAs(tagReader, TagType::ReplayGainTrackGain); diff --git a/src/libs/metadata/include/metadata/Types.hpp b/src/libs/metadata/include/metadata/Types.hpp index 9f69fb19b..5a508413e 100644 --- a/src/libs/metadata/include/metadata/Types.hpp +++ b/src/libs/metadata/include/metadata/Types.hpp @@ -117,6 +117,7 @@ namespace lms::metadata std::optional acoustID; std::string copyright; std::string copyrightURL; + std::vector comments; std::optional replayGain; std::string artistDisplayName; std::vector artists; diff --git a/src/libs/metadata/test/Parser.cpp b/src/libs/metadata/test/Parser.cpp index bbb695322..f2b62d06a 100644 --- a/src/libs/metadata/test/Parser.cpp +++ b/src/libs/metadata/test/Parser.cpp @@ -41,6 +41,7 @@ namespace lms::metadata { TagType::AlbumArtist, { "MyAlbumArtist1 & MyAlbumArtist2" } }, { TagType::AlbumArtists, { "MyAlbumArtist1", "MyAlbumArtist2" } }, { TagType::AlbumArtistsSortOrder, { "MyAlbumArtist1SortName", "MyAlbumArtist2SortName" } }, + { TagType::Comment, { "Comment1", "Comment2" } }, { TagType::Composer, { "MyComposer1", "MyComposer2" } }, { TagType::ComposerSortOrder, { "MyComposerSortOrder1", "MyComposerSortOrder2" } }, { TagType::Conductor, { "MyConductor1", "MyConductor2" } }, @@ -103,6 +104,9 @@ namespace lms::metadata EXPECT_EQ(track->artists[1].name, "MyArtist2"); EXPECT_EQ(track->artists[1].sortName, "MyArtist2SortName"); EXPECT_EQ(track->artists[1].mbid, core::UUID::fromString("5e2cf87f-c8d7-4504-8a86-954dc0840229")); + ASSERT_EQ(track->comments.size(), 2); + EXPECT_EQ(track->comments[0], "Comment1"); + EXPECT_EQ(track->comments[1], "Comment2"); ASSERT_EQ(track->composerArtists.size(), 2); EXPECT_EQ(track->composerArtists[0].name, "MyComposer1"); EXPECT_EQ(track->composerArtists[0].sortName, "MyComposerSortOrder1"); diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index a49d923b5..c3f877a0d 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -710,6 +710,7 @@ namespace lms::scanner track.modify()->setHasCover(trackMetadata->hasCover); track.modify()->setCopyright(trackMetadata->copyright); track.modify()->setCopyrightURL(trackMetadata->copyrightURL); + track.modify()->setComment(!trackMetadata->comments.empty() ? trackMetadata->comments.front() : ""); // only take the first one for now track.modify()->setTrackReplayGain(trackMetadata->replayGain); track.modify()->setArtistDisplayName(trackMetadata->artistDisplayName); diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index d01205e5f..d58b84d94 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -153,6 +153,7 @@ namespace lms::api::subsonic if (!context.enableOpenSubsonic) return trackResponse; + trackResponse.setAttribute("comment", track->getComment()); trackResponse.setAttribute("bitDepth", track->getBitsPerSample()); trackResponse.setAttribute("samplingRate", track->getSampleRate()); trackResponse.setAttribute("channelCount", track->getChannelCount()); diff --git a/src/lms/ui/explore/TrackListHelpers.cpp b/src/lms/ui/explore/TrackListHelpers.cpp index 92000a484..cd118bc39 100644 --- a/src/lms/ui/explore/TrackListHelpers.cpp +++ b/src/lms/ui/explore/TrackListHelpers.cpp @@ -112,7 +112,7 @@ namespace lms::ui::TrackListHelpers { std::unique_ptr artistContainer{ utils::createArtistAnchorList(std::vector(std::cbegin(artistIds), std::cend(artistIds))) }; auto artistsEntry{ std::make_unique