Merge pull request #25371 from notspiff/move_load_tracks_from_cue

changed: move implementation of LoadTracksFromCueDocument to CCueDocument

5 files changed, 154 insertions, 53 deletions

diff --git a/xbmc/CueDocument.cpp b/xbmc/CueDocument.cpp
index 2d09d07f90..2094b02492 100644
--- a/xbmc/CueDocument.cpp
+++ b/xbmc/CueDocument.cpp
@@ -48,6 +48,7 @@
 #include "Util.h"
 #include "filesystem/Directory.h"
 #include "filesystem/File.h"
+#include "music/tags/MusicInfoTag.h"
 #include "settings/AdvancedSettings.h"
 #include "settings/SettingsComponent.h"
 #include "utils/CharsetConverter.h"
@@ -482,3 +483,61 @@ bool CCueDocument::ResolvePath(std::string &strPath, const std::string &strBase)
   return true;
 }
 
+bool CCueDocument::LoadTracks(CFileItemList& scannedItems, const CFileItem& item)
+{
+  const auto& tag = *item.GetMusicInfoTag();
+
+  VECSONGS tracks;
+  this->GetSongs(tracks);
+
+  bool oneFilePerTrack = this->IsOneFilePerTrack();
+
+  int tracksFound = 0;
+  for (auto& song : tracks)
+  {
+    if (song.strFileName == item.GetPath())
+    {
+      if (tag.Loaded())
+      {
+        if (song.strAlbum.empty() && !tag.GetAlbum().empty())
+          song.strAlbum = tag.GetAlbum();
+        //Pass album artist to final MusicInfoTag object via setting song album artist vector.
+        if (song.GetAlbumArtist().empty() && !tag.GetAlbumArtist().empty())
+          song.SetAlbumArtist(tag.GetAlbumArtist());
+        if (song.genre.empty() && !tag.GetGenre().empty())
+          song.genre = tag.GetGenre();
+        //Pass artist to final MusicInfoTag object via setting song artist description string only.
+        //Artist credits not used during loading from cue sheet.
+        if (song.strArtistDesc.empty() && !tag.GetArtistString().empty())
+          song.strArtistDesc = tag.GetArtistString();
+        if (tag.GetDiscNumber())
+          song.iTrack |= (tag.GetDiscNumber() << 16); // see CMusicInfoTag::GetDiscNumber()
+        if (!tag.GetCueSheet().empty())
+          song.strCueSheet = tag.GetCueSheet();
+
+        if (tag.GetYear())
+          song.strReleaseDate = tag.GetReleaseDate();
+        if (song.embeddedArt.Empty() && !tag.GetCoverArtInfo().Empty())
+          song.embeddedArt = tag.GetCoverArtInfo();
+      }
+
+      if (!song.iDuration && tag.GetDuration() > 0)
+      { // must be the last song
+        song.iDuration = CUtil::ConvertMilliSecsToSecsIntRounded(
+            CUtil::ConvertSecsToMilliSecs(tag.GetDuration()) - song.iStartOffset);
+      }
+      if (tag.Loaded() && oneFilePerTrack &&
+          !(tag.GetAlbum().empty() || tag.GetArtist().empty() || tag.GetTitle().empty()))
+      {
+        // If there are multiple files in a cue file, the tags from the files should be preferred if they exist.
+        scannedItems.Add(std::make_shared<CFileItem>(song, tag));
+      }
+      else
+      {
+        scannedItems.Add(std::make_shared<CFileItem>(song));
+      }
+      ++tracksFound;
+    }
+  }
+  return tracksFound != 0;
+}
diff --git a/xbmc/CueDocument.h b/xbmc/CueDocument.h
index 7218a9fe2d..22fa733de2 100644
--- a/xbmc/CueDocument.h
+++ b/xbmc/CueDocument.h
@@ -15,6 +15,8 @@
 
 #define MAX_PATH_SIZE 1024
 
+class CFileItem;
+class CFileItemList;
 class CueReader;
 
 class CCueDocument
@@ -42,6 +44,9 @@ public:
   void UpdateMediaFile(const std::string& oldMediaFile, const std::string& mediaFile);
   bool IsOneFilePerTrack() const;
   bool IsLoaded() const;
+
+  bool LoadTracks(CFileItemList& scannedItems, const CFileItem& item);
+
 private:
   void Clear();
   bool Parse(CueReader& reader, const std::string& strFile = std::string());
diff --git a/xbmc/FileItem.cpp b/xbmc/FileItem.cpp
index 69a23f7ee7..f3fe30b778 100644
--- a/xbmc/FileItem.cpp
+++ b/xbmc/FileItem.cpp
@@ -1908,61 +1908,10 @@ bool CFileItem::LoadTracksFromCueDocument(CFileItemList& scannedItems)
   if (!m_cueDocument)
     return false;
 
-  const CMusicInfoTag& tag = *GetMusicInfoTag();
-
-  VECSONGS tracks;
-  m_cueDocument->GetSongs(tracks);
-
-  bool oneFilePerTrack = m_cueDocument->IsOneFilePerTrack();
+  bool result = m_cueDocument->LoadTracks(scannedItems, *this);
   m_cueDocument.reset();
 
-  int tracksFound = 0;
-  for (VECSONGS::iterator it = tracks.begin(); it != tracks.end(); ++it)
-  {
-    CSong& song = *it;
-    if (song.strFileName == GetPath())
-    {
-      if (tag.Loaded())
-      {
-        if (song.strAlbum.empty() && !tag.GetAlbum().empty())
-          song.strAlbum = tag.GetAlbum();
-        //Pass album artist to final MusicInfoTag object via setting song album artist vector.
-        if (song.GetAlbumArtist().empty() && !tag.GetAlbumArtist().empty())
-          song.SetAlbumArtist(tag.GetAlbumArtist());
-        if (song.genre.empty() && !tag.GetGenre().empty())
-          song.genre = tag.GetGenre();
-        //Pass artist to final MusicInfoTag object via setting song artist description string only.
-        //Artist credits not used during loading from cue sheet.
-        if (song.strArtistDesc.empty() && !tag.GetArtistString().empty())
-          song.strArtistDesc = tag.GetArtistString();
-        if (tag.GetDiscNumber())
-          song.iTrack |= (tag.GetDiscNumber() << 16); // see CMusicInfoTag::GetDiscNumber()
-        if (!tag.GetCueSheet().empty())
-          song.strCueSheet = tag.GetCueSheet();
-
-        if (tag.GetYear())
-          song.strReleaseDate = tag.GetReleaseDate();
-        if (song.embeddedArt.Empty() && !tag.GetCoverArtInfo().Empty())
-          song.embeddedArt = tag.GetCoverArtInfo();
-      }
-
-      if (!song.iDuration && tag.GetDuration() > 0)
-      { // must be the last song
-        song.iDuration = CUtil::ConvertMilliSecsToSecsIntRounded(CUtil::ConvertSecsToMilliSecs(tag.GetDuration()) - song.iStartOffset);
-      }
-      if ( tag.Loaded() && oneFilePerTrack && ! ( tag.GetAlbum().empty() || tag.GetArtist().empty() || tag.GetTitle().empty() ) )
-      {
-        // If there are multiple files in a cue file, the tags from the files should be preferred if they exist.
-        scannedItems.Add(std::make_shared<CFileItem>(song, tag));
-      }
-      else
-      {
-        scannedItems.Add(std::make_shared<CFileItem>(song));
-      }
-      ++tracksFound;
-    }
-  }
-  return tracksFound != 0;
+  return result;
 }
 
 std::string CFileItem::GetUserMusicThumb(bool alwaysCheckRemote /* = false */, bool fallbackToFolder /* = false */) const
diff --git a/xbmc/test/CMakeLists.txt b/xbmc/test/CMakeLists.txt
index 2ee6066aff..41c1b4e21e 100644
--- a/xbmc/test/CMakeLists.txt
+++ b/xbmc/test/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(SOURCES TestBasicEnvironment.cpp
+            TestCueDocument.cpp
             TestFileItem.cpp
             TestURL.cpp
             TestUtil.cpp
diff --git a/xbmc/test/TestCueDocument.cpp b/xbmc/test/TestCueDocument.cpp
new file mode 100644
index 0000000000..e3ce24a0a5
--- /dev/null
+++ b/xbmc/test/TestCueDocument.cpp
@@ -0,0 +1,87 @@
+/*
+ *  Copyright (C) 2024 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "CueDocument.h"
+#include "FileItem.h"
+#include "FileItemList.h"
+#include "music/tags/MusicInfoTag.h"
+
+#include <array>
+#include <string>
+
+#include <gtest/gtest.h>
+
+using namespace KODI;
+
+namespace
+{
+
+const std::string input =
+    R"(PERFORMER "Pink Floyd"
+TITLE "The Dark Side Of The Moon"
+ FILE "The Dark Side Of The Moon.mp3" WAVE
+   TRACK 01 AUDIO
+     TITLE "Speak To Me / Breathe"
+     PERFORMER "Pink Floyd"
+     INDEX 00 00:00:00
+     INDEX 01 00:00:32
+   TRACK 02 AUDIO
+     TITLE "On The Run"
+     PERFORMER "Pink Floyd"
+     INDEX 00 03:58:72
+     INDEX 01 04:00:72
+   TRACK 03 AUDIO
+     TITLE "Time"
+     PERFORMER "Pink Floyd"
+     INDEX 00 07:31:70
+     INDEX 01 07:33:70)";
+
+}
+
+TEST(TestCueDocument, LoadTracks)
+{
+  CCueDocument doc;
+  doc.ParseTag(input);
+
+  using namespace std::string_literals;
+
+  CFileItem item("The Dark Side Of The Moon.mp3", false);
+  auto& tag = *item.GetMusicInfoTag();
+  tag.SetAlbum("TestAlbum"s);
+  tag.SetLoaded(true);
+  tag.SetAlbumArtist("TestAlbumArtist"s);
+  tag.SetGenre("TestGenre"s);
+  tag.SetArtist({"TestArtist1"s, "TestArtist2"s});
+  tag.SetCueSheet("TestCueSheet"s);
+  tag.SetYear(2005);
+  tag.SetDuration(554);
+
+  CFileItemList scannedItems;
+  doc.LoadTracks(scannedItems, item);
+
+  ASSERT_EQ(scannedItems.Size(), 3U);
+
+  static const auto trackNames = std::array{
+      "Speak To Me / Breathe"s,
+      "On The Run"s,
+      "Time"s,
+  };
+  static const auto duration = std::array{241, 213, 100};
+  for (size_t i = 0; i < 3; ++i)
+  {
+    EXPECT_EQ(scannedItems[i]->GetPath(), "The Dark Side Of The Moon.mp3");
+    ASSERT_TRUE(scannedItems[i]->GetMusicInfoTag() != nullptr);
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetArtist(), std::vector{"Pink Floyd"s});
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetAlbumArtist(), std::vector{"Pink Floyd"s});
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetCueSheet(), "TestCueSheet"s);
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetGenre(), std::vector{"TestGenre"s});
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetTitle(), trackNames[i]);
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetTrackNumber(), i + 1);
+    EXPECT_EQ(scannedItems[i]->GetMusicInfoTag()->GetDuration(), duration[i]);
+  }
+}