Merge pull request #10994 from DaveTBlake/SplitPerformerRoleOnly

Fix processing of Performer/TMCL/TIPL Tag values

4 files changed, 59 insertions, 11 deletions

diff --git a/xbmc/music/tags/MusicInfoTag.cpp b/xbmc/music/tags/MusicInfoTag.cpp
index 07967718cf..4f7e79953a 100644
--- a/xbmc/music/tags/MusicInfoTag.cpp
+++ b/xbmc/music/tags/MusicInfoTag.cpp
@@ -1021,7 +1021,7 @@ void CMusicInfoTag::AppendGenre(const std::string &genre)
 
 void CMusicInfoTag::AddArtistRole(const std::string& Role, const std::string& strArtist)
 {
-  if (!strArtist.empty())
+  if (!strArtist.empty() && !Role.empty())
     AddArtistRole(Role, StringUtils::Split(strArtist, g_advancedSettings.m_musicItemSeparator));
 }
 
diff --git a/xbmc/music/tags/TagLoaderTagLib.cpp b/xbmc/music/tags/TagLoaderTagLib.cpp
index ab7085005a..013a9d183f 100644
--- a/xbmc/music/tags/TagLoaderTagLib.cpp
+++ b/xbmc/music/tags/TagLoaderTagLib.cpp
@@ -981,32 +981,60 @@ void CTagLoaderTagLib::AddArtistRole(CMusicInfoTag &tag, const std::vector<std::
   if (values.size() % 2 != 0) // Must contain an even number of entries 
     return;
 
+  // Vector of possible separators
+  const std::vector<std::string> separators{ ";", "/", ",", "&", " and " };
+
   for (size_t i = 0; i + 1 < values.size(); i += 2)
-    tag.AddArtistRole(values[i], StringUtils::Split(values[i + 1], ","));
+  {
+    std::vector<std::string> roles;
+    //Split into individual roles
+    roles = StringUtils::Split(values[i], separators);
+    for (auto role : roles)
+    {
+      StringUtils::Trim(role);
+      StringUtils::ToCapitalize(role);
+      tag.AddArtistRole(role, StringUtils::Split(values[i + 1], ","));
+    }
+  }
 }
 
 void CTagLoaderTagLib::AddArtistInstrument(CMusicInfoTag &tag, const std::vector<std::string> &values)
 {
-  // Values is a musician credits list, each entry is artist name followed by instrument (or function)
-  // e.g. violin, drums, background vocals, solo, orchestra etc. in brackets. This is how Picard uses PERFORMER tag.
-  // If there is not a pair of brackets then role is "performer" by default, and the whole entry is 
-  // taken as artist name.
-  
+  /* Values is a musician credits list, each entry is artist name followed by instrument (or function) 
+     e.g. violin, drums, background vocals, solo, orchestra etc. in brackets. This is how Picard uses 
+     the PERFORMER tag. Multiple instruments may be in one tag 
+     e.g "Pierre Marchand (bass, drum machine and hammond organ)", 
+     these will be separated into individual roles. 
+     If there is not a pair of brackets then role is "performer" by default, and the whole entry is 
+     taken as artist name.
+  */
+  // Vector of possible separators
+  const std::vector<std::string> separators{";", "/", ",", "&", " and "};
+
   for (size_t i = 0; i < values.size(); ++i)
   {
-    std::string strRole = "Performer";
+    std::vector<std::string> roles;    
     std::string strArtist = values[i];
     size_t firstLim = values[i].find_first_of("(");
     size_t lastLim = values[i].find_last_of(")");
     if (lastLim != std::string::npos && firstLim != std::string::npos && firstLim < lastLim - 1)
     {
       //Pair of brackets with something between them
-      strRole = values[i].substr(firstLim + 1, lastLim - firstLim - 1);
-      StringUtils::Trim(strRole);
       strArtist.erase(firstLim, lastLim - firstLim + 1);
+      std::string strRole = values[i].substr(firstLim + 1, lastLim - firstLim - 1);
+      //Split into individual roles
+      roles = StringUtils::Split(strRole, separators);
     }
     StringUtils::Trim(strArtist);
-    tag.AddArtistRole(strRole, strArtist);
+    if (roles.empty())
+      tag.AddArtistRole("Performer", strArtist);
+    else
+      for (auto role : roles)
+      {
+        StringUtils::Trim(role);
+        StringUtils::ToCapitalize(role);
+        tag.AddArtistRole(role, strArtist);
+      }
   }
 }
 
diff --git a/xbmc/utils/StringUtils.cpp b/xbmc/utils/StringUtils.cpp
index 41e9fa5ab5..c8bd861368 100644
--- a/xbmc/utils/StringUtils.cpp
+++ b/xbmc/utils/StringUtils.cpp
@@ -756,6 +756,25 @@ std::vector<std::string> StringUtils::Split(const std::string& input, const char
   return results;
 }
 
+std::vector<std::string> StringUtils::Split(const std::string& input, const std::vector<std::string> &delimiters)
+{
+  std::vector<std::string> results;
+  if (input.empty())
+    return results;
+
+  if (delimiters.empty())
+  {
+    results.push_back(input);
+    return results;
+  }
+  std::string str = input;
+  for (size_t di = 1; di < delimiters.size(); di++)
+    StringUtils::Replace(str, delimiters[di], delimiters[0]);
+  results = Split(str, delimiters[0]);
+
+  return results;
+}
+
 std::vector<std::string> StringUtils::SplitMulti(const std::vector<std::string> &input, const std::vector<std::string> &delimiters, unsigned int iMaxStrings /* = 0 */)
 {
   if (input.empty())
diff --git a/xbmc/utils/StringUtils.h b/xbmc/utils/StringUtils.h
index d16e2b61b9..befe3e9c4e 100644
--- a/xbmc/utils/StringUtils.h
+++ b/xbmc/utils/StringUtils.h
@@ -106,6 +106,7 @@ public:
    */
   static std::vector<std::string> Split(const std::string& input, const std::string& delimiter, unsigned int iMaxStrings = 0);
   static std::vector<std::string> Split(const std::string& input, const char delimiter, size_t iMaxStrings = 0);
+  static std::vector<std::string> Split(const std::string& input, const std::vector<std::string> &delimiters);
   
   /*! \brief Splits the given input strings using the given delimiters into further separate strings.