Skip to content

Add support for lyrics - #519

Merged
sentriz merged 7 commits into
sentriz:masterfrom
danielepintore:master
Aug 26, 2025
Merged

Add support for lyrics#519
sentriz merged 7 commits into
sentriz:masterfrom
danielepintore:master

Conversation

@danielepintore

Copy link
Copy Markdown
Contributor

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

sentriz commented Jun 19, 2024

Copy link
Copy Markdown
Owner

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

@danielepintore

danielepintore commented Jun 19, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown

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

@danielepintore

Copy link
Copy Markdown
Contributor Author

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

  1. Clone the repo: git clone git@github.com: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
Copy Markdown

That's works. Thank you!

@BANanaD3V

BANanaD3V commented Nov 30, 2024

Copy link
Copy Markdown

Any updates? Will this get merged?

@danielepintore

Copy link
Copy Markdown
Contributor 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

@sammy-ette sammy-ette mentioned this pull request Dec 11, 2024
@xxxserxxx

Copy link
Copy Markdown
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!

Comment thread server/ctrlsubsonic/handlers_common.go Outdated
Comment thread server/ctrlsubsonic/handlers_common.go Outdated
@danielepintore

Copy link
Copy Markdown
Contributor Author

Hey @sentriz any update on this getting merged? I've just rebased to master and I think it's ready to be merged

@xxxserxxx

Copy link
Copy Markdown
Contributor

FWI I have a hard fork of stmps that has support for lyrics (and cover art, and song metadata) that depends on (the lyrics part) this feature; my fork is in AUR and Alpine testing. I'm waiting on this being merged before moving the package from Alpine testing to community -- users can use Navidrome, which has this support, but since I use gonic I'm gating on this.

sentriz added a commit that referenced this pull request Aug 26, 2025
Squashed commit of the following:

commit 897fa82
Author: sentriz <senan@senan.xyz>
Date:   Tue Aug 26 11:03:43 2025 +0100

    bump ts

commit bd356a9
Author: Daniele Pintore <daniele.pintore1@gmail.com>
Date:   Thu Jul 24 16:09:28 2025 +0200

    fix(lint): use NewError correctly

commit c0d6cb2
Author: Daniele Pintore <daniele.pintore1@gmail.com>
Date:   Thu Jul 24 15:32:30 2025 +0200

    fix(lyrics): if lrc is formatted with empty spaces remove them

commit ef4f722
Author: Daniele Pintore <daniele.pintore1@gmail.com>
Date:   Thu Jun 20 15:21:02 2024 +0200

    chore: fix tests

commit 24a9439
Author: Daniele Pintore <pintdany@gmail.com>
Date:   Wed Jun 19 17:30:34 2024 +0200

    fix typo

commit 4dfdff1
Author: Daniele Pintore <pintdany@gmail.com>
Date:   Wed Jun 19 17:29:51 2024 +0200

    Add check for getLyrics function

commit e04c6dd
Author: Daniele Pintore <pintdany@gmail.com>
Date:   Wed Jun 19 17:16:22 2024 +0200

    Add support for lyrics

commit 0a745b1
Author: Daniele Pintore <pintdany@gmail.com>
Date:   Wed Jun 19 10:29:03 2024 +0200

    Add songLyrics extension since the getLyricsBySongId method is implemented

Release-As: 0.18.0
@sentriz
sentriz merged commit c6fb8d4 into sentriz:master Aug 26, 2025
1 check passed
sentriz pushed a commit that referenced this pull request Aug 26, 2025
* Add songLyrics extension since the getLyricsBySongId method is implemented

* Add support for lyrics

* Add check for getLyrics function

* fix typo

* chore: fix tests

* fix(lyrics): if lrc is formatted with empty spaces remove them

* fix(lint): use NewError correctly

bump ts
@sentriz

sentriz commented Aug 26, 2025

Copy link
Copy Markdown
Owner

thank you @danielepintore and sorry for the delay!

lurenyang418 pushed a commit to lurenyang418/gonic that referenced this pull request Mar 23, 2026
Closes sentriz#519 to fix popup_menu attempt to subtract with overflow bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants