[vlc-devel] [PATCH 2/4] ytdl: stream filter module for YoutubeDL

[Rémi Denis-Courmont]

Le torstaina 24. syyskuuta 2020, 9.05.34 EEST Steve Lhomme a écrit :
> On 2020-09-23 17:43, Rémi Denis-Courmont wrote:
> > +static void Close(vlc_object_t *obj)
> > +{
> > +    stream_t *s = (stream_t *)obj;
> > +    struct ytdl_playlist *sys = s->p_sys;
> > +
> > +    msg_Dbg(s, "terminating PID %u", (unsigned)sys->pid);
> 
> Do we need a debug message everytime a http URL has been opened ? (no)

Yes. If it gets stuck waiting we know why and on what PID.

And this plugin is one if the quietest to begin with.

> > +    kill(sys->pid, SIGTERM);
> > +    vlc_close(sys->fd);
> > +    vlc_waitpid(sys->pid);
> > +}
> > +
> > +static int OpenFilter(vlc_object_t *obj)
> > +{
> > +    stream_t *s = (stream_t *)obj;
> > +    ssize_t val;
> > +
> > +    if (s->s->pf_readdir != NULL || s->psz_url == NULL)
> > +        return VLC_EGENERIC;
> 
> Why block pf_readdir ? Youtube-dl cannot read Youtube playlists ?

Because Pierre, Thomas and Romain complained about abusively reading 
playlists, and I think they had a point. All other stream filters fail either 
implicitly or explicitly on playlists too.

> > +   if (strncasecmp(s->psz_url, "http:", 5)
> > +    && strncasecmp(s->psz_url, "https:", 6))
> > +       return VLC_EGENERIC;
> > +    if (!var_InheritBool(s, "ytdl"))
> > +        return VLC_EGENERIC;
> > +
> > +    struct ytdl_playlist *sys = vlc_obj_malloc(obj, sizeof (*sys));
> > +    if (unlikely(sys == NULL))
> > +        return VLC_EGENERIC;
> > +
> > +    char *path = config_GetSysPath(VLC_PKG_DATA_DIR, "ytdl-extract.py");
> > +    if (unlikely(path == NULL))
> > +        return VLC_EGENERIC;
> > +
> > +    int fds[2];
> > +
> > +    if (vlc_pipe(fds)) {
> > +        free(path);
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    s->p_sys = sys;
> > +    sys->fd = fds[0];
> > +
> > +    int fdv[] = { -1, fds[1], 2, -1 };
> > +    const char *argv[] = { path, s->psz_url, NULL };
> > +
> > +    val = vlc_spawn(&sys->pid, path, fdv, argv);
> > +    vlc_close(fds[1]);
> > +
> > +    if (val) {
> > +        msg_Dbg(obj, "cannot start %s: %s", path, vlc_strerror_c(val));
> > +        free(path);
> > +        vlc_close(fds[0]);
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    free(path);
> > +
> > +    unsigned char first_byte;
> > +    if (readall(sys->fd, &first_byte, 1) <= 0) {
> > +        /* Location not handled */
> > +        msg_Dbg(s, "cannot extract infos");
> > +        Close(obj);
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    sys->first_byte = first_byte;
> > +    s->pf_read = Read;
> > +    s->pf_control = Control;
> > +    return VLC_SUCCESS;
> > +}
> > +
> > +vlc_module_begin()
> > +    set_shortname("YT-DL")
> > +    set_description("YoutubeDL")
> > +    set_category(CAT_INPUT)
> > +    set_subcategory(SUBCAT_INPUT_STREAM_FILTER)
> > +    set_capability("stream_filter", 305)
> > +    set_callbacks(OpenFilter, Close)
> > +    add_bool("ytdl", true, N_("Enable YT-DL"), N_("Enable YT-DL"), true)
> 
> Defines are usually used for such strings.

And then people forget to delete them, so they stick in the PO files and occupy 
translators needlessly. There are no points in doing that when the string are 
short enough.

> Also you should probably call it Youtube-DL.

I have very mixed feelings about that. First I don't want really to show a 
trademark there. And more importantly, I don't want to mislead that it's 
limited to a specific service.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/