[vlc-devel] [PATCH] ActiveX/Mozilla FullScreen support

[Sergey Radionov]

23.05.2011 18:29, Jean-Paul Saman пишет:
> 2011/5/23 Sergey Radionov<rsatom at gmail.com>:
>> 23.05.2011 15:20, Jean-Paul Saman пишет:
>>> On Sun, May 22, 2011 at 4:10 PM, Sergey Radionov<rsatom at gmail.com>    wrote:
>
>>> ---
>>> 2) What is the purpose of:
>>>
>>> +    STDMETHODIMP get_NewMessageFlag(VARIANT_BOOL* vYes);
>>> +    STDMETHODIMP put_NewMessageFlag(VARIANT_BOOL vYes);
>>>
>>> (I have the impression it is debug code.)
>>>
>>> The methods extent the JavaScript interface and its usage is not clear
>>> nor its purpose.
>>> Please explain.
>>>
>> This methods need for possibility of inform user (in fullscreen mode) about
>> some events wich need user attention (but I forget add changed idl file for
>> it). It needed by project for which I made fullscreen support in VLC. I can
>> remove this, if no need in this function.
>
> I do not see the use case for this from the above description.
Imagine, we have some web site, which embed VLC, and web site has internal messaging sistem (between 
users of site, or between users and administration). User switch to FullScreen and watch video (long 
video, 1 hour or more), and administration send urgent message to user("tornado, escape!" :), but 
user don't see it - he in fullscreen mode... and with this function he has blinking icon on control 
panel, informing him about message, all simple.

>
>>> ---
>>> 3) The methods (on the fullscreen window for activex AND mozilla):
>>>
>>> +void VLCFullScreenWnd::RegisterEvents()
>>> +void VLCFullScreenWnd::UnRegisterEvents()
>>> +void VLCFullScreenWnd::handle_input_state_event(const libvlc_event_t*
>>> event, void *param)
>>>
>> no, it not duplication. FullScreen window is exactly the same client to vlc
>> lib, as VLCPlugin. I made this to minimize changes in main code.
>>
>>> are duplication of methods in file plugin.cpp (for mozilla the file is
>>> vlcplugin.cpp):
>>>
>>> void VLCPlugin::player_register_events()
>>> void VLCPlugin::player_unregister_events()
>>> static void handle_input_state_event(const libvlc_event_t* event, void
>>> *param)
>>>
>>> The fullscreen window should refer to the object VLCPlugin (which
>>> wraps libvlc access) instead of duplicating its
>>> functionality. In case of fullscreen a messsage should also be sent to
>>> the objects in the fullscreen window (if appropriate).
>>
>> I think, this is not nessesary complication, which have no any advantages,
>> but can bring some new bugs. But, may be, I don't understand something :)
>
> It looks to me as duplication, so I would like to move the
> libvlc_event/handle_event code
> to VLCPlugin objects. Since that wraps libVLC code for the webplugins.
>
ok, I see what I can do.

> In the fullscreen window implementation you need a pointer to the
> VLCPlugin instance, hence your defintion of:
>
> +typedef VlcPlugin VLCPlugin;
> +
> +inline libvlc_media_player_t* getMD(VLCPlugin* p_instance)
> +{
> +	return p_instance->getMD();
> +}
>
> Why not make it integral part of the fullscreen class?
>
I try minimize difference in code for ActiveX and Mozilla code.
And may be in future, combine it into one file (may be with some #define, as usual). And for this I 
try localize differences in one place(top of the file).

And also, by this reason, I move npVlcFullScreen.h and axVlcFullScreen.h to "projects folder - 
because, may be in future, they stand one shared file...

But may be you are right, I think about it.

>>> ---
>>> 6) File: projects/mozilla/vlcplugin.h
>>>
>>> Printing an error in this case is necessary IMHO, so please revert this
>>> part:
>>>
>>> @@ -200,7 +204,7 @@ public:
>>>       {
>>>           if( !libvlc_media_player )
>>>           {
>>> -             libvlc_printerr("no mediaplayer");
>>> +             //libvlc_printerr("no mediaplayer");
>>>           }
>>>           return libvlc_media_player;
>>>       }
>>>
>> I use this function too often in fullscreen window, and if no comment, it
>> will be too mach warnings.
>
> Ah, you use the return value of getMD() to decide if the FS button has
> to perform an action.
> Maybe you should remove this part of the method entirely
>
>           if( !libvlc_media_player )
>           {
> -             libvlc_printerr("no mediaplayer");
> +             //libvlc_printerr("no mediaplayer");
>           }
>
Ok, accepted. I only tried make minimal changes in foreign (original) code.

>>> ---
>>> 7)  File: projects/mozilla/vlcplugin.h
>>>
>>> NO functional change here, so revert this part:
>>>
>>> @@ -209,8 +213,8 @@ public:
>>>       char*               getAbsoluteURL(const char *url);
>>>       NPWindow&              getWindow()
>>>                               { return npwindow; };
>>> -    void                setWindow(const NPWindow&window)
>>> -                            { npwindow = window; };
>>> +    void                setWindow(const NPWindow&window);
>>> +                            //{ npwindow = window; };
>>>
>>>       NPClass*            getScriptClass()
>>>                               { return p_scriptClass; };
>>>
>>>
>> I move body of this function to cpp. It become too big for .h file.
>
> ah, ok
>
> Kind regards,
> Jean-Paul Saman.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel

-- 
With best wishes,
Sergey Radionov