From 3ff58a131af14083f311ddcc0fa5b5e8d8ab038f Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 11 Nov 2024 10:02:11 +0100 Subject: [PATCH 1/5] Fixed lyrics parsing on very long tags --- src/libs/metadata/impl/Lyrics.cpp | 81 ++++++++++++++++--------------- src/libs/metadata/test/Lyrics.cpp | 28 +++++++++++ 2 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/libs/metadata/impl/Lyrics.cpp b/src/libs/metadata/impl/Lyrics.cpp index b7fe3e18..ee283d0e 100644 --- a/src/libs/metadata/impl/Lyrics.cpp +++ b/src/libs/metadata/impl/Lyrics.cpp @@ -34,50 +34,53 @@ namespace lms::metadata namespace { - std::string_view getSubmatchString(const std::csub_match& submatch) + // Parse a single line with a tag like [ar: Artist] and set the appropriate fields in the Lyrics object + bool parseTag(std::string_view line, Lyrics& lyrics) { - assert(submatch.matched); - return std::string_view{ submatch.first, static_cast(submatch.length()) }; - } + if (line.empty()) + return false; - // Parse a single line with ID tags like [ar: Artist] and set the appropriate fields in the Lyrics object - bool parseIDTag(std::string_view line, Lyrics& lyrics) - { - static const std::regex idTagRegex{ R"(^\[([a-zA-Z_]+):(.+?)\])" }; - std::cmatch match; + if (line.front() != '[' || line.back() != ']') // consider lines are trimmed + return false; - if (std::regex_search(line.data(), line.data() + line.size(), match, idTagRegex)) + const auto separator{ line.find(':') }; + if (separator == std::string_view::npos) + return false; + + const std::string_view tagType{ core::stringUtils::stringTrim(line.substr(1, separator - 1)) }; + const std::string_view tagValue{ core::stringUtils::stringTrim(line.substr(separator + 1, line.size() - separator - 2)) }; + + if (tagType.empty()) + return false; + + // check for timestamps + if (std::any_of(tagType.begin(), tagType.end(), [](char c) { return std::isdigit(c); })) + return false; + + if (tagType == "ar") { - std::string_view tagType{ getSubmatchString(match[1]) }; - std::string_view tagValue{ core::stringUtils::stringTrim(getSubmatchString(match[2])) }; - - if (tagType == "ar") - { - lyrics.displayArtist = tagValue; - } - else if (tagType == "al") - { - lyrics.displayAlbum = tagValue; - } - else if (tagType == "ti") - { - lyrics.displayTitle = tagValue; - } - else if (tagType == "la") - { - lyrics.language = tagValue; - } - else if (tagType == "offset") - { - if (const auto value{ core::stringUtils::readAs(tagValue) }) - lyrics.offset = std::chrono::milliseconds{ *value }; - } - // not interrested by other tags like 'duration', 'id', etc. - - return true; + lyrics.displayArtist = tagValue; } + else if (tagType == "al") + { + lyrics.displayAlbum = tagValue; + } + else if (tagType == "ti") + { + lyrics.displayTitle = tagValue; + } + else if (tagType == "la") + { + lyrics.language = tagValue; + } + else if (tagType == "offset") + { + if (const auto value{ core::stringUtils::readAs(tagValue) }) + lyrics.offset = std::chrono::milliseconds{ *value }; + } + // not interrested by other tags like 'duration', 'id', etc. - return false; + return true; } // Parse timestamps from a line and return the associated times in milliseconds @@ -172,7 +175,7 @@ namespace lms::metadata if (currentState == State::None && trimmedLine.empty()) continue; - if (parseIDTag(trimmedLine, lyrics)) + if (parseTag(trimmedLine, lyrics)) continue; extractTimestamps(trimmedLine, timestamps); diff --git a/src/libs/metadata/test/Lyrics.cpp b/src/libs/metadata/test/Lyrics.cpp index ee859f88..795391eb 100644 --- a/src/libs/metadata/test/Lyrics.cpp +++ b/src/libs/metadata/test/Lyrics.cpp @@ -72,6 +72,34 @@ namespace lms::metadata::tests EXPECT_EQ(lyrics.synchronizedLines.find(9s + 160ms)->second, "I, I just woke up from a dream"); } + TEST(Lyrics, tagsWithSpaces) + { + std::istringstream is{ R"([al: dqsxdkbu ] +[00:09.16]I, I just woke up from a dream)" }; + + const Lyrics lyrics{ parseLyrics(is) }; + + EXPECT_EQ(lyrics.unsynchronizedLines.size(), 0); + ASSERT_EQ(lyrics.synchronizedLines.size(), 1); + EXPECT_EQ(lyrics.displayAlbum, "dqsxdkbu"); + ASSERT_TRUE(lyrics.synchronizedLines.contains(9s + 160ms)); + EXPECT_EQ(lyrics.synchronizedLines.find(9s + 160ms)->second, "I, I just woke up from a dream"); + } + + TEST(Lyrics, tagIDsWithSpaces) + { + std::istringstream is{ R"([ al : dqsxdkbu ] +[00:09.16]I, I just woke up from a dream)" }; + + const Lyrics lyrics{ parseLyrics(is) }; + + EXPECT_EQ(lyrics.unsynchronizedLines.size(), 0); + ASSERT_EQ(lyrics.synchronizedLines.size(), 1); + EXPECT_EQ(lyrics.displayAlbum, "dqsxdkbu"); + ASSERT_TRUE(lyrics.synchronizedLines.contains(9s + 160ms)); + EXPECT_EQ(lyrics.synchronizedLines.find(9s + 160ms)->second, "I, I just woke up from a dream"); + } + TEST(Lyrics, tagAtTheEndOfLyrics) { std::istringstream is{ R"([00:03.30]Ooh, ooh From 056a9949d4be801f88997ceed58048dfd2a90180 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 11 Nov 2024 10:11:57 +0100 Subject: [PATCH 2/5] Correctly handle lyrics that contain ] chars --- src/libs/metadata/impl/Lyrics.cpp | 15 ++++++--------- src/libs/metadata/test/Lyrics.cpp | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libs/metadata/impl/Lyrics.cpp b/src/libs/metadata/impl/Lyrics.cpp index ee283d0e..f2dc67e6 100644 --- a/src/libs/metadata/impl/Lyrics.cpp +++ b/src/libs/metadata/impl/Lyrics.cpp @@ -83,13 +83,14 @@ namespace lms::metadata return true; } - // Parse timestamps from a line and return the associated times in milliseconds - void extractTimestamps(std::string_view line, std::vector& timestamps) + // Parse timestamps from a line, update the associated times in milliseconds and return the remaining line + std::string_view extractTimestamps(std::string_view line, std::vector& timestamps) { timestamps.clear(); static const std::regex timeTagRegex{ R"(\[(?:(\d{1,2}):)?(\d{1,2}):(\d{1,2})(?:\.(\d{1,3}))?\])" }; std::cregex_iterator regexIt(line.begin(), line.end(), timeTagRegex); std::cregex_iterator regexEnd; + std::string_view::size_type offset{}; while (regexIt != regexEnd) { @@ -110,15 +111,12 @@ namespace lms::metadata currentTimestamp += std::chrono::milliseconds{ fractional }; } + offset = match[0].second - line.data(); timestamps.push_back(currentTimestamp); ++regexIt; } - } - // Extract the lyric text from a line, removing any timestamps - std::string_view extractLyricText(std::string_view line) - { - return line.substr(line.find_last_of(']') + 1); + return line.substr(offset); } } // namespace @@ -178,7 +176,7 @@ namespace lms::metadata if (parseTag(trimmedLine, lyrics)) continue; - extractTimestamps(trimmedLine, timestamps); + const std::string_view lyricText{ extractTimestamps(trimmedLine, timestamps) }; // If there are timestamps, add as synchronized lyrics if (!timestamps.empty()) @@ -189,7 +187,6 @@ namespace lms::metadata currentState = State::SynchronizedLyrics; applyAccumulatedLyrics(); - std::string_view lyricText{ extractLyricText(trimmedLine) }; for (std::chrono::milliseconds timestamp : timestamps) lyrics.synchronizedLines.emplace(timestamp, lyricText); diff --git a/src/libs/metadata/test/Lyrics.cpp b/src/libs/metadata/test/Lyrics.cpp index 795391eb..7756fbd6 100644 --- a/src/libs/metadata/test/Lyrics.cpp +++ b/src/libs/metadata/test/Lyrics.cpp @@ -191,6 +191,22 @@ Some unsynchronized lyrics EXPECT_EQ(lyrics.synchronizedLines.find(3s + 300ms)->second, "Ooh, ooh"); } + TEST(Lyrics, synchronized_withTimestampsDelimiters) + { + std::istringstream is{ R"([00:03.30]Ooh, ooh ] [])" }; + + const Lyrics lyrics{ parseLyrics(is) }; + + EXPECT_TRUE(lyrics.displayArtist.empty()); + EXPECT_TRUE(lyrics.displayAlbum.empty()); + EXPECT_TRUE(lyrics.displayTitle.empty()); + EXPECT_EQ(lyrics.offset, std::chrono::milliseconds{ 0 }); + EXPECT_EQ(lyrics.unsynchronizedLines.size(), 0); + ASSERT_EQ(lyrics.synchronizedLines.size(), 1); + ASSERT_TRUE(lyrics.synchronizedLines.contains(3s + 300ms)); + EXPECT_EQ(lyrics.synchronizedLines.find(3s + 300ms)->second, "Ooh, ooh ] []"); + } + TEST(Lyrics, synchronized_timestampFormats) { std::istringstream is{ R"([00:03.30]First line From 84d57bb82c825d2066741ec5ad7d61f66320df0e Mon Sep 17 00:00:00 2001 From: emeric Date: Wed, 6 Nov 2024 13:59:34 +0100 Subject: [PATCH 3/5] Fixed bad getLyricsBySongId response --- src/libs/subsonic/impl/responses/Lyrics.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/subsonic/impl/responses/Lyrics.cpp b/src/libs/subsonic/impl/responses/Lyrics.cpp index bc00620c..67b91454 100644 --- a/src/libs/subsonic/impl/responses/Lyrics.cpp +++ b/src/libs/subsonic/impl/responses/Lyrics.cpp @@ -83,7 +83,7 @@ namespace lms::api::subsonic if (lyrics->getOffset() != std::chrono::milliseconds{}) lyricsNode.setAttribute("offset", lyrics->getOffset().count()); - lyricsNode.createEmptyArrayChild("lines"); + lyricsNode.createEmptyArrayChild("line"); auto addLine{ [&](std::string&& line, std::optional timestamp = std::nullopt) { Response::Node lineNode; if (timestamp) @@ -98,7 +98,7 @@ namespace lms::api::subsonic lineNode.setValue(std::move(line)); break; } - lyricsNode.addArrayChild("lines", std::move(lineNode)); + lyricsNode.addArrayChild("line", std::move(lineNode)); } }; if (!lyrics->isSynchronized()) From 7bbfe614bdb29dc7db8814eb5826005f0f78b7da Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 5 Nov 2024 22:23:24 +0100 Subject: [PATCH 4/5] Fixed counter in the scanner's discovery step --- .../services/scanner/impl/ScanStepDiscoverFiles.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libs/services/scanner/impl/ScanStepDiscoverFiles.cpp b/src/libs/services/scanner/impl/ScanStepDiscoverFiles.cpp index b9f9acee..325bab0b 100644 --- a/src/libs/services/scanner/impl/ScanStepDiscoverFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepDiscoverFiles.cpp @@ -28,6 +28,14 @@ namespace lms::scanner { context.stats.totalFileCount = 0; + std::vector supportedExtensions; + for (const auto& extension : _settings.supportedAudioFileExtensions) + supportedExtensions.emplace_back(extension); + for (const auto& extension : _settings.supportedImageFileExtensions) + supportedExtensions.emplace_back(extension); + for (const auto& extension : _settings.supportedLyricsFileExtensions) + supportedExtensions.emplace_back(extension); + for (const ScannerSettings::MediaLibraryInfo& mediaLibrary : _settings.mediaLibraries) { std::size_t currentDirectoryProcessElemsCount{}; @@ -36,7 +44,7 @@ namespace lms::scanner if (_abortScan) return false; - if (!ec && (core::pathUtils::hasFileAnyExtension(path, _settings.supportedAudioFileExtensions) || core::pathUtils::hasFileAnyExtension(path, _settings.supportedImageFileExtensions))) + if (!ec && core::pathUtils::hasFileAnyExtension(path, supportedExtensions)) { context.currentStepStats.processedElems++; currentDirectoryProcessElemsCount++; From 017400767aa38f1869a1cfe9ec95a50f7926d52e Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 11 Nov 2024 14:05:38 +0100 Subject: [PATCH 5/5] No need to be user to get scan status --- src/libs/subsonic/impl/SubsonicResource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index bf034115..e12a691f 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -254,7 +254,7 @@ namespace lms::api::subsonic { "/savePlayQueue", { handleNotImplemented } }, // Media library scanning - { "/getScanStatus", { Scan::handleGetScanStatus, { db::UserType::ADMIN } } }, + { "/getScanStatus", { Scan::handleGetScanStatus } }, { "/startScan", { Scan::handleStartScan, { db::UserType::ADMIN } } }, };