Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fix]jsonrpc getting consistent song and album artist data - names, IDs and MusicbrainzIDs #8306

Merged
merged 1 commit into from Oct 31, 2015
Merged

[Fix]jsonrpc getting consistent song and album artist data - names, IDs and MusicbrainzIDs #8306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
107 changes: 57 additions & 50 deletions xbmc/interfaces/json-rpc/AudioLibrary.cpp
View file
Open in desktop
Expand Up @@ -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())
Expand All @@ -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;
Expand Down Expand Up @@ -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())
Expand All @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -685,9 +702,22 @@ bool CAudioLibrary::FillFileItemList(const CVariant &parameterObject, CFileItemL
return success;
}

void CAudioLibrary::FillItemArtistIDs(const std::vector<int> artistids, CFileItemPtr &item)
{
// Add artistIds as separate property as not part of CMusicInfoTag
CVariant artistidObj(CVariant::VariantTypeArray);
for (std::vector<int>::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<int> artistids = album.GetArtistIDArray();
FillItemArtistIDs(artistids, item);
}

JSONRPC_STATUS CAudioLibrary::GetAdditionalDetails(const CVariant &parameterObject, CFileItemList &items)
Expand All @@ -711,7 +741,6 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalAlbumDetails(const CVariant &paramete

std::set<std::string> checkProperties;
checkProperties.insert("genreid");
checkProperties.insert("artistid");
std::set<std::string> additionalProperties;
if (!CheckForAdditionalProperties(parameterObject["properties"], checkProperties, additionalProperties))
return OK;
Expand All @@ -731,18 +760,7 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalAlbumDetails(const CVariant &paramete
item->SetProperty("genreid", genreidObj);
}
}
if (additionalProperties.find("artistid") != additionalProperties.end())
{
std::vector<int> artistids;
if (musicdatabase.GetArtistsByAlbum(item->GetMusicInfoTag()->GetDatabaseId(), true, artistids))
{
CVariant artistidObj(CVariant::VariantTypeArray);
for (std::vector<int>::const_iterator artistid = artistids.begin(); artistid != artistids.end(); ++artistid)
artistidObj.push_back(*artistid);

item->SetProperty("artistid", artistidObj);
}
}

}

return OK;
Expand All @@ -755,8 +773,12 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalSongDetails(const CVariant &parameter

std::set<std::string> 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<std::string> additionalProperties;
if (!CheckForAdditionalProperties(parameterObject["properties"], checkProperties, additionalProperties))
return OK;
Expand All @@ -776,28 +798,13 @@ JSONRPC_STATUS CAudioLibrary::GetAdditionalSongDetails(const CVariant &parameter
item->SetProperty("genreid", genreidObj);
}
}
if (additionalProperties.find("artistid") != additionalProperties.end())
{
std::vector<int> artistids;
if (musicdatabase.GetArtistsBySong(item->GetMusicInfoTag()->GetDatabaseId(), true, artistids))
{
CVariant artistidObj(CVariant::VariantTypeArray);
for (std::vector<int>::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<int> 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<int>::const_iterator albumartistid = albumartistids.begin(); albumartistid != albumartistids.end(); ++albumartistid)
albumartistidObj.push_back(*albumartistid);

item->SetProperty("albumartistid", albumartistidObj);
musicdatabase.GetArtistsByAlbum(item->GetMusicInfoTag()->GetAlbumId(), item.get());
}
}
}
Expand Down
1 change: 1 addition & 0 deletions xbmc/interfaces/json-rpc/AudioLibrary.h
View file
Open in desktop
Expand Up @@ -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<int> artistids, CFileItemPtr &item);

static bool CheckForAdditionalProperties(const CVariant &properties, const std::set<std::string> &checkProperties, std::set<std::string> &foundProperties);
};
Expand Down
2 changes: 1 addition & 1 deletion xbmc/interfaces/json-rpc/schema/version.txt
View file
Open in desktop
@@ -1 +1 @@
6.31.0
6.32.0
14 changes: 9 additions & 5 deletions xbmc/music/Album.cpp
View file
Open in desktop
Expand Up @@ -178,11 +178,6 @@ const std::vector<std::string> 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;
}

Expand All @@ -209,6 +204,15 @@ const std::string CAlbum::GetAlbumArtistString() const
return artistString;
}

const std::vector<int> CAlbum::GetArtistIDArray() const
{
// Get album artist IDs for json rpc
std::vector<int> 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);
Expand Down
5 changes: 5 additions & 0 deletions xbmc/music/Album.h
View file
Open in desktop
Expand Up @@ -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<int> GetArtistIDArray() const;

typedef enum ReleaseType {
Album = 0,
Single
Expand Down