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

[Mike Kazantsev]

On Sun, 30 Jan 2022 00:27:03 +0000
acdw <acdw@acdw.net> wrote:

> Hey there! Thanks for the response ... you're pretty much right on the money.
> 
> >> (emms-player-mpv-proc-init (if track-is-playlist "--playlist" "--")  
> >
> > I'd be surprised if that code line is actually used in your case.
>
> 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.

Actually, about that - turns out you are running the last mpv release
that supports that option (0.32.0).
It was removed in 0.33.0, so setting that to 'file will no longer work there.

I've documented it in the docstring for that option, 
and it should probably be safe to drop it in the future,
as last mpv release where it should be necessary will be 10 years old soon.


> > 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 :)

This should be the default in current EMMS git master,
unless emms-player-mpv-use-playlist-option customizable is set manually.


I think this change can break how things worked before in two cases:

- Very old mpv versions that fail to auto-detect some playlist types
  will fail to play URLs after EMMS update.

  As described earlier, this doesn't seem to be very likely scenario,
  and given that this is a security issue, it might be worth drawing
  user attention to it this way anyway, instead of silently sticking to
  insecure default.

- Playing plaintext "one file per line" playlists will not work by default.

  Hopefully these are rare enough to be an ok drawback here.

Both can be addressed by re-enabling emms-player-mpv-use-playlist-option,
if necessary.


Since mpv 0.33.0 there is also --load-unsafe-playlists flag that allows
to extend compatibility even further (documented in mpv manpage for
--playlist option), but obviously should not be needed normally.

"--playlist file.pls" option syntax was removed in mpv 0.31.0, but as
far as I can tell "--playlist=file.pls" always worked, so will now
be used with both that use-playlists option enabled and 'file ipc
(and "loadlist" command running exactly same unsafe code for socket ipc).


Thank you for reporting the issue.
Wouldn't have noticed the problem and all these changes in mpv otherwise.


-- 
Mike Kazantsev // fraggod.net