[vlc-devel] Support of events in Mozilla plugin - Beta

[Yannick Bréhon]

Anyone?

Yannick Bréhon a écrit :
> See inline
> 
> jpd at videolan.org a écrit :
>> On Tue, Sep 08, 2009 at 10:15:56AM +0200, Yannick Br?hon wrote:
>>> Here is our updated version of our patch to the plugin for enabling
>>> javascript reception of VLC events.
>> Sadly I really don't have time to properly review it, and won't for
>> a week or two at least, but I do have a couple of comments.
>>
>> First, I noticed you did use a bitmap, which is good, but I had
>> something slightly different in mind. I have that code around so I'll
>> drop that in soonish. Same with the list of events, or you could take
>> a look at the lookup function in core.
> 
> We are not quite sure what you want here, can you please expand on this
> topic? Thanks.
> 
>> Second, I'd really rather you didn't traverse string arguments where you
>> could be parsing an array-of-strings passed from javascript instead.
> 
> We have mimicked the mechanism used today to setup a new VLC plugin to
> get the options. We now support both a string of options or an array of
> strings (exactly the same as the options for a new plugin). We have
> deliberately not called the LibvlcPlaylistNPObject::parseOptions method
> for the following two reasons:
> - it is apparently bound to evolve quite a bit according to the number
> of FIXMEs and we are not sure our code will be compatible with such
> evolutions and/or that our code won't prevent such evolutions
> - this method is not naturally accessed from within the plugin and all
> the circonvolutions don't seem appropriate and logical
> 
>> Third, there's still a global that I'd rather see go. Give the map to
>> some object to care for, and since you need to compare against NPUTF8*
>> anyway you could be saving a couple string conversions here.
> 
> We have changed the map of strings for a map of NPUTF8*. We have also
> hidden said map as well as another global (the eventToCatch bitmask) in
> the VlcPLugin object.
> 
>>> The function returns True if all goes well, or a string with the error
>>> message otherwise. If any of the event in eventList does not exist, an
>>> error message is sent back to javascript, and the eventListener is not
>>> added.
>> I think this needs rethinking. It is perhaps more conventional to return
>> true or false and find some other way to return the string or perhaps
>> not return anything (`return void') but throw an exception with the
>> message on error.
> 
> The function now returns void on success, and throws an exception with
> the message on error.
> 
> 
> 
> Moreover, we noticed on *our* development platform that compilation
> fails because the HAS_NPFUNCTIONS_H variable is not #defined for us. We
> had to manually define it, otherwise compiler uses npupp.h instead of
> npfunctions.h, which doesn't define the API functions we need for our patch.
> Possible solutions are:
> - use a better development platform than ours ;)
> - define HAS_NPFUNCTIONS_H manually at compilation time if npfunctions.h
> exists
> - define HAS_NPFUNCTIONS_H in appropriate header file (npapi.h or
> npunix.h, not sure where this should be)
> 
> 
> Awaiting on your comments, thanks for all the help,
> Yannick
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel