SwagLyrics / SwagLyrics-For-Spotify

📃 Get lyrics of currently playing Spotify song so you don't sing along with the wrong ones and embarrass yourself later. Very fast.
https://pypi.org/project/swaglyrics/
MIT License
316 stars 35 forks source link

[FEATURE] multiple lyrics sources #2682

Open martinmake opened 3 years ago

martinmake commented 3 years ago

Many songs that I like to listen to aren't avaible on Genius, but they are avaible on Musixmatch or Tekstowo.pl.

I would like you to add an ordered list of lyrics sources with more than just Genius by default (at least Musixmatch).

aadibajpai commented 3 years ago

hey! thanks for the issue.

yeah, I agree with you that genius lyrics database is somewhat lacking when it comes to non english songs.

however, using genius and not multiple providers is a deliberate design choice—SwagLyrics is able to be this fast because we don't search for the song and are opinionated. Using MusixMatch might increase availability for a few users but would decrease the overall response times for the requests it falls back on MusixMatch for. As for not using MusixMatch as the primary source of lyrics—Genius quality is simply way better than MusixMatch.

Which is why I'm not sure if this is a direction we will be going into.

With that said, I noticed you started working on the integration already at #2683, what I would encourage you to do is to use your fork instead and if it works well I'd love to list it in the README or something for users who would also prefer to have the MusixMatch option.

martinmake commented 3 years ago

Well, I would argue, that a response is better than an error message (no response), when it comes to responsiveness. I've only implemented it as a fallback, so lyrics that are available on Genius should stay really fast.

So I don't really understand why we shouldn't merge. Could you perhaps explain, where this loss of responsiveness comes from? But sure, if we wont find a common ground we could definitely just keep it as a fork and add it to README.

I might open another issue soon. It will be for fuzzy searching on Genius as a fallback to precise url generation from information from Spotify, because I've had some issues with lyrics being avaible on Genius, but only under non-standard url, this is not a problem with how I query Musixmatch (It's slower overall, but failing the fast lookup and then having to go through a slower fallback could end up feeling worse).

For Genius, there is another cool opt-in way we could get better lyrics queries and that's through their API. I assume it would also boost responsiveness. If you want, I could open an issue for it and also implement it in an opt-in non-disruptive way.

My implementation even finds and correctly outputs lyrics for songs with artist and song name being non-latin and thus it breaks one of your tests:

=================================== FAILURES ===================================
_______________________ Tests.test_that_get_lyrics_works _______________________

self = <tests.integration.Tests testMethod=test_that_get_lyrics_works>

    def test_that_get_lyrics_works(self):
        """
        Test that get_lyrics function works
        """
>       self.assertEqual(get_lyrics('果てるまで', 'ハゼ馳せる'), None)  # song and artist non-latin
E       AssertionError: '沈むように溶けてゆくように\n二人だけの空が広がる夜に\n\n「さよなら」だけだ[809 chars]していく' != None

tests/integration.py:23: AssertionError

I had no problems displaying non-latin characters in my terminal emulator and I'm sure other users can either figure out how to get them to display correctly, or they can just ignore them rather than having displayed an error message for all users. image