-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix lrclib lyrics #5406
base: lyrics-refactor-tests
Are you sure you want to change the base?
Fix lrclib lyrics #5406
Conversation
d4bed72
to
829192d
Compare
cb8929f
to
c2807f0
Compare
The build on I will address this in a separate PR and rebase this one accordingly once the fix is merged. Note: this issue popped up now because I added a new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a much better implementation now, well done. I especially like that you've managed to remove the test resources -- I wasn't expecting 2000 lines of lyrics in there.
…5407) See my comment under #5406 for context > The build on win32 is failing to install reflink because it's [only supported until Python 3.7](https://gitlab.com/rubdos/pyreflink/-/blob/master/setup.py?ref_type=heads). > > I will address this in a separate PR and rebase this one accordingly once the fix is merged. > > Note: this issue popped up now because I added a new requests-mock dependency which invalidated cached dependencies.
c2807f0
to
64c0439
Compare
dc02c94
to
622ed3c
Compare
622ed3c
to
3eb04ba
Compare
1ff3ff2
to
4d481d7
Compare
9518b5d
to
27453e3
Compare
Hey, LRCLIB author here 👋. It would be great if you could make the Also, the If you have any ideas for further improvements to LRCLIB's API, feel free to let me know! |
Hi @tranxuanthang, thanks for popping in! Absolutely, that's no problem at all. This should make most lyrics queries even speedier on our side. Now that we're on this topic, I think it may be a good idea to also add caching: for example, if we're getting lyrics for two separate files
Ideally we should only ask for @tranxuanthang thanks for a reliable and performant API! |
6300798
to
200cd19
Compare
7869c53
to
e3fd8ef
Compare
200cd19
to
9e67a56
Compare
e3fd8ef
to
84fbf93
Compare
b4d78da
to
038ca48
Compare
84fbf93
to
0466022
Compare
2fb6682
to
d02e017
Compare
2b21853
to
f9e5374
Compare
54f06f9
to
19dff1c
Compare
f9e5374
to
4af0863
Compare
19dff1c
to
e70f61b
Compare
af40165
to
e914d59
Compare
e914d59
to
73e4a5a
Compare
e70f61b
to
278279e
Compare
8d6762f
to
16188fa
Compare
278279e
to
4970dda
Compare
Adjust the base URL to perform a '/search' instead of attempting to '/get' specific lyrics where we're unlikely to find lyrics for the specific combination of album, artist, track names and the duration (see https://lrclib.net/docs). Since we receive an array of matching lyrics candidates, rank them by their duration similarity to the item's duration, and whether they contain synced lyrics.
16188fa
to
79b2af5
Compare
Fixes #5102
LRCLib lyrics backend fixes
Bug Fixes
lrclib
source. If lyrics for a specific album, artist, and title combination are not found, the plugin now searches for the artist and title and picks the most relevant result, scoring them bysources
configuration to prioritizelrclib
over other sources for faster and more reliable results.Code Improvements
fetch
method in all backends.LRCLyrics
andLRCLibItem
classes to encapsulate lyrics data and improve code structure.LRCLib
backend. These will be added to the rest of the backends in a separate PR.Tests
To Do
docs/
to describe it.)docs/changelog.rst
to the bottom of one of the lists near the top of the document.)