Synced lyrics support

[xxxserxxx]

Implements #49

This was rebased onto main.

This adds support for synchronized lyrics.

It's functional, if not beautiful. mpv gives us ticks every second, on the second, and synchronzed lyrcs rarely line up with the second marks. I've found that a sweet spot is changing lyrics about a half-second in advance, as it nearly always means that at least the right line is highlighted when the vocals happen.

Navidrome doesn't have synchronized lyrics support, that I can tell; @danielepintore has a PR for gonic in the queue waiting for @sentriz to pull it -- that's the fork I'm working and testing with.

As usual, here's the teaser; it's long, but near the end shows me jumping around and having the lyrics match up:

Edit sigh jump forward at the start to 15s; it was apparently recording while waiting for me to respond to a prompt.

[xxxserxxx]

This fixes a bug in the playlist fetching code, where the response can sometimes be nil by the time this is updated -- heavy network congestion, mostly. It doesn't belong in this PR, per se, but if this PR is merged, it addresses an issue. The change should be made regardless.

[xxxserxxx]

I finally set up commit signing. ¯\_(ツ)_/¯

[mahmed2000]

One thing this is missing is checking if the server actually supports lyrics. Lyrics are an extension, not baked into the base API as specified here. A situation like it is now for gonic where the view doesn't exist is still a "valid" implementation of the spec.

I haven't gotten around to testing gonic with lyrics, but without it, browsing the queue gets sluggish (latency dependent). Every movement already triggers a cover art fetch (dealt with by the concurrent album art branch and caching), and also a lyrics fetch with these changes now.

Gonic (currently on main, at least), reports an error 70: view not found for lyrics. Because that response is invalid, nothing gets cached and stmps keeps retrying on each re-selection. This will be fixed when that gonic PR gets merged, but any servers running prior versions, and other servers that don't support the extension will have this issue.

[xxxserxxx]

One thing this is missing is checking if the server actually supports lyrics.

I've pushed changes that account for this into the xxxserxxx branch, and backported them to this branch in fa85bb0.

One thing to note is that just because a server reports that it has the endpoints doesn't mean that the endpoints work: Navidrome claims to support lyrics, and it has the endpoints, but it does not return .lrc files for calls to getLyricsBySongId -- or return anything, from what I can tell. However, this guard should prevent missing endpoint errors for this one endpoint, and it also entirely hides the lyrics widget if the server doesn't support the lyrics extension.

I haven't gotten around to testing gonic with lyrics, but without it, browsing the queue gets sluggish (latency dependent). Every movement already triggers a cover art fetch (dealt with by the concurrent album art branch and caching), and also a lyrics fetch with these changes now.

Even without the concurrent branch, every movement shouldn't cause a fetch; it fetches only if the album art hasn't already been fetched. What the concurrent branch does is fetch the art in the background, so that browsing doesn't affect the latency of moving through the list. Yes, if the art isn't loaded, there might be a lag, but if you're browsing an entire album, there should be no lag after the first.

This is around (depending on your branch) subsonic/api.go#399 (and later line 441, where it's populated). If you're seenig lags on every song in the same album, I wonder if it's because your server is giving the cover art for every song a different ID regardless of whether it's the same image? I can see this happening with cover art embedded in songs; servers are probably not bothering to deduplicate cover art.

In any case, cover art issues are unrelated to lyrics support, right?

Edit

Incidentally, it occurs to me that it may be useful to include in the issue reporting the network layout of the reporter. As I've mentioned elsewhere, I'm running both gonic and Navidrome on a several-year-old weenie AMD A10 Micro-6700T with a bunch of other self-hosted services (mpd, MyMPD, Home Assistant, snapcast server, Jellyfin, a mosquitto server, caddy), and the lag I see on the lyrics branch with cover art and lyrics is a slight hesitation if I hold down the "down" arrow when browsing a large queue. I'm thinking that maybe the difference is that my server is in the basement, and I'm ethernetted (over a coaxial converter, and I get ca. 8Mbps R/W between desktop & server). You (@mahmed2000) and @spezifisch are both reporting much more lag, and I wonder if that's entirely network related or some other issue. In any case, the issue template might include a "profile" attribute, where the reporter says whether the server is accessed through a LAN or WAN.

[mahmed2000]

One thing to note is that just because a server reports that it has the endpoints doesn't mean that the endpoints work: Navidrome claims to support lyrics, and it has the endpoints, but it does not return .lrc files for calls to getLyricsBySongId -- or return anything,

That would be strange. afaik, subsonic isn't supposed to return lrc files. I checked navidrome's source, and that endpoint should be making the structured response. Is the server returning no lyrics in the StructuredLyrics array specifically, or literally just an empty json object at the lyricsList level?

Even without the concurrent branch, every movement shouldn't cause a fetch; it fetches only if the album art hasn't already been fetched.

Right, I'm realizing I could have phrased that a lot better. By fetch I meant just calling GetCoverArt here, so fetching is both from the server or from the cache. There's no lag caused by the cover art after the first time it gets cached.

The only issue with cover art is when there's 50 songs from different albums. That's solved by the concurrent album branch, and has nothing to do with lyrics. I mentioned covers more so as "empty lyrics can fail to cache, unlike cover art".

I'm thinking that maybe the difference is that my server is in the basement, and I'm ethernetted

That would the case, as far as I can tell. I access mine over 2 locations. One is when I'm accessing it over LAN over wifi. The other is over a VPN tunnel with 200-300 ms of latency, and spotty bandwidth anywhere in 100-1k kbps.

Over LAN there's basically zero issues. Over the tunnel, its a hitch-fest.

[xxxserxxx]

That would be strange. afaik, subsonic isn't supposed to return lrc files.

According to what? The only defined metadata tag that stores lyrics, in any format, stores unstructured lyrics. Depending on who and what you read; there are several conversations debating what could go where, but Navidrome actively strips sync data out of synced lyrics stored in the lyrics tag, and it ignores lrc files.

It's going to get worse before it gets better, because there's an emerging spec where synchronization is at the word level, while getLyricsForSongId returns line-level syncing and that's what .lrc supports. There's discussion around the web about adding an "UNSYNCEDLYRICS" tag -- which is what the current "LYRICS" tag is defined to support.

According to the spec,

Retrieves all structured lyrics from the server for a given song. The lyrics can come from embedded tags (SYLT/USLT), LRC file/text file, or any other external source.

SYLT/USLT are not widely supported by encoders -- I've never encountered them in the wild -- and most tools for downloading synced lyrics do so to .lrc files (e.g. https://github.com/moehmeni/syncedlyrics, https://github.com/tranxuanthang/lrcget).

To answer your question, Navidrome just returns an empty structure. If there is a lyrics tag, Navidrome getLyrics/ endpoint strips sync'ing data and returns only the lyrics.

• A discussion about lyrics tags in general
• ID3v2, which while fairly common in mp3 is no ubiquitous, does define unsynced lyrics (ULT in v2.2.0 and USLT in later versions) and synced lyrics (v2.2.0 SLT / later SYLT) tags. It does not clearly specify whether the synced lyrics are word or line synced. Using the ID3v2 lyrics tags in a Vorbis container would be a decidedly ad-hoc, application-specific solution. Specifically, the ID3 FAQ says

  ID3 tags were designed with the MP3 file format in mind. ID3v2 tags will break formats which are container-based such as Ogg Vorbis and WMA

• The OGG Vorbis I spec, which contains no reference to lyrics tags, and is still the spec referenced from various container RFCs.
• The MusicBrainz spec, which states that:

  Note that some of the information such as ‘lyrics’ is not contained within the MusicBrainz database, and will not be provided automatically.
  Interestingly, the Vorbis container spec defines only one tag (ARTIST), claiming that the list of tags is highly dependent on the content, and refers to the MusicBrainz spec for all other tags for music. This may be the most relevant justification for the reason why applications tend to store lyrics data in .lrc files. Where MusicBrainz does mention a lyrics tag, it defines one and merely says:
  Lyrics: The lyrics for the track

• An older discussion about Is the lyrics function working properly? navidrome/navidrome#1840 (comment) mentions that Navidrome doesn't support .lrc files.

Regardless, I don't find the source claiming that "Subsonic isn't supposed to return lrc files". Where did you find that? According to the OpenSubsonic specification for getLyricsForSongId,

Retrieves all structured lyrics from the server for a given song. The lyrics can come from embedded tags (SYLT/USLT), LRC file/text file, or any other external source.

For the purposes of stmps, it doesn't matter where the server gets lyrics from, so long as getLyrics returns unsynced and getLyricsBySongId returns synced lyrics. My code only supports the latter, because the former is not useful without a different representation of the lyrics -- maybe in a pop-up dialog, or something.

To summarize:

• SYLT and USLT are ID3v2 tags, and are not defined for Vorbis containers
• Vorbis does not define a synced lyrics tag; it does define a lyrics tag, which appears to be broadly interpreted by implementors as unsynced lyrics (e.g., Navidrome strips any sync data from lyrics it finds in that tag).
• Tooling that handles lyrics fetching frequently stores lyrics in .lrc files
• The OpenSubsonic spec says synced lyrics can be stored in .lrc files
• Navidrome does not support .lrc files; it may support SYLT and USLT tags, but those would be relevant to only mp3 files. Where it's getting synced lyrics for Vorbis containers (ogg, opus), if not from .lrc files, is not standard.

[mahmed2000]

Regardless, I don't find the source claiming that "Subsonic isn't supposed to return lrc files". Where did you find that?

The fact that any server will never return "text/plain" data on that endpoint, and instead returns the structured xml/json object which contains lyrics as an xml list of tags/json array, as per the response object in that spec page. I think there's been a misunderstanding somewhere? Wherever the server gets its lyrics data from, it cajoles it into the lyrics object (at which point it would stop being a .lrc file). As far as any client is concerned, there need not even be an lrc file, since that data may be from any potential format, or even inside file metadata, or from some third-party service. Only the server's internals need to care and the raw source is never exposed to the API.

On Navidrome, there's 2 lyric endpoints. The original (unused by this PR) /getLyrics, and /getLyricsBySongId (the opensubsonic extension). Navidrome's implementation of the second is here. Its making the Subsonic object, but if the lyrics themselves are empty, that's a different issue that isn't anything stmps needs to worry about:

https://github.com/navidrome/navidrome/blob/master/server/subsonic/media_retrieval.go#L132

Back to the problem this was about, by "returns nothing", do you mean specifically the array of lyrics in the object is empty, or that the server literally just returns "{}"? Because the former would also be valid for just instrumentals or can be chalked up to the server not having any lyrics associated with said song, (unsupported lrc or whatever the case may be). That's still "valid", even if it is technically incorrect. But relying on the server for lyrics means that if the server reports no lyrics, the client must consider that to be "true".

[deluan]

• Navidrome does not support .lrc files; it may support SYLT and USLT tags, but those would be relevant to only mp3 files. Where it's getting synced lyrics for Vorbis containers (ogg, opus), if not from .lrc files, is not standard.

Navidrome currently only reads lyrics from tags, sync and unsync, from both Vorbis and ID3 tags.

If the Vorbis LYRICS tag is synced, Navidrome will return it with timestamps in the getLyricsBySongId endpoint. It only strips timestamps for the getLyrics endpoint, as most clients don't expect this endpoint to return timestamps.

As for supporting .lrc files, we have an open PR to read lyrics from files and from lrclib API: navidrome/navidrome#2897. It will (most probably) be merged for the next release.

[xxxserxxx]

Thank you for providing more detail about this.

Navidrome currently only reads lyrics from tags, sync and unsync, from both Vorbis and ID3 tags.

In no spec I could find were SYLT or USLT referenced for Vorbis, and what information I found about LYRICS was that it was specifically for non-synced lyrics.

If the Vorbis LYRICS tag is synced, Navidrome will return it with timestamps in the getLyricsBySongId endpoint. It only strips timestamps for the getLyrics endpoint, as most clients don't expect this endpoint to return timestamps.

Good to know, thanks. I can at least create an audio file for testing.

As for supporting .lrc files, we have an open PR to read lyrics from files and from lrclib API
...
be merged for the next release.

Outstanding! Given the really poor state of lyrics tag behavior in every container specification, supporting external lrc files is great for users.

[xxxserxxx]

The fact that any server will never return "text/plain" data on that endpoint, and instead returns the structured xml/json object which contains lyrics as an xml list of tags/json array, as per the response object in that spec page. I think there's been a misunderstanding somewhere? Wherever the server gets its lyrics data from, it cajoles it into the lyrics object (at which point it would stop being a .lrc file). As far as any client is concerned, there need not even be an lrc file, since that data may be from any potential format, or even inside file metadata, or from some third-party service. Only the server's internals need to care and the raw source is never exposed to the API.

Are you assuming that because .lrc are not json or XML, that they're not strctured? The information in .lrc files is structured; the patched gonic reads lrc and converts it to what's requested -- in this case, json.

The patch in any case makes no assumption about where the data comes from. It requests and expects the response to be in json format, but how it gets to be that way is irrelevant. The patch does not use getLyrics because -- IMO -- it's not particularly useful since the only thing the UI could do is show it in a window that the user must manually scroll. Eventually it'd be a nice-to-have, in cases where synced lyrics aren't available, but it's not something I'm particularly interested in. Therefore, it focuses on getLyricsBySongId.

On Navidrome, there's 2 lyric endpoints.

On gonic, too.

Back to the problem this was about, by "returns nothing", do you mean specifically the array of lyrics in the object is empty

Yes.

But relying on the server for lyrics means that if the server reports no lyrics, the client must consider that to be "true".

No argument. Which is why the patch updated two weeks ago checks for the extension and disables lyrics if the server doesn't have the lyrics. If it does claim to support the extension, then it should return a structure even if the contents are empty, which is how Navidrome is behaving (currently, with .lrc files -- as explained in more detail above by @deluan). The patch should be fine in this case; by the time of use, an empty array of lyrics is simply not used.