<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 28, 2009, at 3:49 AM, Fabio Ritrovato wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><blockquote type="cite"><blockquote type="cite">You put an item inside as a child to the service discovery, like<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">services_discovery_AddItem, but what you pass is a string that has to<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">be elaborated somehow by the SD<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Ok. What about frontEndRequestsAddItem(input_item_t*) and<br></blockquote><blockquote type="cite">frontEndRequestsAddItemAsSearchString(const char *)?<br></blockquote><font class="Apple-style-span" color="#006312"><br></font>It depends on the module, in the SD one, it needs a file path, but the<br>resulting item, points to the file path on the device, not the<br>original path...<br>So using an input item just to pass a path seems kind of a waste to me...<br></div></blockquote><div><br></div><div>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.</div><br><blockquote type="cite"><div><br><blockquote type="cite"><blockquote type="cite">This happens when you click "save" on an SD item, what happens is what<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">the writer of the modules wants to...<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">For example, in the MTP SD, it's used to transfer files from the<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">device to you hard disk...<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Ok. What about frontEndRequestsSaveItem()? Save is probably vague, and<br></blockquote><blockquote type="cite">the interface would expect an unified experience among service<br></blockquote><blockquote type="cite">discovery. Is SetItemPersistence() more appropriate?<br></blockquote><br>I think it can be hard to have an unified experience, as one can write<br>the module to do anything...<br></div></blockquote><div><br></div><div>We want to refrain that, so that it closely match what the user would expect.</div><br><blockquote type="cite"><div>Also, I'm not sure i get what you mean with SetItemPersistence()...<br></div></blockquote><div><br></div><div>Hum. Wait, after re-reading your mail SaveItem is actually SaveItems!</div><div><br></div><div>The ideal user scenario is:</div><div>- You add a lot of item to the SD</div><div>- You expect it to be in the SD as soon as you add them.</div><div><br></div><div>I guess that we can't achieve the ideal user scenario gracefully with what you have (right?).</div><div><br></div><div>Then, the name I would pick would be SynchronizeItems(), or FlushAddedAndDeletedItems() instead of SaveItems().</div><div><br></div><blockquote type="cite"><div><blockquote type="cite">Given that this can fail, what do you think about:<br></blockquote><blockquote type="cite">services_discovery_FrontEndRequestsDeleteItem();<br></blockquote><br>Ok...<br>But why this one with the services_discovery prefix? Just a typo?<br></div></blockquote><div><br></div><div>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.</div><div><br></div><blockquote type="cite"><div><br><blockquote type="cite"><blockquote type="cite">It's a list, so you are appending the new parameter to that list, you<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">are not setting a value...<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Ok. Should we support remove then. Would the user want to do that?<br></blockquote><br>No, the user does not interact with it, it's read-only for him...<br></div></blockquote><div><br></div><div>oh! So far, after re-reading your patch:</div><div>- 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.</div><div>- All the tree hierarchy that you will end up creating, should be done by adding a node input_item_t.</div><div><br></div><div>Can this work?</div><br><blockquote type="cite"><div>Actually, there was a lock, but courmisch told me to remove it, as it<br>was not needed...<br>But if it is, I can put it in again...<br></div></blockquote><div><br></div>Right, there is no point in adding the lock if it's not needed.</div><div><br></div><div>Anyway, there should be no direct access to the sd structure, and all members should be considered private. A function is required.</div><div><br><blockquote type="cite"><div>Also, those function would have the services_discovery_ prefix, right?<br></div></blockquote><div><br></div><div>Right. They should be named services_discovery_FunctionName(), which makes it a pain to type, but...</div></div><div><br></div>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 :)<div><br><div>Pierre.</div></div></body></html>