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

Add support for lyrics #519

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

danielepintore
Copy link

In this pull request I've added support for lyrics.

When we make a scan now we check for lyrics inside the file using taglib, once the scan process in finished we have all lyrics saved on the db so we can retrieve them faster. When we are looking for a lyrics it first checks if there is a embedded lyric, if there is we return that, otherwise it looks for a file SONG_NAME.lrc and reads the lyrics from it. If there isn't a .lrc file an empty response is provided.

I've added support for both getLyrics and getLyricsBySongId methods. I have even created a migration, but i'm not so sure if that works fine maybe give that a double check.

Let me know if there is something to change.
P.S. I have used some code from #488

@sentriz
Copy link
Owner

sentriz commented Jun 19, 2024

thanks for work here, but is this a dupe of #488 ?

@danielepintore
Copy link
Author

danielepintore commented Jun 19, 2024

Not exactly a dupe since I've added also the possibility of fetching the lyrics from the embedded tag. Since i couldn't work on that pull request I've decided to make a new one. I should have implemented all things that weren't implemented on the other pull request

@gfififid
Copy link

How can i test it? I can't understand how to build it

@danielepintore
Copy link
Author

Hi, I suggest to build a custom docker image and use that in your docker-compose.yaml.

  1. Clone the repo: git clone [email protected]:danielepintore/gonic.git
  2. cd gonic
  3. Build the image: docker build . -t gonic_lyrics:latest
  4. Replace in the docker-compose the image name with gonic_lyrics:latest
  5. Run the container the same way you do for the standard version docker compose up -d

These are the steps needed to build and use the image, but i have noticed that it returns an error. The problem is the one fixed by #526. Changing the Dockerfile with the changes proposed by #526 fixes the issue and let you build the image.

@gfififid
Copy link

That's works. Thank you!

@BANanaD3V
Copy link

BANanaD3V commented Nov 30, 2024

Any updates? Will this get merged?

@danielepintore
Copy link
Author

The feature works fine, I've been using it for a few months without any problems. We should wait for a response from @sentriz

@TorchedSammy TorchedSammy mentioned this pull request Dec 11, 2024
@xxxserxxx
Copy link
Contributor

For client testing, I just pushed a branch onto @spezifisch 's fork of stmps that implements showing synchronized lyrics fetched from @danielepintore 's fork.

It's functional. stmps uses mpv as the player, and gets time sync's every second, which means lyrics timings are always just a little off. I found it's better to switch lyric lines early, rather than later, so that the lyric line is shown when the vocals happen; anyway, it works.

I'd be thrilled to see @danielepintore 's fork merged; now there's one more client that can use it!

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.

5 participants