[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