MusicBrainz & extending the MusicScanner/Scraper

sure - exartistinfo seems to serve a different purpose though. we only take the first artist from the song and create an artistinfo. we create everyone else in the exartistinfo table. if i had to hazard a guess i would say this was added on later so as not to disturb the existing artist relationship but show featuring credits. it also seems to drive the 'Don't show artists that only appear on compilations' tag at a guess?

my thought is removing 'idArtist' from Song and 'idAlbumArtist' from albums and promoting them to link tables. in the 'Mark Knopfler & Chet Atkins' example above Chet Atkins isn't a featured artist, he's a primary artist but i don't think we can support that today. for the 'don't show artists that only appear on compilations' tag my thought would be to use the 'compilation' tag in the songs. i don't think we write that to the db today but i feel we should. this is basically how musicbrainz works. iTunes also sorts its music database this way and with musicbrainz and itunes today the workflow is very natural (to me) - i know not everyone uses the same workflow though. i would re-introduce the 'strArtist' tag read from the file itself as this is the 'genuine' names of the artists as they are presented to the user, e.g. 'Mark Knopfler & Chet Atkins featuring Someone'. the individual artist tags just help to index the tune correctly to show under the relevant artists.

i think that's a side issue though - as with the discussion on github the first challenge seems to be that the we're heavily dependant on the std::vector<string> in CAlbum, etc. this means we only ever use the artist name as the key into the artist table and forego the quality 'idArtist' unique we get when we create the song. :-( I guess it's historical. i think there is some foundation work i propose to do here first to get non-unique named artists & albums to work. otherwise when we introduce MBIDs onto the tables there is a risk of associating MBIDs with the wrong artist/albums:

- in the first instance i would create accessors on CAlbum/CArtist/CSong for artist, album, etc. std::vector<string> GetArtists / SetArtists(std::vector<string> artist).
- replace everywhere in the code we do a StringUtils::JoinString(album.artist, g_musicseparator) with a call to the accessor
- change all the fields from std::vector<string> to std::vector<long> e.g. std::vector<string> artist in CAlbum

this change in itself has no effect on behaviour day 1. the accessors will deal with resolving the key to the artist string. this means some db related code in the CArtist accessors :-( an alternative would be to create an std::vector<string> GetArtistsByID(std::vector<long> artist) in MusicDatabase. I prefer the idea of field accessors personally, and we could remove the DB calls from CArtist/CAlbum later when we get to our destination.

with the accessors in place we can start to work through the functions that call the accessors - if they are internal code that could be converted to use the idArtist correctly then i'll convert them to look up the artist by the ID. if they are serialization or presentation code and genuinely need the artist name etc - then they shouldn't change but we can start to change the accessors to add disambiguation information from CArtistInfo/CAlbumInfo to the generated strings to make the string based lookups more unique.

thoughts? my approach is to take this in small easy digestable chunks that can easily be PRd and integrated. i've seen a bunch of 'redesign the world' schema threads that have been aborted and my thought is not to go that route.

CArtist/CAlbum are go betweens for the UI. You do not want any database stuff in them at all.

CArtist should need only hold a single id, right? Unless we're treating "Mark Knopfler & Chet Atkins" as a collection of artists that would somehow be represented as a single CArtist with separate ids? How does MusicBrainz work in this regard? Do we need an artist ID <-> artist ID link table as well?

Perhaps the best method, then, is for CAlbum (and CSong) to hold a vector< pair<int, string> > for artists/album artists. Using accessors to get the full string seems good to me.

Given this, we'd need to redo the db layout. The first step would be to switch exartistsong (et. al.) to be all artists for the song, rather than just the extra artists - this would simplify the logic a little on fetching songs by artist I think.

Then you can drop idArtist/idAlbumArtist from CSong/CAlbum and replace the strExtraArtists stuff with a denormalised form of the vector< pair<int, string> > for speed.

The MBID stuff needs to be in the appropriate table as well (i.e. no artist/album info for musicbrainz in the song table) - we don't need this at all in the UI imo - it should not be required to be retrieved at any point for the UI, though I have no problem with the song-specific stuff being retrieved in the song queries.

This is quite a bit of work, but I think it's the only way it's really going to work out well, and I don't think it negates any move to another db layout later on - after all, the current musicdb layout is pretty good other than this.

And yes, please keep commits small and self-contained - let's achieve it slowly but surely

Cheers,
Jonathan

I think we're agreed. :-) I have a much better patch coming through now. I went a slightly different way and created a new table album_artist rather than trying to reuse exartistinfo. I'm gradually building out new and then I will demise the old in the patch so that I keep everything working at each stage of the patch. Much smaller commits too now I have the hang of git.

Regarding album->artist this is what I found - CAlbum (today) stores artist as std::vector-string-, but I cannot find anywhere in the code where we actually really support multiple artists except the DB, XML and JSON APIs. None of the tag readers support reading multiple artist tags. So my argument is that album->artist should really be converted back an CStdString, and this would greatly simplify the code. I propose that artist credits are really just a database side things for indexing (so when I hit 'Mark Knopfler' I see all tracks where 'Mark Knopfler' contributed). Everywhere else we need to use album->artist as a string, to get the artists credits as originally written on the track. This is basically how MusicBrainz works. We store a string for the label and tag the string with the relevant artists. Here's a good example: http://musicbrainz.org/artist/e49f69da-1...dcb0e588f5

So basically everywhere in the code today where we do:
StringUtils::Join(album.artist, g_musicitemseparator) should use strAlbum (the original tag read from the file).

Everywhere we use album.artist in a vector context should use a new DB function:
std::vector_long_ GetArtistsForAlbum(idAlbum, bool includeFeatured)

Those places will be fixed to use the ID and lookup the artists by ID rather than String as they do right now.

Now I will write one master 'artist string processor' for non-MB users which deals with as many of the type of featuring credits as I can find:
'Mark Knopfler & Chet Atkins feat. Dizzee Rascal & Emmy Lou Harris' or whatever :-)

And that will be called from AddSong (and temporarily the XML & JSON APIs to maintain compatibility) to create the relevant entities in the DB. If we find MusicBrainz tags in the file we will skip doing this step and just create the artist entities with the MBIDs. The one down side of this is that we would create a bunch of artists with empty strArtist but MBIDs and then call the MB scraper later. I don't think I'm concerned about this on reflection as the scraper runs in the InfoScanner right after we pull in the file... MBIDs will be more reliable than our artist string processor, as sometimes we get a single Artist or Group that has an & in the name ('Myon & Shane 54'). Later I propose to add an advanced settings tag that will let you specify a list of known artists that have an '&' in their name to override the artist string processor for non-MusicBrainz users.

Regards MusicBrainz, 'Mark Knopfler & Chet Atkins' are two artists with two IDs, but the album contains the string 'Mark Knopfler & Chet Atkins'.

See here:
http://musicbrainz.org/artist/e49f69da-1...dcb0e588f5

I'm going to pull out the original MB PR I raised and raise a new PR - apologies for the spam. I also want to move onto a branch in my git repository and I think this is a very different request now.

Once this is done for albums I will start to look at song_artist... :-)

We completely support multiple artist tags, but typically it's done via a separator in a single tag (eg " / " is the default one). Some of these tag formats support a list of strings for these fields, though typically it's a single string that's split using a separator. We don't really utilise it in any way other than to split the artists up for the db, so your plan sounds fine.

I'm not sure what the point of the new table is, but perhaps it will become clear once I see the code

Cheers,
Jonathan

Problem with the splitting of artist is you can only use one seperator. At least supporting multiple would be better

Just chiming in from a user perspective, would these changes allow albums with multiple artists to be grouped under each individual artist? (e.g. this is how MusicBrainz's NGS works). The way I see it, using MBIDs for the artist has three primary uses from a UI/artist browsing point of view.

The first is to have a have a single listing for a primary artist (in the MusicBrainz sense), this gets around the minor variations problem, e.g. "Eno", "Brian Eno" would be listed under a single "Brian Eno", as it is on the MB site:

http://musicbrainz.org/artist/ff95eb47-4...c30f02e872

The second would to allow cross-listing of albums under multiple artists, e.g. the album "Spinner" by Brian Eno & Jah Wobble:

http://musicbrainz.org/release-group/3e5...792874ef79

is displayed under both Brian Eno and Jah Wobble on MB. It would be nice if this was mirrored in the artist hierarchy in XBMC. Of course, there should probably be an option that could switch off the artist MBID mapping, so that the user could default back to the textual representation and ignore the MBIDs for artist groupings if they wanted to opt-out.

The third is to disambiguate identically named artists, perhaps the scraper could pull the "disambiguation" field and optionally display that, e.g. MBID has 5 artists called "Lino":

http://musicbrainz.org/search?query=lino...od=indexed

Of course, in the actual album view (and lists), it would show the actual artist name as it appeared in that release. The Artist MBID would only be used for the grouping purposes (and the standardized name for the Artist MBID in the top-level list of Artists).

XBMC already supports listing an album under more than one artist, it's just that currently it's done using the strings in the (album)artist tags. So yes, using the musicbrainz ids allows better disambiguation.

(2012-06-04 00:36)jmarshall Wrote:  XBMC already supports listing an album under more than one artist, it's just that currently it's done using the strings in the (album)artist tags. So yes, using the musicbrainz ids allows better disambiguation.

Could we do the same for record label perhaps? I know it's not that important however there are quite a lot record labels that have the exact same name and are totally different from each other (different coutry and music genre)

Yes, I want to get record label in too... :-)

A quick update, a lot of work on this.
I've added proper Compilation flag support and I'm just finishing that off, I've converted the exalbumartist and exsongartist tables and updated all of the major dependencies.

This should help me to clean up some of the music database stuff and get acquainted with the codebase..

I much better idea of how I need to add the MusicBrainz IDs now.

Hey.. I think I got the first set of music database patches to the point where I'm happy with them...

These are just foundation stuff at the moment.. Would appreciate your feedback on Pull Request #97.

Basically it:
- Deprecates / Removes the ex* tables, and creates link tables for Song->Artist, Album->Artist, Song->Genre, Album->Genre
- Removes idArtist, strExtraArtist creates strArtists (you'll see all of your artists not just the first)
- Removes idGenre, strExtraGenre creates strGenres (ditto)
- Fully support the Compilation flag and uses the Compilation flag instead of detecting Various Artists releases

The commit history isn't as ordered or structured as I'd like as I was feeling my way through and I made a few mistakes in the way I ordered commits - adding the Compilation flag halfway through, and MBIDs to the tables which made it very difficult to reorder some commits. If I had to do it again it'd be more structured, but I think the end result is good. My artists table looks a lot cleaner with the Compilation flag support instead of the way Various Artists worked before.

I'm going to be running this version on my media centre with 50,000 odd music files to test it further. Need to look at some of the queries with the new table structure - GetArtistsNav() query currently takes around 2.7secs for me to run even with the indexes in place, but I think this query can be optimised to a SELECT /JOIN rather than SELECT .. WHERE idAlbum IN ... I also wonder if some of the GSoC work going on will make this less of a concern?

Let me know your thoughts. If this is good the second part will be to start fixing up some of the GUI / FileItem elements to try and ensure we always query by ID and make use of some of the new queries, rather than query by strings.

You will need to rescan your music to take advantage of the Compilations flag...

---

Pull Request 1040, even :-)