Re: BUG: MPV playlist arguments are out-of-date

[Mike Kazantsev]

On Sat, 29 Jan 2022 00:31:40 +0000
acdw <acdw@acdw.net> wrote:

> Hello devs and users!
> 
> I installed EMMS the other day and was trying to play and set up
> streaming playlists.  However, they wouldn't play at first.  I traced
> the issue to `emms-player-mpv-start' in emms-player-mpv.el:
...
> (emms-player-mpv-proc-init (if track-is-playlist "--playlist" "--")
...
> Due to the changed syntax of the --playlist argument, coupled with
> the warning in MPV's manual, I changed the above highlighted form to
> simply "--" in `emms-player-mpv-start'.

I'd be surprised if that code line is actually used in your case.

It should only run if emms-player-mpv falls back to using FIFO socket
to control very old mpv versions (pre-0.7.0 circa 2014-10-16) or if
you've set emms-player-mpv-ipc-method to 'file manually.

Maybe check "pgrep -fa mpv" output, I'd expect it to be something like this:

  3472 /usr/bin/mpv --quiet --really-quiet --no-audio-display
    --force-window=no --input-ipc-server=.../emms/mpv-ipc.sock --idle

And not have any file/playlist path, "--playlist" or "--" in there.

If that's the case, I think you might've had some unrelated issue
initially, and code tweak didn't really fix it, was something else.


More likely place where emms-player-mpv.el is choosing file-or-list:

  ((,(if track-is-playlist 'loadlist 'loadfile) ,track-name replace))

Maybe you tweaked this check as well?

"loadlist" IPC command should presumably be less secure than loading
playlists via "loadfile" too.


> I'm not sure when, but my version of MPV (0.32.0) doesn't take a
> plain --playlist argument; rather it says this about the --playlist
> argument:

But also, this indeed looks like a bug with that 'file fallback in
modern mpv versions, as they indeed stopped supporting --playlist as a
standalone option like that.

Though again, that fallback should only be used for really old versions
where that option still works in the old way, as a flag.


> With my minimal testing it's working fine, though to change the code
> in the package it probably needs more testing.

I think there are separate dimensions of the issue with playlists:

1. Supporting really old mpv versions that require 'file in insecure way.
2. Supporting somewhat outdated mpv where "loadfile" for playlists might fail.
3. Best-effort for something working vs (error "not supported for security 
reasons").
4. Supporting plain lists and such that don't work with "loadfile" anyway.
5. Maybe file-or-playlist auto-detection is not desirable, even if it works.

An argument can be made to drop (1), or that in such unlikely old+new
setup mpv/ffmpeg/codecs are insecure anyway, or whether there should be
warning/option, or that such setup is likely very intentional anyway, etc.

I'll probably add some let-mpv-decide-what-is-playlist default-on option,
which can be toggled off manually for best compatibility but less security,
and fixing that "--playlist" flag issue by adding a version check to use it
as --playlist=file for newer versions.

Maybe there's a better way to go about resolving this, 
or other factors that I didn't consider in the list above?


-- 
Mike Kazantsev // fraggod.net