[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