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

[Rémi Denis-Courmont]

	Hello,

Le mercredi 22 juillet 2009 02:19:31 Fabio Ritrovato, vous avez écrit :
> Hi everyone,
> here's the first batch of patches for my SoC, hoping they are ok...
> The first one add new the ability to add, get and remove items from
> sds, and create a structure to hold the parameters and sd can
> accept...
> It also create uncachable modules (it will be needed in patch 3)
> Patch 2 is a qt4 dialog to pass data to a sd, but it's not the best in
> design terms... j-b offered to pimp it up, if the offer is still up...
> Patch 3 it's the actual work on lua sd, it's the backend part and 2
> scripts...
>
> I noticed there's a typo in the patch name, I'll probably end up
> resubmitting them anyway, so I'll leave it for now :P
>
> Let me know if there's anything wrong...

Please split 0001. Adding non-cacheable modules has nothing to do with 
extending the service discovery API. Similarly, please split the new scripts 
from the C code in 0003.


struct typedefs should not be added to vlc_common.h anymore these days. This 
is legacy from the times of vlc_symbol.

There is no point in loading and saving the cacheable value in the module 
cache, since it will fundamentally always be false...

Locking in services_discovery_AddParameterValue is weird and inconsistent. The 
va_list variant should take the lock instead. In fact, I couldn't figure out 
what the lock was protecting... ? Also why do you need new functions and new 
events to manage SD items?


In 0003, the closing function will crash if vlc_clone() failed (which can 
happen).

Also, registering to your own event manager seems rather silly. Unless I am 
missing something this is meant for libvlc applications to talk to plugins, 
not for plugins to talk to themselves.

I cannot comment much on the Qt4 and LUA code...

Best regards,

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