[vlc-devel] core playlist

[Pierre d'Herbemont]

On Aug 31, 2009, at 4:15 PM, Jakob Leben wrote:

> On Sun, Aug 30, 2009 at 8:04 PM, Pierre d'Herbemont <pdherbemont at gmail.com 
> > wrote:
>    /* 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.
>
> Yes, I didn't mean the event to be emitted always, but only  
> (asynchronously) in respond to a call to this function. You're  
> right, using a callback function pointer is better, also because  
> otherwise every client would receive events even in response to  
> queries not made by that client.

Why can't the client just listen to the event?

>
> On Sun, Aug 30, 2009 at 8:04 PM, Pierre d'Herbemont <pdherbemont at gmail.com 
> > wrote:
> void (*pf_has_children)
>        ( services_discovery_t *p_sd, services_discovery_item_t  
> *p_item );
>
> To what parent refers has_children? The SD?
>
> It refers to the p_item argument  to the function. The reason for  
> this function is that obtaining a children list may be time  
> consuming (and is therefore asynchronous) while with this funtion a  
> client can check if the item is supposed to have children at all and  
> only if that is so make a query for children list. Also, a GUI might  
> show relevant info based on the sole fact that an item has children  
> at all or not.

Right, I misread the prototype.

>
> On Sun, Aug 30, 2009 at 8:04 PM, Pierre d'Herbemont <pdherbemont at gmail.com 
> > wrote:
> 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()?
>
> We should expect that to be true, yes. The module's "compare"  
> function and the data stored in items should be in such a way that  
> comparing an item with itself (equal pointers) returns true (equal  
> items). I would like to point out again that  
> service_discovery_item_t is only a token or a descriptor or a symbol  
> if you want - that refers to some actual item that the SD module  
> operates on (a database entry, a media file...), so it's expected to  
> have more than one such tokens for the same actual items, so it  
> should be possible to somehow compare them. An example of the use of  
> the compare function is: when a children list is transmitted  
> asynchronously from a SD module, a SD parent item of the children  
> will be passed along, so the client can know which item's children  
> list it received, as the client might have issued multiple requests  
> for different parents before receiving their results. This parent  
> item will be a new instance of services_discovery_item_t, different  
> from the one the client had passed to the pf_children function to  
> issue the request, so comparing pointers will not work, but the SD  
> module shall be able to tell their equivalence based on data it  
> stored in the items.

I'd like it better if this was addressed at the SD level, ie - the SD  
ensure that it does always produce the same services_discovery_item_t  
for a children, not the client. If that's impossible fine.

> Another idea that travels in my brain is to have a "request" object,  
> so the response to a request is associated with the request object,  
> and this might be the way a client knows which parent's children  
> list it received.
>
> There is no "create" function because a valid item can only be  
> obtained from a module and it's the module which will allocate the  
> item and return a pointer to it, as it is also the module who  
> defines the struct services_discovery_item_t. Does that make sense?

It does.

> On the other hand, once the module has returned an item pointer to  
> the client, it should not release that item and data stored in it,  
> but that should be done only on client's request. The free function  
> is there also because a shallow "free" executed by client on the  
> item pointer might not free all the data associated with an item.


Then, there is nothing making the reference ownership clear here. When  
shall an item be _free? The usual convention is that you don't own the  
reference from a parameter unless it is retained/held. I wouldn't go  
too far away from here.

> On Sun, Aug 30, 2009 at 8:04 PM, Pierre d'Herbemont <pdherbemont at gmail.com 
> > wrote:
> /* 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.
> [...]
> 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.
>
> I must say this is a part I am not totally confident about. I would  
> like the API to be so that time consuming item (children) list  
> retrieval is done on request from client (through the function named  
> pf_children (yes, only a work-in-progress name)), but what if an  
> item has been removed from a SD device for example? I supposed a  
> device that a SD module accesses itself sends events (does  
> callbacks) to the module about items being added or removed, and the  
> module should pass those on to the SD client with ItemAdded2 and  
> ItemRemoved2, which would be new ones, besides the current ones, and  
> not sent immediately upon SD module's connection to the source, like  
> the current ones are.
> But you are right, it's more practical if a SD client can register  
> precisely for specific parents whose children it wants to receive  
> those events about, or even about every single item itself.
>
> All in all, the API is supposed to be considered essentially as a  
> way for a client to travel trees of items defined by a module and to  
> do that by (partly) asynchrounous requests from client to module.  
> This is the general idea behind it so this is my humble suggestion  
> as to how it shall be viewed and evaluated.


There should be probably one manual way to browse the tree of a given  
SD plus one way to register event for tree change. If we want to go  
that way.

I was also wondering if the services_discovery_item_t couldn't be  
merged with input_item_t. Currently input items are also used as node,  
but they only send event that indicates a new subitems. They don't  
store their children. If this was changed then you gain tree support.  
This would still be add-only. Unless we add delete, which shouldn't be  
a big deal.

This is probably the best way to actually remove the data model from  
the playlist_t. (And get rid of playlist_item_t).

Pierre.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090901/34282e68/attachment.html>