Add additional lyrics provider methods#93
Conversation
|
Thank you for the pull request. A tangent: First, it seems this part of the codebase is particularly old, and I have doubts about the architecture using an XML file when it is embedded as a resource anyways and not extensible enough to be supplemented by the end-user. I would rather have each provider implemented as its own C++ class, potentially sharing some code. Moving from that, which is something that will have to be handled in a whole other way in the future, it is kludgy to implement a "script-based" lyric provider and have Some feedback:
I do think LRCLib support would be great. But I will not merge this pull request as it is now, it needs some changes. |
|
Thanks for your feedback. |
|
I just want to add that I really appreciate the effort to fix the lyrics feature. I often use it, but have found that more and more the results are incorrect or not found, so thanks for working on it! |
28b6caa to
1339d18
Compare
|
After some consideration, I wonder if it's even worth still having the original lyrics fetching code. After some testing it seems that most of the providers are either dead, the extractor fails, or the wrong part of the website is extracted, and the only source that worked was |
|
I only have three providers enabled and seem to get about 20% or less when it comes to accurate results (azlyrics.com, chartlyrics.com, lyrics.wikia.com) |
This started out as a attempt to add support for fetching lyrics from lrclib.net, but since the response is in json it would not fit the current way of fetching lyrics. As for now json support is still not present, but can be accomplished using the new command provider type, which just passes the data as arguments to a executable, and expects the lyrics to be outputted through stdout.