[vlc-devel] [PATCH 3/4] avahi: Prepare code for support of renderer discovery

[Brüns, Stefan]

On Donnerstag, 3. Januar 2019 18:49:55 CET you wrote:
> Le perjantaina 28. joulukuuta 2018, 17.33.16 EET Stefan Brüns a écrit :
> > On Mittwoch, 26. Dezember 2018 09:03:58 CET you wrote:
> > > Le mercredi 26 décembre 2018, 04:21:42 EET Stefan Brüns a écrit :
> > > > Instead of passing the services_discovery_t as userdata for the
> > > > callbacks,
> > > > use a struct (discovery_sys_t) which can be used regardless of the
> > > > service
> > > > type (input or renderer).
> > > > ---
> > > > 
> > > >  modules/services_discovery/avahi.c | 83
> > > >  ++++++++++++++++++------------
> > > >  1 file changed, 50 insertions(+), 33 deletions(-)
> > > > 
> > > > diff --git a/modules/services_discovery/avahi.c
> > > > b/modules/services_discovery/ avahi.c
> > > > index aa58c7f673..1ea60ab152 100644
> > > > --- a/modules/services_discovery/avahi.c
> > > > +++ b/modules/services_discovery/avahi.c
> > > > @@ -46,8 +46,8 @@
> > > > 
> > > > **********************************************************************
> > > > **
> > > > **
> > > > *
> > > > **/
> > > > 
> > > >  /* Callbacks */
> > > > 
> > > > -static int  Open ( vlc_object_t * );
> > > > -static void Close( vlc_object_t * );
> > > > +static int  OpenSD ( vlc_object_t * );
> > > > +static void CloseSD( vlc_object_t * );
> > > > 
> > > >  VLC_SD_PROBE_HELPER("avahi", N_("Zeroconf network services"),
> > > >  SD_CAT_LAN)
> > > > 
> > > > @@ -57,7 +57,7 @@ vlc_module_begin ()
> > > > 
> > > >      set_category( CAT_PLAYLIST )
> > > >      set_subcategory( SUBCAT_PLAYLIST_SD )
> > > >      set_capability( "services_discovery", 0 )
> > > > 
> > > > -    set_callbacks( Open, Close )
> > > > +    set_callbacks( OpenSD, CloseSD )
> > > > 
> > > >      add_shortcut( "mdns", "avahi" )
> > > >      
> > > >      VLC_SD_PROBE_SUBMODULE
> > > > 
> > > > @@ -72,7 +72,14 @@ typedef struct
> > > > 
> > > >      AvahiThreadedPoll   *poll;
> > > >      AvahiClient         *client;
> > > >      vlc_dictionary_t    services_name_to_input_item;
> > > > 
> > > > -} services_discovery_sys_t;
> > > > +    vlc_object_t        *parent;
> > > > +    bool                renderer;
> > > > +} discovery_sys_t;
> > > > +
> > > > +struct services_discovery_sys_t
> > > > +{
> > > > +    discovery_sys_t s;
> > > > +};
> > > 
> > > Uh, should there not be some SD-specific members there?
> > 
> > vlc_renderer_discovery_t and services_discovery_t both have a private data
> > pointer called p_sys. Unfortunately, one has the type
> > services_discovery_sys_t and the other one is vlc_renderer_discovery_t.
> > 
> > To avoid crosscasting the common userdata struct required for the avahi
> > callbacks, the struct is embedded into both structs.
> 
> If that's your problem, you can convert the (p_)sys fields to void pointers
> in the common headers.
> 
> I just find it suspicious that the content of both compound types is exactly
> the same.

I would find it much more suspicious if:

- struct discovery_sys_t would be static casted to either 
services_discovery_sys_t and vlc_renderer_discovery_sys_t when setting it.
- both structures would be casted to discovery_sys_t on each access.

If p_sys where a void* in both cases, casting *from* void* to discovery_sys_t 
would be fine IMHO, as void* never refers to a concrete type.

Defining struct services_discovery_sys_t and vlc_renderer_discovery_sys_t 
makes it explicit both are incomplete types which have to be defined by the 
implementation.

The fact both structures happen to aggregate the exact same members is just an 
implementation detail. None of the OpenSD/CloseSD/OpenRD/CloseRD functions 
need any specific members.

And again, it is the *exact same* approach as used by the microdns service 
discovery code, see: https://github.com/videolan/vlc/blob/master/modules/
services_discovery/microdns.c#L100

Kind regards,

Stefan