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

[Yannick Bréhon]

Hello,
We put the patch in git format. Hope this can help speed up its acceptance.
Thank you
Yannick



Anthony Loiseau a écrit :
> Hi,
> 
> I have not really read your diff, just looked at it.
> It inserts some tabs and trailing spaces you may want to remove, just
> for you to know.
> 
> Regards,
> Anthony Loiseau
> 
> 
> On Thu, 2009-10-15 at 14:43 +0200, Yannick Bréhon wrote:
>> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Mozilla-plugin-event-listener.patch
Type: text/x-patch
Size: 19279 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20091103/547690ef/attachment.bin>