Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #2974 from night199uk/musicfixes

[musicdb] Fixes for non-MusicBrainz albums
  • Loading branch information...
commit 8193a5359d133a4cbf4858bf2c95c0bf7d86357a 2 parents 7aca6dd + e54553d
@night199uk night199uk authored
View
23 xbmc/music/Album.cpp
@@ -38,6 +38,16 @@ CAlbum::CAlbum(const CFileItem& item)
strMusicBrainzAlbumID = tag.GetMusicBrainzAlbumID();
genre = tag.GetGenre();
artist = tag.GetAlbumArtist();
+ bool hasMusicBrainzAlbumArtist = !tag.GetMusicBrainzAlbumArtistID().empty();
+ const vector<string>& artists = hasMusicBrainzAlbumArtist ? tag.GetMusicBrainzAlbumArtistID() : tag.GetAlbumArtist();
+ for (vector<string>::const_iterator it = artists.begin(); it != artists.end(); ++it)
+ {
+ CStdString artistName = hasMusicBrainzAlbumArtist && !artist.empty() ? artist[0] : *it;
+ CStdString artistId = hasMusicBrainzAlbumArtist ? *it : StringUtils::EmptyString;
+ CStdString strJoinPhrase = (it == --artists.end() ? "" : g_advancedSettings.m_musicItemSeparator);
+ CArtistCredit artistCredit(artistName, artistId, strJoinPhrase);
+ artistCredits.push_back(artistCredit);
+ }
iYear = stTime.wYear;
bCompilation = tag.GetCompilation();
iTimesPlayed = 0;
@@ -55,8 +65,17 @@ CStdString CAlbum::GetGenreString() const
bool CAlbum::operator<(const CAlbum &a) const
{
- if (strAlbum < a.strAlbum) return true;
- if (strAlbum > a.strAlbum) return false;
+ if (strMusicBrainzAlbumID.IsEmpty() && a.strMusicBrainzAlbumID.IsEmpty())
+ {
+ if (strAlbum < a.strAlbum) return true;
+ if (strAlbum > a.strAlbum) return false;
+
+ // This will do an std::vector compare (i.e. item by item)
+ if (artist < a.artist) return true;
+ if (artist > a.artist) return false;
+ return false;
+ }
+
if (strMusicBrainzAlbumID < a.strMusicBrainzAlbumID) return true;
if (strMusicBrainzAlbumID > a.strMusicBrainzAlbumID) return false;
return false;
View
18 xbmc/music/Artist.h
@@ -36,8 +36,13 @@ class CArtist
long idArtist;
bool operator<(const CArtist& a) const
{
- if (strArtist < a.strArtist) return true;
- if (strArtist > a.strArtist) return false;
+ if (strMusicBrainzArtistID.IsEmpty() && a.strMusicBrainzArtistID.IsEmpty())
+ {
+ if (strArtist < a.strArtist) return true;
+ if (strArtist > a.strArtist) return false;
+ return false;
+ }
+
if (strMusicBrainzArtistID < a.strMusicBrainzArtistID) return true;
if (strMusicBrainzArtistID > a.strMusicBrainzArtistID) return false;
return false;
@@ -102,8 +107,13 @@ class CArtistCredit
: m_strArtist(strArtist), m_strMusicBrainzArtistID(strMusicBrainzArtistID), m_strJoinPhrase(strJoinPhrase), m_boolFeatured(false) { }
bool operator<(const CArtistCredit& a) const
{
- if (m_strArtist < a.m_strArtist) return true;
- if (m_strArtist > a.m_strArtist) return false;
+ if (m_strMusicBrainzArtistID.empty() && a.m_strMusicBrainzArtistID.empty())
+ {
+ if (m_strArtist < a.m_strArtist) return true;
+ if (m_strArtist > a.m_strArtist) return false;
+ return false;
+ }
+
if (m_strMusicBrainzArtistID < a.m_strMusicBrainzArtistID) return true;
if (m_strMusicBrainzArtistID > a.m_strMusicBrainzArtistID) return false;
return false;
View
36 xbmc/music/MusicDatabase.cpp
@@ -364,12 +364,15 @@ int CMusicDatabase::AddSong(const int idAlbum,
bHasKaraoke = CKaraokeLyricsFactory::HasLyrics(strPathAndFileName);
#endif
- strSQL=PrepareSQL("SELECT * FROM song WHERE (idAlbum = %i AND strMusicBrainzTrackID = '%s') OR (idAlbum=%i AND dwFileNameCRC='%ul' AND strTitle='%s' AND strMusicBrainzTrackID IS NULL)",
- idAlbum,
- strMusicBrainzTrackID.c_str(),
- idAlbum,
- crc,
- strTitle.c_str());
+ if (!strMusicBrainzTrackID.empty())
+ strSQL = PrepareSQL("SELECT * FROM song WHERE idAlbum = %i AND strMusicBrainzTrackID = '%s'",
+ idAlbum,
+ strMusicBrainzTrackID.c_str());
+ else
+ strSQL = PrepareSQL("SELECT * FROM song WHERE idAlbum=%i AND dwFileNameCRC='%ul' AND strTitle='%s' AND strMusicBrainzTrackID IS NULL",
+ idAlbum,
+ crc,
+ strTitle.c_str());
if (!m_pDS->query(strSQL.c_str()))
return -1;
@@ -497,10 +500,13 @@ int CMusicDatabase::AddAlbum(const CStdString& strAlbum, const CStdString& strMu
if (NULL == m_pDB.get()) return -1;
if (NULL == m_pDS.get()) return -1;
- strSQL=PrepareSQL("SELECT * FROM album WHERE strMusicBrainzAlbumID = '%s' OR (strArtists = '%s' AND strAlbum like '%s' and strMusicBrainzAlbumID IS NULL)",
- strMusicBrainzAlbumID.c_str(),
- strArtist.c_str(),
- strAlbum.c_str());
+ if (!strMusicBrainzAlbumID.empty())
+ strSQL = PrepareSQL("SELECT * FROM album WHERE strMusicBrainzAlbumID = '%s'",
+ strMusicBrainzAlbumID.c_str());
+ else
+ strSQL = PrepareSQL("SELECT * FROM album WHERE strArtists = '%s' AND strAlbum like '%s' and strMusicBrainzAlbumID IS NULL",
+ strArtist.c_str(),
+ strAlbum.c_str());
m_pDS->query(strSQL.c_str());
if (m_pDS->num_rows() == 0)
@@ -607,9 +613,13 @@ int CMusicDatabase::AddArtist(const CStdString& strArtist, const CStdString& str
if (NULL == m_pDB.get()) return -1;
if (NULL == m_pDS.get()) return -1;
- strSQL = PrepareSQL("SELECT * FROM artist WHERE strMusicBrainzArtistID = '%s' OR (strArtist = '%s' AND strMusicBrainzArtistID IS NULL)",
- strMusicBrainzArtistID.IsEmpty() ? "x" : strMusicBrainzArtistID.c_str(),
- strArtist.c_str());
+ if (!strMusicBrainzArtistID.empty())
+ strSQL = PrepareSQL("SELECT * FROM artist WHERE strMusicBrainzArtistID = '%s'",
+ strMusicBrainzArtistID.c_str());
+ else
+ strSQL = PrepareSQL("SELECT * FROM artist WHERE strArtist = '%s' AND strMusicBrainzArtistID IS NULL",
+ strArtist.c_str());
+
m_pDS->query(strSQL.c_str());
if (m_pDS->num_rows() == 0)
View
10 xbmc/music/Song.cpp
@@ -36,6 +36,16 @@ CSong::CSong(CFileItem& item)
strTitle = tag.GetTitle();
genre = tag.GetGenre();
artist = tag.GetArtist();
+ bool hasMusicBrainzArtist = !tag.GetMusicBrainzArtistID().empty();
+ const vector<string>& artists = hasMusicBrainzArtist ? tag.GetMusicBrainzArtistID() : tag.GetArtist();
+ for (vector<string>::const_iterator it = artists.begin(); it != artists.end(); ++it)
+ {
+ CStdString artistName = hasMusicBrainzArtist && !artist.empty() ? artist[0] : *it;
+ CStdString artistId = hasMusicBrainzArtist ? *it : StringUtils::EmptyString;
+ CStdString strJoinPhrase = (it == --artists.end() ? "" : g_advancedSettings.m_musicItemSeparator);
+ CArtistCredit artistCredit(artistName, artistId, strJoinPhrase);
+ artistCredits.push_back(artistCredit);
+ }
strAlbum = tag.GetAlbum();
albumArtist = tag.GetAlbumArtist();
strMusicBrainzTrackID = tag.GetMusicBrainzTrackID();
View
304 xbmc/music/infoscanner/MusicInfoScanner.cpp
@@ -492,18 +492,28 @@ INFO_RET CMusicInfoScanner::ScanTags(const CFileItemList& items, CFileItemList&
return INFO_ADDED;
}
+static bool SortSongsByTrack(const CSong& song, const CSong& song2)
+{
+ return song.iTrack < song2.iTrack;
+}
+
void CMusicInfoScanner::FileItemsToAlbums(CFileItemList& items, VECALBUMS& albums, MAPSONGS* songsMap /* = NULL */)
{
+ /*
+ * Step 1: Convert the FileItems into Songs.
+ * If they're MB tagged, create albums directly from the FileItems.
+ * If they're non-MB tagged, index them by album name ready for step 2.
+ */
+ map<string, VECSONGS> songsByAlbumNames;
for (int i = 0; i < items.Size(); ++i)
{
- CFileItemPtr pItem = items[i];
- CMusicInfoTag& tag = *pItem->GetMusicInfoTag();
- CSong song(*pItem);
+ CMusicInfoTag& tag = *items[i]->GetMusicInfoTag();
+ CSong song(*items[i]);
// keep the db-only fields intact on rescan...
if (songsMap != NULL)
{
- MAPSONGS::iterator it = songsMap->find(pItem->GetPath());
+ MAPSONGS::iterator it = songsMap->find(items[i]->GetPath());
if (it != songsMap->end())
{
song.iTimesPlayed = it->second.iTimesPlayed;
@@ -514,61 +524,143 @@ void CMusicInfoScanner::FileItemsToAlbums(CFileItemList& items, VECALBUMS& album
}
}
- if (!tag.GetMusicBrainzArtistID().empty())
+ if (!tag.GetMusicBrainzAlbumID().empty())
{
- for (vector<string>::const_iterator it = tag.GetMusicBrainzArtistID().begin(); it != tag.GetMusicBrainzArtistID().end(); ++it)
+ VECALBUMS::iterator it;
+ for (it = albums.begin(); it != albums.end(); ++it)
+ if (it->strMusicBrainzAlbumID.Equals(tag.GetMusicBrainzAlbumID()))
+ break;
+
+ if (it == albums.end())
{
- CStdString strJoinPhrase = (it == --tag.GetMusicBrainzArtistID().end() ? "" : g_advancedSettings.m_musicItemSeparator);
- CArtistCredit mbartist(tag.GetArtist().empty() ? "" : tag.GetArtist()[0], *it, strJoinPhrase);
- song.artistCredits.push_back(mbartist);
+ CAlbum album(*items[i]);
+ album.songs.push_back(song);
+ albums.push_back(album);
}
- song.artist = tag.GetArtist();
+ else
+ it->songs.push_back(song);
}
else
+ songsByAlbumNames[tag.GetAlbum()].push_back(song);
+ }
+
+ /*
+ Step 2: Split into unique albums based on album name and album artist
+ In the case where the album artist is unknown, we use the primary artist
+ (i.e. first artist from each song).
+ */
+ for (map<string, VECSONGS>::iterator songsByAlbumName = songsByAlbumNames.begin(); songsByAlbumName != songsByAlbumNames.end(); ++songsByAlbumName)
+ {
+ VECSONGS &songs = songsByAlbumName->second;
+ // sort the songs by tracknumber to identify duplicate track numbers
+ sort(songs.begin(), songs.end(), SortSongsByTrack);
+
+ // map the songs to their primary artists
+ bool tracksOverlap = false;
+ bool hasAlbumArtist = false;
+ bool isCompilation = true;
+
+ map<string, vector<CSong *> > artists;
+ for (VECSONGS::iterator song = songs.begin(); song != songs.end(); ++song)
{
- for (vector<string>::const_iterator it = tag.GetArtist().begin(); it != tag.GetArtist().end(); ++it)
+ // test for song overlap
+ if (song != songs.begin() && song->iTrack == (song - 1)->iTrack)
+ tracksOverlap = true;
+
+ if (!song->bCompilation)
+ isCompilation = false;
+
+ // get primary artist
+ string primary;
+ if (!song->albumArtist.empty())
{
- CStdString strJoinPhrase = (it == --tag.GetArtist().end() ? "" : g_advancedSettings.m_musicItemSeparator);
- CArtistCredit nonmbartist(*it, strJoinPhrase);
- song.artistCredits.push_back(nonmbartist);
+ primary = song->albumArtist[0];
+ hasAlbumArtist = true;
}
- song.artist = tag.GetArtist();
+ else if (!song->artist.empty())
+ primary = song->artist[0];
+
+ // add to the artist map
+ artists[primary].push_back(&(*song));
+ }
+
+ /*
+ We have a compilation if
+ 1. album name is non-empty AND
+ 2a. no tracks overlap OR
+ 2b. all tracks are marked as part of compilation AND
+ 3a. a unique primary artist is specified as "various" or "various artists" OR
+ 3b. we have at least two primary artists and no album artist specified.
+ */
+ bool compilation = !songsByAlbumName->first.empty() && (isCompilation || !tracksOverlap); // 1+2b+2a
+ if (artists.size() == 1)
+ {
+ string artist = artists.begin()->first; StringUtils::ToLower(artist);
+ if (!StringUtils::EqualsNoCase(artist, "various") &&
+ !StringUtils::EqualsNoCase(artist, "various artists")) // 3a
+ compilation = false;
}
+ else if (hasAlbumArtist) // 3b
+ compilation = false;
- bool found = false;
- for (VECALBUMS::iterator it = albums.begin(); it != albums.end(); ++it)
+ if (compilation)
{
- if (it->strAlbum == tag.GetAlbum() && it->strMusicBrainzAlbumID == tag.GetMusicBrainzAlbumID())
+ CLog::Log(LOGDEBUG, "Album '%s' is a compilation as there's no overlapping tracks and %s", songsByAlbumName->first.c_str(), hasAlbumArtist ? "the album artist is 'Various'" : "there is more than one unique artist");
+ artists.clear();
+ std::string various = g_localizeStrings.Get(340); // Various Artists
+ vector<string> va; va.push_back(various);
+ for (VECSONGS::iterator song = songs.begin(); song != songs.end(); ++song)
{
- it->songs.push_back(song);
- found = true;
+ song->albumArtist = va;
+ artists[various].push_back(&(*song));
}
}
- if (!found)
+
+ /*
+ Step 3: Find the common albumartist for each song and assign
+ albumartist to those tracks that don't have it set.
+ */
+ for (map<string, vector<CSong *> >::iterator j = artists.begin(); j != artists.end(); ++j)
{
- CAlbum album(*pItem.get());
- if (!tag.GetMusicBrainzAlbumArtistID().empty())
+ // find the common artist for these songs
+ vector<CSong *> &artistSongs = j->second;
+ vector<string> common = artistSongs.front()->albumArtist.empty() ? artistSongs.front()->artist : artistSongs.front()->albumArtist;
+ for (vector<CSong *>::iterator k = artistSongs.begin() + 1; k != artistSongs.end(); ++k)
{
- for (vector<string>::const_iterator it = tag.GetMusicBrainzAlbumArtistID().begin(); it != tag.GetMusicBrainzAlbumArtistID().end(); ++it)
+ unsigned int match = 0;
+ vector<string> &compare = (*k)->albumArtist.empty() ? (*k)->artist : (*k)->albumArtist;
+ for (; match < common.size() && match < compare.size(); match++)
{
- // Picard always stored the display artist string in the first artist slot, no need to split it
- CStdString strJoinPhrase = (it == --tag.GetMusicBrainzAlbumArtistID().end() ? "" : g_advancedSettings.m_musicItemSeparator);
- CArtistCredit mbartist(tag.GetAlbumArtist().empty() ? "" : tag.GetAlbumArtist()[0], *it, strJoinPhrase);
- album.artistCredits.push_back(mbartist);
+ if (compare[match] != common[match])
+ break;
}
- album.artist = tag.GetAlbumArtist();
+ common.erase(common.begin() + match, common.end());
}
- else
+
+ /*
+ Step 4: Assign the album artist for each song that doesn't have it set
+ and add to the album vector
+ */
+ CAlbum album;
+ album.strAlbum = songsByAlbumName->first;
+ album.artist = common;
+ for (vector<string>::iterator it = common.begin(); it != common.end(); ++it)
{
- for (vector<string>::const_iterator it = tag.GetAlbumArtist().begin(); it != tag.GetAlbumArtist().end(); ++it)
- {
- CStdString strJoinPhrase = (it == --tag.GetAlbumArtist().end() ? "" : g_advancedSettings.m_musicItemSeparator);
- CArtistCredit nonmbartist(*it, strJoinPhrase);
- album.artistCredits.push_back(nonmbartist);
- }
- album.artist = tag.GetAlbumArtist();
+ CStdString strJoinPhrase = (it == --common.end() ? "" : g_advancedSettings.m_musicItemSeparator);
+ CArtistCredit artistCredit(*it, strJoinPhrase);
+ album.artistCredits.push_back(artistCredit);
+ }
+ album.bCompilation = compilation;
+ for (vector<CSong *>::iterator k = artistSongs.begin(); k != artistSongs.end(); ++k)
+ {
+ if ((*k)->albumArtist.empty())
+ (*k)->albumArtist = common;
+ // TODO: in future we may wish to union up the genres, for now we assume they're the same
+ album.genre = (*k)->genre;
+ // in addition, we may want to use year as discriminating for albums
+ album.iYear = (*k)->iYear;
+ album.songs.push_back(**k);
}
- album.songs.push_back(song);
albums.push_back(album);
}
}
@@ -588,8 +680,6 @@ int CMusicInfoScanner::RetrieveMusicInfo(const CStdString& strDirectory, CFileIt
VECALBUMS albums;
FileItemsToAlbums(scannedItems, albums, &songsMap);
- if (!(m_flags & SCAN_ONLINE))
- FixupAlbums(albums);
FindArtForAlbums(albums, items.GetPath());
int numAdded = 0;
@@ -805,140 +895,6 @@ int CMusicInfoScanner::RetrieveMusicInfo(const CStdString& strDirectory, CFileIt
return numAdded;
}
-static bool SortSongsByTrack(const CSong& song, const CSong& song2)
-{
- return song.iTrack < song2.iTrack;
-}
-
-void CMusicInfoScanner::FixupAlbums(VECALBUMS &albums)
-{
- /*
- Step 2: Split into unique albums based on album name and album artist
- In the case where the album artist is unknown, we use the primary artist
- (i.e. first artist from each song).
- */
- for (VECALBUMS::iterator album = albums.begin(); album != albums.end(); ++album)
- {
- /*
- * If we have a valid MusicBrainz tag for the album, assume everything
- * is okay with our tags, as Picard should set everything up correctly.
- */
- if (!album->strMusicBrainzAlbumID.IsEmpty())
- continue;
-
- VECSONGS &songs = album->songs;
- // sort the songs by tracknumber to identify duplicate track numbers
- sort(songs.begin(), songs.end(), SortSongsByTrack);
-
- // map the songs to their primary artists
- bool tracksOverlap = false;
- bool hasAlbumArtist = false;
- bool isCompilation = true;
-
- map<string, vector<CSong *> > artists;
- for (VECSONGS::iterator song = songs.begin(); song != songs.end(); ++song)
- {
- // test for song overlap
- if (song != songs.begin() && song->iTrack == (song - 1)->iTrack)
- tracksOverlap = true;
-
- if (!song->bCompilation)
- isCompilation = false;
-
- // get primary artist
- string primary;
- if (!song->albumArtist.empty())
- {
- primary = song->albumArtist[0];
- hasAlbumArtist = true;
- }
- else if (!song->artist.empty())
- primary = song->artist[0];
-
- // add to the artist map
- artists[primary].push_back(&(*song));
- }
-
- /*
- We have a compilation if
- 1. album name is non-empty AND
- 2a. no tracks overlap OR
- 2b. all tracks are marked as part of compilation AND
- 3a. a unique primary artist is specified as "various" or "various artists" OR
- 3b. we have at least two primary artists and no album artist specified.
- */
- bool compilation = !album->strAlbum.empty() && (isCompilation || !tracksOverlap); // 1+2b+2a
- if (artists.size() == 1)
- {
- string artist = artists.begin()->first; StringUtils::ToLower(artist);
- if (!StringUtils::EqualsNoCase(artist, "various") &&
- !StringUtils::EqualsNoCase(artist, "various artists")) // 3a
- compilation = false;
- }
- else if (hasAlbumArtist) // 3b
- compilation = false;
-
- if (compilation)
- {
- CLog::Log(LOGDEBUG, "Album '%s' is a compilation as there's no overlapping tracks and %s", album->strAlbum.c_str(), hasAlbumArtist ? "the album artist is 'Various'" : "there is more than one unique artist");
- artists.clear();
- std::string various = g_localizeStrings.Get(340); // Various Artists
- vector<string> va; va.push_back(various);
- for (VECSONGS::iterator song = songs.begin(); song != songs.end(); ++song)
- {
- song->albumArtist = va;
- artists[various].push_back(&(*song));
- }
- }
-
- /*
- Step 3: Find the common albumartist for each song and assign
- albumartist to those tracks that don't have it set.
- */
- for (map<string, vector<CSong *> >::iterator j = artists.begin(); j != artists.end(); ++j)
- {
- // find the common artist for these songs
- vector<CSong *> &artistSongs = j->second;
- vector<string> common = artistSongs.front()->albumArtist.empty() ? artistSongs.front()->artist : artistSongs.front()->albumArtist;
- for (vector<CSong *>::iterator k = artistSongs.begin() + 1; k != artistSongs.end(); ++k)
- {
- unsigned int match = 0;
- vector<string> &compare = (*k)->albumArtist.empty() ? (*k)->artist : (*k)->albumArtist;
- for (; match < common.size() && match < compare.size(); match++)
- {
- if (compare[match] != common[match])
- break;
- }
- common.erase(common.begin() + match, common.end());
- }
-
- /*
- Step 4: Assign the album artist for each song that doesn't have it set
- and add to the album vector
- */
- album->artistCredits.clear();
- for (vector<string>::iterator it = common.begin(); it != common.end(); ++it)
- {
- CStdString strJoinPhrase = (it == --common.end() ? "" : g_advancedSettings.m_musicItemSeparator);
- CArtistCredit artistCredit(*it, strJoinPhrase);
- album->artistCredits.push_back(artistCredit);
- }
- album->bCompilation = compilation;
- for (vector<CSong *>::iterator k = artistSongs.begin(); k != artistSongs.end(); ++k)
- {
- if ((*k)->albumArtist.empty())
- (*k)->albumArtist = common;
- // TODO: in future we may wish to union up the genres, for now we assume they're the same
- if (album->genre.empty())
- album->genre = (*k)->genre;
- // in addition, we may want to use year as discriminating for albums
- if (album->iYear == 0)
- album->iYear = (*k)->iYear;
- }
- }
- }
-}
-
void CMusicInfoScanner::FindArtForAlbums(VECALBUMS &albums, const CStdString &path)
{
/*
Please sign in to comment.
Something went wrong with that request. Please try again.