moehmeni / syncedlyrics

Get an LRC format (synchronized) lyrics for your music
MIT License
188 stars 18 forks source link

Providers assume all search results sane, fragile #15

Closed hseg closed 9 months ago

hseg commented 9 months ago

Trying to diagnose why #14 doesn't work, it appears that besides using recent songs (more likely to be subject to licensing restrictions and thus not available in certain providers), certain providers assume all tracks returned have synced lyrics (or, more specifically, that always picking the first track from a list of matches is correct). Offending lines are https://github.com/txtsd/syncedlyrics/blob/9bd9ddcf92c01f7090927b42827aaa4c11710afe/syncedlyrics/providers/deezer.py#L49 https://github.com/txtsd/syncedlyrics/blob/9bd9ddcf92c01f7090927b42827aaa4c11710afe/syncedlyrics/providers/lrclib.py#L34 https://github.com/txtsd/syncedlyrics/blob/9bd9ddcf92c01f7090927b42827aaa4c11710afe/syncedlyrics/providers/musixmatch.py#L92 https://github.com/txtsd/syncedlyrics/blob/9bd9ddcf92c01f7090927b42827aaa4c11710afe/syncedlyrics/providers/megalobiz.py#L25

(noticed when testing against a query of ave maria -- it exists on lrclib, but the first match doesn't have synced lyrics)

moehmeni commented 9 months ago

Thanks for the notice. The CI problem is solved now since it was because of a bug in the NetEase provider which was failing.

About your concern, I suppose the provider's sorting system is enough, but in the case of wrong results could you give more examples? I could not find the mentioned song for evaluation (is it an artist or a track?)

hseg commented 9 months ago

https://en.wikipedia.org/wiki/Hail_Mary#Musical_settings It's a prayer that's been covered by many classical composers, hence should usually have a good version in the database. In this case, checked with curl https://lrclib.net/api/search?q='ave+maria' | jq '.[0] , which lists track 15003, however replacing the jq filter with map(select(.syncedLyrics)).[0] returns track 131735, which indeed does have synced lyrics.

moehmeni commented 9 months ago

Yes, you were right. I hope this commit solves the issue. Feel free to reopen in case of any problems.