[vlc-devel] core playlist

[Pierre d'Herbemont]

On Aug 29, 2009, at 6:30 PM, Jakob Leben wrote:

> On Fri, Aug 28, 2009 at 5:26 PM, Pierre d'Herbemont <pdherbemont at free.fr 
> > wrote:
> You should probably post here your API (not the patch first) once you
> have a good vision of it so it can be discussed.
>
> There it goes, change proposals stripped down into one file.

Hi Jakob,

     void (*pf_has_children)
         ( services_discovery_t *p_sd, services_discovery_item_t  
*p_item );
     /* pf_children must be asynchronous, it must result in
        a vlc_Services_DiscoveryItemChildren event. If called with  
p_parent = NULL,
        it must return the top level items. */

If it is asynchronous, I would probably use a function pointer to the  
callback to make this clear, instead of relying onto an event. If the  
event is always emitted, then this function is useless.

To what parent refers has_children? The SD?

I would also suffix it by _async(), if it was to be used.

>     /* (shall also some others or all of them be asynchronous??) */
>     void (*pf_children)
>         ( services_discovery_t *p_sd, services_discovery_item_t  
> *p_parent );
>     bool (*pf_has_media)
>         ( services_discovery_t *p_sd, services_discovery_item_t  
> *p_item );
>     input_item_t * (*pf_media)
>         ( services_discovery_t *p_sd, services_discovery_item_t  
> *p_item );
>     bool (*pf_compare)
>         ( services_discovery_t *p_sd,
>           services_discovery_item_t *p_item1,  
> services_discovery_item_t *p_item2 );
This can make the API a pain to use. Can we expect that "p_item1 ==  
p_item2 <=> p_item1 equals p_item2"?

>     void (*pf_free)
>         ( services_discovery_t *p_sd, services_discovery_item_t  
> *p_item );
There is no create/new, so why is there a _free()?

>     /* Moreover, a sd module may still (like in the current API)
>        inform clients dynamically about single items being added or  
> removed
>        with vlc_ServicesDiscoveryItemAdded and  
> vlc_ServicesDiscoveryItemRemoved
>        events. However, these events shall not be sent for starting  
> info retrieval
>        but only for changes that happen to services after a sd  
> module has
>        already connected to the services source. Does this make  
> sense? */
>
I don't understand what is the rationale behind that... And I don't  
understand what is "already connected to the services source". I would  
expect those event to be sent whenever someone is listening to it.


>     vlc_Services_DiscoveryItemChildren, //new
>

ItemChildren is not a name describing an event. We should look for  
something more explicit.

>         {
>             services_discovery_item_t * p_parent;
>             vlc_array_t * p_children;
>             /*maybe better to make a
>               "typedef services_discovery_item_list_t vlc_array_t"
>               and implement wrapper functions to work on the  
> typedef? */
>         } services_discovery_item_children;
>

Well, how do you deal with children changes then? Do you resend the  
full array?

I think I would expect either to reuse the ItemAdded and ItemRemoved  
for children addition, or create new one that applies locally to  
children of the specific item you are interested in.

Thanks for your work! :)

Pierre.