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.

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

Feature/lyrics #1123

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Feature/lyrics #1123

wants to merge 7 commits into from

Conversation

Naapperas
Copy link

@Naapperas Naapperas commented Apr 15, 2023

This PR adds a new screen for the user to see the currently playing track's lyrics and optionally save them to their system.
Closes #1076.

This behavior should be configurable by the user, i.e.:

  • the location for saving lyrics should be configurable;
  • the users preferred fetcher(s) should be configurable. For instance, they may want to only use MusixMatch or they may want to have several options available, with the next fetcher being used in case the previous is unavailable/invalid

Missing features:

  • Lyric fetching (need to think about how to correctly abstract the fetching mechanism to use several implementations)
    • Although this is currently working, there are formatting issues which I believe are easy to fix. Only tested for a single API, which does not contain everything.
  • Lyric caching (for performance reasons, re-fetching the same lyrics over and over is not ideal)
  • Configurable Lyrics fetcher
  • Lyrics page update (no access to music change event, at least none I could find, need to "manually" update, aka re-enter the view)
  • Asynchronous lyrics fetching (I looked into Cursive Async View but could not get it to work, still need to investigate further)

This is a follow up on #1108

We are subject to the availability of that API. I still need to investigate whether this breaks TOS.
Currently needing to handle line-feeds (and carriage returns) when rendering text in cursive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add time-synced lyrics tab
1 participant