REQ: Server-side playlist import?

[xxxserxxx]

It's common among servers to have a directory in which m3u playlists are stored as files; this location is specified by a path. LMS stores user-created playlists (with absolute filesystem paths) in this directory, and loads them from there. Navidrome and mpd will also pick up playlists from this directory.

If gonic supports this, I've missed it. I didn't see an environment variable that would point gonic to the playlist directory, nor did I find code that would load m3u playlists from a directory in a quick scan. I may have missed it, though.

If gonic can't do this thing, I'd like to request it. gonic already has m3u8 playlist support; to support a playlists directory, gonic would need:

• A configuration variable for the user to provide the path to the playlists, e.g. GONIC_PLAYLIST_PATH
• Code to read in files from that directory
• Code to trigger a rescan of the directory (I think most simple would be hooking into the existing "scan for changes" template function)

[sentriz]

interesting idea

maybe it could even be taken a step further to make things simpler, like a directory /playlists/<user>/<playlist name>.m3u8

then when a getPlaylist or getPlaylists etc called, we just read that directory and generate a response on the fly. it would be plenty fast

then there would be no need for importing, scanning, or any syncing with the database. and we would also solve #54 for free

thoughts @brian-doherty ?

[brian-doherty]

Well playlists need IDs so any way you look at it I think you need to involve the DB. I think xxxserxxx's proposal makes sense given that those imported playlists become public playlists available to all users.

Oddly, the Subsonic API spec has a notion of a list of allowed users for playlists (http://www.subsonic.org/pages/inc/api/examples/playlists_example_1.xml) but no API to set that list of users. Under the API playlists are either personal to the user or public for all.

[sentriz]

if the playlist id only needs to survive between getPlaylists and updatePlaylist?id=<id>, it could just be hash(path)

for public playlists we could have

/playlists/public-a.m3u8
/playlists/public-b.m3u8
/playlists/joe/private-a.m3u8
/playlists/joe/private-b.m3u8

maybe it's a little crazy but it would reduce complexity quite a bit

[brian-doherty]

You could. It would make the playlist IDs out of sync with the rest of the ID system. You'd also have to convert your existing playlist IDs to the new system. I suspect it'd be less work to use the existing logic to just import them.

[xxxserxxx]

I have a patch that implements this; I'm testing it now. Basically, it:

• Adds a GONIC_PLAYLIST_PATH
• Reads all of the files ending in .m3u or .m3u8, and for each one of those,
• calls ctrladmin.playlistParseLine()

To enable this, I had to move some code from ctrladmin to scanner, but I think it makes sense. You could even call it solving some feature envy on the part of ctrladmin... but you can make your own call on that when you see the patch.

It's not going to be ready for a little while, but I expect you'll see a PR a bit later today.

[sentriz]

@xxxserxxx my idea was about the playlists not being in the scanner or the database, instead we would just read the m3u files when getPlaylist etc is called

and yep Brian it would involve creating playlist files for existing playlists in the database. but I don't think there's any need to do anything with playlist ids

[sentriz]

would also mean we don't need the "upload m3u8" button / code anymore

[xxxserxxx]

I have it working with loading playlists into the DB, but the issue that playlists are locked to (individual) users does make limit the implementation.

Are you thinking that, perhaps, playlists get taken out of the DB entirely? While it doesn't seem to bad (to me) to load playlists into the DB from a directory (vs uploading via the API), having an entire side-channel where there are playlists in both the filesystem and the DB seems fraught. If the filesystem becomes a canonical source -- as opposed to just another loading channel -- it would seem there's significantly more work than what I've done. That smells like a DB migration requiring re-persisting information from DB to disk; new data structures or possibly an entire package for playlists-on-FS; an API for treating the FS as a DB for playlists, even if a rather limited one. This sounds like a design discussion, rather than me just hacking in a feature :-)

Don't get me wrong: I think playlists-as-files-in-directories is the right way to go. Mainly, because that's how almost every other music server does it, so having it that way would make migrating to gonic a lot easier for users: it'd be a migration with data-in-place. However, it's a big mouthful for a first contribution, and as I said, it feels like it'd need some formalization of the plan first.

Since what I hacked together works, I'm going to submit a PR anyway. Maybe it'll be of use to someone until a larger change can be rolled out. And since changing how playlists are stored is going to require a data migration anyway, I can't see it doing much harm.

[sentriz]

@brian-doherty and @xxxserxxx thanks for you work on #307 👍 though i decided to use the filesystem as the source of truth as mentioned #311

i think maybe it's bit unorthodox, but we would finally have a solution to #54 (that's been an issue forever) and also we get playlist import and export for free

also no need for playlist upload and parsing logic on the admin ui

what do you think? thanks!

PS it also features a migration that creates some m3u files for you from the database. but that could do with some more testing

[xxxserxxx]

@sentriz can you explain the thought behind playlist.go#108? It looks like you're getting the playlist file names and trying to turn them into ints? I'm getting an error on (I think) this line on my server:

023/04/25 09:19:01 subsonic error code 0: error reading playlist "Christmas Sampler.m3u": convert id to str: strconv.Atoi: parsing "A Very Random Christmas Sampler.m3u": invalid syntax

This is from the head of the playlist-m3u branch.

[sentriz]

@xxxserxxx oh yes, you're trying to use an existing playlist folder? i should have documented it better:

the playlists should look something like

playlists_dir/<user_id>/some/path/whatever.m3u

for example, the admin user:

playlists_dir/1/some/path/whatever.m3u

[xxxserxxx]

So, the second-to-last path element must be an ID, and it's an internal gonic ID? Is it necessary to leak gonic internals to the filesystem?

[sentriz]

to clarify, the id is not the second-to-last path element, but rather the first folder that's located underneath the folder specified by GONIC_PLAYLISTS_DIR or -playlists-dir

regarding the concern about leaking gonic internals, the idea is that gonic takes ownership of the directory, as it's responsible for updating the playlists entries through the subsonic api. so i don't think that leaking internals is an issue in this case

also, if you meant whether just the id should be included in the folder name, there's also the option of using the user's name instead of their id. but that would break if a user were to change their username since gonic wouldn't be able to find their playlists anymore

[xxxserxxx]

Ah. Is it not common for people to also run, e.g., mpd or LMS on the same server?

If gonic is going to own the playlists, why not just store them in the DB? Why leave them in directories if not to allow interoperability and migrate-ability?

Oh, no; I'm not suggesting using user names. I was hoping that the directory structure could be left alone, to avoid having to restructure directory layout every time a user changes tools. I understand that there's a question of how to segregate user playlists, but I'd still prefer to not have to restructure directory layouts every time I try new server software. Maybe that's just me.

[sentriz]

I suppose if you're using docker and want even better interoperability without having to think about the user id stuff, then you could mount your existing playlists dir into the container as /playlists/1/

then outside the container you don't need consider gonic user stuff at all

would that be a reasonable compromise? keeping in mind the benefits that this whole new mechanism gives us - outlined in the PR

[xxxserxxx]

That would work, certainly. Would it be possible to share playlists?

Does a user see only their playlists, or everyone's? Maybe I'm doing an unusual thing, here, but we don't have per-person playlists in my house. Of the 92 playlists we have, there are only one or two that are specific to a person, and these we just name "myWifes playlist.m3u". Would I need to symlink to multiple UIDs?

What's the use case for per-user playlists? Everyone shares the music itself, but not the lists?

I suspect my use is pretty different that the way you're thinking about it. It probably isn't worth worrying about.

[sentriz]

in line with the subsonic api, every playlist has an assigned user. however some playlists can be marked as public (this is still supported with the new system via m3u comments) so that any user can see the playlist

were you using playlists before the playlist-m3u branch? hopefully nothing has changed

for your usecase you could just have 1 user with all the playlists marked as public

[xxxserxxx]

Two users using the same credentials‽ Horror!

But, yes, that would be fine. I can look at the code to figure out the m3u comment feature -- that seems entirely reasonable, given that m3u is already a format that relies heavily on metadata. I wasn't using playlists (with gonic) prior to the playlist-m3u branch, but then, I haven't been using gonic for all that long and am still easing into it.

Thanks for your help; since I opened this, and playlist-m3u branch is likely to be merged, I'll close this ticket.

[brian-doherty]

Just got around to upgrading. Would it be possible to make -playlists-path an optional parameter? As it is right now upgrading breaks every installation until they add one.

…

On Fri, May 5, 2023 at 8:55 AM xxxserxxx ***@***.***> wrote: Closed #306 <#306> as completed. — Reply to this email directly, view it on GitHub <#306 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASFD42MYYXR6WYRCVVK6KVLXEUBF7ANCNFSM6AAAAAAV3TALQ4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[brian-doherty]

A couple more comments: 1) User IDs are hard for end users to come by. It would be nice to name the directories by username, or at least have the option. 2) It would be nice if the system didn't throw Go errors and fail to migrate the DB if you have a directory by the wrong name in there. Just output a warning and move on. On Fri, May 12, 2023 at 10:41 AM Brian Doherty ***@***.***> wrote:

…

Just got around to upgrading. Would it be possible to make -playlists-path an optional parameter? As it is right now upgrading breaks every installation until they add one. On Fri, May 5, 2023 at 8:55 AM xxxserxxx ***@***.***> wrote: > Closed #306 <#306> as completed. > > — > Reply to this email directly, view it on GitHub > <#306 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ASFD42MYYXR6WYRCVVK6KVLXEUBF7ANCNFSM6AAAAAAV3TALQ4> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

[sentriz]

thanks for testing 👍

re points 1 & 3, I think its important that gonic fails to start if there is no playlists for provided, or mounted into a docker volume. since the user may loose their playlists if it migrates into a volume that doesn't exist

and point 2, its a good idea it was mentioned before to use usernames. it makes thing a bit difficult if a user decides to change their name but maybe the tradeoff is worth it

[xxxserxxx]

A couple of other ideas -- before I dig into these, could I get a ruling if PRs would be accepted?

Q1: I'd like to make any playlists in the top-level directory global. That is, any user can see any playlist in GONIC_PLAYLISTS_PATH -- non-recursive. This would allow per-user playlists, but also the global playlists I've been arguing for. I'm trying to avoid having to muck around with things at the filesystem level, which could break other applications and would impact things like backups. Would a patch for this be accepted?

Q2: What if the algorithm were: users see any playlists in subdirectories matching by either username or user ID. I hostile admin could expose users' playlists by swapping around usernames, but if you have a hostile admin, I think you're screwed no matter what. Although I have no use for per-user playlist lists, I'd be willing to work on a PR for this, as well just to support not exposing gonic internal IDs at the filesystem level.

[sentriz]

these both sound nice and reasonable to me. I would surely accept if the implementations didn't add too much complexity

what I'm trying to minimise is code that I would need to maintain while not personally using

[sentriz]

or maybe they make the implementation even simpler - then even better