Merge branch 'lyrics-fixes' for release v3.60.1

epoupon · Nov 11, 2024 · 8bbee8a · 8bbee8a
2 parents 1842c0e + 0174007
commit 8bbee8a
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 51 deletions.
diff --git a/src/libs/metadata/impl/Lyrics.cpp b/src/libs/metadata/impl/Lyrics.cpp
@@ -34,59 +34,63 @@ 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<std::string_view::size_type>(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))
-            {
-                std::string_view tagType{ getSubmatchString(match[1]) };
-                std::string_view tagValue{ core::stringUtils::stringTrim(getSubmatchString(match[2])) };
+            const auto separator{ line.find(':') };
+            if (separator == std::string_view::npos)
+                return false;
 
-                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<int>(tagValue) })
-                        lyrics.offset = std::chrono::milliseconds{ *value };
-                }
-                // not interrested by other tags like 'duration', 'id', etc.
+            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)) };
 
-                return true;
+            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")
+            {
+                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<int>(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
-        void extractTimestamps(std::string_view line, std::vector<std::chrono::milliseconds>& 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<std::chrono::milliseconds>& 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)
             {
@@ -107,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
 
@@ -172,10 +173,10 @@ namespace lms::metadata
             if (currentState == State::None && trimmedLine.empty())
                 continue;
 
-            if (parseIDTag(trimmedLine, lyrics))
+            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())
@@ -186,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
@@ -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
@@ -163,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

diff --git 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<std::filesystem::path> 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++;

diff --git 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 } } },
         };
 

diff --git 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<std::chrono::milliseconds> 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())