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.

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

Fix musicbrainz resolution with python based scrapers #11632

Merged
merged 2 commits into from Feb 14, 2017

Conversation

@notspiff
Copy link
Contributor

commented Feb 7, 2017

This fixes mbrainz resolution using python based scrapers. Previously it tried to load the py file as xml, then crashed and burned.

@ronie

@ronie

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

thx! works as expected.

i do have some additional questions on the scraping process:
http://forum.kodi.tv/showthread.php?tid=306218
^ ping @DaveTBlake as well

@ronie

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

jenkins build this please

@notspiff is that unrelated commit supposed to go in as well?

@notspiff

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2017

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

jenkins build this please

@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Feb 14, 2017
@MartijnKaijser MartijnKaijser merged commit 7262e71 into xbmc:master Feb 14, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins4kodi You did a great job. Have a cookie.
Details
@DaveTBlake

This comment has been minimized.

Copy link
Member

commented Jun 8, 2017

Late in the day I know but the CScraper::ResolveIDToUrl change seems an odd thing to do.

For the XML music scrapers the "ResolveIDToUrl" function called by CScraper::ResolveIDToUrl is used to create the URL for that artist or album mbid, it gets put into the scraper list of albums or artists, and is subsequently used to get the details. It does not make any server requests, or validate the mbid in any way, it just does some string manipulation.

The Python music scrapers "getdetails" action, called from PythonDetails, is simply passed the mbid. So why need the Python scraper to have a "resolveid" action to put a fake album or artist into the list so that a count check avoids lookup by name etc. ?

It makes more sense to me to change CMusicInfoScanner::DownloadAlbumInfo and DownloadArtistInfo to check preferredScraper->IsPython() and act accordingly. Or am I missing something?

@akva2

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

as i said in the description - i have gone through stupid hoops to operate within the current scraper system. it does not make sense, it's just to map to the old.

@DaveTBlake

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

Fair enough :)

I have also discovered that XML music scrapers "ResolveIDToUrl" is not just string parsing. It takes the Musicbrainz release id that Kodi has from music file tags (strMusicBrainzAlbumID in the album table), and looks up the Musicbrainz release group id that is needed to look up the album in Fanart.TV and TADB. Python scraper needs to do the same.

EDIT: Correction, no it doesn't - the regexp parsing of xml is so darn hard to follow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.