[vlc-devel] [RFC] NOT FOR MERGE event: single handler per object
Thomas Guillem
thomas at gllm.fr
Sun May 19 21:16:45 CEST 2019
OK for me.
On Sun, May 19, 2019, at 14:11, Rémi Denis-Courmont wrote:
> This drastically simplifies the LibVLC event handling by keeping only
> one handler per object, rather than n handlers per event type and per
> object.
>
> In practice, applications/libraries built on LibVLC subscribe to each
> event type zero or once per object. And then bindings to other languages
> or frameworks typically subscribe exactly once to every event, then
> re-expose the events using proper mechanisms of the other language or
> the framework. Either way, there are little to no use for variable set
> of handlers for a given event type.
>
> As such keeping a single handler avoids difficult or impossible to
> handle errors in libvlc_event_attach() and simplifies the code at no
> practical cost.
>
> Then using the same handler for all events simplifies backward/forward
> compatibility. Luckily, the event type is already conveyed within the
> event structure, so no changes are needed in this respect.
> (It could be argued that the type should be a parameter of the callback
> but that would be beyond the scope of this changeset.)
>
> This also makes the event mutex non-recursive, because recursion
> becomes completely pointless with the above changes.
>
> BIG FAT WARNING: The internal user (media list player) and the test
> cases are not updated. This version of the patch will not build.
> ---
> include/vlc/libvlc.h | 34 ++++++------
> lib/event.c | 120 +++++-------------------------------------
> lib/libvlc_internal.h | 3 +-
> 3 files changed, 33 insertions(+), 124 deletions(-)
>
> diff --git a/include/vlc/libvlc.h b/include/vlc/libvlc.h
> index 0ec0045c96..a23eb80f93 100644
> --- a/include/vlc/libvlc.h
> +++ b/include/vlc/libvlc.h
> @@ -308,33 +308,33 @@ typedef int libvlc_event_type_t;
> typedef void ( *libvlc_callback_t )( const struct libvlc_event_t
> *p_event, void *p_data );
>
> /**
> - * Register for an event notification.
> + * Register the event handler.
> + *
> + * This function sets or resets the event handler. If the handler was already
> + * set, it waits for any pending event handler invocation to complete.
> *
> * \param p_event_manager the event manager to which you want to attach to.
> * Generally it is obtained by vlc_my_object_event_manager() where
> * my_object is the object you want to listen to.
> - * \param i_event_type the desired event to which we want to listen
> - * \param f_callback the function to call when i_event_type occurs
> + * \param f_callback event handler (or NULL to ignore events)
> * \param user_data user provided data to carry with the event
> - * \return 0 on success, ENOMEM on error
> */
> -LIBVLC_API int libvlc_event_attach( libvlc_event_manager_t *p_event_manager,
> - libvlc_event_type_t i_event_type,
> - libvlc_callback_t f_callback,
> - void *user_data );
> +LIBVLC_API void libvlc_event_attach(libvlc_event_manager_t *p_event_manager,
> + libvlc_callback_t f_callback,
> + void *user_data);
>
> /**
> - * Unregister an event notification.
> + * Unregister the event handler.
> + *
> + * This function removes the event handler, and waits for any pending event
> + * handler invocation to complete.
> *
> - * \param p_event_manager the event manager
> - * \param i_event_type the desired event to which we want to unregister
> - * \param f_callback the function to call when i_event_type occurs
> - * \param p_user_data user provided data to carry with the event
> + * \param p_em the event manager
> */
> -LIBVLC_API void libvlc_event_detach( libvlc_event_manager_t *p_event_manager,
> - libvlc_event_type_t i_event_type,
> - libvlc_callback_t f_callback,
> - void *p_user_data );
> +static inline void libvlc_event_detach(libvlc_event_manager_t *p_em)
> +{
> + libvlc_event_attach(p_em, NULL, NULL);
> +}
>
> /** @} */
>
> diff --git a/lib/event.c b/lib/event.c
> index 28d3d0c232..51078f5607 100644
> --- a/lib/event.c
> +++ b/lib/event.c
> @@ -37,65 +37,25 @@
> * Event Handling
> */
>
> -/* Example usage
> - *
> - * struct libvlc_cool_object_t
> - * {
> - * ...
> - * libvlc_event_manager_t event_manager;
> - * ...
> - * }
> - *
> - * libvlc_my_cool_object_new()
> - * {
> - * ...
> - * libvlc_event_manager_init(&p_self->event_manager, p_self)
> - * ...
> - * }
> - *
> - * libvlc_my_cool_object_release()
> - * {
> - * ...
> - * libvlc_event_manager_release(&p_self->event_manager);
> - * ...
> - * }
> - *
> - * libvlc_my_cool_object_do_something()
> - * {
> - * ...
> - * libvlc_event_t event;
> - * event.type = libvlc_MyCoolObjectDidSomething;
> - * event.u.my_cool_object_did_something.what_it_did = kSomething;
> - * libvlc_event_send(&p_self->event_manager, &event);
> - * }
> - * */
> -
> -typedef struct libvlc_event_listener_t
> -{
> - libvlc_event_type_t event_type;
> - void * p_user_data;
> - libvlc_callback_t pf_callback;
> -} libvlc_event_listener_t;
> -
> /*
> * Internal libvlc functions
> */
>
> +static void dummy_callback(const struct libvlc_event_t *event, void *data)
> +{
> + (void) event; (void) data;
> +}
> +
> void libvlc_event_manager_init(libvlc_event_manager_t *em, void *obj)
> {
> em->p_obj = obj;
> - vlc_array_init(&em->listeners);
> - vlc_mutex_init_recursive(&em->lock);
> + em->callback = dummy_callback;
> + vlc_mutex_init(&em->lock);
> }
>
> void libvlc_event_manager_destroy(libvlc_event_manager_t *em)
> {
> vlc_mutex_destroy(&em->lock);
> -
> - for (size_t i = 0; i < vlc_array_count(&em->listeners); i++)
> - free(vlc_array_item_at_index(&em->listeners, i));
> -
> - vlc_array_clear(&em->listeners);
> }
>
> /**************************************************************************
> @@ -110,14 +70,7 @@ void libvlc_event_send( libvlc_event_manager_t * p_em,
> p_event->p_obj = p_em->p_obj;
>
> vlc_mutex_lock(&p_em->lock);
> - for (size_t i = 0; i < vlc_array_count(&p_em->listeners); i++)
> - {
> - libvlc_event_listener_t *listener;
> -
> - listener = vlc_array_item_at_index(&p_em->listeners, i);
> - if (listener->event_type == p_event->type)
> - listener->pf_callback(p_event, listener->p_user_data);
> - }
> + p_em->callback(p_event, p_em->user_data);
> vlc_mutex_unlock(&p_em->lock);
> }
>
> @@ -125,59 +78,14 @@ void libvlc_event_send( libvlc_event_manager_t * p_em,
> * Public libvlc functions
> */
>
> -/**************************************************************************
> - * libvlc_event_attach (public) :
> - *
> - * Add a callback for an event.
> - **************************************************************************/
> -int libvlc_event_attach(libvlc_event_manager_t *em, libvlc_event_type_t type,
> - libvlc_callback_t callback, void *opaque)
> +void libvlc_event_attach(libvlc_event_manager_t *em,
> + libvlc_callback_t callback, void *opaque)
> {
> - libvlc_event_listener_t *listener = malloc(sizeof (*listener));
> - if (unlikely(listener == NULL))
> - return ENOMEM;
> -
> - listener->event_type = type;
> - listener->p_user_data = opaque;
> - listener->pf_callback = callback;
> + if (callback == NULL)
> + callback = dummy_callback;
>
> - int i_ret;
> vlc_mutex_lock(&em->lock);
> - if(vlc_array_append(&em->listeners, listener) != 0)
> - {
> - i_ret = VLC_EGENERIC;
> - free(listener);
> - }
> - else
> - i_ret = VLC_SUCCESS;
> + em->callback = callback;
> + em->user_data = opaque;
> vlc_mutex_unlock(&em->lock);
> - return i_ret;
> -}
> -
> -/**************************************************************************
> - * libvlc_event_detach (public) :
> - *
> - * Remove a callback for an event.
> - **************************************************************************/
> -void libvlc_event_detach(libvlc_event_manager_t *em, libvlc_event_type_t type,
> - libvlc_callback_t callback, void *opaque)
> -{
> - vlc_mutex_lock(&em->lock);
> - for (size_t i = 0; i < vlc_array_count(&em->listeners); i++)
> - {
> - libvlc_event_listener_t *listener;
> -
> - listener = vlc_array_item_at_index(&em->listeners, i);
> -
> - if (listener->event_type == type
> - && listener->pf_callback == callback
> - && listener->p_user_data == opaque)
> - { /* that's our listener */
> - vlc_array_remove(&em->listeners, i);
> - vlc_mutex_unlock(&em->lock);
> - free(listener);
> - return;
> - }
> - }
> - abort();
> }
> diff --git a/lib/libvlc_internal.h b/lib/libvlc_internal.h
> index 5c1107b84f..f11089e543 100644
> --- a/lib/libvlc_internal.h
> +++ b/lib/libvlc_internal.h
> @@ -78,7 +78,8 @@ struct libvlc_instance_t
> struct libvlc_event_manager_t
> {
> void * p_obj;
> - vlc_array_t listeners;
> + void *user_data;
> + libvlc_callback_t callback;
> vlc_mutex_t lock;
> };
>
> --
> 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