From 1c98775046b351b0c5e009da1e647769daa770db Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 3 Aug 2024 10:05:48 +0200 Subject: [PATCH 1/7] Aiff files: get bits per sample + cover, fixes #509 --- src/libs/metadata/impl/TagLibTagReader.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index cdf176fc6..1ddb40d00 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -300,6 +301,16 @@ namespace lms::metadata if (!opusFile->tag()->pictureList().isEmpty()) _hasEmbeddedCover = true; } + else if (TagLib::RIFF::AIFF::File * aiffFile{ dynamic_cast(_file.file()) }) + { + if (aiffFile->hasID3v2Tag()) + { + const auto& frameListMap{ aiffFile->tag()->frameListMap() }; + + if (!frameListMap["APIC"].isEmpty()) + _hasEmbeddedCover = true; + } + } if (debug && core::Service::get()->isSeverityActive(core::logging::Severity::DEBUG)) { @@ -334,6 +345,8 @@ namespace lms::metadata _audioProperties.bitsPerSample = mp4Properties->bitsPerSample(); else if (const auto* wavePackProperties{ dynamic_cast(properties) }) _audioProperties.bitsPerSample = wavePackProperties->bitsPerSample(); + else if (const auto* aiffProperties{ dynamic_cast(properties) }) + _audioProperties.bitsPerSample = aiffProperties->bitsPerSample(); #if TAGLIB_MAJOR_VERSION >= 2 else if (const auto* dsfProperties{ dynamic_cast(properties) }) _audioProperties.bitsPerSample = dsfProperties->bitsPerSample(); From 6494f3f783d0f67b344e8601d1736494055f1c5d Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 16 Aug 2024 16:05:21 +0200 Subject: [PATCH 2/7] Workaround original date parsing with taglib, fixes #506 --- src/libs/metadata/impl/TagLibTagReader.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index 1ddb40d00..89c7f45b6 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -274,6 +274,27 @@ namespace lms::metadata TagLib::MP4::CoverArtList coverArtList{ coverItem.toCoverArtList() }; if (!coverArtList.isEmpty()) _hasEmbeddedCover = true; + + if (!_propertyMap.contains("ORIGINALDATE")) + { + // For now: + // * TagLib 2.0 only parses ----:com.apple.iTunes:ORIGINALDATE + // / TagLib <2.0 only parses ----:com.apple.iTunes:originaldate + const auto& tags{ mp4File->tag()->itemMap() }; + for (const auto& origDateString : { "----:com.apple.iTunes:originaldate", "----:com.apple.iTunes:ORIGINALDATE" }) + { + auto itOrigDateTag{ tags.find(origDateString) }; + if (itOrigDateTag != std::cend(tags)) + { + const TagLib::StringList dates{ itOrigDateTag->second.toStringList() }; + if (!dates.isEmpty()) + { + _propertyMap["ORIGINALDATE"] = dates.front(); + break; + } + } + } + } } // MPC else if (TagLib::MPC::File * mpcFile{ dynamic_cast(_file.file()) }) From bf125817b0cdb5416775db6f07f03826f1f5c49e Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 19 Aug 2024 19:34:19 +0200 Subject: [PATCH 3/7] Improved the display artist name construction when tags are not set as expected, ref #513 --- src/libs/metadata/impl/Parser.cpp | 25 +++++++++--- src/libs/metadata/test/Parser.cpp | 64 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp index d99ffec57..c33b39b58 100644 --- a/src/libs/metadata/impl/Parser.cpp +++ b/src/libs/metadata/impl/Parser.cpp @@ -297,11 +297,26 @@ namespace lms::metadata track.medium = getMedium(tagReader); track.artists = getArtists(tagReader, { TagType::Artists, TagType::Artist }, { TagType::ArtistSortOrder }, { TagType::MusicBrainzArtistID }, _artistTagDelimiters); - // We consider the artist display name is put in the Artist tag (picard case) - // But to please most users, if we find a custom delimiter in the Artist tag, we construct the artist diplay string with a "nicer" join - if (!_artistTagDelimiters.empty() - && track.artists.size() > 1 - && getTagValuesAs(tagReader, TagType::Artist, _artistTagDelimiters).size() > 1) + auto needReconstructArtistDisplayName{ [&] { + // We consider the artist display name is put in the Artist tag (picard case) + + // To please most users, if we find a custom delimiter in the Artist tag, we construct the artist display string with a "nicer" join + if (!_artistTagDelimiters.empty() + && track.artists.size() > 1 + && getTagValuesAs(tagReader, TagType::Artist, _artistTagDelimiters).size() > 1) + { + return true; + } + // We have (true) multiple entries in the Artist tag or nothing + else if (getTagValuesAs(tagReader, TagType::Artist, {}).size() != 1) + { + return true; + } + + return false; + } }; + + if (needReconstructArtistDisplayName()) { std::vector artistNames; std::transform(std::cbegin(track.artists), std::cend(track.artists), std::back_inserter(artistNames), [](const Artist& artist) -> std::string_view { return artist.name; }); diff --git a/src/libs/metadata/test/Parser.cpp b/src/libs/metadata/test/Parser.cpp index 0b4833b91..bbb695322 100644 --- a/src/libs/metadata/test/Parser.cpp +++ b/src/libs/metadata/test/Parser.cpp @@ -252,4 +252,68 @@ namespace lms::metadata EXPECT_EQ(track->artists[1].name, "Other Artist"); EXPECT_EQ(track->artistDisplayName, "This / is ; One Artist, Other Artist"); // reconstruct artist display name since a custom delimiter is hit } + + TEST(Parser, noArtistInArtist) + { + const TestTagReader testTags{ + { + // nothing in Artist! + } + }; + + std::unique_ptr track{ Parser{}.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 0); + EXPECT_EQ(track->artistDisplayName, ""); + } + + TEST(Parser, singleArtistInArtist) + { + const TestTagReader testTags{ + { + // nothing in Artist! + { TagType::Artists, { "Artist1" } }, + } + }; + + std::unique_ptr track{ Parser{}.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 1); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artistDisplayName, "Artist1"); + } + + TEST(Parser, multipleArtistsInArtist) + { + const TestTagReader testTags{ + { + // nothing in Artists! + { TagType::Artist, { "Artist1", "Artist2" } }, + } + }; + + std::unique_ptr track{ Parser{}.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artists[1].name, "Artist2"); + EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct artist display name since multiple entries are found + } + + TEST(Parser, multipleArtistsInArtists) + { + const TestTagReader testTags{ + { + // nothing in Artist! + { TagType::Artists, { "Artist1", "Artist2" } }, + } + }; + + std::unique_ptr track{ Parser{}.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artists[1].name, "Artist2"); + EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct artist display name since multiple entries are found and nothing is set in artist + } } // namespace lms::metadata From d50f680f532bdfe7ce6cca6956ed15ae218fb99c Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 19 Aug 2024 21:40:01 +0200 Subject: [PATCH 4/7] Better wav support, fixes #512 --- src/libs/metadata/impl/TagLibTagReader.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index 89c7f45b6..0730eb324 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include "core/ILogger.hpp" #include "core/ITraceLogger.hpp" @@ -332,6 +333,16 @@ namespace lms::metadata _hasEmbeddedCover = true; } } + else if (TagLib::RIFF::WAV::File * wavFile{ dynamic_cast(_file.file()) }) + { + if (wavFile->hasID3v2Tag()) + { + const auto& frameListMap{ wavFile->tag()->frameListMap() }; + + if (!frameListMap["APIC"].isEmpty()) + _hasEmbeddedCover = true; + } + } if (debug && core::Service::get()->isSeverityActive(core::logging::Severity::DEBUG)) { @@ -368,6 +379,8 @@ namespace lms::metadata _audioProperties.bitsPerSample = wavePackProperties->bitsPerSample(); else if (const auto* aiffProperties{ dynamic_cast(properties) }) _audioProperties.bitsPerSample = aiffProperties->bitsPerSample(); + else if (const auto* wavProperties{ dynamic_cast(properties) }) + _audioProperties.bitsPerSample = wavProperties->bitsPerSample(); #if TAGLIB_MAJOR_VERSION >= 2 else if (const auto* dsfProperties{ dynamic_cast(properties) }) _audioProperties.bitsPerSample = dsfProperties->bitsPerSample(); From a231b4a12ad8a7ca6dfe1e732a555f03e8e31054 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 20 Aug 2024 09:05:29 +0200 Subject: [PATCH 5/7] Fixed build --- src/libs/metadata/impl/TagLibTagReader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index 0730eb324..20a409fe1 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -337,7 +337,7 @@ namespace lms::metadata { if (wavFile->hasID3v2Tag()) { - const auto& frameListMap{ wavFile->tag()->frameListMap() }; + const auto& frameListMap{ wavFile->ID3v2Tag()->frameListMap() }; if (!frameListMap["APIC"].isEmpty()) _hasEmbeddedCover = true; From ce8d75d71d3101c0d014e25d79923d080edf7b02 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 20 Aug 2024 09:14:49 +0200 Subject: [PATCH 6/7] Fixed format --- src/libs/metadata/impl/TagLibTagReader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index 20a409fe1..6a4d08310 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -38,8 +38,8 @@ #include #include #include -#include #include +#include #include "core/ILogger.hpp" #include "core/ITraceLogger.hpp" From cd0e88d28d693ec335716369e7571e619bbc4428 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 20 Aug 2024 19:03:28 +0200 Subject: [PATCH 7/7] 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}
+ ${
}