metadata/Music: upgrade to latest CU LRC lyrics grabbers

[twitham1]

(replaced by #847)

CU LRC is now used unmodified thanks to slightly modified Kodistubs mocking just the calls the grabbers need. Common CLI code moves to common/main.py reducing the grabber definitions to only 20 lines each.

There are currently 12 working grabbers that pass their tests via common/testall.py. Local embeded and file lyrics are also updated to the new layout.

The first 2 commits are the new imported code unmodified.

The last 2 commits are patches to Kodistubs and then to our code to make it work with current CU LRC.

Fixes #427

Checklist

• contribution does not duplicate one of our existing pull requests
• contribution is in a branch rebased against master
• code compiles successfully without errors
• code follows the MythTV Coding Standards
• documentation added/updated/removed where necessary
• commits are logically organised and have good commit messages

[hambre]

Thanks for doing this! I think it important that the grabbers themselves are unchanged when possible, so syncing is easier in the future. Great for accomplishing that.

[paul-h]

I'll have comeback to this when I have more time but my initial thought is I don't like having to pull in all the extra Kodi stuff that we don't really need. I would have much preferred to just sync the various LyricsFetcher classes since that is all we really need?

[twitham1]

Thanks Paul,

I found our copy and CU LRC very different and difficult to reconcile. So as @hambre mentioned, my goal was to use CU LRC unmodified so that upgrades are as easy as overwriting lib/ with the latest from CU LRC. I stumbled on Kodistubs, do-nothing function definitions that let all xbmc imports "work" but with zero code/functionality. Then it was a simple matter to simulate the few calls used by CU LRC by adding tiny code to the stubs. Now we can even use CU LRC's scrapertest.py to test all grabbers at once.

But now that this works, I could very well remove almost all of Kodistubs to just the bare minimum few we need. I thought there might be some value in leaving all stubs with minimum change in case future grabbers use more xbmc calls. You can see all CU LRC currently needs in ff98399.

I just realized we do have a make install problem though. Broken grabbers that need removed are left in place. Makefile can't remove everything and install current as that would lose any local custom grabbers the user added. Might need to keep broken ones but move them to priority 999 and exit 1 right away.

Getting the menu from Lyrics View follows a noticeable delay. I believe this is because we have to run all grabbers to find their names to build the menu. Similar delay before searching as we have to run them all to find their order. Ideally we could cache the names and priorities until a grabber file timestamp changes and this would speed up the interface.

[paul-h]

Unless something has changed all we actually need is the get_lyrics() functions and any other functions it calls which typically just means you just have to copy the entire LyricsFetcher class into our grabber. From what I recall the only other changes were to call our log function from common. I'm pretty sure non of the existing grabbers need anything else from KODI so the stubs aren't required. To my mind it is just another external dependency that we are pulling in which will have to be maintained even though we don't really need any of it.

The problem with old grabbers not being removed I'm assuming you are taking about if you do a make install it doesn't remove any old grabbers? That shouldn't be a problem for anyone who uses packages though since I think they do clean the directories before installing stuff? Not sure the best way to fix this for anyone installing from source. I think make uninstall should clean stuff up before you make install. It wouldn't bother me if we always clear the lyrics directory before installing the new ones to be honest but maybe I'm evil :). Failing that we could use a 999 priority to mean 'don't use this one' and just don't include them in the menu and ignore them when doing the lookup.

[twitham1]

Ok, I cut all unused Kodistubs leaving only the few lines we use for the CU LRC to MythTV translator. As written, we can recursive diff lib/ to preview what has changed in a new CU LRC and then simply copy it in to upgrade. This is impossible if we cut-n-paste and edit the 12 LyricsFetchers into our own files in root directory as before. Using CU LRC unmodified is easier.

That's right, I tested by moving my production directory aside and using this one in its place, worked great. Then I reverted and did "make install" instead. That's when old broken grabbers remained and lower priority ones preempted the new grabbers and their new order.

Good point that packages should be more careful to remove old files they installed that no longer exist in the next version. Those doing make install might be smart enough (I failed) to make uninstall first. I did the 999 hack but now I'm thinking remove would be better.

[paul-h]

Hope you don't mind I managed to sync the existing grabbers with the upstream ones and also add the new ones. Since I know how I created them in the first place it was easy enough to just update them. I prefer that than add a load of stuff we don't really need but would still have to maintain. It also means things like the token caching work properly and in a way consistent with elsewhere in MythTV.

I haven't done any testing in MythMusic at all. I did however use each grabbers self test option to make sure they worked.

[twitham1]

Thank you Paul!

Ideally we would have done this 2 years ago after #427 was submitted. I wonder if it was skipped since it is non-trivial for others to do the individual edits that you did. My contribution fixes that by making upgrades trivial for anyone to do. It is now in the cleaned up #847.

I'll be glad to maintain the < 100 lines I wrote in Kodistubs, but it is so simple others could do it too. Chances are it will just work with new grabbers and not need any changes. Thanks to your example, the token is stored correctly in ~/.mythtv/cache/.

[twitham1]

I cleaned #847 further to an import commit that is zero code change and then our code change in a separate commit. It turns out that we now maintain < 25% of the original lines of code, explained in my comment on that PR.