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

[Yannick Bréhon]

ping?

Yannick Bréhon a écrit :
> 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
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> http://mailman.videolan.org/listinfo/vlc-devel