From 3830fa368c2b0f334605978ad09fd47b1f82fd58 Mon Sep 17 00:00:00 2001 From: DaveTBlake Date: Mon, 19 Oct 2015 20:13:00 +0100 Subject: [PATCH] JSON when getting album or song ensure underlying query uses song_artist or album_artist tables to fully populate artist credits. Hence artist names and id arrays will be consistent. As a bonus now gets MBID (as a vector to match names and ids) In CMusicInfoTag set "FieldArtist" = GetArtistString so sorts work on strArtists from Album and song tables. Hence not need fix in CAlbum or Csong populating artist vector from artistDesc when artist credits empty. Rework underlying queries for GetAlbums, GetSongs, GetRecentlyAddedAlbums, GetRecentlyPlayedAlbums, GetRecentlyPlayedAlbumSongs, GetRecentlyAddedAlbumSongs, GetTop100Albums to fully populate artist credits Bump JSON version for now artist data returned is consistent --- xbmc/interfaces/json-rpc/AudioLibrary.cpp | 107 ++--- xbmc/interfaces/json-rpc/AudioLibrary.h | 1 + xbmc/interfaces/json-rpc/schema/version.txt | 2 +- xbmc/music/Album.cpp | 14 +- xbmc/music/Album.h | 5 + xbmc/music/MusicDatabase.cpp | 448 ++++++++++++++++++-- xbmc/music/MusicDatabase.h | 5 +- xbmc/music/Song.cpp | 14 +- xbmc/music/Song.h | 5 + xbmc/music/tags/MusicInfoTag.cpp | 8 +- 10 files changed, 508 insertions(+), 101 deletions(-) diff --git a/xbmc/interfaces/json-rpc/AudioLibrary.cpp b/xbmc/interfaces/json-rpc/AudioLibrary.cpp index b52b106253859..d5d3d6b7f3631 100644 --- a/xbmc/interfaces/json-rpc/AudioLibrary.cpp +++ b/xbmc/interfaces/json-rpc/AudioLibrary.cpp @@ -137,18 +137,16 @@ JSONRPC_STATUS CAudioLibrary::GetAlbums(const std::string &method, ITransportLay if (!musicUrl.FromString("musicdb://albums/")) return InternalError; - if (parameterObject["includesingles"].asBoolean()) musicUrl.AddOption("show_singles", true); - int artistID = -1, genreID = -1; const CVariant &filter = parameterObject["filter"]; if (filter.isMember("artistid")) - artistID = (int)filter["artistid"].asInteger(); + musicUrl.AddOption("artistid", (int)filter["artistid"].asInteger()); else if (filter.isMember("artist")) musicUrl.AddOption("artist", filter["artist"].asString()); else if (filter.isMember("genreid")) - genreID = (int)filter["genreid"].asInteger(); + musicUrl.AddOption("genreid", (int)filter["genreid"].asInteger()); else if (filter.isMember("genre")) musicUrl.AddOption("genre", filter["genre"].asString()); else if (filter.isObject()) @@ -165,17 +163,32 @@ JSONRPC_STATUS CAudioLibrary::GetAlbums(const std::string &method, ITransportLay if (!ParseSorting(parameterObject, sorting.sortBy, sorting.sortOrder, sorting.sortAttributes)) return InvalidParams; - CFileItemList items; - if (!musicdatabase.GetAlbumsNav(musicUrl.ToString(), items, genreID, artistID, CDatabase::Filter(), sorting)) + int total; + VECALBUMS albums; + if (!musicdatabase.GetAlbumsByWhere(musicUrl.ToString(), CDatabase::Filter(), albums, total, sorting)) return InternalError; + CFileItemList items; + items.Reserve(albums.size()); + for (unsigned int index = 0; index < albums.size(); index++) + { + CMusicDbUrl itemUrl = musicUrl; + std::string path = StringUtils::Format("%i/", albums[index].idAlbum); + itemUrl.AppendPath(path); + + CFileItemPtr pItem; + FillAlbumItem(albums[index], itemUrl.ToString(), pItem); + items.Add(pItem); + } + + //Get Genre IDs JSONRPC_STATUS ret = GetAdditionalAlbumDetails(parameterObject, items, musicdatabase); if (ret != OK) return ret; int size = items.Size(); - if (items.HasProperty("total") && items.GetProperty("total").asInteger() > size) - size = (int)items.GetProperty("total").asInteger(); + if (total > size) + size = total; HandleFileItemList("albumid", false, "albums", items, parameterObject, result, size, false); return OK; @@ -221,22 +234,20 @@ JSONRPC_STATUS CAudioLibrary::GetSongs(const std::string &method, ITransportLaye if (!musicUrl.FromString("musicdb://songs/")) return InternalError; - if (!parameterObject["includesingles"].asBoolean()) musicUrl.AddOption("singles", false); - int genreID = -1, albumID = -1, artistID = -1; const CVariant &filter = parameterObject["filter"]; if (filter.isMember("artistid")) - artistID = (int)filter["artistid"].asInteger(); + musicUrl.AddOption("artistid", (int)filter["artistid"].asInteger()); else if (filter.isMember("artist")) musicUrl.AddOption("artist", filter["artist"].asString()); else if (filter.isMember("genreid")) - genreID = (int)filter["genreid"].asInteger(); + musicUrl.AddOption("genreid", (int)filter["genreid"].asInteger()); else if (filter.isMember("genre")) musicUrl.AddOption("genre", filter["genre"].asString()); else if (filter.isMember("albumid")) - albumID = (int)filter["albumid"].asInteger(); + musicUrl.AddOption("albumid", (int)filter["albumid"].asInteger()); else if (filter.isMember("album")) musicUrl.AddOption("album", filter["album"].asString()); else if (filter.isObject()) @@ -254,8 +265,8 @@ JSONRPC_STATUS CAudioLibrary::GetSongs(const std::string &method, ITransportLaye return InvalidParams; CFileItemList items; - if (!musicdatabase.GetSongsNav(musicUrl.ToString(), items, genreID, artistID, albumID, sorting)) - return InternalError; + if (!musicdatabase.GetSongsFullByWhere(musicUrl.ToString(), CDatabase::Filter(), items, sorting, true)) + return InternalError; JSONRPC_STATUS ret = GetAdditionalSongDetails(parameterObject, items, musicdatabase); if (ret != OK) @@ -282,7 +293,10 @@ JSONRPC_STATUS CAudioLibrary::GetSongDetails(const std::string &method, ITranspo return InvalidParams; CFileItemList items; - items.Add(CFileItemPtr(new CFileItem(song))); + CFileItemPtr item = CFileItemPtr(new CFileItem(song)); + FillItemArtistIDs(song.GetArtistIDArray(), item); + items.Add(item); + JSONRPC_STATUS ret = GetAdditionalSongDetails(parameterObject, items, musicdatabase); if (ret != OK) return ret; @@ -601,9 +615,11 @@ bool CAudioLibrary::FillFileItem(const std::string &strFilename, CFileItemPtr &i if (musicdatabase.GetAlbum(albumid, album, false)) { item->SetFromAlbum(album); + FillItemArtistIDs(album.GetArtistIDArray(), item); CFileItemList items; items.Add(item); + if (GetAdditionalAlbumDetails(parameterObject, items, musicdatabase) == OK) filled = true; } @@ -614,6 +630,7 @@ bool CAudioLibrary::FillFileItem(const std::string &strFilename, CFileItemPtr &i if (musicdatabase.GetSongByFileName(strFilename, song)) { item->SetFromSong(song); + FillItemArtistIDs(song.GetArtistIDArray(), item); CFileItemList items; items.Add(item); @@ -685,9 +702,22 @@ bool CAudioLibrary::FillFileItemList(const CVariant ¶meterObject, CFileItemL return success; } +void CAudioLibrary::FillItemArtistIDs(const std::vector artistids, CFileItemPtr &item) +{ + // Add artistIds as separate property as not part of CMusicInfoTag + CVariant artistidObj(CVariant::VariantTypeArray); + for (std::vector::const_iterator artistid = artistids.begin(); artistid != artistids.end(); ++artistid) + artistidObj.push_back(*artistid); + + item->SetProperty("artistid", artistidObj); +} + void CAudioLibrary::FillAlbumItem(const CAlbum &album, const std::string &path, CFileItemPtr &item) { item = CFileItemPtr(new CFileItem(path, album)); + // Add album artistIds as separate property as not part of CMusicInfoTag + std::vector artistids = album.GetArtistIDArray(); + FillItemArtistIDs(artistids, item); } JSONRPC_STATUS CAudioLibrary::GetAdditionalDetails(const CVariant ¶meterObject, CFileItemList &items) @@ -711,7 +741,6 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalAlbumDetails(const CVariant ¶mete std::set checkProperties; checkProperties.insert("genreid"); - checkProperties.insert("artistid"); std::set additionalProperties; if (!CheckForAdditionalProperties(parameterObject["properties"], checkProperties, additionalProperties)) return OK; @@ -731,18 +760,7 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalAlbumDetails(const CVariant ¶mete item->SetProperty("genreid", genreidObj); } } - if (additionalProperties.find("artistid") != additionalProperties.end()) - { - std::vector artistids; - if (musicdatabase.GetArtistsByAlbum(item->GetMusicInfoTag()->GetDatabaseId(), true, artistids)) - { - CVariant artistidObj(CVariant::VariantTypeArray); - for (std::vector::const_iterator artistid = artistids.begin(); artistid != artistids.end(); ++artistid) - artistidObj.push_back(*artistid); - - item->SetProperty("artistid", artistidObj); - } - } + } return OK; @@ -755,8 +773,12 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalSongDetails(const CVariant ¶meter std::set checkProperties; checkProperties.insert("genreid"); - checkProperties.insert("artistid"); + // Query (songview join songartistview) returns song.strAlbumArtists = CMusicInfoTag.m_strAlbumArtistDesc only + // Actual album artist data, if required, comes from album_artist and artist tables. + // It may differ from just splitting album artist description string + checkProperties.insert("albumartist"); checkProperties.insert("albumartistid"); + checkProperties.insert("musicbrainzalbumartistid"); std::set additionalProperties; if (!CheckForAdditionalProperties(parameterObject["properties"], checkProperties, additionalProperties)) return OK; @@ -776,28 +798,13 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalSongDetails(const CVariant ¶meter item->SetProperty("genreid", genreidObj); } } - if (additionalProperties.find("artistid") != additionalProperties.end()) - { - std::vector artistids; - if (musicdatabase.GetArtistsBySong(item->GetMusicInfoTag()->GetDatabaseId(), true, artistids)) - { - CVariant artistidObj(CVariant::VariantTypeArray); - for (std::vector::const_iterator artistid = artistids.begin(); artistid != artistids.end(); ++artistid) - artistidObj.push_back(*artistid); - - item->SetProperty("artistid", artistidObj); - } - } - if (additionalProperties.find("albumartistid") != additionalProperties.end() && item->GetMusicInfoTag()->GetAlbumId() > 0) + if (item->GetMusicInfoTag()->GetAlbumId() > 0) { - std::vector albumartistids; - if (musicdatabase.GetArtistsByAlbum(item->GetMusicInfoTag()->GetAlbumId(), true, albumartistids)) + if (additionalProperties.find("albumartist") != additionalProperties.end() || + additionalProperties.find("albumartistid") != additionalProperties.end() || + additionalProperties.find("musicbrainzalbumartistid") != additionalProperties.end()) { - CVariant albumartistidObj(CVariant::VariantTypeArray); - for (std::vector::const_iterator albumartistid = albumartistids.begin(); albumartistid != albumartistids.end(); ++albumartistid) - albumartistidObj.push_back(*albumartistid); - - item->SetProperty("albumartistid", albumartistidObj); + musicdatabase.GetArtistsByAlbum(item->GetMusicInfoTag()->GetAlbumId(), item.get()); } } } diff --git a/xbmc/interfaces/json-rpc/AudioLibrary.h b/xbmc/interfaces/json-rpc/AudioLibrary.h index c44dddb9bfa1c..73cbd25db9215 100644 --- a/xbmc/interfaces/json-rpc/AudioLibrary.h +++ b/xbmc/interfaces/json-rpc/AudioLibrary.h @@ -62,6 +62,7 @@ namespace JSONRPC private: static void FillAlbumItem(const CAlbum &album, const std::string &path, CFileItemPtr &item); + static void FillItemArtistIDs(const std::vector artistids, CFileItemPtr &item); static bool CheckForAdditionalProperties(const CVariant &properties, const std::set &checkProperties, std::set &foundProperties); }; diff --git a/xbmc/interfaces/json-rpc/schema/version.txt b/xbmc/interfaces/json-rpc/schema/version.txt index d2715cd5cece9..d6b71af5291dd 100644 --- a/xbmc/interfaces/json-rpc/schema/version.txt +++ b/xbmc/interfaces/json-rpc/schema/version.txt @@ -1 +1 @@ -6.31.0 +6.32.0 diff --git a/xbmc/music/Album.cpp b/xbmc/music/Album.cpp index 866959e321f88..540c05ec7d532 100644 --- a/xbmc/music/Album.cpp +++ b/xbmc/music/Album.cpp @@ -178,11 +178,6 @@ const std::vector CAlbum::GetAlbumArtist() const { albumartists.push_back(artistCredit->GetArtist()); } - //When artist credits have not been populated attempt to build an artist vector from the descrpition string - //This is a tempory fix, in the longer term other areas should query the album_artist table and populate - //artist credits. Note that splitting the string may not give the same artists as held in the album_artist table - if (albumartists.empty() && !strArtistDesc.empty()) - albumartists = StringUtils::Split(strArtistDesc, g_advancedSettings.m_musicItemSeparator); return albumartists; } @@ -209,6 +204,15 @@ const std::string CAlbum::GetAlbumArtistString() const return artistString; } +const std::vector CAlbum::GetArtistIDArray() const +{ + // Get album artist IDs for json rpc + std::vector artistids; + for (VECARTISTCREDITS::const_iterator artistCredit = artistCredits.begin(); artistCredit != artistCredits.end(); ++artistCredit) + artistids.push_back(artistCredit->GetArtistId()); + return artistids; +} + std::string CAlbum::GetReleaseType() const { return ReleaseTypeToString(releaseType); diff --git a/xbmc/music/Album.h b/xbmc/music/Album.h index fee9116958ce4..2a2239492e2a1 100644 --- a/xbmc/music/Album.h +++ b/xbmc/music/Album.h @@ -87,6 +87,11 @@ class CAlbum */ const std::string GetAlbumArtistString() const; + /*! \brief Get album artist IDs (for json rpc) from the vector of artistcredits objects + \return album artist IDs as a vector of integers + */ + const std::vector GetArtistIDArray() const; + typedef enum ReleaseType { Album = 0, Single diff --git a/xbmc/music/MusicDatabase.cpp b/xbmc/music/MusicDatabase.cpp index d2281ebe35582..ef3583513c669 100644 --- a/xbmc/music/MusicDatabase.cpp +++ b/xbmc/music/MusicDatabase.cpp @@ -1421,17 +1421,13 @@ bool CMusicDatabase::GetAlbumsByArtist(int idArtist, bool includeFeatured, std:: return false; } -bool CMusicDatabase::GetArtistsByAlbum(int idAlbum, bool includeFeatured, std::vector &artists) +bool CMusicDatabase::GetArtistsByAlbum(int idAlbum, CFileItem* item) { try { std::string strSQL, strPrepSQL; - - strPrepSQL = "select idArtist from album_artist where idAlbum=%i"; - if (includeFeatured == false) - strPrepSQL += " AND boolFeatured = 0"; - - strSQL=PrepareSQL(strPrepSQL, idAlbum); + + strSQL = PrepareSQL("SELECT * FROM albumartistview WHERE idAlbum = %i", idAlbum); if (!m_pDS->query(strSQL)) return false; if (m_pDS->num_rows() == 0) @@ -1440,12 +1436,31 @@ bool CMusicDatabase::GetArtistsByAlbum(int idAlbum, bool includeFeatured, std::v return false; } + // Get album artist credits + VECARTISTCREDITS artistCredits; while (!m_pDS->eof()) { - artists.push_back(m_pDS->fv("idArtist").get_asInt()); + artistCredits.push_back(GetArtistCreditFromDataset(m_pDS->get_sql_record(), 0)); m_pDS->next(); } m_pDS->close(); + + // Populate item with song albumartist credits + std::vector musicBrainzID; + std::vector albumartists; + CVariant artistidObj(CVariant::VariantTypeArray); + for (VECARTISTCREDITS::const_iterator artistCredit = artistCredits.begin(); artistCredit != artistCredits.end(); ++artistCredit) + { + artistidObj.push_back(artistCredit->GetArtistId()); + albumartists.push_back(artistCredit->GetArtist()); + if (!artistCredit->GetMusicBrainzArtistID().empty()) + musicBrainzID.push_back(artistCredit->GetMusicBrainzArtistID()); + } + item->GetMusicInfoTag()->SetAlbumArtist(albumartists); + item->GetMusicInfoTag()->SetMusicBrainzAlbumArtistID(musicBrainzID); + // Add song albumartistIds as separate property as not part of CMusicInfoTag + item->SetProperty("albumartistid", artistidObj); + return true; } catch (...) @@ -1672,8 +1687,8 @@ void CMusicDatabase::GetFileItemFromDataset(CFileItem* item, const CMusicDbUrl & void CMusicDatabase::GetFileItemFromDataset(const dbiplus::sql_record* const record, CFileItem* item, const CMusicDbUrl &baseUrl) { - // get the artist string from song (not the song_artist and artist tables) - item->GetMusicInfoTag()->SetArtist(record->at(song_strArtists).get_asString()); + // get the artist string from songview (not the song_artist and artist tables) + item->GetMusicInfoTag()->SetArtistDesc(record->at(song_strArtists).get_asString()); // and the full genre string item->GetMusicInfoTag()->SetGenre(record->at(song_strGenres).get_asString()); // and the rest... @@ -1700,6 +1715,7 @@ void CMusicDatabase::GetFileItemFromDataset(const dbiplus::sql_record* const rec std::string strRealPath = URIUtils::AddFileToFolder(record->at(song_strPath).get_asString(), record->at(song_strFileName).get_asString()); item->GetMusicInfoTag()->SetURL(strRealPath); item->GetMusicInfoTag()->SetCompilation(record->at(song_bCompilation).get_asInt() == 1); + // get the album artist string from songview (not the album_artist and artist tables) item->GetMusicInfoTag()->SetAlbumArtist(record->at(song_strAlbumArtists).get_asString()); item->GetMusicInfoTag()->SetAlbumReleaseType(CAlbum::ReleaseTypeFromString(record->at(song_strAlbumReleaseType).get_asString())); item->GetMusicInfoTag()->SetLoaded(true); @@ -1717,6 +1733,25 @@ void CMusicDatabase::GetFileItemFromDataset(const dbiplus::sql_record* const rec } } +void CMusicDatabase::GetFileItemFromArtistCredits(VECARTISTCREDITS& artistCredits, CFileItem* item) +{ + // Populate fileitem with artists from vector of artist credits + std::vector musicBrainzID; + std::vector songartists; + CVariant artistidObj(CVariant::VariantTypeArray); + for (VECARTISTCREDITS::const_iterator artistCredit = artistCredits.begin(); artistCredit != artistCredits.end(); ++artistCredit) + { + artistidObj.push_back(artistCredit->GetArtistId()); + songartists.push_back(artistCredit->GetArtist()); + if (!artistCredit->GetMusicBrainzArtistID().empty()) + musicBrainzID.push_back(artistCredit->GetMusicBrainzArtistID()); + } + item->GetMusicInfoTag()->SetArtist(songartists); + item->GetMusicInfoTag()->SetMusicBrainzArtistID(musicBrainzID); + // Add album artistIds as separate property as not part of CMusicInfoTag + item->SetProperty("artistid", artistidObj); +} + CAlbum CMusicDatabase::GetAlbumFromDataset(dbiplus::Dataset* pDS, int offset /* = 0 */, bool imageURL /* = false*/) { return GetAlbumFromDataset(pDS->get_sql_record(), offset, imageURL); @@ -1987,12 +2022,14 @@ bool CMusicDatabase::GetTop100Albums(VECALBUMS& albums) if (NULL == m_pDB.get()) return false; if (NULL == m_pDS.get()) return false; - // NOTE: The song.idAlbum is needed for the group by, as for some reason group by albumview.idAlbum doesn't work - // consistently - possibly an SQLite bug, as it works fine in SQLiteSpy (v3.3.17) - std::string strSQL = "select albumview.* from albumview " - "where albumview.iTimesPlayed>0 and albumview.strAlbum != '' " - "order by albumview.iTimesPlayed desc " - "limit 100 "; + // Get data from album and album_artist tables to fully populate albums + std::string strSQL = "SELECT albumview.*, albumartistview.* FROM albumview " + "LEFT JOIN albumartistview ON albumview.idAlbum = albumartistview.idAlbum " + "WHERE albumartistview.idAlbum in " + "(SELECT albumview.idAlbum FROM albumview " + "WHERE albumview.strAlbum != '' AND albumview.iTimesPlayed>0 " + "ORDER BY albumview.iTimesPlayed DESC LIMIT 100) " + "ORDER BY albumview.iTimesPlayed DESC, albumartistview.iOrder"; CLog::Log(LOGDEBUG, "%s query: %s", __FUNCTION__, strSQL.c_str()); if (!m_pDS->query(strSQL)) return false; @@ -2002,9 +2039,21 @@ bool CMusicDatabase::GetTop100Albums(VECALBUMS& albums) m_pDS->close(); return true; } + + int albumArtistOffset = album_enumCount; + int albumId = -1; while (!m_pDS->eof()) { - albums.push_back(GetAlbumFromDataset(m_pDS.get())); + const dbiplus::sql_record* const record = m_pDS->get_sql_record(); + + if (albumId != record->at(album_idAlbum).get_asInt()) + { // New album + albumId = record->at(album_idAlbum).get_asInt(); + albums.push_back(GetAlbumFromDataset(record)); + } + // Get artist details + albums.back().artistCredits.push_back(GetArtistCreditFromDataset(record, albumArtistOffset)); + m_pDS->next(); } @@ -2070,7 +2119,14 @@ bool CMusicDatabase::GetRecentlyPlayedAlbums(VECALBUMS& albums) if (NULL == m_pDB.get()) return false; if (NULL == m_pDS.get()) return false; - std::string strSQL = StringUtils::Format("select distinct albumview.* from song join albumview on albumview.idAlbum=song.idAlbum where song.lastplayed IS NOT NULL order by song.lastplayed desc limit %i", RECENTLY_PLAYED_LIMIT); + // Get data from album and album_artist tables to fully populate albums + std::string strSQL = PrepareSQL("SELECT albumview.*, albumartistview.* FROM " + "(SELECT idAlbum FROM albumview WHERE albumview.lastplayed IS NOT NULL " + "ORDER BY albumview.lastplayed DESC LIMIT %u) as playedalbums " + "JOIN albumview ON albumview.idAlbum = playedalbums.idAlbum " + "LEFT JOIN albumartistview ON albumview.idAlbum = albumartistview.idAlbum " + "ORDER BY albumview.lastplayed DESC, albumartistview.iorder ", RECENTLY_PLAYED_LIMIT); + CLog::Log(LOGDEBUG, "%s query: %s", __FUNCTION__, strSQL.c_str()); if (!m_pDS->query(strSQL)) return false; int iRowsFound = m_pDS->num_rows(); @@ -2079,12 +2135,23 @@ bool CMusicDatabase::GetRecentlyPlayedAlbums(VECALBUMS& albums) m_pDS->close(); return true; } + + int albumArtistOffset = album_enumCount; + int albumId = -1; while (!m_pDS->eof()) { - albums.push_back(GetAlbumFromDataset(m_pDS.get())); + const dbiplus::sql_record* const record = m_pDS->get_sql_record(); + + if (albumId != record->at(album_idAlbum).get_asInt()) + { // New album + albumId = record->at(album_idAlbum).get_asInt(); + albums.push_back(GetAlbumFromDataset(record)); + } + // Get artist details + albums.back().artistCredits.push_back(GetArtistCreditFromDataset(record, albumArtistOffset)); + m_pDS->next(); } - m_pDS->close(); // cleanup recordset data return true; } @@ -2107,7 +2174,12 @@ bool CMusicDatabase::GetRecentlyPlayedAlbumSongs(const std::string& strBaseDir, if (!strBaseDir.empty() && !baseUrl.FromString(strBaseDir)) return false; - std::string strSQL = StringUtils::Format("SELECT songview.*, albumview.* FROM songview JOIN albumview ON (songview.idAlbum = albumview.idAlbum) JOIN (SELECT DISTINCT album.idAlbum FROM album JOIN song ON album.idAlbum = song.idAlbum WHERE song.lastplayed IS NOT NULL ORDER BY song.lastplayed DESC LIMIT %i) AS _albumlimit ON (albumview.idAlbum = _albumlimit.idAlbum)", g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); + std::string strSQL = PrepareSQL("SELECT songview.*, songartistview.* FROM " + "(SELECT idAlbum FROM albumview WHERE albumview.lastplayed IS NOT NULL " + "ORDER BY albumview.lastplayed DESC LIMIT %u) as playedalbums " + "JOIN songview ON songview.idAlbum = playedalbums.idAlbum " + "LEFT JOIN songartistview ON songview.idSong = songartistview.idSong ", + g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); CLog::Log(LOGDEBUG,"GetRecentlyPlayedAlbumSongs() query: %s", strSQL.c_str()); if (!m_pDS->query(strSQL)) return false; @@ -2118,15 +2190,38 @@ bool CMusicDatabase::GetRecentlyPlayedAlbumSongs(const std::string& strBaseDir, return true; } - // get data from returned rows - items.Reserve(iRowsFound); + // Needs a separate query to determine number of songs to set items size. + // Get songs from returned rows. Join means there is a row for every song artist + int songArtistOffset = song_enumCount; + int songId = -1; + VECARTISTCREDITS artistCredits; while (!m_pDS->eof()) { - CFileItemPtr item(new CFileItem); - GetFileItemFromDataset(item.get(), baseUrl); - items.Add(item); + const dbiplus::sql_record* const record = m_pDS->get_sql_record(); + + if (songId != record->at(song_idSong).get_asInt()) + { //New song + if (songId > 0 && !artistCredits.empty()) + { + //Store artist credits for previous song + GetFileItemFromArtistCredits(artistCredits, items[items.Size() - 1].get()); + artistCredits.clear(); + } + songId = record->at(song_idSong).get_asInt(); + CFileItemPtr item(new CFileItem); + GetFileItemFromDataset(record, item.get(), baseUrl); + items.Add(item); + } + // Get song artist credits + artistCredits.push_back(GetArtistCreditFromDataset(record, songArtistOffset)); m_pDS->next(); } + if (!artistCredits.empty()) + { + //Store artist credits for final song + GetFileItemFromArtistCredits(artistCredits, items[items.Size() - 1].get()); + artistCredits.clear(); + } // cleanup m_pDS->close(); @@ -2147,7 +2242,15 @@ bool CMusicDatabase::GetRecentlyAddedAlbums(VECALBUMS& albums, unsigned int limi if (NULL == m_pDB.get()) return false; if (NULL == m_pDS.get()) return false; - std::string strSQL = StringUtils::Format("select * from albumview where strAlbum != '' order by idAlbum desc limit %u", limit ? limit : g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); + // Get data from album and album_artist tables to fully populate albums + // Use idAlbum to determine the recently added albums + // (not "dateAdded" as this is file time stamp and nothing to do with when albums added to library) + std::string strSQL = PrepareSQL("SELECT albumview.*, albumartistview.* FROM " + "(SELECT idAlbum FROM album WHERE strAlbum != '' ORDER BY idAlbum DESC LIMIT %u) AS recentalbums " + "JOIN albumview ON albumview.idAlbum = recentalbums.idAlbum " + "LEFT JOIN albumartistview ON albumview.idAlbum = albumartistview.idAlbum " + "ORDER BY albumview.idAlbum desc, albumartistview.iOrder ", + limit ? limit : g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); CLog::Log(LOGDEBUG, "%s query: %s", __FUNCTION__, strSQL.c_str()); if (!m_pDS->query(strSQL)) return false; @@ -2158,12 +2261,22 @@ bool CMusicDatabase::GetRecentlyAddedAlbums(VECALBUMS& albums, unsigned int limi return true; } + int albumArtistOffset = album_enumCount; + int albumId = -1; while (!m_pDS->eof()) { - albums.push_back(GetAlbumFromDataset(m_pDS.get())); + const dbiplus::sql_record* const record = m_pDS->get_sql_record(); + + if (albumId != record->at(album_idAlbum).get_asInt()) + { // New album + albumId = record->at(album_idAlbum).get_asInt(); + albums.push_back(GetAlbumFromDataset(record)); + } + // Get artist details + albums.back().artistCredits.push_back(GetArtistCreditFromDataset(record, albumArtistOffset)); + m_pDS->next(); } - m_pDS->close(); // cleanup recordset data return true; } @@ -2186,8 +2299,16 @@ bool CMusicDatabase::GetRecentlyAddedAlbumSongs(const std::string& strBaseDir, C if (!strBaseDir.empty() && !baseUrl.FromString(strBaseDir)) return false; + // Get data from song and song_artist tables to fully populate songs + // Use idAlbum to determine the recently added albums + // (not "dateAdded" as this is file time stamp and nothing to do with when albums added to library) std::string strSQL; - strSQL = PrepareSQL("SELECT songview.* FROM (SELECT idAlbum FROM albumview ORDER BY idAlbum DESC LIMIT %u) AS recentalbums JOIN songview ON songview.idAlbum=recentalbums.idAlbum", limit ? limit : g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); + strSQL = PrepareSQL("SELECT songview.*, songartistview.* FROM " + "(SELECT idAlbum FROM album ORDER BY idAlbum DESC LIMIT %u) AS recentalbums " + "JOIN songview ON songview.idAlbum = recentalbums.idAlbum " + "JOIN songartistview ON songview.idSong = songartistview.idSong " + "ORDER BY songview.idAlbum desc, songview.itrack, songartistview.iOrder ", + limit ? limit : g_advancedSettings.m_iMusicLibraryRecentlyAddedItems); CLog::Log(LOGDEBUG,"GetRecentlyAddedAlbumSongs() query: %s", strSQL.c_str()); if (!m_pDS->query(strSQL)) return false; @@ -2198,15 +2319,39 @@ bool CMusicDatabase::GetRecentlyAddedAlbumSongs(const std::string& strBaseDir, C return true; } - // get data from returned rows - items.Reserve(iRowsFound); + // Needs a separate query to determine number of songs to set items size. + // Get songs from returned rows. Join means there is a row for every song artist + int songArtistOffset = song_enumCount; + int songId = -1; + VECARTISTCREDITS artistCredits; while (!m_pDS->eof()) { - CFileItemPtr item(new CFileItem); - GetFileItemFromDataset(item.get(), baseUrl); - items.Add(item); + const dbiplus::sql_record* const record = m_pDS->get_sql_record(); + + if (songId != record->at(song_idSong).get_asInt()) + { //New song + if (songId > 0 && !artistCredits.empty()) + { + //Store artist credits for previous song + GetFileItemFromArtistCredits(artistCredits, items[items.Size() - 1].get()); + artistCredits.clear(); + } + songId = record->at(song_idSong).get_asInt(); + CFileItemPtr item(new CFileItem); + GetFileItemFromDataset(record, item.get(), baseUrl); + items.Add(item); + } + // Get song artist credits + artistCredits.push_back(GetArtistCreditFromDataset(record, songArtistOffset)); + m_pDS->next(); } + if (!artistCredits.empty()) + { + //Store artist credits for final song + GetFileItemFromArtistCredits(artistCredits, items[items.Size() - 1].get()); + artistCredits.clear(); + } // cleanup m_pDS->close(); @@ -3533,6 +3678,239 @@ bool CMusicDatabase::GetAlbumsByWhere(const std::string &baseDir, const Filter & return false; } +bool CMusicDatabase::GetAlbumsByWhere(const std::string &baseDir, const Filter &filter, VECALBUMS& albums, int& total, const SortDescription &sortDescription /* = SortDescription() */, bool countOnly /* = false */) +{ + albums.erase(albums.begin(), albums.end()); + if (NULL == m_pDB.get()) return false; + if (NULL == m_pDS.get()) return false; + + try + { + total = -1; + // Get data from album and album_artist tables to fully populate albums + std::string strSQL = "SELECT %s FROM albumview LEFT JOIN albumartistview on albumartistview.idalbum = albumview.idalbum "; + + Filter extFilter = filter; + CMusicDbUrl musicUrl; + SortDescription sorting = sortDescription; + if (!musicUrl.FromString(baseDir) || !GetFilter(musicUrl, extFilter, sorting)) + return false; + + // if there are extra WHERE conditions we might need access + // to songview for these conditions + if (extFilter.where.find("songview") != std::string::npos) + { + extFilter.AppendJoin("JOIN songview ON songview.idAlbum = albumview.idAlbum"); + extFilter.AppendGroup("albumview.idAlbum"); + } + + std::string strSQLExtra; + if (!BuildSQL(strSQLExtra, extFilter, strSQLExtra)) + return false; + + // Count and return number of albums that satisfy selection criteria + total = (int)strtol(GetSingleValue("SELECT COUNT(1) FROM albumview " + strSQLExtra, m_pDS).c_str(), NULL, 10); + if (countOnly) + return true; + + // Apply the limiting directly here if there's no special sorting but limiting + if (extFilter.limit.empty() && + sortDescription.sortBy == SortByNone && + (sortDescription.limitStart > 0 || sortDescription.limitEnd > 0)) + { + strSQLExtra += DatabaseUtils::BuildLimitClause(sortDescription.limitEnd, sortDescription.limitStart); + albums.reserve(sortDescription.limitEnd - sortDescription.limitStart); + } + else + albums.reserve(total); + + strSQL = PrepareSQL(strSQL, !filter.fields.empty() && filter.fields.compare("*") != 0 ? filter.fields.c_str() : "albumview.*, albumartistview.* ") + strSQLExtra; + + CLog::Log(LOGDEBUG, "%s query: %s", __FUNCTION__, strSQL.c_str()); + // run query + unsigned int time = XbmcThreads::SystemClockMillis(); + if (!m_pDS->query(strSQL)) + return false; + CLog::Log(LOGDEBUG, "%s - query took %i ms", + __FUNCTION__, XbmcThreads::SystemClockMillis() - time); time = XbmcThreads::SystemClockMillis(); + + int iRowsFound = m_pDS->num_rows(); + if (iRowsFound <= 0) + { + m_pDS->close(); + return true; + } + + //Sort the results set - need to add sort by iOrder to maintain artist name order?? + DatabaseResults results; + results.reserve(iRowsFound); + if (!SortUtils::SortFromDataset(sortDescription, MediaTypeAlbum, m_pDS, results)) + return false; + + // Get albums from returned rows. Join means there is a row for every album artist + int albumArtistOffset = album_enumCount; + int albumId = -1; + + const dbiplus::query_data &data = m_pDS->get_result_set().records; + for (DatabaseResults::const_iterator it = results.begin(); it != results.end(); ++it) + { + unsigned int targetRow = (unsigned int)it->at(FieldRow).asInteger(); + const dbiplus::sql_record* const record = data.at(targetRow); + + if (albumId != record->at(album_idAlbum).get_asInt()) + { // New album + albumId = record->at(album_idAlbum).get_asInt(); + albums.push_back(GetAlbumFromDataset(record)); + } + // Get album artist credits + albums.back().artistCredits.push_back(GetArtistCreditFromDataset(record, albumArtistOffset)); + } + + m_pDS->close(); // cleanup recordset data + return true; + } + catch (...) + { + m_pDS->close(); + CLog::Log(LOGERROR, "%s (%s) failed", __FUNCTION__, filter.where.c_str()); + } + return false; +} + +bool CMusicDatabase::GetSongsFullByWhere(const std::string &baseDir, const Filter &filter, CFileItemList &items, const SortDescription &sortDescription /* = SortDescription() */, bool artistData /* = false*/) +{ + if (m_pDB.get() == NULL || m_pDS.get() == NULL) + return false; + + try + { + unsigned int time = XbmcThreads::SystemClockMillis(); + int total = -1; + + std::string strSQL = "SELECT %s FROM songview "; + if (artistData) // Get data from song and song_artist tables to fully populate songs with artists + strSQL = "SELECT %s FROM songview JOIN songartistview on songartistview.idsong = songview.idsong "; + + Filter extFilter = filter; + CMusicDbUrl musicUrl; + SortDescription sorting = sortDescription; + if (!musicUrl.FromString(baseDir) || !GetFilter(musicUrl, extFilter, sorting)) + return false; + + // if there are extra WHERE conditions we might need access + // to songview for these conditions + if (extFilter.where.find("albumview") != std::string::npos) + { + extFilter.AppendJoin("JOIN albumview ON albumview.idAlbum = songview.idAlbum"); + extFilter.AppendGroup("songview.idSong"); + } + + std::string strSQLExtra; + if (!BuildSQL(strSQLExtra, extFilter, strSQLExtra)) + return false; + + // Count number of songs that satisfy selection criteria + total = (int)strtol(GetSingleValue("SELECT COUNT(1) FROM songview " + strSQLExtra, m_pDS).c_str(), NULL, 10); + + // Apply the limiting directly here if there's no special sorting but limiting + if (extFilter.limit.empty() && + sortDescription.sortBy == SortByNone && + (sortDescription.limitStart > 0 || sortDescription.limitEnd > 0)) + strSQLExtra += DatabaseUtils::BuildLimitClause(sortDescription.limitEnd, sortDescription.limitStart); + + if (artistData) + strSQL = PrepareSQL(strSQL, !filter.fields.empty() && filter.fields.compare("*") != 0 ? filter.fields.c_str() : "songview.*, songartistview.* ") + strSQLExtra; + else + strSQL = PrepareSQL(strSQL, !filter.fields.empty() && filter.fields.compare("*") != 0 ? filter.fields.c_str() : "songview.* ") + strSQLExtra; + + CLog::Log(LOGDEBUG, "%s query = %s", __FUNCTION__, strSQL.c_str()); + // run query + if (!m_pDS->query(strSQL)) + return false; + + int iRowsFound = m_pDS->num_rows(); + if (iRowsFound == 0) + { + m_pDS->close(); + return true; + } + + // Store the total number of songs as a property + items.SetProperty("total", total); + + DatabaseResults results; + results.reserve(iRowsFound); + if (!SortUtils::SortFromDataset(sortDescription, MediaTypeSong, m_pDS, results)) + return false; + + // Get songs from returned rows. If join songartistview then there is a row for every album artist + items.Reserve(total); + int songArtistOffset = song_enumCount; + int songId = -1; + VECARTISTCREDITS artistCredits; + const dbiplus::query_data &data = m_pDS->get_result_set().records; + int count = 0; + for (DatabaseResults::const_iterator it = results.begin(); it != results.end(); ++it) + { + unsigned int targetRow = (unsigned int)it->at(FieldRow).asInteger(); + const dbiplus::sql_record* const record = data.at(targetRow); + + try + { + if (songId != record->at(song_idSong).get_asInt()) + { //New song + if (songId > 0 && !artistCredits.empty()) + { + //Store artist credits for previous song + GetFileItemFromArtistCredits(artistCredits, items[items.Size()-1].get()); + artistCredits.clear(); + } + songId = record->at(song_idSong).get_asInt(); + CFileItemPtr item(new CFileItem); + GetFileItemFromDataset(record, item.get(), musicUrl); + // HACK for sorting by database returned order + item->m_iprogramCount = ++count; + items.Add(item); + } + // Get song artist credits + if (artistData) + artistCredits.push_back(GetArtistCreditFromDataset(record, songArtistOffset)); + } + catch (...) + { + m_pDS->close(); + CLog::Log(LOGERROR, "%s: out of memory loading query: %s", __FUNCTION__, filter.where.c_str()); + return (items.Size() > 0); + } + + + } + if (!artistCredits.empty()) + { + //Store artist credits for final song + GetFileItemFromArtistCredits(artistCredits, items[items.Size() - 1].get()); + artistCredits.clear(); + } + // cleanup + m_pDS->close(); + + // Load some info from embedded cuesheet if present (now only ReplayGain) + CueInfoLoader cueLoader; + for (int i = 0; i < items.Size(); ++i) + cueLoader.Load(LoadCuesheet(items[i]->GetMusicInfoTag()->GetURL()), items[i]); + + CLog::Log(LOGDEBUG, "%s(%s) - took %d ms", __FUNCTION__, filter.where.c_str(), XbmcThreads::SystemClockMillis() - time); + return true; + } + catch (...) + { + // cleanup + m_pDS->close(); + CLog::Log(LOGERROR, "%s(%s) failed", __FUNCTION__, filter.where.c_str()); + } + return false; +} + bool CMusicDatabase::GetSongsByWhere(const std::string &baseDir, const Filter &filter, CFileItemList &items, const SortDescription &sortDescription /* = SortDescription() */) { if (m_pDB.get() == NULL || m_pDS.get() == NULL) diff --git a/xbmc/music/MusicDatabase.h b/xbmc/music/MusicDatabase.h index 1f07d42e9743f..b16cb6c4e542b 100644 --- a/xbmc/music/MusicDatabase.h +++ b/xbmc/music/MusicDatabase.h @@ -293,7 +293,7 @@ class CMusicDatabase : public CDatabase ///////////////////////////////////////////////// bool AddAlbumArtist(int idArtist, int idAlbum, std::string strArtist, std::string joinPhrase, bool featured, int iOrder); bool GetAlbumsByArtist(int idArtist, bool includeFeatured, std::vector& albums); - bool GetArtistsByAlbum(int idAlbum, bool includeFeatured, std::vector& artists); + bool GetArtistsByAlbum(int idAlbum, CFileItem* item); bool DeleteAlbumArtistsByAlbum(int idAlbum); bool AddSongArtist(int idArtist, int idSong, std::string strArtist, std::string joinPhrase, bool featured, int iOrder); @@ -354,7 +354,9 @@ class CMusicDatabase : public CDatabase bool GetSongsNav(const std::string& strBaseDir, CFileItemList& items, int idGenre, int idArtist,int idAlbum, const SortDescription &sortDescription = SortDescription()); bool GetSongsByYear(const std::string& baseDir, CFileItemList& items, int year); bool GetSongsByWhere(const std::string &baseDir, const Filter &filter, CFileItemList& items, const SortDescription &sortDescription = SortDescription()); + bool GetSongsFullByWhere(const std::string &baseDir, const Filter &filter, CFileItemList& items, const SortDescription &sortDescription = SortDescription(), bool artistData = false); bool GetAlbumsByWhere(const std::string &baseDir, const Filter &filter, CFileItemList &items, const SortDescription &sortDescription = SortDescription(), bool countOnly = false); + bool GetAlbumsByWhere(const std::string &baseDir, const Filter &filter, VECALBUMS& albums, int& total, const SortDescription &sortDescription = SortDescription(), bool countOnly = false); bool GetArtistsByWhere(const std::string& strBaseDir, const Filter &filter, CFileItemList& items, const SortDescription &sortDescription = SortDescription(), bool countOnly = false); bool GetRandomSong(CFileItem* item, int& idSong, const Filter &filter); int GetSongsCount(const Filter &filter = Filter()); @@ -493,6 +495,7 @@ class CMusicDatabase : public CDatabase void UpdateFileDateAdded(int songId, const std::string& strFileNameAndPath); void GetFileItemFromDataset(CFileItem* item, const CMusicDbUrl &baseUrl); void GetFileItemFromDataset(const dbiplus::sql_record* const record, CFileItem* item, const CMusicDbUrl &baseUrl); + void GetFileItemFromArtistCredits(VECARTISTCREDITS& artistCredits, CFileItem* item); CSong GetAlbumInfoSongFromDataset(const dbiplus::sql_record* const record, int offset = 0); bool CleanupSongs(); bool CleanupSongsByIds(const std::string &strSongIds); diff --git a/xbmc/music/Song.cpp b/xbmc/music/Song.cpp index 50cf0f6c00029..01ea30accbdc1 100644 --- a/xbmc/music/Song.cpp +++ b/xbmc/music/Song.cpp @@ -162,11 +162,6 @@ const std::vector CSong::GetArtist() const { songartists.push_back(artistCredit->GetArtist()); } - //When artist credits have not been populated attempt to build an artist vector from the descrpition string - //This is a tempory fix, in the longer term other areas should query the song_artist table and populate - //artist credits. Note that splitting the string may not give the same artists as held in the song_artist table - if (songartists.empty() && !strArtistDesc.empty()) - songartists = StringUtils::Split(strArtistDesc, g_advancedSettings.m_musicItemSeparator); return songartists; } @@ -193,6 +188,15 @@ const std::string CSong::GetArtistString() const return artistString; } +const std::vector CSong::GetArtistIDArray() const +{ + // Get song artist IDs for json rpc + std::vector artistids; + for (VECARTISTCREDITS::const_iterator artistCredit = artistCredits.begin(); artistCredit != artistCredits.end(); ++artistCredit) + artistids.push_back(artistCredit->GetArtistId()); + return artistids; +} + bool CSong::HasArt() const { if (!strThumb.empty()) return true; diff --git a/xbmc/music/Song.h b/xbmc/music/Song.h index 14b32bc47bfee..5caf404ee01bf 100644 --- a/xbmc/music/Song.h +++ b/xbmc/music/Song.h @@ -88,6 +88,11 @@ class CSong: public ISerializable */ const std::string GetArtistString() const; + /*! \brief Get song artist IDs (for json rpc) from the vector of artistcredits objects + \return album artist IDs as a vector of integers + */ + const std::vector GetArtistIDArray() const; + /*! \brief Get album artist names associated with song from tag data Note for initial album processing only, normalised album artist data belongs to album and is stored in album artist credits diff --git a/xbmc/music/tags/MusicInfoTag.cpp b/xbmc/music/tags/MusicInfoTag.cpp index 08ac5f0f26c85..fc8dfbe3669f8 100644 --- a/xbmc/music/tags/MusicInfoTag.cpp +++ b/xbmc/music/tags/MusicInfoTag.cpp @@ -706,9 +706,9 @@ void CMusicInfoTag::Serialize(CVariant& value) const value["loaded"] = m_bLoaded; value["year"] = m_dwReleaseDate.wYear; value["musicbrainztrackid"] = m_strMusicBrainzTrackID; - value["musicbrainzartistid"] = StringUtils::Join(m_musicBrainzArtistID, " / "); + value["musicbrainzartistid"] = m_musicBrainzArtistID; value["musicbrainzalbumid"] = m_strMusicBrainzAlbumID; - value["musicbrainzalbumartistid"] = StringUtils::Join(m_musicBrainzAlbumArtistID, " / "); + value["musicbrainzalbumartistid"] = m_musicBrainzAlbumArtistID; value["musicbrainztrmid"] = m_strMusicBrainzTRMID; value["comment"] = m_strComment; value["mood"] = m_strMood; @@ -738,9 +738,9 @@ void CMusicInfoTag::ToSortable(SortItem& sortable, Field field) const sortable[FieldTitle] = title; break; } - case FieldArtist: sortable[FieldArtist] = m_artist; break; + case FieldArtist: sortable[FieldArtist] = m_strArtistDesc; break; case FieldAlbum: sortable[FieldAlbum] = m_strAlbum; break; - case FieldAlbumArtist: sortable[FieldAlbumArtist] = m_albumArtist; break; + case FieldAlbumArtist: sortable[FieldAlbumArtist] = m_strAlbumArtistDesc; break; case FieldGenre: sortable[FieldGenre] = m_genre; break; case FieldTime: sortable[FieldTime] = m_iDuration; break; case FieldTrackNumber: sortable[FieldTrackNumber] = m_iTrack; break;