cantata icon indicating copy to clipboard operation
cantata copied to clipboard

Add additional lyrics provider methods

Open Bartkk0 opened this issue 4 months ago • 5 comments

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.

Bartkk0 avatar Oct 15 '25 04:10 Bartkk0

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 curl in it when Qt already has plenty of facilities to get HTML and parse JSON.

Some feedback:

  • The XML stuff will probably all be gone in the future. I would suggest you implement an LRCLibLyricsProvider class without any connection to the XML using Qt web and json support.
  • Qt has good JSON support: https://doc.qt.io/qt-6/json.html https://doc.qt.io/qt-6/qjsondocument.html#fromJson
  • And also plenty of support for doing web requests, which you have seen in the existing code: https://doc.qt.io/qt-6/qtnetwork-programming.html https://doc.qt.io/qt-6/qnetworkrequest.html

I do think LRCLib support would be great. But I will not merge this pull request as it is now, it needs some changes.

nullobsi avatar Oct 17 '25 04:10 nullobsi

Thanks for your feedback. I mostly did it that way, to fit the existing codebase, and I didn't do JSON directly because I didn't know how to fit that into XML, but if it's better to just have the provider directly in C++, I'll just do that. As for the script-based lyrics provider, I think it might be useful for some users, who might want to add a custom provider without compiling the entire project.

Bartkk0 avatar Oct 17 '25 10:10 Bartkk0

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!

lokke3 avatar Oct 17 '25 18:10 lokke3

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 letras.mus.br.

Bartkk0 avatar Oct 18 '25 14:10 Bartkk0

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) I didn't dig too deeply but to me it looked like some of the providers are completely dead, some have changed their query format (produce no results) and some are actively hostile to the query (they produce results which seem to intentionally be incorrect.) I could possibly be paranoid when it comes to the hostile results.

lokke3 avatar Oct 19 '25 14:10 lokke3

@Bartkk0, thank you for the PR and great suggestion. I will close this PR as it is superseded by #122; there can also be discussion in #94.

nullobsi avatar Dec 19 '25 04:12 nullobsi