MusicPlayerDaemon / MPD

Music Player Daemon
https://www.musicpd.org/
GNU General Public License v2.0
2.18k stars 352 forks source link

Support other playlist formats in playlist_directory as read-only #870

Open eMPee584 opened 4 years ago

eMPee584 commented 4 years ago

This 8 year old devel mailing list post is the only documentation I found about why my attempts to convince mpd to accept somafm's pls playlist were failing:

In the playlist directory, MPD reads only the format that it can write
- and that's plain M3U, not ExtM3U. At that point, MPD does not care
if the link points to a stream or a local file.
[…]
You can have (read-only) playlist files in the music directory in any
subdirectory. The playlist directory is just for playlists that are
managed by MPD.

Rather than documenting it better, it would be nice if mpd would apply its playlist format wit within the playlists directory, too. Non-m3u playlist could just be hidden from the playlist editor.

MaxKellermann commented 4 years ago

How is that supposed to work? How can MPD "hide" things fromt he "playlist editor", and what is the "playlist editor"? How are pls files in the playlist_directory announced to the client?

eMPee584 commented 4 years ago

Well how would I know, I'm just a user. I just see the symptom, that common playlist formats generally supported by mpd will not work when put in /var/lib/mpd/playlists. IMHO, this limitation strongly defies user expectations and either should be documented or, much more preferable, overcome.

This is the code for the playlist editor of my favorite player, ncmpcpp: https://github.com/ncmpcpp/ncmpcpp/blame/master/src/screens/playlist_editor.cpp The playlists seem to be fetched here https://github.com/ncmpcpp/ncmpcpp/blame/master/src/mpdpp.cpp#L703 by invoking mpd_send_list_playlists() which is implemented here https://github.com/MusicPlayerDaemon/libmpdclient/blame/b4791fcff9eb30c9566f04f118269921a65c87c2/src/cplaylist.c#L44 which ultimately invokes ListPlaylistFiles() https://github.com/MusicPlayerDaemon/MPD/blame/5ccfcffcc124e406233359fe8fe65b704b98b8c8/src/PlaylistFile.cxx#L160 ..

MaxKellermann commented 4 years ago

Documented how? This is the documentation about playlist_directory:

Stored playlists are some kind of secondary playlists which can be created, saved, edited and deleted by the client. They are addressed by their names. Its contents can be loaded into the queue, to be played back. The playlist_directory setting specifies where those playlists are stored.

This doesn't say you should copy existing playlists there. It doesn't even specify what file format is used. The fact that MPD uses m3u is just an implementation detail which can change any time.

And here it says:

Playlist plugins are used to load remote playlists (protocol commands load, listplaylist and listplaylistinfo). This is not related to MPD’s playlist directory.

If you believe that these sentences raised any false expectations, submit a documentation improvement.

Your pointers to ncmpcpp source code do not answer my questions. It answers "what is the playlist editor", but the answer is that you're not talking about MPD, because that feature is not a MPD feature.

So you found the source code of ncmpcpp which obtains a list of playlists. And you said MPD should "hide" PLS files from the ncmpcpp playlist editor. So MPD could just omit PLS files from the "listplaylists" command. This would hide PLS files from the playlist editor, but it would also hide those files from all MPD clients, because MPD cannot know if this is a playlist listing for the playlist editor or for regular playback. So this would effectively make PLS files inaccessible.

See my last question: "How are pls files in the playlist_directory announced to the client?" Your implicit response (deducted from your code pointers) was: "not at all".

I believe this whole feature you're requestion doesn't makse sense, and we're better off not doing it. It's a bad idea and a bad design - or rather: no design at all. I asked for your design, and you replied "how would I know". If you, as the guy who wants the feature, don't know, then who does?

This is the problem with this "feature": it just doesn't fit into the existing design, because it contradicts with the existing design. Nobody can tell how to properly integrate it. Just saying "hey I want this feature" isn't going to magically make it fit.

But yes, documentation can always be improved, and maybe the existing design needs a better explanation. (Documentation is always and infinitely in need of improvement)