Skip to content

Conversation

@Bartkk0
Copy link

@Bartkk0 Bartkk0 commented Oct 15, 2025

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.

@nullobsi
Copy link
Owner

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:

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 nullobsi added the enhancement New feature or request label Oct 17, 2025
@Bartkk0
Copy link
Author

Bartkk0 commented Oct 17, 2025

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.

@lokke3
Copy link

lokke3 commented Oct 17, 2025

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!

@Bartkk0 Bartkk0 force-pushed the feat/additional-lyrics-providers branch from 28b6caa to 1339d18 Compare October 18, 2025 12:21
@Bartkk0
Copy link
Author

Bartkk0 commented Oct 18, 2025

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.

@lokke3
Copy link

lokke3 commented Oct 19, 2025

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.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants