[vlc-devel] [RFC] NOT FOR MERGE event: single handler per object

RĂ©mi Denis-Courmont remi at remlab.net
Sun May 19 14:10:52 CEST 2019


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



More information about the vlc-devel mailing list