[vlc-devel] [PATCH] GSoC: lua services discovery, take two

[Pierre d'Herbemont]

On Jul 28, 2009, at 3:49 AM, Fabio Ritrovato wrote:

>>> You put an item inside as a child to the service discovery, like
>>> services_discovery_AddItem, but what you pass is a string that has  
>>> to
>>> be elaborated somehow by the SD
>>
>> Ok. What about frontEndRequestsAddItem(input_item_t*) and
>> frontEndRequestsAddItemAsSearchString(const char *)?
>
> It depends on the module, in the SD one, it needs a file path, but the
> resulting item, points to the file path on the device, not the
> original path...
> So using an input item just to pass a path seems kind of a waste to  
> me...

An input_item_t is what represents path. But it can even represent  
more complex item with meta data. It makes sense to use it here.

>
>>> This happens when you click "save" on an SD item, what happens is  
>>> what
>>> the writer of the modules wants to...
>>> For example, in the MTP SD, it's used to transfer files from the
>>> device to you hard disk...
>>
>> Ok. What about frontEndRequestsSaveItem()? Save is probably vague,  
>> and
>> the interface would expect an unified experience among service
>> discovery. Is SetItemPersistence() more appropriate?
>
> I think it can be hard to have an unified experience, as one can write
> the module to do anything...

We want to refrain that, so that it closely match what the user would  
expect.

> Also, I'm not sure i get what you mean with SetItemPersistence()...

Hum. Wait, after re-reading your mail SaveItem is actually SaveItems!

The ideal user scenario is:
- You add a lot of item to the SD
- You expect it to be in the SD as soon as you add them.

I guess that we can't achieve the ideal user scenario gracefully with  
what you have (right?).

Then, the name I would pick would be SynchronizeItems(), or  
FlushAddedAndDeletedItems() instead of SaveItems().

>> Given that this can fail, what do you think about:
>> services_discovery_FrontEndRequestsDeleteItem();
>
> Ok...
> But why this one with the services_discovery prefix? Just a typo?

Depends if it is in vlc_services_discovery.h In this case you want the  
prefix. If this is the object pointer callback, then you want to stick  
to the usual coding style.

>
>>> It's a list, so you are appending the new parameter to that list,  
>>> you
>>> are not setting a value...
>>
>> Ok. Should we support remove then. Would the user want to do that?
>
> No, the user does not interact with it, it's read-only for him...

oh! So far, after re-reading your patch:
- This parameter function doesn't really belong to the  
services_discovery API. It should pobably be some kind of helper in  
lua sd I believe. From what I understand this is just a convenience so  
that all lua SD can describe their hierarchy, and the associated query.
- All the tree hierarchy that you will end up creating, should be done  
by adding a node input_item_t.

Can this work?

> Actually, there was a lock, but courmisch told me to remove it, as it
> was not needed...
> But if it is, I can put it in again...

Right, there is no point in adding the lock if it's not needed.

Anyway, there should be no direct access to the sd structure, and all  
members should be considered private. A function is required.

> Also, those function would have the services_discovery_ prefix, right?

Right. They should be named services_discovery_FunctionName(), which  
makes it a pain to type, but...

Will it be only a services_discovery API now? The current name is  
wrong as this is just a media_discovery API. Now, it would be more a  
media_database API if it starts to be writable :)

Pierre.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090729/c779bfc4/attachment.html>