[vlc-devel] [PATCH] GSoC: lua services discovery, first try

[Rémi Denis-Courmont]

Le jeudi 23 juillet 2009 19:19:18 Fabio Ritrovato, vous avez écrit :
> >> What's protected is the parameter list, one thing i probably forgot is
> >> to actually use the lock in the interface...
> >
> > If the item itself is not protected, why does the parameter list need to
> > be? Or to put it another way, if the parameter list needs to be protected
> > why does the rest not need to be too?
>
> I think because the services_discovery_t is not access in a way that
> needs a lock...
> I'm accessing the parameter list from 2 different threads, the SD
> thread to set it, and the interface thread to read it, so I thought a
> lock was needed...
> If I'm wrong, I'll remove it, no problem...

I just don't see the point. By the time "another" thread can access your item, 
it ought to have all parameters set for good. Otherwise, you'll get really 
weird side effects, if you did not add all parameters yet, but someone else 
tries to use the item anyway.

> >> Because you can't manage SD items right now; or, to be right, you can
> >> only manage the actual playlist items, but you can't do anything with
> >> them...
> >> With the new functions, you can dinamically add items to the SD, for
> >> example in the youtube script, you can search for a string and the
> >> videos are added to the playlist, or in the mtp SD you can transfer
> >> song to the device, that's what the new functions and events are
> >> for...
> >
> > The whole point of SD is to add (and remove) items. I don't see that.
>
> The point of the SD is to add items, but from the code, the user can't
> interact with them...

First it's called Services Discovery, not Services Announce. Second, if you 
really want to extend them to support addition, you should use synchronous 
(and optional) pf_add/pf_delete callbacks into the structure as with every 
other plugin types, or whatever fits.

> >> > In 0003, the closing function will crash if vlc_clone() failed (which
> >> > can happen).
> >>
> >> Is checking if p_sys->p_thread != NULL enough?
>
> if( p_sd->p_sys->thread != NULL )
> {
>     vlc_cancel (p_sd->p_sys->thread);
>     vlc_join (p_sd->p_sys->thread, NULL);
> }

vlc_thread_t can be an aggregate type.

> module_need is exactly the one that's allocating stuff and it's not
> fast enough...
> Without the event, it crash on module_get_name, because p_module it's
> still not allocated...

It does not make sense to use module_get_name() on your own self. When a 
module wants to know its own name for aliasing purpose, we add it in the 
object typedef (e.g. access_t.psz_access, demux_t.psz_demux...) and set it 
before calling module_need(). It's just that SD never needed it this far. 
Using event handlers seems backward.

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