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

[acdw]

Hey there! Thanks for the response ... you're pretty much right on the money.

On 2022-01-29 (Saturday) at 06:30, Mike Kazantsev <mk.fraggod@gmail.com> wrote:

> 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.

I didn't check the pgrep -fa mpv output, because I did indeed set 
emms-player-mpv-ipc-method to 'file manually, since mpv wasn't playing.  So my 
emms was running that codeline.

> 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?

After replacing the above with simply ((loadfile ,track-name replace)) and 
reverting emms-player-mpv-ipc-method to the default, EMMS now plays the streams 
using MPV perfectly :)

>> 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.

Yes, this makes sense to me.

> 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?

I'll be honest -- I don't know enough about the internals of mpv and emms to 
know what the best option here is, which is a big reason I didn't try to send a 
patch. Removing the if form where you mentioned it and replacing it with simply 
'loadfile has worked on my system, however I'm not sure how secure or good that 
is.

Thanks for your help!

-- 
~ acdw
acdw.net | breadpunk.club/~breadw