[vlc-devel] [PATCH 01/20] input: item: add input_item_Parse

Thomas Guillem thomas at gllm.fr
Mon Jun 3 07:48:56 CEST 2019



On Sat, Jun 1, 2019, at 00:03, Romain Vimont wrote:
> On Fri, May 31, 2019 at 03:59:27PM +0200, Thomas Guillem wrote:
> > This function doesn't use a background thread, contrary to
> > libvlc_MetadataRequest(). It will be used by the Medialibray that has its own
> 
> (typo: Medialibrary)
> 
> > background thread.
> 
> Cool feature!
> 
> > ---
> >  include/vlc_input_item.h | 60 +++++++++++++++++++++++++++++++++++
> >  src/input/item.c         | 67 ++++++++++++++++++++++++++++++++++++++++
> >  src/libvlccore.sym       |  2 ++
> >  3 files changed, 129 insertions(+)
> > 
> > diff --git a/include/vlc_input_item.h b/include/vlc_input_item.h
> > index 5ae437c13f..272f774306 100644
> > --- a/include/vlc_input_item.h
> > +++ b/include/vlc_input_item.h
> > @@ -371,6 +371,66 @@ VLC_API input_item_t *input_item_Hold(input_item_t *);
> >  /** Releases an input item, i.e. decrements its reference counter. */
> >  VLC_API void input_item_Release(input_item_t *);
> >  
> > +/**
> > + * input item parser opaque structure
> > + */
> > +typedef struct input_item_parser_t input_item_parser_t;
> > +
> > +/**
> > + * input item parser callbacks
> > + */
> > +typedef struct input_item_parser_cbs_t
> > +{
> > +    /**
> > +     * Event received when the parser ends
> > +     *
> > +     * @note this callback is mandatory.
> > +     *
> > +     * @param item the parsed item
> > +     * @param ret VLC_SUCCESS in case of success, an error otherwise
> > +     * @param userdata user data set by input_item_Parse()
> > +     */
> > +    void (*on_ended)(input_item_t *item, int ret, void *userdata);
> > +
> > +    /**
> > +     * Event received when a new subtree is added
> > +     *
> > +     * @note this callback is optional.
> > +     *
> > +     * @param item the parsed item
> > +     * @param subtree sub items of the current item
> > +     * @param userdata user data set by input_item_Parse()
> > +     */
> > +    void (*on_subtree_added)(input_item_t *item, input_item_node_t *subtree, void *userdata);
> > +} input_item_parser_cbs_t;
> > +
> > +/**
> > + * Parse an item
> > + *
> > + * @note the parsing is done asynchronously. The user can call
> > + * input_item_parser_Release() before receiving the on_ended() event in order
> > + * to interrupt it.
> > + *
> > + * @param item the item to parse
> > + * @param parent the parent obj
> > + * @param cbs callbacks to be notified of the end of the parsing
> > + * @param userdata opaque data user by parser callbacks
> > + *
> > + * @return a parser instance or NULL in case of error, the parser needs to be
> > + * released with input_item_parser_Release()
> > + */
> > +VLC_API input_item_parser_t *
> > +input_item_Parse(input_item_t *item, vlc_object_t *parent,
> > +                 const input_item_parser_cbs_t *cbs, void *userdata) VLC_USED;
> > +
> > +/**
> > + * Release (and interrupt if needed) a parser
> > + *
> > + * @param parser the parser returned by input_item_Parse
> > + */
> > +VLC_API void
> > +input_item_parser_Release(input_item_parser_t *parser);
> > +
> >  typedef enum input_item_meta_request_option_t
> >  {
> >      META_REQUEST_OPTION_NONE          = 0x00,
> > diff --git a/src/input/item.c b/src/input/item.c
> > index 830c79895f..ce58b90122 100644
> > --- a/src/input/item.c
> > +++ b/src/input/item.c
> > @@ -1325,6 +1325,73 @@ void input_item_UpdateTracksInfo(input_item_t *item, const es_format_t *fmt)
> >      vlc_mutex_unlock( &item->lock );
> >  }
> >  
> > +struct input_item_parser_t
> > +{
> > +    input_thread_t *input;
> > +    input_state_e state;
> > +    const input_item_parser_cbs_t *cbs;
> > +    void *userdata;
> > +};
> > +
> > +static void
> > +input_item_parser_InputEvent(input_thread_t *input,
> > +                             const struct vlc_input_event *event, void *parser_)
> > +{
> > +    input_item_parser_t *parser = parser_;
> > +
> > +    switch (event->type)
> > +    {
> > +        case INPUT_EVENT_STATE:
> > +            parser->state = event->state;
> > +            break;
> > +        case INPUT_EVENT_DEAD:
> > +        {
> > +            int ret = parser->state == END_S ? VLC_SUCCESS : VLC_EGENERIC;
> > +            parser->cbs->on_ended(input_GetItem(input), ret, parser->userdata);
> > +            break;
> > +        }
> > +        case INPUT_EVENT_SUBITEMS:
> > +            if (parser->cbs->on_subtree_added)
> > +                parser->cbs->on_subtree_added(input_GetItem(input),
> > +                                              event->subitems, parser->userdata);
> > +            break;
> > +        default:
> > +            break;
> > +    }
> > +}
> > +
> > +input_item_parser_t *
> > +input_item_Parse(input_item_t *item, vlc_object_t *obj,
> > +                 const input_item_parser_cbs_t *cbs, void *userdata)
> > +{
> > +    assert(cbs && cbs->on_ended);
> > +    input_item_parser_t *parser = malloc(sizeof(*parser));
> > +    if (!parser)
> > +        return NULL;
> > +
> > +    parser->state = INIT_S;
> > +    parser->cbs = cbs;
> > +    parser->userdata = userdata;
> > +    parser->input = input_CreatePreparser(obj, input_item_parser_InputEvent,
> > +                                          parser, item);
> > +    if (!parser->input || input_Start(parser->input))
> > +    {
> > +        if (parser->input)
> > +            input_Close(parser->input);
> > +        free(parser);
> > +        return NULL;
> > +    }
> > +    return parser;
> > +}
> > +
> > +void
> > +input_item_parser_Release(input_item_parser_t *parser)
> > +{
> > +    input_Stop(parser->input);
> 
> Isn't it asynchronous?

Parsing is asynchronous, Release may be not if it wait for the input thread to stop.

> 
> If it causes input_item_parser_InputEvent() to be called asynchronously,
> parser will be used-after-free.

input_item_parser_InputEvent() could be called while input_item_parser_Release is called, yes.
I could add a input_item_parser_Stop() function but most users will just call input_item_parser_Stop() and input_item_parser_Release().

I don't see the use-after-free, the userdata set in input_item_Parse() must be released after 
input_item_parser_Release() returns.

> 
> > +    input_Close(parser->input);
> > +    free(parser);
> > +}
> > +
> >  static int rdh_compar_type(input_item_t *p1, input_item_t *p2)
> >  {
> >      if (p1->i_type != p2->i_type)
> > diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> > index 87190a3573..ae7cc6d8b4 100644
> > --- a/src/libvlccore.sym
> > +++ b/src/libvlccore.sym
> > @@ -207,6 +207,8 @@ input_item_WriteMeta
> >  input_item_slave_GetType
> >  input_item_slave_New
> >  input_item_AddSlave
> > +input_item_Parse
> > +input_item_parser_Release
> >  input_Read
> >  input_resource_New
> >  input_resource_Release
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list