search

LMS-Community / slimserver

Server for Squeezebox and compatible players. This server is also called Lyrion Music Server.
https://lyrion.org
Other
1.1k stars 279 forks source link

Supporting Disc Subtitles (TSST tag) #1111

Closed chrober closed 1 week ago

chrober commented 1 month ago

It would be great for multi-disc releases if the UI would show the disc subtitle (if present in the TSST tag). Is there any chance (or even a plan) to implement this?

michaelherger commented 1 month ago

@chrober what LMS version are you using?

@darrell-k - isn't this something you've been working on?

chrober commented 1 month ago

@chrober what LMS version are you using?

I am using Logitech Media Server Version: 8.5.1 - 1713072758 @ Mon 15 Apr 2024 11:43:22 AM CEST.

darrell-k commented 1 month ago

I added a subtitle column to the albums table in 9.0, but we're not doing anything with it yet.

The subtitle column in the tracks table is populated from the scan (with TIT3 in mp3-land).

CDrummond commented 4 weeks ago

I'd also support this. Currently Material can be configured to use the comment field for this, but that's always felt a bit hacky. Would be great if LMS supported the actual tag. So, am I correct in understanding that the subtitle is there, just not supplied to the clients?

darrell-k commented 4 weeks ago

Currently the scanner maps SUBTITLE/TIT3 to tracks.subtitle in the database.

The column albums.subtitle exists, but is not yet populated by the scan.

CDrummond commented 4 weeks ago

Any chance of having this in soon? Would be good to have to deprecate Material's work-around - and next release seems a good time for this.

chrober commented 4 weeks ago

Here is a list of disc-subtitle tags for various audio formats used by Musicbrainz Picard: https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#disc-subtitle

darrell-k commented 4 weeks ago

Should be simple enough to add to the scanner. Presumably you would want the value to be returned in the albums message?

I can add it to this PR, best keep the changes that require release coordination and retagging together.

CDrummond commented 4 weeks ago

In the albums message would be great. I currently just get all tracks - but having this in the albums command would reduce complexity.

darrell-k commented 4 weeks ago

In the albums message would be great. I currently just get all tracks - but having this in the albums command would reduce complexity.

@CDrummond Actually, thinking about this, it's not album level data is it? ie in a multidisc set, there might be a different discsubtitle for each disc in the set, though it is a single album?

In which case I'd need to add a new column to the tracks table, and you'd have to do what you're doing now, retrieve all the tracks?

Is this correct?

CDrummond commented 4 weeks ago

@darrell-k Yeah, adding to the tracks probably makes more sense - as, AFAIK, there is no reliable way to return a list (LMS uses a comma separated string, but that's prone to failures).

darrell-k commented 4 weeks ago

@CDrummond at a database level, it has to be in the Tracks table, as it's an attribute of track, not of album. I misunderstood before.

So simplest from the LMS point of view is to return discsubtitle for each track in the tracks response, and leave you to do with it what you're already doing with comments.

darrell-k commented 4 weeks ago

@CDrummond do you only need this from the tracks command, or also the status command?

CDrummond commented 4 weeks ago

Just tracks should be enough.

darrell-k commented 3 weeks ago

@CDrummond changes made in https://github.com/LMS-Community/slimserver/pull/1109

CDrummond commented 2 weeks ago

With https://github.com/LMS-Community/slimserver/pull/1109 isn't this issue now resolved?

darrell-k commented 2 weeks ago

I think so, I wonder if the issue raiser agrees?

michaelherger commented 2 weeks ago

@chrober would you be willing to give LMS 9 a try (https://lyrion.org/downloads/#lyrion-music-server-v900-development-build)? Or we can close this and you re-open or open a new issue once LMS9 has been released and you see need for more changes?

chrober commented 2 weeks ago

@chrober would you be willing to give LMS 9 a try (https://lyrion.org/downloads/#lyrion-music-server-v900-development-build)?

Sure, I am looking forward to try this! Thanks, for implementing this so quickly!

Or we can close this and you re-open or open a new issue once LMS9 has been released and you see need for more changes?

Whatever you prefer to keep your issue list and backlog groomed. I will not have time for setting up LMS 9 in the next three days, but I will definitely reserve some time during the weekend.

chrober commented 1 week ago

Hi @darrell-k, @CDrummond, I've just installed LMS 9 (Lyrion Music Server Version: 9.0.0 - 1718877384 @ Fri 21 Jun 2024 02:38:05 AM CEST) on a clone of my pCP card ... and waited for the re-scan to complete ... and upgraded to latest versions of plugins (e.g. Material Skin 5.0.3) ...and... ta-da!

grafik

The album view gives me disc titles instead of disc numbers if TSST is present!

Thank you so much guys, it looks fantastic!

michaelherger commented 1 week ago

Thanks for the feedback, @chrober!