Added support for comment tag, fixes #510

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`
-  * `displayArtist`
-  * `releaseTypes`
-  * `moods`
-  * `originalReleaseDate`
-  * `isCompilation`
   * `discTitles`: discs with no subtitle are omitted
-* `Child` response:
-  * `bitDepth`
-  * `samplingRate`
-  * `channelCount`
-  * `mediaType`
-  * `played`
-  * `musicBrainzId`: note this is actually the recording MBID when this response refers to a song
-  * `genres`
-  * `artists`
   * `displayArtist`
-  * `albumArtists`
+  * `genres`
-  * `displayAlbumArtist`
+  * `isCompilation`
-  * `contributors`
+  * `played`
+  * `mediaType`
   * `moods`
+  * `musicBrainzId`
+  * `originalReleaseDate`
+  * `releaseTypes`
+* `Child` response:
+  * `albumArtists`
+  * `artists`
+  * `bitDepth`
+  * `channelCount`
+  * `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`

approot/tracks.xml

@@ -125,6 +125,10 @@
 								${playcount}
 							</div>
 						</div>
+						${<if-has-comment>}
+						<hr/>
+						<pre>${comment}</pre>
+						${</if-has-comment>}
 					</div>
 				</div>
 				<div class="modal-footer">

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{};

src/libs/database/include/database/Track.hpp

@@ -228,6 +228,7 @@ namespace lms::db
         void setTrackReplayGain(std::optional<float> replayGain) { _trackReplayGain = replayGain; }
         void setReleaseReplayGain(std::optional<float> 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<TrackArtistLink>& artistLink);
         void setRelease(ObjectPtr<Release> release) { _release = getDboPtr(release); }
@@ -264,6 +265,8 @@ namespace lms::db
         std::optional<float> getTrackReplayGain() const { return _trackReplayGain; }
         std::optional<float> 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<ObjectPtr<Artist>> getArtists(core::EnumSet<TrackArtistLinkType> artistLinkTypes) const; // no type means all
         std::vector<ArtistId> getArtistIds(core::EnumSet<TrackArtistLinkType> 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<float> _trackReplayGain;
         std::optional<float> _releaseReplayGain;
         std::string _artistDisplayName;
+        std::string _comment;
 
         Wt::Dbo::ptr<Release> _release;
         Wt::Dbo::ptr<MediaLibrary> _mediaLibrary;

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

src/libs/metadata/impl/Parser.cpp

@@ -273,6 +273,7 @@ namespace lms::metadata
             track.originalYear = utils::parseYear(*dateStr);
         }
 
+        track.comments = getTagValuesAs<std::string>(tagReader, TagType::Comment, {} /* no custom delimiter on comments */);
         track.copyright = getTagValueAs<std::string>(tagReader, TagType::Copyright).value_or("");
         track.copyrightURL = getTagValueAs<std::string>(tagReader, TagType::CopyrightURL).value_or("");
         track.replayGain = getTagValueAs<float>(tagReader, TagType::ReplayGainTrackGain);

src/libs/metadata/include/metadata/Types.hpp

@@ -117,6 +117,7 @@ namespace lms::metadata
         std::optional<core::UUID> acoustID;
         std::string copyright;
         std::string copyrightURL;
+        std::vector<std::string> comments;
         std::optional<float> replayGain;
         std::string artistDisplayName;
         std::vector<Artist> artists;

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");

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);
 

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());

src/lms/ui/explore/TrackListHelpers.cpp

@@ -112,7 +112,7 @@ namespace lms::ui::TrackListHelpers
             {
                 std::unique_ptr<Wt::WContainerWidget> artistContainer{ utils::createArtistAnchorList(std::vector(std::cbegin(artistIds), std::cend(artistIds))) };
                 auto artistsEntry{ std::make_unique<Template>(Wt::WString::tr("Lms.Explore.template.info.artists")) };
-                artistsEntry->bindString("type", role);
+                artistsEntry->bindString("type", role, Wt::TextFormat::Plain);
                 artistsEntry->bindWidget("artist-container", std::move(artistContainer));
                 artistTable->addWidget(std::move(artistsEntry));
             }
@@ -124,7 +124,7 @@ namespace lms::ui::TrackListHelpers
             if (audioStream)
             {
                 trackInfo->setCondition("if-has-codec", true);
-                trackInfo->bindString("codec", audioStream->codecName);
+                trackInfo->bindString("codec", audioStream->codecName, Wt::TextFormat::Plain);
             }
         }
 
@@ -137,6 +137,12 @@ namespace lms::ui::TrackListHelpers
 
         trackInfo->bindInt("playcount", core::Service<scrobbling::IScrobblingService>::get()->getCount(LmsApp->getUserId(), track->getId()));
 
+        if (std::string_view comment{ track->getComment() }; !comment.empty())
+        {
+            trackInfo->setCondition("if-has-comment", true);
+            trackInfo->bindString("comment", Wt::WString::fromUTF8(std::string{ comment }), Wt::TextFormat::Plain);
+        }
+
         Wt::WContainerWidget* clusterContainer{ trackInfo->bindWidget("clusters", utils::createFilterClustersForTrack(track, filters)) };
         if (clusterContainer->count() > 0)
             trackInfo->setCondition("if-has-clusters", true);

src/tools/metadata/LmsMetadata.cpp

@@ -216,6 +216,9 @@ namespace lms::metadata
         if (!track->copyright.empty())
             std::cout << "Copyright: " << track->copyright << std::endl;
 
+        for (const auto& comment : track->comments)
+            std::cout << "Comment: '" << comment << "'" << std::endl;
+
         if (!track->copyrightURL.empty())
             std::cout << "CopyrightURL: " << track->copyrightURL << std::endl;