[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: BUG: MPV playlist arguments are out-of-date
From: |
Mike Kazantsev |
Subject: |
Re: BUG: MPV playlist arguments are out-of-date |
Date: |
Sat, 29 Jan 2022 11:30:25 +0500 |
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