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

Alexandre Janniaux ajanni at videolabs.io
Sat Jun 1 11:37:38 CEST 2019


Hi,

Nice, I have some minor nitpicking inline remarks.

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

Nit, missing capital letter for "this".

> +     *
> +     * @param item the parsed item
> +     * @param ret VLC_SUCCESS in case of success, an error otherwise

Would "status" or "parse_status" be a better name than ret ?

> +     * @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.

Nit, missing capital letter for "this".

> +     *
> +     * @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

The brief text seems to mismatch the function, as it creates the parser
but doesn't do the parsing. Maybe "parsing" could be explained too.

> + *
> + * @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.

Nit, missing capital letter for "the".

> + *
> + * @param item the item to parse
> + * @param parent the parent obj
> + * @param cbs callbacks to be notified of the end of the parsing

It's not only the end of parsing but also subtree right ?

> + * @param userdata opaque data user by parser callbacks

I believed you meant s/user/used/.

> + *
> + * @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);
> +    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


More information about the vlc-devel mailing list