search

arcnmx / MPD

MPD fork adding a youtube-dl plugin
https://github.com/MusicPlayerDaemon/MPD/pull/223
GNU General Public License v2.0
7 stars 2 forks source link

Handle `mpc add playlist_url` #4

Closed Rio6 closed 4 years ago

Rio6 commented 4 years ago

Makes it so MPD skips the song when a playlist url is added as song url.

arcnmx commented 4 years ago

Oh huh, bailing out seems reasonable. It seems to fallback to dumping the playlist even though --no-playlist was requested, since it's the only data it can reasonably return.

However, it defaults to full mode, so it takes a while before parsing completes (and then errors). I think that means we want to change the arguments to youtube-dl --flat-playlist --no-playlist so that if this happens, it doesn't do the recursive fetching.

Rio6 commented 4 years ago

I think that means we want to change the arguments to youtube-dl --flat-playlist --no-playlist so that if this happens

That's a good idea. Although right now in YtdlProcess::Invoke it only accept one PlaylistMode argument. Maybe we can change it so it always uses --flat-playlist? Or maybe turn PlaylistMode into bit masks?

arcnmx commented 4 years ago

I kind of expanded on it in the notes in #5, but I think it basically just comes down to:

  1. PlaylistMode::SINGLE: We want a URL/info for a single URL via input plugin, invoke should use --flat-playlist --no-playlist
  2. PlaylistMode::FLAT: Playlist plugin wants info on a playlist, invoke should use --flat-playlist --yes-playlist
  3. PlaylistMode::FULL: Playlist plugin wants the full playlist, invoke should just use --yes-playlist and omit the flat flag

The argument doesn't really need to change, those are still the only 3 request modes that can be made. I don't think FULL is even used right now, because flat is much faster and the RemoteTagScanner fills in the individual song metadata asynchronously instead. So there really are just the two modes, song or playlist. Full is mainly still there just in case certain extractors would benefit from its use, I figure it could be requested as part of the URL like ytdl://https://whatever.url/#full or something if really needed. Not sure that it ever will be useful/needed though, so maybe it should just be removed.

Rio6 commented 4 years ago

Sounds good. I think we can keep FULL, it's not much of the code anyways.

I'll make a PR that adds --flat-playlist to SINGLE and --yes-playlist to FLAT.