Fetch full song info and art types info, and set user rating as separate jobs from UI to avoid locking. Wait for fetched data, cancel possible while waiting should fetch be slow. Job populates the dialog item and art items list, then triggers dialog event to indicate completion.

Support selection of any type of art. OnGetArt replaces OnGetThumb, and art type selection in separate class for use by other music info dialog later. Handling fallback art, showing it as the current art when there is no specific song art.

Improve how CMusicInfoLoader::LoadAdditionalTagInfo fetches artist and album data as song item properties.

Album info button enabled only when album known (library item).

Use GUI_MSG_UPDATE_ITEM to efficiently keep GUI item art consistent.

10 files changed, 734 insertions, 207 deletions

diff --git a/addons/resource.language.en_gb/resources/strings.po b/addons/resource.language.en_gb/resources/strings.po
index a971560c9a..32177fece6 100644
--- a/addons/resource.language.en_gb/resources/strings.po
+++ b/addons/resource.language.en_gb/resources/strings.po
@@ -2589,6 +2589,7 @@ msgstr ""
 
 #: xbmc/dialogs/GUIDialogMediaFilter.cpp
 #: xbmc/playlists/SmartPlaylist.cpp
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 #: addons/skin.estuary/xml/DialogVideoInfo.xml
 #: addons/skin.estuary/xml/DialogFullScreenInfo.xml
@@ -3955,7 +3956,6 @@ msgstr ""
 
 #: xbmc/dialogs/GUIDialogFavourites.cpp
 #: xbmc/music/dialogs/GUIDialogMusicInfo.cpp
-#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 msgctxt "#1030"
 msgid "Browse for image"
 msgstr ""
@@ -7171,11 +7171,13 @@ msgctxt "#13510"
 msgid "Sync playback to display"
 msgstr ""
 
+#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#13511"
 msgid "Choose art"
 msgstr ""
 
+#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#13512"
 msgid "Current art"
@@ -7191,14 +7193,17 @@ msgctxt "#13514"
 msgid "Local art"
 msgstr ""
 
+#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#13515"
 msgid "No art"
 msgstr ""
 
+#. Button to add a new type of art e.g. "thumb", "poster", "fanart" etc. 
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#13516"
-msgid "Add art"
+msgid "Add art type"
 msgstr ""
 
 #: system/settings/settings.xml
@@ -7221,7 +7226,13 @@ msgctxt "#13520"
 msgid "Embedded fanart"
 msgstr ""
 
-#empty strings from id 13521 to 13549
+#. Heading for selection of a type of art e.g. "thumb", "poster", "fanart" etc. 
+#: xbmc/music/MusicUtils.cpp
+msgctxt "#13521"
+msgid "Choose art type"
+msgstr ""
+
+#empty strings from id 13522 to 13549
 
 #: system/settings/settings.xml
 msgctxt "#13550"
@@ -11804,7 +11815,6 @@ msgstr ""
 
 #: xbmc/dialogs/GUIDialogContextMenu.cpp
 #: xbmc/music/dialogs/GUIDialogMusicInfo.cpp
-#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 msgctxt "#20016"
 msgid "Current thumb"
 msgstr ""
@@ -11818,7 +11828,6 @@ msgstr ""
 
 #: xbmc/dialogs/GUIDialogContextMenu.cpp
 #: xbmc/music/dialogs/GUIDialogMusicInfo.cpp
-#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
 msgctxt "#20018"
 msgid "No thumb"
 msgstr ""
@@ -11830,6 +11839,7 @@ msgid "Choose thumbnail"
 msgstr ""
 
 #. Name of an artwork type
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 #: addons/skin.estuary/xml/View_501_Banner.xml
 msgctxt "#20020"
@@ -11837,6 +11847,7 @@ msgid "Banner"
 msgstr ""
 
 #. Name of an artwork type
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 #: addons/skin.estuary/xml/View_51_Poster.xml
 msgctxt "#20021"
@@ -13397,6 +13408,8 @@ msgctxt "#20444"
 msgid "Add to library"
 msgstr ""
 
+#. Name of an artwork type
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/music/dialogs/GUIDialogMusicInfo.cpp
 #: addons/skin.estuary/xml/View_502_FanArt.xml
 msgctxt "#20445"
@@ -13669,6 +13682,11 @@ msgctxt "#21370"
 msgid "Play GUI sounds during media playback"
 msgstr ""
 
+#. Name of an artwork type
+#: xbmc/music/MusicUtils.cpp
+#: xbmc/music/dialogs/GUIDialogSongInfo.cpp
+#: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
+#: xbmc/dialogs/GUIDialogFileBrowser.h
 msgctxt "#21371"
 msgid "Thumbnail"
 msgstr ""
@@ -20849,12 +20867,16 @@ msgstr ""
 
 #empty strings from id 38019 to 38021
 
-#. Used for the viewstate selection
+#. Used for the rating selection
+#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#38022"
 msgid "No rating"
 msgstr ""
 
+#. Used for the rating selection
+#: xbmc/music/MusicUtils.cpp
+#: xbmc/video/dialogs/GUIDialogVideoInfo.cpp
 msgctxt "#38023"
 msgid "Set my rating"
 msgstr ""
diff --git a/xbmc/music/CMakeLists.txt b/xbmc/music/CMakeLists.txt
index 8b80e19669..5e1fd420fa 100644
--- a/xbmc/music/CMakeLists.txt
+++ b/xbmc/music/CMakeLists.txt
@@ -7,6 +7,7 @@ set(SOURCES Album.cpp
             MusicInfoLoader.cpp
             MusicLibraryQueue.cpp
             MusicThumbLoader.cpp
+            MusicUtils.cpp
             Song.cpp)
 
 set(HEADERS Album.h
@@ -18,6 +19,7 @@ set(HEADERS Album.h
             MusicInfoLoader.h
             MusicLibraryQueue.h
             MusicThumbLoader.h
+            MusicUtils.h
             Song.h)
 
 core_add_library(music)
diff --git a/xbmc/music/MusicDatabase.cpp b/xbmc/music/MusicDatabase.cpp
index d1324e3528..0b37940520 100644
--- a/xbmc/music/MusicDatabase.cpp
+++ b/xbmc/music/MusicDatabase.cpp
@@ -5724,6 +5724,39 @@ int CMusicDatabase::GetArtistByMatch(const CArtist& artist)
   return -1;
 }
 
+bool CMusicDatabase::GetArtistFromSong(int idSong, CArtist &artist)
+{
+  try
+  {
+    if (NULL == m_pDB.get()) return false;
+    if (NULL == m_pDS.get()) return false;
+
+    std::string strSQL = PrepareSQL(
+      "SELECT artistview.* FROM song_artist "
+      "JOIN artistview ON song_artist.idArtist = artistview.idArtist "
+      "WHERE song_artist.idSong= %i AND song_artist.idRole = 1 AND song_artist.iOrder = 0", 
+      idSong);
+    if (!m_pDS->query(strSQL)) return false;
+    int iRowsFound = m_pDS->num_rows();
+    if (iRowsFound != 1)
+    {
+      m_pDS->close();
+      return false;
+    }
+
+    artist = GetArtistFromDataset(m_pDS.get());
+
+    m_pDS->close();
+    return true;
+
+  }
+  catch (...)
+  {
+    CLog::Log(LOGERROR, "%s failed", __FUNCTION__);
+  }
+  return false;
+}
+
 int CMusicDatabase::GetAlbumByName(const std::string& strAlbum, const std::string& strArtist)
 {
   try
@@ -6185,15 +6218,31 @@ bool CMusicDatabase::SetSongUserrating(const std::string &filePath, int userrati
     if (NULL == m_pDS.get()) return false;
 
     int songID = GetSongIDFromPath(filePath);
-    if (-1 == songID) return false;
+    if (-1 == songID) return false;    
+    
+    return SetSongUserrating(songID, userrating);
+  }
+  catch (...)
+  {
+    CLog::Log(LOGERROR, "%s (%s,%i) failed", __FUNCTION__, filePath.c_str(), userrating);
+  }
+  return false;
+}
 
-    std::string sql = PrepareSQL("UPDATE song SET userrating='%i' WHERE idSong = %i", userrating, songID);
+bool CMusicDatabase::SetSongUserrating(int idSong, int userrating)
+{
+  try
+  {
+    if (NULL == m_pDB.get()) return false;
+    if (NULL == m_pDS.get()) return false;
+
+    std::string sql = PrepareSQL("UPDATE song SET userrating='%i' WHERE idSong = %i", userrating, idSong);
     m_pDS->exec(sql);
     return true;
   }
   catch (...)
   {
-    CLog::Log(LOGERROR, "%s (%s,%i) failed", __FUNCTION__, filePath.c_str(), userrating);
+    CLog::Log(LOGERROR, "%s (%i,%i) failed", __FUNCTION__, idSong, userrating);
   }
   return false;
 }
@@ -7230,6 +7279,38 @@ bool CMusicDatabase::RemoveArtForItem(int mediaId, const MediaType & mediaType,
   return result;
 }
 
+bool CMusicDatabase::GetArtTypes(const MediaType &mediaType, std::vector<std::string> &artTypes)
+{
+  try
+  {
+    if (NULL == m_pDB.get()) return false;
+    if (NULL == m_pDS.get()) return false;
+
+    std::string strSQL = PrepareSQL("SELECT DISTINCT type FROM art WHERE media_type='%s'", mediaType.c_str());
+
+    if (!m_pDS->query(strSQL)) return false;
+    int iRowsFound = m_pDS->num_rows();
+    if (iRowsFound == 0)
+    {
+      m_pDS->close();
+      return false;
+    }
+
+    while (!m_pDS->eof())
+    {
+      artTypes.emplace_back(m_pDS->fv(0).get_asString());
+      m_pDS->next();
+    }
+    m_pDS->close();
+    return true;
+  }
+  catch (...)
+  {
+    CLog::Log(LOGERROR, "%s(%s) failed", __FUNCTION__, mediaType.c_str());
+  }
+  return false;
+}
+
 bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription &sorting)
 {
   if (!musicUrl.IsValid())
diff --git a/xbmc/music/MusicDatabase.h b/xbmc/music/MusicDatabase.h
index 66f967c1a9..deea2a1c56 100644
--- a/xbmc/music/MusicDatabase.h
+++ b/xbmc/music/MusicDatabase.h
@@ -208,6 +208,7 @@ public:
   bool Search(const std::string& search, CFileItemList &items);
   bool RemoveSongsFromPath(const std::string &path, MAPSONGS& songs, bool exact=true);
   bool SetSongUserrating(const std::string &filePath, int userrating);
+  bool SetSongUserrating(int idSong, int userrating);
   bool SetSongVotes(const std::string &filePath, int votes);
   int  GetSongByArtistAndAlbumAndTitle(const std::string& strArtist, const std::string& strAlbum, const std::string& strTitle);
 
@@ -321,6 +322,7 @@ public:
   std::string GetArtistById(int id);
   int GetArtistByName(const std::string& strArtist);
   int GetArtistByMatch(const CArtist& artist);
+  bool GetArtistFromSong(int idSong, CArtist &artist);
   std::string GetRoleById(int id);
 
   /*! \brief Propagate artist sort name into the concatenated artist sort name strings
@@ -552,6 +554,13 @@ public:
   */
   bool RemoveArtForItem(int mediaId, const MediaType &mediaType, const std::set<std::string> &artTypes);
 
+  /*! \brief Fetch the distinct types of art held in the database for a type of media.
+  \param mediaType the type of media, which corresponds to the table the item resides in (song/artist/album).
+  \param artTypes [out] the types of art e.g. "thumb", "fanart", etc. 
+  \return true if art is found, false if no art is found.
+  */
+  bool GetArtTypes(const MediaType &mediaType, std::vector<std::string> &artTypes);
+
   /////////////////////////////////////////////////
   // Tag Scan Version
   /////////////////////////////////////////////////
diff --git a/xbmc/music/MusicInfoLoader.cpp b/xbmc/music/MusicInfoLoader.cpp
index ca1d9b49f2..d8bd95a980 100644
--- a/xbmc/music/MusicInfoLoader.cpp
+++ b/xbmc/music/MusicInfoLoader.cpp
@@ -91,20 +91,36 @@ bool CMusicInfoLoader::LoadAdditionalTagInfo(CFileItem* pItem)
     return false; // already have the information
 
   std::string path(pItem->GetPath());
-  if (pItem->IsMusicDb())
+  // For songs in library set the (primary) song artist and album properties 
+  // Use song Id (not path) as called for items from either library or file view 
+  if (pItem->GetMusicInfoTag()->GetDatabaseId() > 0)
   {
-    // set the artist / album properties
-    XFILE::MUSICDATABASEDIRECTORY::CQueryParams param;
-    XFILE::MUSICDATABASEDIRECTORY::CDirectoryNode::GetDatabaseInfo(pItem->GetPath(),param);
-    CArtist artist;
     CMusicDatabase database;
     database.Open();
-    if (database.GetArtist(param.GetArtistId(), artist, false))
-      CMusicDatabase::SetPropertiesFromArtist(*pItem,artist);
+    // May already have song artist ids as item property, otherwise fetch from song id
+    CArtist artist;
+    bool artistfound = false;
+    if (pItem->HasProperty("artistid"))
+    {
+      CVariant::const_iterator_array varid = pItem->GetProperty("artistid").begin_array();
+      int idArtist = varid->asInteger();
+      artistfound = database.GetArtist(idArtist, artist, false);
+    }
+    else
+      artistfound = database.GetArtistFromSong(pItem->GetMusicInfoTag()->GetDatabaseId(), artist);
+    if (artistfound)
+      CMusicDatabase::SetPropertiesFromArtist(*pItem, artist);
 
+    // May already have album id, otherwise fetch album from song id
     CAlbum album;
-    if (database.GetAlbum(param.GetAlbumId(), album, false))
-      CMusicDatabase::SetPropertiesFromAlbum(*pItem,album);
+    bool albumfound = false;
+    int idAlbum = pItem->GetMusicInfoTag()->GetAlbumId();
+    if (idAlbum > 0)
+      albumfound = database.GetAlbum(idAlbum, album, false);
+    else
+      albumfound = database.GetAlbumFromSong(pItem->GetMusicInfoTag()->GetDatabaseId(), album);
+    if (albumfound)
+      CMusicDatabase::SetPropertiesFromAlbum(*pItem, album);
 
     path = pItem->GetMusicInfoTag()->GetURL();
   }
diff --git a/xbmc/music/MusicUtils.cpp b/xbmc/music/MusicUtils.cpp
new file mode 100644
index 0000000000..fc200aa24c
--- /dev/null
+++ b/xbmc/music/MusicUtils.cpp
@@ -0,0 +1,240 @@
+/*
+ *      Copyright (C) 2018 Team Kodi
+ *      http://kodi.tv
+ *
+ *  This Program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2, or (at your option)
+ *  any later version.
+ *
+ *  This Program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with XBMC; see the file COPYING.  If not, see
+ *  <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "MusicUtils.h"
+#include "dialogs/GUIDialogSelect.h"
+#include "guilib/GUIKeyboardFactory.h"
+#include "guilib/GUIWindowManager.h"
+#include "guilib/LocalizeStrings.h"
+#include "input/Key.h"
+#include "music/MusicDatabase.h"
+#include "music/tags/MusicInfoTag.h"
+#include "Util.h"
+#include "utils/JobManager.h"
+
+using namespace MUSIC_INFO;
+using namespace XFILE;
+
+namespace MUSIC_UTILS
+{
+  class CSetArtJob : public CJob
+  {
+    CFileItemPtr pItem;
+    std::string m_artType;
+    std::string m_newArt;
+  public:
+    CSetArtJob(const CFileItemPtr item, const std::string& type, const std::string& newArt) :
+      pItem(item),
+      m_artType(type),
+      m_newArt(newArt)
+    { }
+
+    ~CSetArtJob(void) override = default;
+
+    // Asynchronously update song, album or artist art in library
+    bool DoWork(void) override
+    {
+      CMusicInfoTag& tag = *pItem->GetMusicInfoTag();
+      CMusicDatabase db;
+      if (db.Open())
+      {
+        if (!m_newArt.empty())
+          db.SetArtForItem(tag.GetDatabaseId(), tag.GetType(), m_artType, m_newArt);
+        else
+          db.RemoveArtForItem(tag.GetDatabaseId(), tag.GetType(), m_artType);
+        db.Close();
+      }
+      return true;
+    }
+  };
+
+  class CSetSongRatingJob : public CJob
+  {
+    std::string strPath;
+    int idSong;
+    int iUserrating;
+  public:
+    CSetSongRatingJob(const std::string& filePath, int userrating) :
+      strPath(filePath),
+      idSong(-1),
+      iUserrating(userrating)
+    { }
+
+    CSetSongRatingJob(int songId, int userrating) :
+      strPath(),
+      idSong(songId),
+      iUserrating(userrating)
+    { }
+
+    ~CSetSongRatingJob(void) override = default;
+
+    bool DoWork(void) override
+    {
+      // Asynchronously update song userrating in library
+      CMusicDatabase db;
+      if (db.Open())
+      {
+        if (idSong > 0)
+          db.SetSongUserrating(idSong, iUserrating);
+        else
+          db.SetSongUserrating(strPath, iUserrating);
+        db.Close();
+      }
+
+      return true;
+    }
+  };
+
+  void UpdateArtJob(const CFileItemPtr pItem, const std::string& strType, const std::string& strArt)
+  {
+    // Asynchronously update that type of art in the database
+    CSetArtJob *job = new CSetArtJob(pItem, strType, strArt);
+    CJobManager::GetInstance().AddJob(job, NULL);
+  }
+
+  bool FillArtTypesList(CFileItem& musicitem, CFileItemList& artlist)
+  {
+    CMusicInfoTag &tag = *musicitem.GetMusicInfoTag();
+    if (tag.GetDatabaseId() < 1 || tag.GetType().empty())
+      return false;
+    if (tag.GetType() != MediaTypeArtist && tag.GetType() != MediaTypeAlbum && tag.GetType() != MediaTypeSong)
+      return false;
+
+    artlist.Clear();
+    // Songs, albums and artists all  have thumbs by default
+    std::vector<std::string> artTypes = { "thumb" };
+    if (tag.GetType() == MediaTypeArtist)
+    {
+      artTypes.emplace_back("fanart");
+    }
+
+    CMusicDatabase db;
+    db.Open();
+
+    // Add in any stored art for this item that is non-empty.
+    std::map<std::string, std::string> currentArt;
+    db.GetArtForItem(tag.GetDatabaseId(), tag.GetType(), currentArt);
+    for (const auto art : currentArt)
+    {
+      if (!art.second.empty() && find(artTypes.begin(), artTypes.end(), art.first) == artTypes.end())
+        artTypes.push_back(art.first);
+    }
+
+    // Add any art types that exist for other media items of the same type
+    std::vector<std::string> dbArtTypes;
+    db.GetArtTypes(tag.GetType(), dbArtTypes);
+    for (const auto it : dbArtTypes)
+    {
+      if (find(artTypes.begin(), artTypes.end(), it) == artTypes.end())
+        artTypes.push_back(it);
+    }
+    db.Close();
+
+    for (const auto type : artTypes)
+    {
+      CFileItemPtr artitem(new CFileItem(type, false));
+      // Localise the names of common types of art
+      if (type == "banner")
+        artitem->SetLabel(g_localizeStrings.Get(20020));
+      else if (type == "fanart")
+        artitem->SetLabel(g_localizeStrings.Get(20445));
+      else if (type == "poster")
+        artitem->SetLabel(g_localizeStrings.Get(20021));
+      else if (type == "thumb")
+        artitem->SetLabel(g_localizeStrings.Get(21371));
+      else
+        artitem->SetLabel(type);
+      // Set art type as art item property
+      artitem->SetProperty("arttype", type);
+      // Set current art as art item thumb
+      if (musicitem.HasArt(type))
+        artitem->SetArt("thumb", musicitem.GetArt(type));
+      artlist.Add(artitem);
+    }
+
+    return !artlist.IsEmpty();
+  }
+
+  std::string ShowSelectArtTypeDialog(CFileItemList& artitems)
+  {
+    // Prompt for choice
+    CGUIDialogSelect *dialog = g_windowManager.GetWindow<CGUIDialogSelect>(WINDOW_DIALOG_SELECT);
+    if (!dialog)
+      return "";
+
+    dialog->SetHeading(CVariant{ 13521 });
+    dialog->Reset();
+    dialog->SetUseDetails(true);
+    dialog->EnableButton(true, 13516);
+
+    dialog->SetItems(artitems);
+    dialog->Open();
+
+    if (dialog->IsButtonPressed())
+    {
+      // Get the new art type name
+      std::string strArtTypeName;
+      if (!CGUIKeyboardFactory::ShowAndGetInput(strArtTypeName, CVariant{ g_localizeStrings.Get(13516) }, false))
+        return "";
+      // Add new type to the list of art types
+      CFileItemPtr artitem(new CFileItem(strArtTypeName, false));
+      artitem->SetLabel(strArtTypeName);
+      artitem->SetProperty("arttype", strArtTypeName);
+      artitems.Add(artitem);
+
+      return strArtTypeName;
+    }
+
+    return dialog->GetSelectedFileItem()->GetProperty("arttype").asString();
+  }
+
+  int ShowSelectRatingDialog(int iSelected)
+  {
+    CGUIDialogSelect *dialog = g_windowManager.GetWindow<CGUIDialogSelect>(WINDOW_DIALOG_SELECT);
+    if (dialog)
+    {
+      dialog->SetHeading(CVariant{ 38023 });
+      dialog->Add(g_localizeStrings.Get(38022));
+      for (int i = 1; i <= 10; i++)
+        dialog->Add(StringUtils::Format("%s: %i", g_localizeStrings.Get(563).c_str(), i));
+      dialog->SetSelected(iSelected);
+      dialog->Open();
+
+      int userrating = dialog->GetSelectedItem();
+      userrating = std::max(userrating, -1);
+      userrating = std::min(userrating, 10);
+      return userrating;
+    }
+    return -1;
+  }
+
+  void UpdateSongRatingJob(const CFileItemPtr pItem, int userrating)
+  {
+    // Asynchronously update the song user rating in music library
+    const CMusicInfoTag *tag = pItem->GetMusicInfoTag();
+    CSetSongRatingJob *job;
+    if (tag && tag->GetType() == MediaTypeSong && tag->GetDatabaseId() > 0)
+      // Use song ID when known
+      job = new CSetSongRatingJob(tag->GetDatabaseId(), userrating);
+    else 
+      job = new CSetSongRatingJob(pItem->GetPath(), userrating);
+    CJobManager::GetInstance().AddJob(job, NULL);
+  }
+}
diff --git a/xbmc/music/MusicUtils.h b/xbmc/music/MusicUtils.h
new file mode 100644
index 0000000000..44a1467b86
--- /dev/null
+++ b/xbmc/music/MusicUtils.h
@@ -0,0 +1,69 @@
+#pragma once
+
+/*
+ *      Copyright (C) 2018 Team Kodi
+ *      http://kodi.tv
+ *
+ *  This Program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2, or (at your option)
+ *  any later version.
+ *
+ *  This Program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with XBMC; see the file COPYING.  If not, see
+ *  <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "FileItem.h"
+
+namespace MUSIC_UTILS
+{
+  /*! \brief Show a dialog to allow the selection of type of art from a list.
+  Input is a fileitem list, with each item having an "arttype" property 
+  e.g. "thumb", current art URL (if art exists), and label. One of these art types
+  can be selected, or a new art type added. The new art type is added as a new item
+  in the list, as well as retuned as the selected art type.
+  \param artitems [in/out] a fileitem list to display
+  \return the selected art type e.g. "fanart" or empty string when cancelled.
+  \sa FillArtTypesList
+  */
+  std::string ShowSelectArtTypeDialog(CFileItemList& artitems);
+
+  /*! \brief Helper function to build a list of art types for a music library item.
+  This fetches the possible types of art for a song, album or artist, and the 
+  current art URL (if the item has art of that type), for display on a dialog.
+  \param musicitem a music CFileItem (song, album or artist)
+  \param artitems [out] a fileitem list,  each item having "arttype" property 
+  e.g. "thumb", current art URL (if art exists), and localized label (for common arttypes)
+  \return true if art types are retrieved, false if none is found.
+  \sa ShowSelectArtTypeDialog
+  */
+  bool FillArtTypesList(CFileItem& musicitem, CFileItemList& artlist);
+
+  /*! \brief Helper function to asynchronously update art in the music database
+  For the song, album or artist this adds a job to the queue to update the art table
+  modifying, adding or deleting that type of art,
+  \param item a shared pointer to a music CFileItem (song, album or artist)
+  \param strType the type of art e.g. "fanart" or "thumb" etc.
+  \param strArt art URL, when empty the entry for that type of art is deleted. 
+  */
+  void UpdateArtJob(const CFileItemPtr pItem, const std::string& strType, const std::string& strArt);
+
+  /*! \brief Show a dialog to allow the selection of user rating.
+  \param iSelected the rating to show initially
+  \return the selected rating, 0 (no rating), 1 to 10 or -1 no rating selected
+  */
+  int ShowSelectRatingDialog(int iSelected);
+
+/*! \brief Helper function to asynchronously update the user rating of a song
+\param pItem pointer to song item being rated
+\param userrating the userrating 0 = no rating, 1 to 10
+*/
+  void UpdateSongRatingJob(const CFileItemPtr pItem, int userrating);
+}
diff --git a/xbmc/music/dialogs/GUIDialogSongInfo.cpp b/xbmc/music/dialogs/GUIDialogSongInfo.cpp
index 4cc89cb065..be8025193b 100644
--- a/xbmc/music/dialogs/GUIDialogSongInfo.cpp
+++ b/xbmc/music/dialogs/GUIDialogSongInfo.cpp
@@ -1,5 +1,5 @@
 /*
- *      Copyright (C) 2005-2013 Team XBMC
+ *      Copyright (C) 2005-2018 Team XBMC
  *      http://kodi.tv
  *
  *  This Program is free software; you can redistribute it and/or modify
@@ -19,26 +19,23 @@
  */
 
 #include "GUIDialogSongInfo.h"
-#include "utils/URIUtils.h"
-#include "utils/StringUtils.h"
-#include "utils/Variant.h"
+#include "dialogs/GUIDialogBusy.h"
 #include "dialogs/GUIDialogFileBrowser.h"
 #include "dialogs/GUIDialogSelect.h"
-#include "GUIPassword.h"
+#include "filesystem/File.h"
+#include "guilib/GUIWindowManager.h"
+#include "guilib/LocalizeStrings.h"
+#include "GUIDialogMusicInfo.h"
 #include "GUIUserMessages.h"
+#include "input/Key.h"
+#include "music/Album.h"
 #include "music/MusicDatabase.h"
-#include "music/windows/GUIWindowMusicBase.h"
+#include "music/MusicUtils.h"
 #include "music/tags/MusicInfoTag.h"
-#include "guilib/GUIWindowManager.h"
-#include "input/Key.h"
-#include "filesystem/File.h"
-#include "filesystem/CurlFile.h"
-#include "FileItem.h"
+#include "music/windows/GUIWindowMusicBase.h"
 #include "settings/MediaSourceSettings.h"
-#include "guilib/LocalizeStrings.h"
-#include "music/Album.h"
 #include "storage/MediaManager.h"
-#include "GUIDialogMusicInfo.h"
+#include "Util.h"
 
 using namespace XFILE;
 
@@ -49,14 +46,82 @@ using namespace XFILE;
 
 #define CONTROL_LIST              50
 
+#define TIME_TO_BUSY_DIALOG 500
+
+
+
+class CGetSongInfoJob : public CJob
+{
+public:
+  ~CGetSongInfoJob(void) override = default;
+
+  // Fetch full song information including art types list
+  bool DoWork() override
+  {
+    CGUIDialogSongInfo *dialog = g_windowManager.GetWindow<CGUIDialogSongInfo>(WINDOW_DIALOG_SONG_INFO);
+    if (!dialog)
+      return false; 
+    if (dialog->IsCancelled())
+      return false;
+    CFileItemPtr m_song = dialog->GetCurrentListItem();
+
+    // Fetch tag data from library using filename, or scanning file (if item not already have this loaded)
+    m_song->LoadMusicTag();
+    if (dialog->IsCancelled())
+      return false;
+    // Fetch album and primary song artist data from library as properties
+    // and lyrics by scanning tags from file 
+    MUSIC_INFO::CMusicInfoLoader::LoadAdditionalTagInfo(m_song.get());
+    if (dialog->IsCancelled())
+      return false;
+
+    // Load song art. 
+    // For songs in library this includes related album and artist(s) art, 
+    // otherwise just embedded or cached thumb is fetched.
+    CMusicThumbLoader loader;
+    loader.LoadItem(m_song.get());
+    if (dialog->IsCancelled())
+      return false;
+
+    // For songs in library fill vector of possible art types, with current art when it exists
+    // for display on the art type selection dialog
+    CFileItemList artlist;
+    MUSIC_UTILS::FillArtTypesList(*m_song, artlist);
+    dialog->SetArtTypeList(artlist);
+    if (dialog->IsCancelled())
+      return false;
+
+    // Fetch artist art for non db songs when artist can be found uniquely by name
+    if (!m_song->IsMusicDb() && m_song->HasMusicInfoTag() && !m_song->GetMusicInfoTag()->GetArtist().empty())
+    {
+      CMusicDatabase db;
+      db.Open();
+      int idArtist = db.GetArtistByName(m_song->GetMusicInfoTag()->GetArtist()[0]);
+      if (idArtist > 0)
+      {
+        std::map<std::string, std::string> art;
+        if (db.GetArtForItem(idArtist, MediaTypeArtist, art))
+          m_song->AppendArt(art, "artist");
+      }
+      db.Close();
+    }
+
+    // Tell waiting SongInfoDialog that job is complete
+    dialog->FetchComplete();
+
+    return true;
+  }
+};
+
 CGUIDialogSongInfo::CGUIDialogSongInfo(void)
     : CGUIDialog(WINDOW_DIALOG_SONG_INFO, "DialogMusicInfo.xml")
     , m_song(new CFileItem)
     , m_albumId(-1)
 {
   m_cancelled = false;
-  m_needsUpdate = false;
+  m_hasUpdatedUserrating = false;
   m_startUserrating = -1;
+  m_artTypeList.Clear();
   m_loadType = KEEP_IN_MEMORY;
 }
 
@@ -68,15 +133,20 @@ bool CGUIDialogSongInfo::OnMessage(CGUIMessage& message)
   {
   case GUI_MSG_WINDOW_DEINIT:
     {
+      m_artTypeList.Clear();
       if (m_startUserrating != m_song->GetMusicInfoTag()->GetUserrating())
       {
-        CMusicDatabase db;
-        if (db.Open())
-        {
-          m_needsUpdate = true;
-          db.SetSongUserrating(m_song->GetPath(), m_song->GetMusicInfoTag()->GetUserrating());
-          db.Close();
-        }
+        m_hasUpdatedUserrating = true;
+
+        // Asynchronously update song userrating in library
+        MUSIC_UTILS::UpdateSongRatingJob(m_song, m_song->GetMusicInfoTag()->GetUserrating());
+
+        // Send a message to all windows to tell them to update the fileitem
+        // This communicates the rating change to the music lib window, current playlist and OSD.
+        // The music lib window item is updated to but changes to the rating when it is the sort 
+        // do not show on screen until refresh() that fetchs the list from scratch, sorts etc.
+        CGUIMessage msg(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_UPDATE_ITEM, 0, m_song);
+        g_windowManager.SendMessage(msg);
       }
       CGUIMessage msg(GUI_MSG_LABEL_RESET, GetID(), CONTROL_LIST);
       OnMessage(msg);
@@ -110,7 +180,7 @@ bool CGUIDialogSongInfo::OnMessage(CGUIMessage& message)
       }
       else if (iControl == CONTROL_BTN_GET_THUMB)
       {
-        OnGetThumb();
+        OnGetArt();
         return true;
       }
       else if (iControl == CONTROL_LIST)
@@ -146,7 +216,7 @@ bool CGUIDialogSongInfo::OnMessage(CGUIMessage& message)
   return CGUIDialog::OnMessage(message);
 }
 
-bool CGUIDialogSongInfo::OnAction(const CAction &action)
+bool CGUIDialogSongInfo::OnAction(const CAction& action)
 {
   int userrating = m_song->GetMusicInfoTag()->GetUserrating();
   if (action.GetID() == ACTION_INCREASE_RATING)
@@ -173,29 +243,18 @@ bool CGUIDialogSongInfo::OnBack(int actionID)
   return CGUIDialog::OnBack(actionID);
 }
 
+void CGUIDialogSongInfo::FetchComplete()
+{
+  //Trigger the event to indicate data has been fetched
+  m_event.Set();
+}
+
 void CGUIDialogSongInfo::OnInitWindow()
 {
-  // Normally have album id from song
+  // Enable album info button when we know album
   m_albumId = m_song->GetMusicInfoTag()->GetAlbumId();
-  if (m_albumId < 0)
-  {
-    CMusicDatabase db;
-    db.Open();
-
-    // no known db info - check if parent dir is an album
-    if (m_song->GetMusicInfoTag()->GetDatabaseId() == -1)
-    {
-      std::string path = URIUtils::GetDirectory(m_song->GetPath());
-      m_albumId = db.GetAlbumIdByPath(path);
-    }
-    else
-    {
-      CAlbum album;
-      db.GetAlbumFromSong(m_song->GetMusicInfoTag()->GetDatabaseId(), album);
-      m_albumId = album.idAlbum;
-    }
-  }
-  CONTROL_ENABLE_ON_CONDITION(CONTROL_ALBUMINFO, m_albumId > -1);
+  
+  CONTROL_ENABLE_ON_CONDITION(CONTROL_ALBUMINFO, m_albumId > 0);
 
   // Disable music user rating button for plugins as they don't have tables to save this
   if (m_song->IsPlugin())
@@ -205,7 +264,7 @@ void CGUIDialogSongInfo::OnInitWindow()
 
   SET_CONTROL_HIDDEN(CONTROL_BTN_REFRESH);
   SET_CONTROL_LABEL(CONTROL_USERRATING, 38023);
-  SET_CONTROL_LABEL(CONTROL_BTN_GET_THUMB, 13405);
+  SET_CONTROL_LABEL(CONTROL_BTN_GET_THUMB, 13511);
   SET_CONTROL_LABEL(CONTROL_ALBUMINFO, 10523);
 
   CGUIDialog::OnInitWindow();
@@ -227,179 +286,198 @@ void CGUIDialogSongInfo::Update()
 
 void CGUIDialogSongInfo::SetUserrating(int userrating)
 {
-  if (userrating < 0) userrating = 0;
-  if (userrating > 10) userrating = 10;
+  userrating = std::max(userrating, 0);
+  userrating = std::min(userrating, 10);
   if (userrating != m_song->GetMusicInfoTag()->GetUserrating())
   {
     m_song->GetMusicInfoTag()->SetUserrating(userrating);
-    // send a message to all windows to tell them to update the fileitem (eg playlistplayer, media windows)
-    CGUIMessage msg(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_UPDATE_ITEM, 0, m_song);
-    g_windowManager.SendMessage(msg);
   }
 }
 
-void CGUIDialogSongInfo::SetSong(CFileItem *item)
+bool CGUIDialogSongInfo::SetSong(CFileItem* item)
 {
   *m_song = *item;
-  m_song->LoadMusicTag();
-  m_startUserrating = m_song->GetMusicInfoTag()->GetUserrating();
-  MUSIC_INFO::CMusicInfoLoader::LoadAdditionalTagInfo(m_song.get());
-
-  // Load song art. 
-  // For songs in library this includes related album and artist(s) art, 
-  // otherwise just embedded or cached thumb is fetched.
-  CMusicThumbLoader loader;
-  loader.LoadItem(m_song.get());
-
-   // Set artist art for non db songs when artist can be found uniquely by name
-  if (!m_song->IsMusicDb() && m_song->HasMusicInfoTag() && !m_song->GetMusicInfoTag()->GetArtist().empty())
+  m_event.Reset();
+  m_cancelled = false;  // SetSong happens before win_init
+  // In a separate job fetch song info and fill list of art types.
+  int jobid = CJobManager::GetInstance().AddJob(new CGetSongInfoJob(), nullptr, CJob::PRIORITY_LOW);
+  
+  // Wait to get all data before show, allowing user to to cancel if fetch is slow
+  if (!CGUIDialogBusy::WaitOnEvent(m_event, TIME_TO_BUSY_DIALOG))
   {
-    CMusicDatabase db;
-    db.Open();
-    int idArtist = db.GetArtistByName(m_song->GetMusicInfoTag()->GetArtist()[0]);
-    if (idArtist > 0)
-    {
-      std::map<std::string, std::string> art;
-      if (db.GetArtForItem(idArtist, MediaTypeArtist, art))
-        m_song->SetArt(art);
-    }
+    // Cancel job still waiting in queue (unlikely)
+    CJobManager::GetInstance().CancelJob(jobid);
+    // Flag to stop job already in progress
+    m_cancelled = true; 
+    return false;
   }
-  m_needsUpdate = false;
+
+  // CurrentDirectory() returns m_artTypeList (a convenient CFileItemList)
+  // Set content so can return dialog CONTAINER_CONTENT as "songs"
+  m_artTypeList.SetContent("songs");
+  // Copy art from ListItem so CONTAINER_ART returns song art
+  m_artTypeList.SetArt(m_song->GetArt());
+
+  // Store initial userrating
+  m_startUserrating = m_song->GetMusicInfoTag()->GetUserrating();
+  m_hasUpdatedUserrating = false;
+  return true;
 }
 
-CFileItemPtr CGUIDialogSongInfo::GetCurrentListItem(int offset)
+void CGUIDialogSongInfo::SetArtTypeList(CFileItemList& artlist)
 {
-  return m_song;
+  m_artTypeList.Copy(artlist);
 }
 
-bool CGUIDialogSongInfo::DownloadThumbnail(const std::string &thumbFile)
+CFileItemPtr CGUIDialogSongInfo::GetCurrentListItem(int offset)
 {
-  //! @todo Obtain the source...
-  std::string source;
-  CCurlFile http;
-  http.Download(source, thumbFile);
-  return true;
+  return m_song;
 }
 
-// Get Thumb from user choice.
-// Options are:
-// 1.  Current thumb
-// 2.  AllMusic.com thumb
-// 3.  Local thumb
-// 4.  No thumb (if no Local thumb is available)
-
-//! @todo Currently no support for "embedded thumb" as there is no easy way to grab it
-//!       without sending a file that has this as it's album to this class
-void CGUIDialogSongInfo::OnGetThumb()
+/*
+  Allow user to choose artwork for the song
+  For each type of art the options are:
+  1.  Current art
+  2.  Local art (thumb found by filename)
+  3.  Embedded art (@todo)
+  4.  None 
+  Note that songs are not scraped, hence there is no list of urls for possible remote art
+*/
+void CGUIDialogSongInfo::OnGetArt()
 {
-  CFileItemList items;
+  std::string type = MUSIC_UTILS::ShowSelectArtTypeDialog(m_artTypeList);
+  if (type.empty())
+    return; // Cancelled
 
-
-  // Grab the thumbnail from the web
-  /*
-  std::string thumbFromWeb;
-  thumbFromWeb = URIUtils::AddFileToFolder(g_advancedSettings.m_cachePath, "allmusicThumb.jpg");
-  if (DownloadThumbnail(thumbFromWeb))
+  CFileItemList items;
+  CGUIListItem::ArtMap primeArt = m_song->GetArt(); // Song art without fallbacks
+  bool bHasArt = m_song->HasArt(type);
+  bool bFallback(false);
+  if (bHasArt)
   {
-    CFileItemPtr item(new CFileItem("thumb://allmusic.com", false));
-    item->SetArt("thumb", thumbFromWeb);
-    item->SetLabel(g_localizeStrings.Get(20055));
-    items.Add(item);
-  }*/
+    // Check if that type of art is actually a fallback, e.g. album thumb or artist fanart
+    CGUIListItem::ArtMap::const_iterator i = primeArt.find(type);
+    bFallback = (i == primeArt.end());
+  }
 
-  // Current thumb
-  if (CFile::Exists(m_song->GetArt("thumb")))
+  // Build list of possible images of that art type
+  if (bHasArt)
   {
+    // Add item for current artwork, could a fallback from album/artist
     CFileItemPtr item(new CFileItem("thumb://Current", false));
-    item->SetArt("thumb", m_song->GetArt("thumb"));
-    item->SetLabel(g_localizeStrings.Get(20016));
+    item->SetArt("thumb", m_song->GetArt(type));
+    item->SetIconImage("DefaultPicture.png");    
+    item->SetLabel(g_localizeStrings.Get(13512));  //! @todo: label fallback art so user knows?
     items.Add(item);
   }
-
-  // local thumb
-  std::string cachedLocalThumb;
-  std::string localThumb(m_song->GetUserMusicThumb(true));
-  if (m_song->IsMusicDb())
-  {
-    CFileItem item(m_song->GetMusicInfoTag()->GetURL(), false);
-    localThumb = item.GetUserMusicThumb(true);
+  else if (m_song->HasArt("thumb"))
+  { // For missing art of that type add the thumb (when it exists and not a fallback)
+    CGUIListItem::ArtMap::const_iterator i = primeArt.find("thumb");
+    if (i != primeArt.end())
+    {
+      CFileItemPtr item(new CFileItem("thumb://Thumb", false));
+      item->SetArt("thumb", m_song->GetArt("thumb"));
+      item->SetIconImage("DefaultAlbumCover.png");
+      item->SetLabel(g_localizeStrings.Get(21371));
+      items.Add(item);
+    }
   }
-  if (CFile::Exists(localThumb))
-  {
-    CFileItemPtr item(new CFileItem("thumb://Local", false));
-    item->SetArt("thumb", localThumb);
-    item->SetLabel(g_localizeStrings.Get(20017));
-    items.Add(item);
+
+  std::string localThumb;
+  if (type == "thumb")
+  { // Local thumb type art held in <filename>.tbn (for non-library items)
+    localThumb = m_song->GetUserMusicThumb(true);
+    if (m_song->IsMusicDb())
+    {
+      CFileItem item(m_song->GetMusicInfoTag()->GetURL(), false);
+      localThumb = item.GetUserMusicThumb(true);
+    }
+    if (CFile::Exists(localThumb))
+    {
+      CFileItemPtr item(new CFileItem("thumb://Local", false));
+      item->SetArt("thumb", localThumb);
+      item->SetLabel(g_localizeStrings.Get(20017));
+      items.Add(item);
+    }
   }
-  else
-  { // no local thumb exists, so we are just using the allmusic.com thumb or cached thumb
-    // which is probably the allmusic.com thumb.  These could be wrong, so allow the user
-    // to delete the incorrect thumb
+
+  if (bHasArt && !bFallback)
+  { // Actually has this type of art (not a fallback) so 
+    // allow the user to delete it by selecting "no art".
     CFileItemPtr item(new CFileItem("thumb://None", false));
     item->SetArt("thumb", "DefaultAlbumCover.png");
-    item->SetLabel(g_localizeStrings.Get(20018));
+    item->SetLabel(g_localizeStrings.Get(13515));
     items.Add(item);
   }
 
+  //! @todo: Add support for extracting embedded art
+
+  // Show list of possible art for user selection
   std::string result;
   VECSOURCES sources(*CMediaSourceSettings::GetInstance().GetSources("music"));
   CGUIDialogMusicInfo::AddItemPathToFileBrowserSources(sources, *m_song);
   g_mediaManager.GetLocalDrives(sources);
-  if (!CGUIDialogFileBrowser::ShowAndGetImage(items, sources, g_localizeStrings.Get(1030), result))
-    return;   // user cancelled
-
-  if (result == "thumb://Current")
-    return;   // user chose the one they have
-
-  // delete the thumbnail if that's what the user wants, else overwrite with the
-  // new thumbnail
-
-  std::string newThumb;
-  if (result == "thumb://None")
-    newThumb = "";
-  else if (result == "thumb://allmusic.com")
-    newThumb.clear();
-  else if (result == "thumb://Local")
-    newThumb = localThumb;
-  else
-    newThumb = result;
-
-  // update thumb in the database
-  CMusicDatabase db;
-  if (db.Open())
+  if (CGUIDialogFileBrowser::ShowAndGetImage(items, sources, g_localizeStrings.Get(13511), result) &&
+    result != "thumb://Current") 
   {
-    db.SetArtForItem(m_song->GetMusicInfoTag()->GetDatabaseId(), m_song->GetMusicInfoTag()->GetType(), "thumb", newThumb);
-    db.Close();
-  }
+    // User didn't choose the one they have, or the fallback image. 
+    // Overwrite with the new art or clear it
+    std::string newArt;
+    if (result == "thumb://Thumb")
+      newArt = m_song->GetArt("thumb");
+    else if (result == "thumb://Local")
+      newArt = localThumb;
+//    else if (result == "thumb://Embedded")
+//      newArt = embeddedArt;
+    else if (CFile::Exists(result))
+      newArt = result;
+    else // none
+      newArt.clear();
+
+    // Asynchronously update that type of art in the database
+    MUSIC_UTILS::UpdateArtJob(m_song, type, newArt);
+  
+    // Update local song with current art
+    if (newArt.empty())
+    {
+      // Remove that type of art from the song
+      primeArt.erase(type);
+      m_song->SetArt(primeArt);
+    }
+    else    
+      // Add or modify the type of art
+      m_song->SetArt(type, newArt);
+
+    // Update local art list with current art
+    // Show any fallback art when song art removed
+    if (newArt.empty() && m_song->HasArt(type))
+      newArt = m_song->GetArt(type);
+    for (const auto artitem : m_artTypeList)
+    {
+      if (artitem->GetProperty("artType") == type)
+      {
+        artitem->SetArt("thumb", newArt);
+        break;
+      }
+    }
 
-  m_song->SetArt("thumb", newThumb);
+    // Get new artwork to show in other places e.g. on music lib window, 
+    // current playlist and player OSD.
+    CGUIMessage msg(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_UPDATE_ITEM, 0, m_song);
+    g_windowManager.SendMessage(msg);
 
-  // tell our GUI to completely reload all controls (as some of them
-  // are likely to have had this image in use so will need refreshing)
-  CGUIMessage msg(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_REFRESH_THUMBS);
-  g_windowManager.SendMessage(msg);
+  }
 
-//  m_hasUpdatedThumb = true;
+  // Re-open the art type selection dialog as we come back from
+  // the image selection dialog
+  OnGetArt();
 }
 
 void CGUIDialogSongInfo::OnSetUserrating()
 {
-  CGUIDialogSelect *dialog = g_windowManager.GetWindow<CGUIDialogSelect>(WINDOW_DIALOG_SELECT);
-  if (dialog)
-  {
-    dialog->SetHeading(CVariant{ 38023 });
-    dialog->Add(g_localizeStrings.Get(38022));
-    for (int i = 1; i <= 10; i++)
-      dialog->Add(StringUtils::Format("%s: %i", g_localizeStrings.Get(563).c_str(), i));
-
-    dialog->SetSelected(m_song->GetMusicInfoTag()->GetUserrating());
-
-    dialog->Open();
+  int userrating = MUSIC_UTILS::ShowSelectRatingDialog(m_song->GetMusicInfoTag()->GetUserrating());
+  if (userrating < 0) // Nothing selected, so rating unchanged
+    return;
 
-    int iItem = dialog->GetSelectedItem();
-    if (iItem < 0)
-      return;
-
-    SetUserrating(iItem);
-  }
-}
+  SetUserrating(userrating);
+}
\ No newline at end of file
diff --git a/xbmc/music/dialogs/GUIDialogSongInfo.h b/xbmc/music/dialogs/GUIDialogSongInfo.h
index 6237070a27..61928f4dfd 100644
--- a/xbmc/music/dialogs/GUIDialogSongInfo.h
+++ b/xbmc/music/dialogs/GUIDialogSongInfo.h
@@ -1,7 +1,7 @@
 #pragma once
 
 /*
- *      Copyright (C) 2005-2013 Team XBMC
+ *      Copyright (C) 2005-2018 Team XBMC
  *      http://kodi.tv
  *
  *  This Program is free software; you can redistribute it and/or modify
@@ -21,8 +21,8 @@
  */
 
 #include "guilib/GUIDialog.h"
-
-class CFileItem;
+#include "FileItem.h"
+#include "threads/Event.h"
 
 class CGUIDialogSongInfo :
       public CGUIDialog
@@ -31,24 +31,31 @@ public:
   CGUIDialogSongInfo(void);
   ~CGUIDialogSongInfo(void) override;
   bool OnMessage(CGUIMessage& message) override;
-  void SetSong(CFileItem *item);
-  bool OnAction(const CAction &action) override;
+  bool SetSong(CFileItem* item);
+  void SetArtTypeList(CFileItemList& artlist);
+  bool OnAction(const CAction& action) override;
   bool OnBack(int actionID) override;
-  bool NeedsUpdate() const { return m_needsUpdate; };
+  bool HasUpdatedUserrating() const { return m_hasUpdatedUserrating; };
 
   bool HasListItems() const override { return true; };
   CFileItemPtr GetCurrentListItem(int offset = 0) override;
+  const CFileItemList& CurrentDirectory() const { return m_artTypeList; };
+  bool IsCancelled() const { return m_cancelled; };
+  void FetchComplete();
+
 protected:
   void OnInitWindow() override;
   void Update();
-  bool DownloadThumbnail(const std::string &thumbFile);
-  void OnGetThumb();
+  void OnGetArt();
   void SetUserrating(int userrating);
   void OnSetUserrating();
 
   CFileItemPtr m_song;
+  CFileItemList m_artTypeList;
+  CEvent m_event;
   int m_startUserrating;
   bool m_cancelled;
-  bool m_needsUpdate;
+  bool m_hasUpdatedUserrating;
   long m_albumId;
+
 };
diff --git a/xbmc/music/windows/GUIWindowMusicBase.cpp b/xbmc/music/windows/GUIWindowMusicBase.cpp
index 058da61f32..d87e3c86d2 100644
--- a/xbmc/music/windows/GUIWindowMusicBase.cpp
+++ b/xbmc/music/windows/GUIWindowMusicBase.cpp
@@ -174,7 +174,7 @@ bool CGUIWindowMusicBase::OnMessage(CGUIMessage& message)
 
   // update the display
   case GUI_MSG_SCAN_FINISHED:
-  case GUI_MSG_REFRESH_THUMBS:
+  case GUI_MSG_REFRESH_THUMBS: // Never called as is secondary msg sent as GUI_MSG_NOTIFY_ALL
     Refresh();
     break;
 
@@ -529,10 +529,13 @@ void CGUIWindowMusicBase::ShowSongInfo(CFileItem* pItem)
     if (!pItem->HasMusicInfoTag())
       return;
 
-    dialog->SetSong(pItem);
-    dialog->Open();
-    if (dialog->NeedsUpdate())
-      Refresh(true); // update our file list
+    if (dialog->SetSong(pItem))  // Fetch full song info asynchronously
+    { 
+      dialog->Open();
+      if (dialog->HasUpdatedUserrating() && m_vecItems->GetSortMethod() == SortByUserRating)
+        // Refresh list to sort items and show new userrating sort label
+        Refresh();
+    }
   }
 }