[vlc-devel] [PATCH 1/9] core: Add a services advertisement API
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Tue Sep 8 13:44:33 CEST 2020
On Fri, Sep 4, 2020, at 3:44 PM, Rémi Denis-Courmont wrote:
> Le keskiviikkona 2. syyskuuta 2020, 16.47.23 EEST Hugo Beauzée-Luyssen a écrit
> :
> > Refs #18090
> >
> > Co-authored-by: Roland Bewick <roland.bewick at gmail.com>
> > ---
> > include/vlc_services_advertisement.h | 119 +++++++++++++++++
> > src/Makefile.am | 2 +
> > src/libvlc.c | 8 ++
> > src/libvlc.h | 2 +
> > src/libvlccore.sym | 11 ++
> > src/misc/services_advertisement.c | 192 +++++++++++++++++++++++++++
> > 6 files changed, 334 insertions(+)
> > create mode 100644 include/vlc_services_advertisement.h
> > create mode 100644 src/misc/services_advertisement.c
> >
> > diff --git a/include/vlc_services_advertisement.h
> > b/include/vlc_services_advertisement.h new file mode 100644
> > index 0000000000..cf713d017a
> > --- /dev/null
> > +++ b/include/vlc_services_advertisement.h
> > @@ -0,0 +1,119 @@
> > +/**************************************************************************
> > *** + * vlc_services_advertisement.h : Services Advertisement functions +
> > ***************************************************************************
> > ** + * Copyright (C) 1999-2019 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_SERVICES_ADVERTISEMENT_H_
> > +#define VLC_SERVICES_ADVERTISEMENT_H_
> > +
> > +#include <vlc_common.h>
> > +
> > +/**
> > + * \file
> > + * This file lists functions and structures for service advertisement (SA)
> > in vlc
> > + */
>
> Do all those Doxygen comment lay out nicely?
> They look a little suspiciously long for first lines.
>
I'll update those
> > +
> > +# ifdef __cplusplus
> > +extern "C" {
> > +# endif
> > +
> > +/**
> > + * @{
> > + */
> > +
> > +typedef struct services_advertisement_entry_t
> > services_advertisement_entry_t; +
> > +/**
> > + * Main service advertisement structure to build a SA module
> > + */
> > +typedef struct services_advertisement_t
> > +{
> > + struct vlc_object_t obj;
> > +
> > + module_t * p_module; /**< Loaded module */
> > +
> > + char *psz_name; /**< Main name of the SA */
> > + config_chain_t *p_cfg; /**< Configuration for the SA
> > */ +
> > + const char *description; /**< Human-readable name */
> > +
> > + void *p_sys; /**< Custom private data */
> > +
> > + int (*pf_announce)( struct services_advertisement_t*,
> > + services_advertisement_entry_t* );
> > + int (*pf_conceal)( struct services_advertisement_t*,
> > + services_advertisement_entry_t* );
> > +} services_advertisement_t;
>
> I would expect new plugin type to follow new conventions, in particular pass
> close and other callbacks via a pointer to const structure.
>
Indeed, fixed locally
> > +
> > +/***********************************************************************
> > + * Service Advertisement
> > + ***********************************************************************/
> > +
> > +/**
> > + * Stops and unloads services advertisement module if it was loaded.
> > + *
> > + * @param libvlc the LibVLC instance
> > + */
> > +void vlc_sa_Destroy( libvlc_int_t *libvlc );
> > +
> > +/**
> > + * @brief vlc_sa_Announce Announces a new service
> > + * @param libvlc The LibVLC instance
> > + * @param entry The entry to announce
> > + * @return VLC_SUCCESS or an error code
> > + */
> > +VLC_API int vlc_sa_Announce( libvlc_int_t *libvlc,
> > services_advertisement_entry_t* entry );
> > +
> > +/**
> > + * @brief vlc_sa_Conceal Conceals an announced service
> > + * @param libvlc The LibVLC instance
> > + * @param entry The entry to conceal
> > + * @return VLC_SUCCESS or an error code
> > + */
> > +VLC_API int vlc_sa_Conceal( libvlc_int_t *libvlc,
> > services_advertisement_entry_t* entry );
> > +
> > +/**
> > + * @brief vlc_sa_entry_New Creates a new service advertisement entry
> > + * @param name The service name
> > + * @param type The service type, eg. _http._tcp (without the .local suffix)
> > + * @param port The port this service runs on
> > + *
> > + * @return A services_advertisement_entry_t on success or NULL in case of
> > error + */
> > +VLC_API services_advertisement_entry_t*
> > +vlc_sa_entry_New( const char* name, const char* type, uint16_t port );
>
> This has the same problem as the existing SA plugin API:
> It's totally hard-wired to a specific advertisement protocol...
>
> And it's not even up-front about it. At least the other one does say 'SDP'.
>
I renamed to vlc_mdns_sa_*
> > +
> > +VLC_API services_advertisement_entry_t*
> > +vlc_sa_entry_Hold( services_advertisement_entry_t *entry );
> > +VLC_API void vlc_sa_entry_Release( services_advertisement_entry_t *entry );
> > +
> > +VLC_API const char* vlc_sa_entry_Name( services_advertisement_entry_t
> > *entry ); +VLC_API const char* vlc_sa_entry_Type(
> > services_advertisement_entry_t *entry ); +VLC_API uint16_t
> > vlc_sa_entry_Port( services_advertisement_entry_t *entry ); +
> > +VLC_API size_t vlc_sa_entry_Count_TXT( services_advertisement_entry_t*
> > entry ); +VLC_API const char** vlc_sa_entry_Get_TXT(
> > services_advertisement_entry_t* entry ); +VLC_API bool
> > vlc_sa_entry_Add_TXT( services_advertisement_entry_t* entry, +
> > const char* txt );
> > +
> > +/** @} */
> > +# ifdef __cplusplus
> > +}
> > +# endif
> > +
> > +#endif
> > diff --git a/src/libvlc.c b/src/libvlc.c
> > index c1dcde276d..558b146561 100644
> > --- a/src/libvlc.c
> > +++ b/src/libvlc.c
> > @@ -63,6 +63,7 @@
> > #include <vlc_modules.h>
> > #include <vlc_media_library.h>
> > #include <vlc_thumbnailer.h>
> > +#include <vlc_services_advertisement.h>
> >
> > #include "libvlc.h"
> >
> > @@ -92,6 +93,7 @@ libvlc_int_t * libvlc_InternalCreate( void )
> > priv = libvlc_priv (p_libvlc);
> > vlc_mutex_init(&priv->lock);
> > priv->interfaces = NULL;
> > + priv->services_advertisements = NULL;
>
> No thanks. I think we have enough of those singletons for backward
> compatibility.
>
While I agree we have a bunch of those, I'm not sure where to store the object. It seems we should have a single instance as soon as we start to advertise a service, and use the same one from there on, but if we don't store that instance in the libvlc object, I'm not really sure where to put it.
> And I don't even see the point. Owner of an announce should know to remove it.
> The existing API does not have such hack.
>
I don't understand that remark
> > priv->main_playlist = NULL;
> > priv->p_vlm = NULL;
> > priv->media_source_provider = NULL;
>
Regards,
--
Hugo Beauzée-Luyssen
hugo at beauzee.fr
More information about the vlc-devel
mailing list