Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Lyrics: Refactor Genius, Google backends, and consolidate common functionality #5474

Merged
merged 25 commits into from
Jan 27, 2025
Merged

Lyrics: Refactor Genius, Google backends, and consolidate common functionality #5474

merged 25 commits into from
Jan 27, 2025

Conversation

snejus
Copy link
Member

@snejus snejus commented Oct 19, 2024

Description

I've made several updates to the lyrics plugin to enhance its functionality and address a few bugs. Here's a quick overview of what has changed:

Bug Fixes

Plugin Enhancements

  • Session Management: Introduced a TimeoutSession to enable connection pooling and maintain consistent configuration across requests.
  • Error Handling: Centralized error handling logic in a new RequestsHandler class, which includes methods for retrieving either HTML text or JSON data.
  • Logging: Added methods to ensure the backend name is included in log messages.

Configuration Changes

  • Added a new dist_thresh field to the configuration, allowing users to control the maximum tolerable mismatch between the artist and title of the lyrics search result and their item. Interestingly, this field was previously available (though undocumented) and used in the Tekstowo backend. Now, this threshold has also been applied to Genius and Google search logic.

Backend Updates

  • All backends that perform searches now validate each result against the configured dist_thresh.

Genius

  • Removed the need to scrape HTML tags for lyrics; instead, lyrics are now parsed from the JSON data embedded in the HTML. This change should reduce our vulnerability to Genius' frequent alterations in their HTML structure.
  • Documented the structure of their search JSON data.

Google

  • Typed the response data returned by the Google Custom Search API.
  • Excluded certain pages under https://letras.mus.br that do not contain lyrics.
  • Excluded all results from MusiXmatch, as we cannot access their pages.
  • Improved parsing of URL titles (used for matching item/lyrics artist/title):
    • Handled results from long search queries where URL titles are truncated with an ellipsis.
    • Enhanced URL title cleanup logic.
    • Added functionality to determine (or rather, guess) not only the track title but also the artist from the URL title.
  • Similar to Fix lrclib lyrics #5406, search results are now compared to the original item and sorted by distance. Results exceeding the configured dist_thresh value are discarded. The previous functionality simply selected the first result containing the track's title in its URL, which often led to returning lyrics for the wrong artist, particularly for short track titles.
  • Since we now fetch lyrics confidently, redundant checks for valid lyrics and credits cleanup have been removed.

HTML Cleanup

  • Organized regex patterns into a new Html class.
  • Adjusted patterns to ensure new lines between blocks of lyrics text scraped from letras.mus.br and musica.com.
  • Modified patterns to scrape missing lyrics text on paroles.net and lacoccinelle.net. See the diff in test/plugins/lyrics_page.py.

To Do

  • Documentation: If you've added a new command-line flag, please find the appropriate page under docs/ to describe it.
  • Changelog: Add an entry to docs/changelog.rst at the bottom of one of the lists near the top of the document.
  • Tests: While tests are highly encouraged, they are not strictly required.

Note: If you find this PR too large to manage, I am happy to split it up!

@snejus snejus self-assigned this Oct 19, 2024
@snejus snejus force-pushed the lyrics-general-improvements branch 5 times, most recently from 13b52ec to fed32d9 Compare October 20, 2024 17:05
@snejus snejus force-pushed the fix-lrclib-lyrics branch from e914d59 to 73e4a5a Compare October 23, 2024 04:01
@snejus snejus force-pushed the lyrics-general-improvements branch 5 times, most recently from 1497893 to cb559f0 Compare October 26, 2024 16:32
@snejus snejus force-pushed the fix-lrclib-lyrics branch from 8d6762f to 16188fa Compare October 30, 2024 19:23
@snejus snejus force-pushed the lyrics-general-improvements branch from cb559f0 to 95c954a Compare October 30, 2024 19:24
@snejus snejus mentioned this pull request Nov 22, 2024
Closed
3 tasks
@snejus snejus linked an issue Nov 22, 2024 that may be closed by this pull request
lyrics: In Genius backend, tolerate artist disambiguation markers #4791
Closed
@snejus snejus force-pushed the lyrics-general-improvements branch from 95c954a to ebf136f Compare November 22, 2024 07:45
@snejus snejus force-pushed the fix-lrclib-lyrics branch from 79b2af5 to b879e9c Compare December 7, 2024 16:57
@snejus snejus force-pushed the lyrics-general-improvements branch from ebf136f to 09c0b9f Compare December 7, 2024 16:59
@edgars-supe edgars-supe mentioned this pull request Dec 7, 2024
Merged
3 tasks
@snejus snejus force-pushed the fix-lrclib-lyrics branch from b879e9c to 645c12e Compare January 12, 2025 22:39
@snejus snejus force-pushed the lyrics-general-improvements branch from 09c0b9f to 83c125c Compare January 12, 2025 22:39
@snejus snejus force-pushed the fix-lrclib-lyrics branch from 645c12e to 121ae5f Compare January 12, 2025 23:12
@snejus snejus force-pushed the lyrics-general-improvements branch from 83c125c to 4531297 Compare January 12, 2025 23:12
@snejus snejus force-pushed the fix-lrclib-lyrics branch from 121ae5f to 788a1d7 Compare January 13, 2025 22:55
snejus added 8 commits January 27, 2025 08:50
@snejus
Include class name in the log messages
cb29605
@snejus
Leave a single chef in the kitchen
7c2fb31
@snejus
Do not try to strip cruft from the parsed lyrics text.
dd9f178 
Having removed it I fuond that only the Genius lyrics changed: it had en
extra new line. Thus I defined a function 'collapse_newlines' which now
gets called for the Genius lyrics.
@snejus
Use a single slug implementation
f94d276 
Tidy up 'Google.is_page_candidate' method and remove 'Google.sluggify'
method which was a duplicate of 'slug'.

Since 'GeniusFetchTest' only tested whether the artist name is cleaned
up (the rest of the functionality is patched), remove it and move its
test cases to the 'test_slug' test.
@snejus
lyrics: Add symbols for better visual feedback in the logs
8bdc2c6
@snejus
lyrics: Do not write item unless lyrics have changed
8a1ce27
@snejus
Replace custom unescape implementation by html.unescape
55b7824
@snejus
Remove extract_text_between
54fc67b
@snejus snejus force-pushed the lyrics-general-improvements branch 4 times, most recently from 7f51480 to 50ac6ca Compare January 27, 2025 10:40
@snejus snejus mentioned this pull request Jan 27, 2025
Closed
3 tasks
@snejus snejus linked an issue Jan 27, 2025 that may be closed by this pull request
Beets import crash with lyrics plugin for file w/o lyrics #5583
Closed
snejus added 13 commits January 27, 2025 10:56
@snejus
Genius: refactor and simplify
745c5eb
@snejus
Unite Genius, Tekstowo and Google backends under the same interface
12c5eaa
@snejus
Google: Refactor and improve
c5c4138 
* Type the response data that Google Custom Search API return.
* Exclude some 'letras.mus.br' pages that do not contain lyric.
* Exclude results from Musixmatch as we cannot access their pages.
* Improve parsing of the URL title:
  - Handle long URL titles that get truncated (end with ellipsis) for
    long searches
  - Remove domains starting with 'www'
  - Parse the title AND the artist. Previously this would only parse the
    title, and fetch lyrics even when the artist did not match.
* Remove now redundant credits cleanup and checks for valid lyrics.
@snejus
Create Html class for cleaning up the html text
7055464 
Additionally, improve HTML pre-processing:

* Ensure a new line between blocks of lyrics text from letras.mus.br.
* Parse a missing last block of lyrics text from lacocinelle.net.
* Parse a missing last block of lyrics text from paroles.net.
* Fix encoding issues with AZLyrics by setting response encoding to
  None, allowing `requests` to handle it.
@snejus
Google: prioritise Songlyrics and AZlyrics sources
07d372c
@snejus
Google: make sure we do not return the captcha text
b2402b1 
If we get caught by Cloudfare, it forwards our request somewhere else
and returns some validation text response. To make sure that this text
does not get assumed for lyrics, we can disable redirects for the Google
backend, check the response code and raise if there's a redirect
attempt. This source will then be skipped and the backend continues with
the next one.
@snejus
Remove dependency existence checks
04054ca 
I think we can make our life easier by removing these checks assuming
that users follow the instructions in the docs.
@snejus
Tidy up handling of backends
bdc564a
@snejus
Append source to the lyrics
734bcc2
@snejus
Xfail Songlyrics source
858c135
@snejus
Google: add support for dainuzodziai.lt
39c479f
@snejus
Do not search for Various Artists, split titles by ' / '
7389f24
@snejus
Bring back Tekstowo search
dab9a0d 
It was my mistake to remove search earlier - I found that in many cases
it works fine.
@snejus snejus force-pushed the lyrics-general-improvements branch from 50ac6ca to dab9a0d Compare January 27, 2025 10:57
@snejus snejus merged commit a1c0ebd into master Jan 27, 2025
13 checks passed
@snejus snejus deleted the lyrics-general-improvements branch January 27, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bal-e bal-e Awaiting requested review from bal-e

@jackwilsdon jackwilsdon Awaiting requested review from jackwilsdon

@RollingStar RollingStar Awaiting requested review from RollingStar

@Serene-Arc Serene-Arc Awaiting requested review from Serene-Arc

Assignees

@snejus snejus

Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@snejus