finamp icon indicating copy to clipboard operation
finamp copied to clipboard

[Feature Request] Lyric support

Open LordZeuss opened this issue 1 year ago • 20 comments

Finamp is a fantastic project. Not sure how difficult it is to implement, but to be able to read downloaded lyrics files, or somehow have them pull from an external source on the fly, would be amazing.

LordZeuss avatar Sep 26 '22 12:09 LordZeuss

Embedded lyrics display is very necessary for music library。

koollylee avatar Oct 14 '22 06:10 koollylee

Great news in this topic!!! Last month Jellyfin has merged lyric support in their official API! The feature should be available in the next releases. https://github.com/jellyfin/jellyfin/pull/8381 I've build Jellyfin from Source and the API is there!!!

This means that we could implement this feature now. Maybe I'll take a look at it, I've already forked Finamp.

However, I have no experience in Flutter yet. Could therefore also become nothing.

iPaulTech avatar Nov 15 '22 19:11 iPaulTech

Awesome! I guess combined with this new API and the fact that any files that have embedded lyrics tags can also be used to acquire lyrics, this should cover almost all cases.

The implementation could first test if there are (mp3) tags embedded with lyrics info, and if there are try to parse them with an LRC parser. If there are no tags or parsing fails, the API could be tried next.

Ideally, this should probably be done when a song is fetched for playback, or downloaded for offline use.

@iPaulTech if you need any help with the UI stuff just ask me. I'd be happy to design some mockups for you :)
But getting the lyrics data into the app somehow, even without displaying them, would already be a massive leap forward!

Chaphasilor avatar Nov 15 '22 20:11 Chaphasilor

Oh you've right... There is a download function 😄. I mostly streamt directly, because this works so good :)

I think it would be better, if the lyric comes only from the Jellyfin API. And would be saved for offline playback in the Finamp database (I think this is SQLite).

I hope i can experiment with flutter soon. Because last weekend i've had build problems... But this should be fixed in the next days. I wait for the commits :).

And for the UI, i like the implementation from the App "musicolet" (not really Opensource).

iPaulTech avatar Nov 15 '22 20:11 iPaulTech

I think it would be better, if the lyric comes only from the Jellyfin API

The potential problem with that is that is only exposes .lrc and .txt files and not any embedded tags. The embedded tags solution already works in other apps and would be great to have here as well, otherwise some people might be missing out, depening on how their media is organized :)

Chaphasilor avatar Nov 15 '22 21:11 Chaphasilor

Mh... Thats not good. But i think this should not be a "Finamp problem"... It would be better if the user would use some script for extract the lyrics to seperated files on the server side... I also do not know how this would be with transcoded streams... If the metadata are not in the transcoded files you would habe a Problem...

People should do the backend rightly and not a simple client... What is with the Web Client for Jellyfin... This would be bad if only Finamp has the lyrics from inside the .mp3 or .FLAC files...

iPaulTech avatar Nov 15 '22 21:11 iPaulTech

You're right of course, the API should ideally support embedded tags as well. I'd suggest to start with using just the API and then seeing if we can also add client-side support for embedded tags until the backend catches up.
Removing code once it's not needed anymore shouldn't be a problem.

Also, I'm curious what @jmshrv's opinion is on this 🤔

Chaphasilor avatar Nov 15 '22 21:11 Chaphasilor

This is really exciting! Awesome to see that its becoming part of Jellyfin.

just_audio currently doesn't support viewing a song's metadata tags, but it should be possible to add (although it would require me writing Objective-C 🤢) - https://github.com/ryanheise/just_audio/issues/88

Ideally, Jellyfin itself should handle this. I'd assume it's possible as Jellyfin already looks at tags to organise the library. It also looks like the code is pretty easy to extend as LRC and TXT implementations are nicely split into providers, so it'll probably be easier to add a tag provider than to add tag extraction support inside Finamp/just_audio.

As for downloads, Finamp's download system is a mess that I've been meaning to strip out for a while. I don't have the time to do it now, so I guess we can't wait for that to happen. It's currently spread out across 5 key value databases (+ the SQLite DB that flutter_downloader creates), and its a horrible blob of functions that just have to manage everything themselves with no security if anything goes wrong - I'm amazed I don't get any bug reports mentioning downloads going missing or something. Since the lyric endpoint returns its own type instead of adding it to BaseItemDto (which is a way better solution - BaseItemDto is silly), we'd have to get it separately when adding downloads. It could probably be added into DownloadedSong pretty easily, and addDownloads already needs to get other item metadata so we could do it there. I'll be happy to handle the download stuff since it's pretty impossible to understand.

As for implementing the API bit, these are the relevant files:

  • services/jellyfin_api.dart - This actually handles sending HTTP requests to and from Jellyfin. It's pretty much just a simple Chopper thing that returns JSON.
  • services/jellyfin_api_helper.dart - This is what the whole app uses to interact with Jellyfin (and by extension the offline stuff, but that's all obscured). In this, you'll call the method you make in jellyfin_api.dart and return a nice class instead of some random JSON
  • models/jellyfin_models.dart - This is where we store classes from the Jellyfin API. The Hive stuff is related to offline storage, so it's not essential for basic API stuff. I usually just hand write classes from the API documentation, which is extremely fun and not draining at all :)

jmshrv avatar Nov 16 '22 02:11 jmshrv

Ideally, Jellyfin itself should handle this. I'd assume it's possible as Jellyfin already looks at tags to organise the library. It also looks like the code is pretty easy to extend as LRC and TXT implementations are nicely split into providers, so it'll probably be easier to add a tag provider than to add tag extraction support inside Finamp/just_audio.

I added support for embedded lyrics in this PR: https://github.com/jellyfin/jellyfin/pull/9220

Works transparently for the client so just calling the one lyrics endpoint is fine.

DomiStyle avatar Feb 02 '23 22:02 DomiStyle

That's awesome, saves me a lot of work. Thanks!

jmshrv avatar Feb 03 '23 02:02 jmshrv

Thanks @DomiStyle! Just wondering, does this PR work for both downloaded and streamed songs? And pardon my lack of understanding—to get the lyrics in Jellyfin Server is that enabled by default, or something you need to set up before this PR would read them from the server?

ryanwwest avatar Mar 07 '23 01:03 ryanwwest

That PR is for Jellyfin itself, which previously had no way for asking for lyrics. Finamp will need to add support for the new endpoint, which I plan to do in the redesign :)

jmshrv avatar Mar 07 '23 08:03 jmshrv

Ha, right, I should have clicked the link to see where the PR originated from! Thanks for letting me know.

ryanwwest avatar Mar 07 '23 09:03 ryanwwest

I'll probably start working on this once I've finished implementing the new queueing system. This will happen in the redesign though, and I probably won't backport it myself. But backporting should be possible, if someone wants lyrics asap.
ETA is probably... in a month? 😅

Chaphasilor avatar May 21 '23 10:05 Chaphasilor

Jellyfin releases are super slow so the redesign will probably be out by time lyrics are available in the server lol

jmshrv avatar Jul 26 '23 19:07 jmshrv

That's what I'm hoping for. Still haven't started, because the queue is driving me insane...
I'm looking into setting up a Jellyfin beta server for testing though, and as soon as the queue stuff is done I'm tackling lyrics!

Chaphasilor avatar Jul 26 '23 19:07 Chaphasilor

+1 :)

stefan1983 avatar Feb 28 '24 17:02 stefan1983

ETA is probably... in a month?

Well that didn't work out so well...
I'm keeping up with @jmshrv's traditions it seems 😁

Now that the beta is out, I'll first focus on the imminent issues, and then see when I have some time to tackle this. There's still no ETA for Jellyfin 10.9, but I'd like to make sure Finamp is ready for that release!

Chaphasilor avatar Feb 29 '24 01:02 Chaphasilor

I'm keeping up with @jmshrv's traditions it seems 😁

hey you actually deliver on most of your promises :)

jmshrv avatar Feb 29 '24 01:02 jmshrv

Lyrics support is basically done at this point, just in time for the Jellyfin 10.9 update next week! It will be part of the next beta update

Chaphasilor avatar Apr 28 '24 23:04 Chaphasilor