[vlc-devel] [PATCH 1/2] core: Add a vlc_instance_holder API
Rémi Denis-Courmont
remi at remlab.net
Sat Jul 1 17:07:43 CEST 2017
Le 1 juillet 2017 11:22:25 GMT+02:00, "Hugo Beauzée-Luyssen" <hugo at beauzee.fr> a écrit :
>This API can be used to store an instance in the core, removing the use
>for some special shared libraries/shared variables without refcounting
>---
> include/vlc_instance_holder.h | 74 +++++++++++++++++
> include/vlc_threads.h | 1 +
> src/Makefile.am | 4 +-
> src/libvlc.c | 4 +
> src/libvlc.h | 2 +
> src/libvlccore.sym | 3 +
>src/misc/instance_holder.c | 181
>++++++++++++++++++++++++++++++++++++++++++
> src/misc/threads.c | 1 +
> 8 files changed, 269 insertions(+), 1 deletion(-)
> create mode 100644 include/vlc_instance_holder.h
> create mode 100644 src/misc/instance_holder.c
>
>diff --git a/include/vlc_instance_holder.h
>b/include/vlc_instance_holder.h
>new file mode 100644
>index 0000000000..72f21e1e95
>--- /dev/null
>+++ b/include/vlc_instance_holder.h
>@@ -0,0 +1,74 @@
>+/*****************************************************************************
>+ * vlc_instance_holder.h: Provide functions to hold a unique
>refcounted opaque
>+ * instance.
>+
>*****************************************************************************
>+ * Copyright (C) 2004-2016 VLC authors, VideoLAN and VideoLabs
>+ *
>+ * Authors: Hugo Beauzée-Luyssen <hugo at beauzee.fr>
>+ *
>+ * This program is free software; you can redistribute it and/or
>modify it
>+ * under the terms of the GNU Lesser General Public License as
>published by
>+ * the Free Software Foundation; either version 2.1 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>License
>+ * along with this program; if not, write to the Free Software
>Foundation,
>+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>+
>*****************************************************************************/
>+
>+#ifndef VLC_INSTANCE_HOLDER_H
>+#define VLC_INSTANCE_HOLDER_H
>+
>+#include <vlc_common.h>
>+
>+/**
>+ * Will acquire the instance with the given name.
>+ * \param p_obj A vlc_object_t instance
>+ * \param psz_name The instance name
>+ * \param pf_constructor Will be invoked to create the instance when
>the provided
>+ * name doesn't refer to an existing instance.
>If the
>+ * constructor function pointer is NULL, the
>instance won't
>+ * be created.
>+ * \param pf_destructor Will be invoked to destroy the instance when
>its
>+ * refount reaches 0. If the opaque context was
>allocated
>+ * the destructor is expected to release it as
>well. This
>+ * can be NULL.
>+ * \param p_opaque An opaque context that will be owned by the
>instance
>+ * holder. It is expected to be release by
>pf_destructor.
>+ * This can be NULL.
>+ */
>+VLC_API void* vlc_instance_holder_acquire( vlc_object_t* p_obj,
>+ const char* psz_name, void* (*pf_constructor)( void*
>),
>+ void (*pf_destructor)( void*, void* ), void* p_opaque
>);
>+
>+/**
>+ * Return an existing instance, or NULL.
>+ *
>+ * This is equivalent to calling vlc_instance_holder_acquire with a
>+ * NULL constructor, a NULL destructor, and a NULL context.
>+ * This is guaranteed not to create a new instance.
>+ * If an instance is returned, its refcount will be incremented.
>+ * \param p_obj A vlc_object_t
>+ * \param psz_name The instance name
>+ */
>+VLC_API void* vlc_instance_holder_get( vlc_object_t* p_obj,
>+ const char* psz_name );
>+
>+/**
>+ * Decrease the instance refcount.
>+ *
>+ * When the refcount reaches 0, the destructor provided upon first
>creation will
>+ * be invoked.
>+ */
>+VLC_API void vlc_instance_holder_release( vlc_object_t* p_obj,
>+ const char*
>psz_name );
>+
>+int libvlc_InstanceHolderCreate( libvlc_int_t* p_libvlc );
>+void libvlc_InstanceHolderRelease( libvlc_int_t* p_libvlc );
>+
>+#endif // VLC_INSTANCE_HOLDER_H
>diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>index 960d458bb2..55b5753234 100644
>--- a/include/vlc_threads.h
>+++ b/include/vlc_threads.h
>@@ -1053,6 +1053,7 @@ enum
> VLC_XLIB_MUTEX,
> VLC_MOSAIC_MUTEX,
> VLC_HIGHLIGHT_MUTEX,
>+ VLC_INSTANCE_HOLDER_MUTEX,
> #ifdef _WIN32
> VLC_MTA_MUTEX,
> #endif
>diff --git a/src/Makefile.am b/src/Makefile.am
>index c93a4021ee..24970b0bec 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -57,6 +57,7 @@ pluginsinclude_HEADERS = \
> ../include/vlc_inhibit.h \
> ../include/vlc_input.h \
> ../include/vlc_input_item.h \
>+ ../include/vlc_instance_holder.h \
> ../include/vlc_interface.h \
> ../include/vlc_keys.h \
> ../include/vlc_keystore.h \
>@@ -348,7 +349,8 @@ libvlccore_la_SOURCES = \
> misc/fingerprinter.c \
> misc/text_style.c \
> misc/subpicture.c \
>- misc/subpicture.h
>+ misc/subpicture.h \
>+ misc/instance_holder.c
> libvlccore_la_LIBADD = $(LIBS_libvlccore) \
> ../compat/libcompat.la \
> $(LTLIBINTL) $(LTLIBICONV) \
>diff --git a/src/libvlc.c b/src/libvlc.c
>index c7e7b9013d..46e97cfd04 100644
>--- a/src/libvlc.c
>+++ b/src/libvlc.c
>@@ -61,6 +61,7 @@
> #include <vlc_cpu.h>
> #include <vlc_url.h>
> #include <vlc_modules.h>
>+#include <vlc_instance_holder.h>
>
> #include "libvlc.h"
> #include "playlist/playlist_internal.h"
>@@ -225,6 +226,8 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc,
>int i_argc,
> goto error;
> if( libvlc_InternalKeystoreInit( p_libvlc ) != VLC_SUCCESS )
> msg_Warn( p_libvlc, "memory keystore init failed" );
>+ if ( libvlc_InstanceHolderCreate( p_libvlc ) != VLC_SUCCESS )
>+ goto error;
>
> vlc_CPU_dump( VLC_OBJECT(p_libvlc) );
>
>@@ -390,6 +393,7 @@ void libvlc_InternalCleanup( libvlc_int_t *p_libvlc
>)
> msg_Dbg( p_libvlc, "removing all interfaces" );
> intf_DestroyAll( p_libvlc );
>
>+ libvlc_InstanceHolderRelease( p_libvlc );
> libvlc_InternalDialogClean( p_libvlc );
> libvlc_InternalKeystoreClean( p_libvlc );
>
>diff --git a/src/libvlc.h b/src/libvlc.h
>index 299795c549..4866e0da5f 100644
>--- a/src/libvlc.h
>+++ b/src/libvlc.h
>@@ -177,6 +177,7 @@ void vlc_objres_remove(vlc_object_t *obj, void
>*data,
> */
> typedef struct vlc_dialog_provider vlc_dialog_provider;
> typedef struct vlc_keystore vlc_keystore;
>+typedef struct vlc_instance_holder vlc_instance_holder;
>
> typedef struct libvlc_priv_t
> {
>@@ -190,6 +191,7 @@ typedef struct libvlc_priv_t
> vlm_t *p_vlm; ///< the VLM singleton (or NULL)
> vlc_dialog_provider *p_dialog_provider; ///< dialog provider
> vlc_keystore *p_memory_keystore; ///< memory keystore
>+ vlc_instance_holder *p_instance_holder; ///< Store global opaque
>instances
> struct playlist_t *playlist; ///< Playlist for interfaces
> struct playlist_preparser_t *parser; ///< Input item meta data handler
> struct vlc_actions *actions; ///< Hotkeys handler
>diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>index 58a54bb0f0..2d809d0b39 100644
>--- a/src/libvlccore.sym
>+++ b/src/libvlccore.sym
>@@ -755,3 +755,6 @@ vlc_rd_get_names
> vlc_rd_new
> vlc_rd_release
> vlc_rd_probe_add
>+vlc_instance_holder_acquire
>+vlc_instance_holder_release
>+vlc_instance_holder_get
>diff --git a/src/misc/instance_holder.c b/src/misc/instance_holder.c
>new file mode 100644
>index 0000000000..c236366fcc
>--- /dev/null
>+++ b/src/misc/instance_holder.c
>@@ -0,0 +1,181 @@
>+/*****************************************************************************
>+ * instance_holder.c: Holds a unique refcounted opaque instance.
>+
>*****************************************************************************
>+ * Copyright (C) 2004-2016 VLC authors, VideoLAN and VideoLabs
>+ *
>+ * Authors: Hugo Beauzée-Luyssen <hugo at beauzee.fr>
>+ *
>+ * This program is free software; you can redistribute it and/or
>modify it
>+ * under the terms of the GNU Lesser General Public License as
>published by
>+ * the Free Software Foundation; either version 2.1 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>License
>+ * along with this program; if not, write to the Free Software
>Foundation,
>+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>+
>*****************************************************************************/
>+
>+#ifdef HAVE_CONFIG_H
>+# include "config.h"
>+#endif
>+
>+#include <vlc_instance_holder.h>
>+#include <vlc_threads.h>
>+#include "libvlc.h"
>+
>+#include <assert.h>
>+#ifdef HAVE_SEARCH_H
>+# include <search.h>
>+#endif
>+
>+typedef struct instance
>+{
>+ char* psz_name;
>+ void* p_instance;
>+ void* p_opaque;
>+ unsigned int i_refcount;
>+ void (*pf_destructor)( void*, void* );
>+ vlc_mutex_t lock;
>+} instance;
>+
>+typedef struct vlc_instance_holder
>+{
>+ void* p_root;
>+} vlc_instance_holder;
>+
>+static vlc_instance_holder* instance_holder_create( void )
>+{
>+ vlc_instance_holder* p_holder = malloc( sizeof( *p_holder ) );
>+ if( unlikely( p_holder == NULL ) )
>+ return NULL;
>+ p_holder->p_root = NULL;
>+ return p_holder;
>+}
>+
>+int libvlc_InstanceHolderCreate( libvlc_int_t* p_libvlc )
>+{
>+ libvlc_priv_t* p_priv = libvlc_priv( p_libvlc );
>+ p_priv->p_instance_holder = instance_holder_create();
>+ if( unlikely( p_priv->p_instance_holder == NULL ) )
>+ return VLC_ENOMEM;
>+ return VLC_SUCCESS;
>+}
>+
>+void libvlc_InstanceHolderRelease( libvlc_int_t* p_libvlc )
>+{
>+ libvlc_priv_t* p_priv = libvlc_priv( p_libvlc );
>+ // We expect all instances to have been released when the instance
>holder
>+ // gets released.
>+ assert( p_priv->p_instance_holder->p_root == NULL );
>+ free( p_priv->p_instance_holder );
>+ p_priv->p_instance_holder = NULL;
>+}
>+
>+static int instance_key_compare( const void* l, const void* r )
>+{
>+ const instance* p_left = l, *p_right = r;
>+ return strcmp( p_left->psz_name, p_right->psz_name );
>+}
>+
>+void* vlc_instance_holder_acquire( vlc_object_t* p_obj,
>+ const char* psz_name, void*(*pf_constructor)( void* ),
>+ void(*pf_destructor)( void*, void* ), void* p_opaque )
>+{
>+ vlc_instance_holder* p_holder = libvlc_priv( p_obj->obj.libvlc
>)->p_instance_holder;
>+ assert( p_holder != NULL );
>+ instance* p_instance = malloc( sizeof( *p_instance ) );
>+ if( unlikely( p_instance == NULL ) )
>+ return NULL;
>+ /* Simply copy the pointer to the string until we know if the
>variable
>+ * was created or not: */
>+ p_instance->psz_name = (char*)psz_name;
>+ vlc_global_lock( VLC_INSTANCE_HOLDER_MUTEX );
>+ void** pp_key = tsearch( p_instance, &p_holder->p_root,
>+ &instance_key_compare );
>+ /* If the instance just got created, fully initialize it */
>+ if( pp_key != NULL && *pp_key == p_instance )
>+ {
>+ p_instance->psz_name = strdup( psz_name );
>+ if( p_instance->psz_name != NULL )
>+ {
>+ if ( pf_constructor == NULL )
>+ {
>+ tdelete( p_instance, &p_holder->p_root,
>&instance_key_compare );
>+ free( p_instance );
>+ vlc_global_unlock( VLC_INSTANCE_HOLDER_MUTEX );
>+ return NULL;
>+ }
>+ p_instance->p_instance = pf_constructor( p_opaque );
>+ }
>+ if( p_instance->psz_name == NULL || p_instance->p_instance ==
>NULL )
>+ {
>+ tdelete( p_instance, &p_holder->p_root,
>&instance_key_compare );
>+ free( p_instance );
>+ vlc_global_unlock( VLC_INSTANCE_HOLDER_MUTEX );
>+ return NULL;
>+ }
>+ vlc_mutex_init( &p_instance->lock );
>+ p_instance->i_refcount = 1;
>+ p_instance->pf_destructor = pf_destructor;
>+ p_instance->p_opaque = p_opaque;
>+ }
>+ vlc_global_unlock( VLC_INSTANCE_HOLDER_MUTEX );
>+
>+ // Failed to create a new instance:
>+ if( unlikely( pp_key == NULL ) )
>+ return NULL;
>+ // Got an already existing instance back:
>+ if( *pp_key == p_instance )
>+ return p_instance->p_instance;
>+ // The instance was created:
>+ free( p_instance );
>+ instance* p_existing_instance = (instance*)*pp_key;
>+ vlc_mutex_lock( &p_existing_instance->lock );
>+ p_existing_instance->i_refcount++;
>+ vlc_mutex_unlock( &p_existing_instance->lock );
>+ return p_existing_instance->p_instance;
>+}
>+
>+void* vlc_instance_holder_get( vlc_object_t* p_obj,
>+ const char* psz_name )
>+{
>+ return vlc_instance_holder_acquire( p_obj, psz_name, NULL,
>+ NULL, NULL );
>+}
>+
>+void vlc_instance_holder_release( vlc_object_t* p_obj,
>+ const char *psz_name )
>+{
>+ vlc_instance_holder* p_holder = libvlc_priv( p_obj->obj.libvlc
>)->p_instance_holder;
>+ assert( p_holder != NULL );
>+ instance inst = { .psz_name = (char*)psz_name };
>+ vlc_global_lock( VLC_INSTANCE_HOLDER_MUTEX );
>+ instance** pp_inst = tfind( &inst, &p_holder->p_root,
>&instance_key_compare );
>+ instance* p_inst = NULL;
>+ bool b_delete = false;
>+ if( pp_inst != NULL )
>+ {
>+ p_inst = *pp_inst;
>+ vlc_mutex_lock( &p_inst->lock );
>+ if( --p_inst->i_refcount == 0 )
>+ {
>+ tdelete( p_inst, &p_holder->p_root, &instance_key_compare
>);
>+ b_delete = true;
>+ }
>+ vlc_mutex_unlock( &p_inst->lock );
>+ }
>+ vlc_global_unlock( VLC_INSTANCE_HOLDER_MUTEX );
>+ if( b_delete == true )
>+ {
>+ if( p_inst->pf_destructor != NULL )
>+ p_inst->pf_destructor( p_inst->p_instance,
>p_inst->p_opaque );
>+ free( p_inst->psz_name );
>+ vlc_mutex_destroy( &p_inst->lock );
>+ free( p_inst );
>+ }
>+}
>diff --git a/src/misc/threads.c b/src/misc/threads.c
>index 3d099db6cb..f0d709e300 100644
>--- a/src/misc/threads.c
>+++ b/src/misc/threads.c
>@@ -37,6 +37,7 @@ void vlc_global_mutex (unsigned n, bool acquire)
> VLC_STATIC_MUTEX,
> VLC_STATIC_MUTEX,
> VLC_STATIC_MUTEX,
>+ VLC_STATIC_MUTEX,
> #ifdef _WIN32
> VLC_STATIC_MUTEX, // For MTA holder
> #endif
>--
>2.11.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
In general, singletons should be avoided. And when they are used, they should be static to whatever source module needs them and wrapped with function calls. VDPAU already solves this just as well without leaking anything to the core, for instance. Qt also has quite a few singletons that wouldn't benefit from this.
So I can't really see the point.
Also this will deadlock if a singleton depends on another one.
Furthermore using a name is really pointlessly slow and intricate as opposed to a global or static variable.
And depending on VLC object cannot be right, since there may be multiple independent vlc instances in a process.
--
Rémi Denis-Courmont
Typed on an inconvenient virtual keyboard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170701/258cb3e3/attachment-0001.html>
More information about the vlc-devel
mailing list