Skip to content
This repository

MusicBrainz support #1213

Closed
wants to merge 18 commits into from

4 participants

night199uk jmarshallnz fiveisalive macmenot
night199uk
Collaborator

This is the rest of the MusicBrainz support for XBMC. It's pretty complete now I feel. It has hard-coded the universal music scraper in there right now, but I need to demise that - I'm working on a small restructure & tidy up to do that. But basically this shouldn't change besides for your feedback now.

It seems to show up the TagLib changes in this stream on GitHub as well. Not sure how I can remove those from this stream.

jmarshallnz
Owner

I notice that artist name is set to the musicbrainz id when not scraped yet - perhaps it should instead be set to the artist info from the tag so that if/when scraping fails, at least we have something useful in the UI?

and others added some commits July 03, 2012
night199uk [musicdb] Demise CSongMap and just use an std::map 0eac69a
night199uk [musicdb] Make the karaoke table use a generated primary key for cons…
…istency with other tables - seed it with an initial index value to maintain current behaviour
b6f4065
night199uk [musicdb] make the song path index unique on path + filename a54ee88
night199uk [musicdb] Modify album/artist getters to use int instead of long dcebded
night199uk [musicdb] Add UpdateArtist function 4dbfb80
night199uk [musicdb] Add HasArtistInfo function and use it to detect artistinfo,…
… make GetArtistInfo use the views
51f8c54
night199uk [musicscanner] Separate tag scanning into a separate function 7ede3d5
night199uk [musicgui] Simplify the functions to show & find artist/album data add311a
night199uk [musicloader] Reroll the MusicLoader to remove dependencies on InfoSc…
…anner
7761a6c
night199uk [musicscanner] cosmetic: shuffle some code to a place where it makes …
…more sense
d2f6df1
[musicdb] musicdb restructure for musicbrainz artist IDs - remove art…
…ist/album ID from CSong and place into artist/album
5d87a9d
night199uk [advancedsettings] Add an advanced setting to use MusicBrainz tags in…
… the library, currently default false
5f38417
night199uk [musicscanner+db] reroll the infoscanner & music file addition to sto…
…p CSong being a copy of CMusicInfoTag
932444e
night199uk [tags] Make MusicBrainzArtist & AlbumIDs a vector of strings 8d3bb51
night199uk [musicdb] Change the way we add artists to use MusicBrainzIDs f9c5d00
night199uk [musicdb] Add MusicBrainz IDs when creating an album e8384d3
night199uk [musicscanner] Change the way we find & download information to take …
…a full CAlbum or CArtist, so we can pass through all details to the scrapers.
ddc1499
night199uk [musicbrainz] Update the Scanner & Scrapers to support ID resolution …
…for MusicBrainzIDs
ad4ec1f
night199uk
Collaborator

@jmarshallnz can you take another swipe at reviewing this please? I fixed all of the stuff I think you posted in your previously detailed commits. i'm not happy 100% with the way this deal with embedded artwork now though, but it seems to work... appreciate your thoughts.

jmarshallnz

You can drop the GetArtistInfo() by the looks

jmarshallnz

Move this inside the class perhaps?

jmarshallnz

You don't need the Get, right?

jmarshallnz

For db items, I don't think you need to do anything, as the CMusicThumbLoader should run on them anyway at the appropriate time? Not sure this happens in the now playing window (GUIWindowMusicPlaylist.cpp) as that one is an odd one in that tag reading is done in the bg.

For non-db items, this isn't enough. The whole point of calling the scanner, is that if we have no information from the database, the only way to accurately set the art is to check for embedded art, and then assign the folder art if all the songs are from the same album. Thus the calls to the infoscanner helpers.

jmarshallnz

Why allow the latter, invalid XML?

fiveisalive

This used to be in the October milestone, I guess it's been removed? Does this mean we'll have to wait until the next major release to get this full MusicBrainz support in XBMC?

jmarshallnz
Owner

Correct.

night199uk
Collaborator
fiveisalive

So could "next release" be a point release 12.1, or would it have to wait until the next "major" release like 13.0?

jmarshallnz
Owner

Any releases in the 12.x line are bugfixes only, so you'll need to wait until 13.x if you want it in a final (else, I'm sure it'll be merged as soon as Frodo is out the door).

fiveisalive

I was afraid of that. Bummer that it didn't make it in to Frodo and won't be in any 12.x releases. Hopefully @night199uk will be willing to backport this patch to 12.x so I people will to build their own version can test out the MusicBrainz support on the same Frodo release.

macmenot

is it possible to get this rebased and re-evaluated for a coming merge window?

night199uk
Collaborator

I will start taking a look at this again soon - it stalled while we had feature freeze for Frodo and I've been incredibly tied up with real-life work outside XBMC for the last few months. Hopefully the next month or two should give me some more time to pick this up again. Its good to know there is support for the MusicBrainz stuff :-)

fiveisalive

@night199uk thanks for keeping this alive! it will be very useful when it gets in. looking forward ultimately to true disambiguation of identically named artists

night199uk
Collaborator

See the newer PR.

night199uk night199uk closed this April 23, 2013
fiveisalive

I assume that the newer PR is #2626 ?

night199uk
Collaborator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 18 unique commits by 2 authors.

Sep 21, 2012
night199uk [musicdb] Demise CSongMap and just use an std::map 0eac69a
night199uk [musicdb] Make the karaoke table use a generated primary key for cons…
…istency with other tables - seed it with an initial index value to maintain current behaviour
b6f4065
night199uk [musicdb] make the song path index unique on path + filename a54ee88
night199uk [musicdb] Modify album/artist getters to use int instead of long dcebded
night199uk [musicdb] Add UpdateArtist function 4dbfb80
night199uk [musicdb] Add HasArtistInfo function and use it to detect artistinfo,…
… make GetArtistInfo use the views
51f8c54
night199uk [musicscanner] Separate tag scanning into a separate function 7ede3d5
night199uk [musicgui] Simplify the functions to show & find artist/album data add311a
night199uk [musicloader] Reroll the MusicLoader to remove dependencies on InfoSc…
…anner
7761a6c
night199uk [musicscanner] cosmetic: shuffle some code to a place where it makes …
…more sense
d2f6df1
[musicdb] musicdb restructure for musicbrainz artist IDs - remove art…
…ist/album ID from CSong and place into artist/album
5d87a9d
night199uk [advancedsettings] Add an advanced setting to use MusicBrainz tags in…
… the library, currently default false
5f38417
night199uk [musicscanner+db] reroll the infoscanner & music file addition to sto…
…p CSong being a copy of CMusicInfoTag
932444e
night199uk [tags] Make MusicBrainzArtist & AlbumIDs a vector of strings 8d3bb51
night199uk [musicdb] Change the way we add artists to use MusicBrainzIDs f9c5d00
night199uk [musicdb] Add MusicBrainz IDs when creating an album e8384d3
night199uk [musicscanner] Change the way we find & download information to take …
…a full CAlbum or CArtist, so we can pass through all details to the scrapers.
ddc1499
night199uk [musicbrainz] Update the Scanner & Scrapers to support ID resolution …
…for MusicBrainzIDs
ad4ec1f
Something went wrong with that request. Please try again.