[vlc-devel] [PATCH 1/9] core: Add a services advertisement API

[Rémi Denis-Courmont]

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.

> +
> +# 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.

> +
> +/***********************************************************************
> + * 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'.

> +
> +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.

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.

>      priv->main_playlist = NULL;
>      priv->p_vlm = NULL;
>      priv->media_source_provider = NULL;

-- 
Rémi Denis-Courmont
http://www.remlab.net/