[vlc-devel] [PATCH v3 1/3] Introduce media source and media tree API
Romain Vimont
rom1v at videolabs.io
Fri Jun 29 15:57:28 CEST 2018
Thank you for your review. Answers inlined.
On Fri, Jun 29, 2018 at 03:10:11PM +0300, Rémi Denis-Courmont wrote:
> Le jeudi 28 juin 2018, 11:55:30 EEST Romain Vimont a écrit :
> > Add an API to manage "services discovery" out of the playlist.
> >
> > A "media source provider", associated to the libvlc instance, allows to
> > retrieve media sources (each associated to a services discovery module).
> >
> > Requesting a services discovery that is not open will automatically open
> > it. If several "clients" request the same media source (i.e. by
> > requesting the same name), they will receive the same (refcounted) media
> > source instance.
> >
> > As soon as a media source is released by all its clients, the associated
> > services discovery is closed.
> >
> > Each media source holds a media tree, independant of the playlist, used
> > to store the input items detected by the services discovery. Clients may
> > connect callbacks to the tree to be notified of changes.
> > ---
> > include/vlc_media_source.h | 85 ++++++++++
> > include/vlc_media_tree.h | 129 +++++++++++++++
> > include/vlc_services_discovery.h | 2 +
> > src/Makefile.am | 6 +
> > src/input/services_discovery.c | 1 +
> > src/libvlc.c | 9 +
> > src/libvlc.h | 2 +
> > src/libvlccore.sym | 12 ++
> > src/media_source/media_source.c | 257 +++++++++++++++++++++++++++++
> > src/media_source/media_source.h | 29 ++++
> > src/media_tree/media_tree.c | 272 +++++++++++++++++++++++++++++++
> > src/media_tree/media_tree.h | 30 ++++
> > 12 files changed, 834 insertions(+)
> > create mode 100644 include/vlc_media_source.h
> > create mode 100644 include/vlc_media_tree.h
> > create mode 100644 src/media_source/media_source.c
> > create mode 100644 src/media_source/media_source.h
> > create mode 100644 src/media_tree/media_tree.c
> > create mode 100644 src/media_tree/media_tree.h
> >
> > diff --git a/include/vlc_media_source.h b/include/vlc_media_source.h
> > new file mode 100644
> > index 00000000000..d554fed41bf
> > --- /dev/null
> > +++ b/include/vlc_media_source.h
> > @@ -0,0 +1,85 @@
> > +/**************************************************************************
> > *** + * vlc_media_source.h : Media source
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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_MEDIA_SOURCE_H
> > +#define VLC_MEDIA_SOURCE_H
> > +
> > +#include <vlc_common.h>
> > +
> > +typedef struct vlc_media_tree_t vlc_media_tree_t;
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
>
> Please group in Doxygen properly. Ditto below.
OK, I added something like:
/**
* \defgroup media_tree Media tree
* \ingroup input
* @{
*/
/** @} */
Does it look ok to you?
> Also watch out for overly long summary lines.
Which ones? It seems that all the doxygen "comments" fit in 80 chars.
> > +
> > +/**
> > + * Media source.
> > + *
> > + * A media source is associated to a "service discovery". It stores the
> > + * detected media in a media tree.
> > + */
> > +typedef struct vlc_media_source_t
> > +{
> > + vlc_media_tree_t *p_tree;
> > + const char *psz_description;
> > +} vlc_media_source_t;
> > +
> > +/**
> > + * Increase the media source reference count.
> > + */
> > +VLC_API void vlc_media_source_Hold( vlc_media_source_t * );
> > +
> > +/**
> > + * Decrease the media source reference count.
> > + *
> > + * Destroy the media source and close the associated "service discovery" if
> > it + * reaches 0.
> > + */
> > +VLC_API void vlc_media_source_Release( vlc_media_source_t * );
> > +
> > +/**
> > + * Media source provider, used to get media sources.
> > + *
> > + * It's typically used as an opaque pointer.
> > + */
> > +typedef struct vlc_media_source_provider_t
> > +{
> > + struct vlc_common_members obj;
> > + /* all other fields are private */
> > +} vlc_media_source_provider_t;
> > +
> > +/**
> > + * Return the media source provider associated to the libvlc instance.
> > + */
> > +VLC_API vlc_media_source_provider_t *vlc_media_source_provider_Get(
> > libvlc_int_t * ); +
> > +/**
> > + * Return the media source identified by psz_name.
> > + *
> > + * The resulting media source must be released by
> > vlc_media_source_Release(). + */
> > +VLC_API vlc_media_source_t *vlc_media_source_provider_GetMediaSource(
> > vlc_media_source_provider_t *, const char *psz_name ); +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > +
> > diff --git a/include/vlc_media_tree.h b/include/vlc_media_tree.h
> > new file mode 100644
> > index 00000000000..b265060abb5
> > --- /dev/null
> > +++ b/include/vlc_media_tree.h
> > @@ -0,0 +1,129 @@
> > +/**************************************************************************
> > *** + * vlc_media_tree.h : Media tree
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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_MEDIA_TREE_H
> > +#define VLC_MEDIA_TREE_H
> > +
> > +#include <vlc_common.h>
> > +#include <vlc_arrays.h>
> > +#include <vlc_input_item.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * Media tree.
> > + *
> > + * Nodes must be traversed with locked held (vlc_media_tree_Lock()).
> > + */
> > +typedef struct vlc_media_tree_t {
> > + input_item_node_t root;
> > +} vlc_media_tree_t;
> > +
> > +/**
> > + * Opaque type to identify a "listener" connection.
> > + */
> > +typedef struct vlc_media_tree_connection_t vlc_media_tree_connection_t;
> > +
> > +/**
> > + * Callbacks to listen to media tree events.
> > + */
> > +typedef struct vlc_media_tree_callbacks_t
> > +{
> > + /**
> > + * Called on vlc_media_tree_Connect(), with lock held.
> > + *
> > + * Use vlc_media_tree_connect_default implementation to call
> > pf_node_added()
> > + * for every node.
> > + */
> > + void ( *pf_tree_connected )( vlc_media_tree_t *, void *userdata );
> > +
> > + /**
> > + * Called when a new node is added to the media tree, with lock held.
>
> It cannot be called "when" AFAICT. It is either before or after... Please
> clarify. Ditto below.
OK (it's "after" for both cases).
> > + */
> > + void ( *pf_node_added )( vlc_media_tree_t *, const input_item_node_t
> > *p_parent, + const input_item_node_t *, void
> > *userdata ); +
> > + /**
> > + * Called when a node is removed from the media tree, with lock held.
> > + */
> > + void ( *pf_node_removed )( vlc_media_tree_t *, const input_item_node_t
> > *p_parent,
> > + const input_item_node_t *, void
> > *userdata );
>
> While const is semantically correct, won't it prevent some "legitimate" use of
> the objects from the callbacks?
I don't think so. The caller should never write to these nodes directly.
> > +} vlc_media_tree_callbacks_t;
>
> I would argue that pf_ is utmostly redundant in an ops table that by
> definition contains function pointers :/
Yes, but many of these hungarian notation prefixes are useless/redundant
too (TBH, I don't like those prefixes).
So I just use them for consistency.
> > +
> > +/**
> > + * Default implementation for pf_tree_connected(), which calls
> > pf_node_added()
> > + * for every existing node.
> > + **/
> > +VLC_API void vlc_media_tree_connected_default( vlc_media_tree_t *, void
> > *userdata );
> > +
> > +/**
> > + * Increase the media tree reference count.
> > + */
> > +VLC_API void vlc_media_tree_Hold( vlc_media_tree_t * );
>
> Shouldn't that be merged with callback registration (what you call connection,
> IIUC)?
I don't think so, reference counting and listeners are different things.
Of course, adding a listener implies we should hold the object, but not
necessarily the other way around.
> Is there a use case for referencing a tree without registering?
Currently, maybe not, but in theory it may happen (e.g. one-shot
search in the tree).
> > +
> > +/**
> > + * Decrease the media tree reference count.
> > + *
> > + * Destroy the media tree if it reaches 0.
> > + */
> > +VLC_API void vlc_media_tree_Release( vlc_media_tree_t * );
> > +
> > +/**
> > + * Connect callbacks. The lock must NOT be held.
> > + *
> > + * \return a connection to be used in vlc_media_tree_Disconnect()
> > + */
> > +VLC_API vlc_media_tree_connection_t *vlc_media_tree_Connect(
> > vlc_media_tree_t *, const vlc_media_tree_callbacks_t *, void *userdata ); +
> > +/**
> > + * Disconnect callbacks. The lock must NOT be held.
> > + */
> > +VLC_API void vlc_media_tree_Disconnect( vlc_media_tree_t *,
> > vlc_media_tree_connection_t * ); +
> > +/**
> > + * Lock the media tree (non-recursive).
> > + */
> > +VLC_API void vlc_media_tree_Lock( vlc_media_tree_t * );
> > +
> > +/**
> > + * Unlock the media tree.
> > + */
> > +VLC_API void vlc_media_tree_Unlock( vlc_media_tree_t * );
> > +
> > +/**
> > + * Find the node containing the requested input item (and its parent).
> > + *
> > + * \param pp_result point to the matching node if the function returns true
> > + * [OUT]
> > + * \param pp_result_parent if not NULL, point to the matching node parent
> > + * if the function returns true [OUT]
> > + *
> > + * \return true if found, false otherwise.
> > + */
> > +VLC_API bool vlc_media_tree_Find( vlc_media_tree_t *, const input_item_t *,
> > + input_item_node_t **pp_result,
> > input_item_node_t **pp_result_parent ); +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/include/vlc_services_discovery.h
> > b/include/vlc_services_discovery.h index 1f5305b788e..e117a1c0a52 100644
> > --- a/include/vlc_services_discovery.h
> > +++ b/include/vlc_services_discovery.h
> > @@ -149,6 +149,8 @@ VLC_API char ** vlc_sd_GetNames( vlc_object_t *, char
> > ***, int ** ) VLC_USED; VLC_API services_discovery_t
> > *vlc_sd_Create(vlc_object_t *parent, const char *chain, const struct
> > services_discovery_owner_t *owner) VLC_USED;
> > +#define vlc_sd_Create( obj, a, b ) \
> > + vlc_sd_Create( VLC_OBJECT( obj ), a, b )
> >
> > VLC_API void vlc_sd_Destroy( services_discovery_t * );
> >
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 897c557e55d..76fe6dff0a9 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -62,10 +62,12 @@ pluginsinclude_HEADERS = \
> > ../include/vlc_keystore.h \
> > ../include/vlc_list.h \
> > ../include/vlc_md5.h \
> > + ../include/vlc_media_source.h \
> > ../include/vlc_messages.h \
> > ../include/vlc_meta.h \
> > ../include/vlc_meta_fetcher.h \
> > ../include/vlc_media_library.h \
> > + ../include/vlc_media_tree.h \
> > ../include/vlc_memstream.h \
> > ../include/vlc_mime.h \
> > ../include/vlc_modules.h \
> > @@ -203,6 +205,10 @@ libvlccore_la_SOURCES = \
> > config/getopt.c \
> > config/vlc_getopt.h \
> > extras/libc.c \
> > + media_source/media_source.c \
> > + media_source/media_source.h \
> > + media_tree/media_tree.c \
> > + media_tree/media_tree.h \
> > modules/modules.h \
> > modules/modules.c \
> > modules/bank.c \
> > diff --git a/src/input/services_discovery.c b/src/input/services_discovery.c
> > index 12a029ef6e6..7d43e4ee7ab 100644
> > --- a/src/input/services_discovery.c
> > +++ b/src/input/services_discovery.c
> > @@ -103,6 +103,7 @@ char **vlc_sd_GetNames (vlc_object_t *obj, char
> > ***pppsz_longnames, int **pp_cat * That's how the playlist get's Service
> > Discovery information
> > */
> >
> > +#undef vlc_sd_Create
> > services_discovery_t *vlc_sd_Create(vlc_object_t *parent, const char *cfg,
> > const struct services_discovery_owner_t *restrict owner)
> > {
> > diff --git a/src/libvlc.c b/src/libvlc.c
> > index cbb4cd5a65c..483827f5a39 100644
> > --- a/src/libvlc.c
> > +++ b/src/libvlc.c
> > @@ -43,6 +43,7 @@
> > #include "modules/modules.h"
> > #include "config/configuration.h"
> > #include "playlist/preparser.h"
> > +#include "media_source/media_source.h"
> >
> > #include <stdio.h> /*
> > sprintf() */ #include <string.h>
> > @@ -93,6 +94,7 @@ libvlc_int_t * libvlc_InternalCreate( void )
> > priv = libvlc_priv (p_libvlc);
> > priv->playlist = NULL;
> > priv->p_vlm = NULL;
> > + priv->p_media_source_provider = NULL;
> >
> > vlc_ExitInit( &priv->exit );
> >
> > @@ -230,6 +232,10 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int
> > i_argc, if( !priv->parser )
> > goto error;
> >
> > + priv->p_media_source_provider = vlc_media_source_provider_Create(
> > VLC_OBJECT( p_libvlc ) ); + if( !priv->p_media_source_provider )
> > + goto error;
> > +
> > /* variables for signalling creation of new files */
> > var_Create( p_libvlc, "snapshot-file", VLC_VAR_STRING );
> > var_Create( p_libvlc, "record-file", VLC_VAR_STRING );
> > @@ -363,6 +369,9 @@ void libvlc_InternalCleanup( libvlc_int_t *p_libvlc )
> > msg_Dbg( p_libvlc, "removing all interfaces" );
> > intf_DestroyAll( p_libvlc );
> >
> > + if( priv->p_media_source_provider )
> > + vlc_media_source_provider_Destroy( priv->p_media_source_provider );
> > +
> > libvlc_InternalDialogClean( p_libvlc );
> > libvlc_InternalKeystoreClean( p_libvlc );
> >
> > diff --git a/src/libvlc.h b/src/libvlc.h
> > index a8f50404b25..b1f29b72786 100644
> > --- a/src/libvlc.h
> > +++ b/src/libvlc.h
> > @@ -172,6 +172,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_actions_t vlc_actions_t;
> > +typedef struct vlc_media_source_provider_t vlc_media_source_provider_t;
> >
> > typedef struct libvlc_priv_t
> > {
> > @@ -184,6 +185,7 @@ typedef struct libvlc_priv_t
> > vlc_keystore *p_memory_keystore; ///< memory keystore
> > struct playlist_t *playlist; ///< Playlist for interfaces
> > struct playlist_preparser_t *parser; ///< Input item meta data handler
> > + vlc_media_source_provider_t *p_media_source_provider;
> > vlc_actions_t *actions; ///< Hotkeys handler
> >
> > /* Exit callback */
> > diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> > index 0378c96e1bf..0a658ef22b2 100644
> > --- a/src/libvlccore.sym
> > +++ b/src/libvlccore.sym
> > @@ -752,3 +752,15 @@ vlc_rd_get_names
> > vlc_rd_new
> > vlc_rd_release
> > vlc_rd_probe_add
> > +vlc_media_source_Hold
> > +vlc_media_source_Release
> > +vlc_media_source_provider_Get
> > +vlc_media_source_provider_GetMediaSource
> > +vlc_media_tree_Hold
> > +vlc_media_tree_Release
> > +vlc_media_tree_Connect
> > +vlc_media_tree_Disconnect
> > +vlc_media_tree_Lock
> > +vlc_media_tree_Unlock
> > +vlc_media_tree_Find
> > +vlc_media_tree_connected_default
> > diff --git a/src/media_source/media_source.c
> > b/src/media_source/media_source.c new file mode 100644
> > index 00000000000..14a9e16dddb
> > --- /dev/null
> > +++ b/src/media_source/media_source.c
> > @@ -0,0 +1,257 @@
> > +/**************************************************************************
> > *** + * media_source.c : Media source
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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. +
> > ***************************************************************************
> > **/ +
> > +#include "media_source.h"
> > +
> > +#ifdef HAVE_CONFIG_H
> > +# include "config.h"
> > +#endif
> > +
> > +#include <assert.h>
> > +#include <stdatomic.h>
> > +#include <vlc_media_tree.h>
> > +#include <vlc_playlist.h>
> > +#include <vlc_services_discovery.h>
> > +#include "libvlc.h"
> > +#include "playlist/playlist_internal.h"
> > +#include "media_tree/media_tree.h"
> > +
> > +typedef struct
> > +{
> > + vlc_media_source_t public_data;
> > +
> > + services_discovery_t *p_sd;
> > + atomic_uint refs;
> > + vlc_media_source_provider_t *p_owner;
> > + struct vlc_list siblings;
> > + char psz_name[];
> > +} media_source_private_t;
> > +
> > +#define ms_priv( ms ) container_of( ms, media_source_private_t, public_data
> > ) +
> > +typedef struct
> > +{
> > + vlc_media_source_provider_t public_data;
> > +
> > + vlc_mutex_t lock;
> > + struct vlc_list media_sources;
> > +} media_source_provider_private_t;
> > +
> > +#define msp_priv( msp ) container_of( msp, media_source_provider_private_t,
> > public_data ) +
> > +/* A new item has been added to a certain services discovery */
> > +static void services_discovery_item_added( services_discovery_t *p_sd,
> > + input_item_t *p_parent,
> > input_item_t *p_input, + const
> > char *psz_cat )
> > +{
> > + assert( !p_parent || !psz_cat );
> > + VLC_UNUSED( psz_cat );
> > +
> > + vlc_media_source_t *p_ms = p_sd->owner.sys;
> > + vlc_media_tree_t *p_tree = p_ms->p_tree;
> > +
> > + msg_Dbg( p_sd, "adding: %s", p_input->psz_name ? p_input->psz_name :
> > "(null)" ); +
> > + vlc_media_tree_Lock( p_tree );
> > +
> > + input_item_node_t *p_parent_node;
> > + if( p_parent )
> > + vlc_media_tree_Find( p_tree, p_parent, &p_parent_node, NULL );
> > + else
> > + p_parent_node = &p_tree->root;
> > +
> > + vlc_media_tree_Add( p_tree, p_parent_node, p_input );
> > +
> > + vlc_media_tree_Unlock( p_tree );
> > +}
> > +
> > +static void services_discovery_item_removed( services_discovery_t *p_sd,
> > input_item_t *p_input ) +{
> > + vlc_media_source_t *p_ms = p_sd->owner.sys;
> > + vlc_media_tree_t *p_tree = p_ms->p_tree;
> > +
> > + msg_Dbg( p_sd, "removing: %s", p_input->psz_name ? p_input->psz_name :
> > "(null)" ); +
> > + vlc_media_tree_Lock( p_tree );
> > + bool removed = vlc_media_tree_Remove( p_tree, p_input );
> > + vlc_media_tree_Unlock( p_tree );
> > +
> > + if( unlikely( !removed) )
> > + {
> > + msg_Err( p_sd, "removing item not added"); /* SD plugin bug */
> > + return;
> > + }
> > +}
> > +
> > +static const struct services_discovery_callbacks
> > media_source_provider_sd_cbs = { + .item_added =
> > services_discovery_item_added,
> > + .item_removed = services_discovery_item_removed,
> > +};
> > +
> > +static inline void AssertLocked( vlc_media_source_provider_t *p_msp )
> > +{
> > + media_source_provider_private_t *p_priv = msp_priv( p_msp );
> > + vlc_assert_locked( &p_priv->lock );
> > +}
> > +
> > +static vlc_media_source_t *MediaSourceCreate( vlc_media_source_provider_t
> > *p_msp, const char *psz_name ) +{
> > + media_source_private_t *p_priv = malloc( sizeof( *p_priv ) + strlen(
> > psz_name ) + 1 ); + if( unlikely( !p_priv ) )
> > + return NULL;
> > +
> > + atomic_init( &p_priv->refs, 1 );
> > +
> > + vlc_media_source_t *p_ms = &p_priv->public_data;
> > +
> > + /* vlc_sd_Create() may call services_discovery_item_added(), which will
> > read its + * p_tree, so it must be initialized first */
> > + p_ms->p_tree = vlc_media_tree_Create();
> > + if( unlikely( !p_ms->p_tree ) )
> > + {
> > + free( p_ms );
> > + return NULL;
> > + }
> > +
> > + strcpy( p_priv->psz_name, psz_name );
> > +
> > + struct services_discovery_owner_t owner = {
> > + .cbs = &media_source_provider_sd_cbs,
> > + .sys = p_ms,
> > + };
> > +
> > + p_priv->p_sd = vlc_sd_Create( p_msp, psz_name, &owner );
> > + if( unlikely( !p_priv->p_sd ) )
> > + {
> > + vlc_media_tree_Release( p_ms->p_tree );
> > + free( p_ms );
> > + return NULL;
> > + }
> > +
> > + /* p_sd->description is set during vlc_sd_Create() */
> > + p_ms->psz_description = p_priv->p_sd->description;
> > +
> > + p_priv->p_owner = p_msp;
> > +
> > + return p_ms;
> > +}
> > +
> > +static void Remove( vlc_media_source_provider_t *, vlc_media_source_t * );
> > +
> > +static void MediaSourceDestroy( vlc_media_source_t *p_ms )
> > +{
> > + media_source_private_t *p_priv = ms_priv( p_ms );
> > + Remove( p_priv->p_owner, p_ms );
> > + vlc_sd_Destroy( p_priv->p_sd );
> > + vlc_media_tree_Release( p_ms->p_tree );
> > + free( p_priv );
> > +}
> > +
> > +void vlc_media_source_Hold( vlc_media_source_t *p_ms )
> > +{
> > + media_source_private_t *p_priv = ms_priv( p_ms );
> > + atomic_fetch_add( &p_priv->refs, 1 );
> > +}
> > +
> > +void vlc_media_source_Release( vlc_media_source_t *p_ms )
> > +{
> > + media_source_private_t *p_priv = ms_priv( p_ms );
> > + if( atomic_fetch_sub( &p_priv->refs, 1 ) == 1 )
> > + MediaSourceDestroy( p_ms );
> > +}
> > +
> > +static vlc_media_source_t *FindByName( media_source_provider_private_t
> > *p_priv, const char *psz_name ) +{
> > + vlc_assert_locked( &p_priv->lock );
> > + media_source_private_t *p_entry;
> > + vlc_list_foreach( p_entry, &p_priv->media_sources, siblings )
> > + if( !strcmp( psz_name, p_entry->psz_name ) )
> > + return &p_entry->public_data;
> > + return NULL;
> > +}
> > +
> > +vlc_media_source_provider_t *vlc_media_source_provider_Get( libvlc_int_t
> > *libvlc ) +{
> > + return libvlc_priv( libvlc )->p_media_source_provider;
> > +}
> > +
> > +vlc_media_source_provider_t *vlc_media_source_provider_Create( vlc_object_t
> > *p_parent ) +{
> > + media_source_provider_private_t *p_priv = vlc_custom_create( p_parent,
> > sizeof( *p_priv ), "media-source-provider" ); + if( unlikely( !p_priv )
> > )
> > + return NULL;
> > +
> > + vlc_mutex_init( &p_priv->lock );
> > + vlc_list_init( &p_priv->media_sources );
> > + return &p_priv->public_data;
> > +}
> > +
> > +void vlc_media_source_provider_Destroy( vlc_media_source_provider_t *p_msp
> > ) +{
> > + media_source_provider_private_t *p_priv = msp_priv( p_msp );
> > +
> > + vlc_mutex_destroy( &p_priv->lock );
> > + vlc_object_release( p_msp );
> > +}
> > +
> > +static vlc_media_source_t *AddServiceDiscovery( vlc_media_source_provider_t
> > *p_msp, const char *psz_name ) +{
> > + AssertLocked( p_msp );
> > +
> > + vlc_media_source_t *p_ms = MediaSourceCreate( p_msp, psz_name );
> > + if( unlikely( !p_ms ) )
> > + return NULL;
> > +
> > + media_source_provider_private_t *p_priv = msp_priv( p_msp );
> > +
> > + vlc_list_append( &ms_priv( p_ms )->siblings, &p_priv->media_sources );
> > + return p_ms;
> > +}
> > +
> > +vlc_media_source_t *vlc_media_source_provider_GetMediaSource(
> > vlc_media_source_provider_t *p_msp, const char *psz_name ) +{
> > + media_source_provider_private_t *p_priv = msp_priv( p_msp );
> > +
> > + vlc_mutex_lock( &p_priv->lock );
> > +
> > + vlc_media_source_t *p_ms = FindByName( p_priv, psz_name );
> > + if( !p_ms )
> > + {
> > + p_ms = AddServiceDiscovery( p_msp, psz_name );
> > + if( unlikely( !p_ms ) )
> > + {
> > + vlc_mutex_unlock( &p_priv->lock );
> > + return NULL;
> > + }
> > + }
> > +
> > + vlc_mutex_unlock( &p_priv->lock );
> > +
> > + return p_ms;
> > +}
> > +
> > +static void Remove( vlc_media_source_provider_t *p_msp, vlc_media_source_t
> > *p_ms ) +{
> > + media_source_provider_private_t *p_priv = msp_priv( p_msp );
> > +
> > + vlc_mutex_lock( &p_priv->lock );
> > + vlc_list_remove( &ms_priv( p_ms )->siblings );
> > + vlc_mutex_unlock( &p_priv->lock );
> > +}
> > diff --git a/src/media_source/media_source.h
> > b/src/media_source/media_source.h new file mode 100644
> > index 00000000000..821d0a8bd9c
> > --- /dev/null
> > +++ b/src/media_source/media_source.h
> > @@ -0,0 +1,29 @@
> > +/**************************************************************************
> > *** + * media_source.h : Media source
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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 _MEDIA_SOURCE_H
> > +#define _MEDIA_SOURCE_H
> > +
> > +#include <vlc_media_source.h>
> > +
> > +vlc_media_source_provider_t *vlc_media_source_provider_Create( vlc_object_t
> > *p_parent ); +void vlc_media_source_provider_Destroy(
> > vlc_media_source_provider_t * ); +
> > +#endif
> > diff --git a/src/media_tree/media_tree.c b/src/media_tree/media_tree.c
> > new file mode 100644
> > index 00000000000..3cedddb8085
> > --- /dev/null
> > +++ b/src/media_tree/media_tree.c
> > @@ -0,0 +1,272 @@
> > +/**************************************************************************
> > *** + * media_tree.c : Media tree
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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. +
> > ***************************************************************************
> > **/ +
> > +#include "media_tree.h"
> > +
> > +#include <assert.h>
> > +#include <stdatomic.h>
> > +#include <vlc_common.h>
> > +#include <vlc_input_item.h>
> > +#include <vlc_threads.h>
> > +#include "libvlc.h"
> > +
> > +struct vlc_media_tree_connection_t
> > +{
> > + const vlc_media_tree_callbacks_t *cbs;
> > + void *userdata;
> > + struct vlc_list siblings;
> > +};
> > +
> > +typedef struct
> > +{
> > + vlc_media_tree_t public_data;
> > +
> > + struct vlc_list connections;
> > + vlc_mutex_t lock;
> > + atomic_uint refs;
> > +} media_tree_private_t;
> > +
> > +#define mt_priv( mt ) container_of( mt, media_tree_private_t, public_data
> > ); +
> > +vlc_media_tree_t *vlc_media_tree_Create( void )
> > +{
> > + media_tree_private_t *p_priv = malloc( sizeof( *p_priv ) );
> > + if( unlikely( !p_priv ) )
> > + return NULL;
> > +
> > + vlc_mutex_init( &p_priv->lock );
> > + atomic_init( &p_priv->refs, 1 );
> > + vlc_list_init( &p_priv->connections );
> > +
> > + vlc_media_tree_t *p_tree = &p_priv->public_data;
> > + input_item_node_t *p_root = &p_tree->root;
> > + p_root->p_item = NULL;
> > + TAB_INIT( p_root->i_children, p_root->pp_children );
> > +
> > + return p_tree;
> > +}
> > +
> > +static inline void AssertLocked( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_assert_locked( &p_priv->lock );
> > +}
> > +
> > +static void NotifyTreeConnected( vlc_media_tree_t *p_tree )
> > +{
> > + AssertLocked( p_tree );
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
>
> Let the code breath. empty line is customary here for readability.
OK.
> > + vlc_media_tree_connection_t *p_conn;
> > + vlc_list_foreach( p_conn, &p_priv->connections, siblings )
> > + if( p_conn->cbs->pf_tree_connected )
> > + p_conn->cbs->pf_tree_connected( p_tree, p_conn->userdata );
> > +}
> > +
> > +static void NotifyNodeAdded( vlc_media_tree_t *p_tree, const
> > input_item_node_t *p_parent, + const
> > input_item_node_t *p_node )
> > +{
> > + AssertLocked( p_tree );
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_media_tree_connection_t *p_conn;
> > + vlc_list_foreach( p_conn, &p_priv->connections, siblings )
> > + if( p_conn->cbs->pf_node_added )
> > + p_conn->cbs->pf_node_added( p_tree, p_parent, p_node,
> > p_conn->userdata ); +}
> > +
> > +static void NotifyNodeRemoved( vlc_media_tree_t *p_tree, const
> > input_item_node_t *p_parent, + const
> > input_item_node_t *p_node )
> > +{
> > + AssertLocked( p_tree );
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_media_tree_connection_t *p_conn;
> > + vlc_list_foreach( p_conn, &p_priv->connections, siblings )
> > + if( p_conn->cbs->pf_node_removed )
> > + p_conn->cbs->pf_node_removed( p_tree, p_parent, p_node,
> > p_conn->userdata ); +}
> > +
> > +static bool FindNodeByInput( input_item_node_t *p_parent, const
> > input_item_t *p_input, + input_item_node_t
> > **pp_result, input_item_node_t **pp_result_parent ) +{
> > + for( int i = 0; i < p_parent->i_children; ++i )
> > + {
> > + input_item_node_t *p_child = p_parent->pp_children[i];
> > + if( p_child->p_item == p_input )
> > + {
> > + *pp_result = p_child;
> > + if( pp_result_parent )
> > + *pp_result_parent = p_parent;
> > + return true;
> > + }
> > +
> > + if( FindNodeByInput( p_child, p_input, pp_result, pp_result_parent
> > ) ) + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static void DestroyRootNode( vlc_media_tree_t *p_tree )
> > +{
> > + input_item_node_t *p_root = &p_tree->root;
> > + for( int i = 0; i < p_root->i_children; ++i )
> > + input_item_node_Delete( p_root->pp_children[i] );
> > +
> > + free( p_root->pp_children );
> > +}
> > +
> > +static void Destroy( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_media_tree_connection_t *p_conn;
> > + vlc_list_foreach( p_conn, &p_priv->connections, siblings )
> > + free( p_conn );
> > + vlc_list_init( &p_priv->connections ); /* reset */
> > + DestroyRootNode( p_tree );
> > + vlc_mutex_destroy( &p_priv->lock );
> > + free( p_tree );
> > +}
> > +
> > +void vlc_media_tree_Hold( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + atomic_fetch_add( &p_priv->refs, 1 );
>
> Could assert that refs was not zero.
>
> Also could relax memory ordering but whatever.
I changed to:
atomic_fetch_add_explicit( &p_priv->refs, 1, memory_order_relaxed );
and:
if( atomic_fetch_sub_explicit( &p_priv->refs, 1, memory_order_acq_rel ) == 1 )
I think we should add a vlc_refcounter_t or similar, to provide a simple
refcount interface which hides these non-trivial memory order details to
the caller (and add the assertion you suggest).
> > +}
> > +
> > +void vlc_media_tree_Release( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + if( atomic_fetch_sub( &p_priv->refs, 1 ) == 1 )
> > + Destroy( p_tree );
> > +}
> > +
> > +void vlc_media_tree_Lock( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_mutex_lock( &p_priv->lock );
> > +}
> > +
> > +void vlc_media_tree_Unlock( vlc_media_tree_t *p_tree )
> > +{
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_mutex_unlock( &p_priv->lock );
> > +}
> > +
> > +static input_item_node_t *AddChild( input_item_node_t *p_parent,
> > input_item_t *p_input ) +{
> > + input_item_node_t *p_node = input_item_node_Create( p_input );
> > + if( unlikely( !p_node ) )
> > + return NULL;
> > +
> > + input_item_node_AppendNode( p_parent, p_node );
> > +
> > + return p_node;
> > +}
> > +
> > +static void NotifyChildren( vlc_media_tree_t *p_tree, const
> > input_item_node_t *p_node, + const
> > vlc_media_tree_connection_t *p_conn ) +{
> > + AssertLocked( p_tree );
> > + for( int i = 0; i < p_node->i_children; ++i )
> > + {
> > + input_item_node_t *p_child = p_node->pp_children[i];
> > + p_conn->cbs->pf_node_added( p_tree, p_node, p_child,
> > p_conn->userdata );
> > + NotifyChildren( p_tree, p_child, p_conn );
> > + }
> > +}
> > +
> > +void vlc_media_tree_connected_default( vlc_media_tree_t *p_tree, void
> > *userdata ) +{
> > + VLC_UNUSED( userdata );
> > + AssertLocked( p_tree );
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_media_tree_connection_t *p_conn;
> > + vlc_list_foreach( p_conn, &p_priv->connections, siblings )
> > + {
> > + if( !p_conn->cbs->pf_node_added)
> > + break; /* nothing to do for this listener */
>
> Did you mean 'continue' ?
Wow, of course :) Thank you for your vigilance.
(I reversed the condition instead).
> > + /* notify "node added" for every node */
> > + NotifyChildren( p_tree, &p_tree->root, p_conn );
> > + }
> > +}
> > +
> > +vlc_media_tree_connection_t *vlc_media_tree_Connect( vlc_media_tree_t
> > *p_tree, const vlc_media_tree_callbacks_t *p_callbacks, void *userdata ) +{
> > + vlc_media_tree_connection_t *p_conn = malloc( sizeof( *p_conn ) );
> > + if( !p_conn )
> > + return NULL;
> > + p_conn->cbs = p_callbacks;
> > + p_conn->userdata = userdata;
> > +
> > + media_tree_private_t *p_priv = mt_priv( p_tree );
> > + vlc_media_tree_Lock( p_tree );
> > + vlc_list_append( &p_conn->siblings, &p_priv->connections );
> > + NotifyTreeConnected( p_tree );
> > + vlc_media_tree_Unlock( p_tree );
> > +
> > + return p_conn;
> > +}
>
> Maybe expose the structure layout to registrant and make this fail-free?
It is expected to be an opaque pointer.
And it would only move the problem to the caller.
> > +
> > +void vlc_media_tree_Disconnect( vlc_media_tree_t *p_tree,
> > vlc_media_tree_connection_t *p_connection ) +{
> > + vlc_media_tree_Lock( p_tree );
> > + vlc_list_remove( &p_connection->siblings );
> > + vlc_media_tree_Unlock( p_tree );
> > +
> > + free( p_connection );
> > +}
> > +
> > +input_item_node_t *vlc_media_tree_Add( vlc_media_tree_t *p_tree,
> > + input_item_node_t *p_parent,
> > + input_item_t *p_input )
> > +{
> > + AssertLocked( p_tree );
> > +
> > + input_item_node_t *p_node = AddChild( p_parent, p_input );
> > + if( unlikely( !p_node ) )
> > + return NULL;
> > +
> > + NotifyNodeAdded( p_tree, p_parent, p_node );
> > +
> > + return p_node;
> > +}
> > +
> > +bool vlc_media_tree_Find( vlc_media_tree_t *p_tree, const input_item_t
> > *p_input, + input_item_node_t **pp_result,
> > input_item_node_t **pp_result_parent ) +{
> > + AssertLocked( p_tree );
> > +
> > + /* quick & dirty depth-first O(n) implementation, with n the number of
> > nodes in the tree */ + return FindNodeByInput( &p_tree->root, p_input,
> > pp_result, pp_result_parent ); +}
> > +
> > +bool vlc_media_tree_Remove( vlc_media_tree_t *p_tree, input_item_t *p_input
> > ) +{
> > + AssertLocked( p_tree );
> > +
> > + input_item_node_t *p_node;
> > + input_item_node_t *p_parent;
> > + if( !FindNodeByInput( &p_tree->root, p_input, &p_node, &p_parent ) )
> > + return false;
> > +
> > + input_item_node_RemoveNode( p_parent, p_node );
> > + NotifyNodeRemoved( p_tree, p_parent, p_node );
> > + input_item_node_Delete( p_node );
> > + return true;
> > +}
> > diff --git a/src/media_tree/media_tree.h b/src/media_tree/media_tree.h
> > new file mode 100644
> > index 00000000000..8f827350453
> > --- /dev/null
> > +++ b/src/media_tree/media_tree.h
> > @@ -0,0 +1,30 @@
> > +/**************************************************************************
> > *** + * media_tree.h : Media tree
> > +
> > ***************************************************************************
> > ** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * 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 _MEDIA_TREE_H
> > +#define _MEDIA_TREE_H
> > +
> > +#include <vlc_media_tree.h>
> > +
> > +vlc_media_tree_t *vlc_media_tree_Create( void );
> > +input_item_node_t *vlc_media_tree_Add( vlc_media_tree_t *,
> > input_item_node_t *p_parent, input_item_t * ); +bool vlc_media_tree_Remove(
> > vlc_media_tree_t *, input_item_t *p_input ); +
> > +#endif
>
>
> --
> Rémi Denis-Courmont
More information about the vlc-devel
mailing list