[vlc-devel] [PATCH] Adding metadata support to web-plugin

[Besnard Jean-Baptiste]

Hi,

Le samedi 19 septembre 2009 à 18:19 +0300, Rémi Denis-Courmont a écrit :

> In retrospect, I don't understand why you are passing a string at all, instead 
> of exporting the enumeration to the caller and passing an integer. I might be 
> missing something.


This allows named symbols, passing an integer is not as obvious as I
think a javascript API should be, even if it is well documented (see
libvlc_state_t for input.state). 

The idea is to keep the enum values synced by using their symbol
somewhere in the "calling chain". This has two major advantages :
-The insertion of symbols in the middle of the enum does not break the
API
-The deletion of used symbols will generate an error on the API side


An alternative to this string comparison hell would be to create a
"const" node in the javascript API which exports the values of the
enums. This would allow the direct naming of the members, the use of
switch, and the previously mentioned advantages.

Up to now libvlc_state_t is addressed this way :

newState == 0
..
newState == 7

Creating getters will make the code like this :

newState == vlc.const.STOPPED
...
newState == vlc.const.ERROR

The drawback is that it supposes a bunch of getters which will be
redundant in Mozilla and ActiveX Plugin.

Is there any objection on this vlc.const node?
I may propose a patch for the state enum in a first approach.


> +        while (!libvlc_media_is_preparsed (media, p_e) && i < 500)
> +        {
> 
> This is not an appropriate to wait for an asynchronous event:
> 
> +            /* Here we wait for the meta */
> +            msleep (10000);
> +            i++;
> +        }
> 

I don't like this part of my code because polling is EVIL. I must admit
that I ended with this ugly solution because I couldn't find out how to
recover those nested syncing primitives (playlist_internal.h).

Ideally it would have done something like WaitPreparsed
in ./src/playlist/fetcher.c:347 which uses a vlc_cond_timedwait.
I abandoned the callback solution because I would end with a similar
polling method to implement this barrier on the get_metadata call.

I think I need some hints on this one to make things cleaner.

> Also, what happens in error cases? Are we sure that is-preparsed always become 
> eventually true?

If is-preparsed does not become true there is a timeout of 5 seconds
using the i variable which will lead to a null meta. 
Again this is not optimal ...


Best regards,

BESNARD Jean-Baptiste