[vlc-devel] [PATCH] ActiveX FullScreen support

[Sergey Radionov]

2011/6/1 Jean-Baptiste Kempf <jb at videolan.org>

> On Wed, May 25, 2011 at 10:31:44PM +0700, Sergey Radionov wrote :
> > Reformated version
>
> Review:
>
> > +class VLCHolderWnd
> > +{
> > +public:
> > +    static void RegisterWndClassName(HINSTANCE hInstance);
> > +    static void UnRegisterWndClassName();
> > +    static VLCHolderWnd* CreateHolderWindow(VLCPlugin* p_instance);
> > +    void DestroyWindow()
> > +        {::DestroyWindow(_hWnd);};
> Ok.
>
> > +private:
> > +    static LPCTSTR getClassName(void)  { return TEXT("VLC ActiveX Media
> Window Holder Class"); };
> Text isn't good. "VLC ActiveX Window Holder Class" or something similar.
> "Media" is out of context here.
>
Ok, accepted.


> > +private:
> Why again a private: ?
>
> > +private:
> idem?
>
> > +public:
> idem?
>
I am usually split class members into logical groups, and
private/protected/public serves as separators... I think it helps understanding
code...


> > +HINSTANCE VLCHolderWnd::_hinstance=0;
> > +ATOM VLCHolderWnd::_holder_wndclass_atom=0;
> spaces around = , please.
>
Ok, accepted.


>
> > +void VLCHolderWnd::RegisterWndClassName(HINSTANCE hInstance)
> Shouldn't implementations be in .cpp?
>
> > +void VLCHolderWnd::UnRegisterWndClassName()
> idem
>
> > +VLCHolderWnd* VLCHolderWnd::CreateHolderWindow(VLCPlugin* p_instance)
> idem.
>
Ok, accepted.

> +LRESULT CALLBACK VLCHolderWnd::VLCHolderClassWndProc(HWND hWnd, UINT
> uMsg, WPARAM wParam, LPARAM lParam)
> > +{
> > +    VLCHolderWnd* h_data =
> reinterpret_cast<VLCHolderWnd*>(GetWindowLongPtr(hWnd, GWLP_USERDATA));
> > +    VLCPlugin* p_instance = h_data ? h_data->_p_instance : 0;
>
> Please use NULL for pointers;
>
 Bjarne Stroustrup: http://www2.research.att.com/~bs/bs_faq2.html#null


> > +class VLCFullScreenWnd
> Exactly the same remarks as above :D
>
> > +#define ID_FS_SWITCH_FS 1
> > +#define ID_FS_PLAY_PAUSE 2
> > +#define ID_FS_VIDEO_POS_SCROLL 3
> > +#define ID_FS_MUTE 4
> > +#define ID_FS_VOLUME 5
> > +#define ID_FS_LABEL 5
> No. Use enums.
>
Ok, accepted.


> > diff --git a/projects/activex/axvlc.idl b/projects/activex/axvlc.idl
> > index de7562d..21a3495 100644
> > --- a/projects/activex/axvlc.idl
> > +++ b/projects/activex/axvlc.idl
> > @@ -644,6 +644,11 @@ library AXVLC
> >
> >          [propget, helpstring("Returns the audio object.")]
> >          HRESULT video([out, retval] IVLCVideo** obj);
> > +
> > +        [propget]
> > +        HRESULT NewMessageFlag([out, retval] VARIANT_BOOL* vYes);
> > +        [propput]
> > +        HRESULT NewMessageFlag([in] VARIANT_BOOL vYes);
> >      };
> >
> >      [
>
> I still think this part should be in a separate commit, but well...
>
> Ok, I'll try split it to different commits (if I can)...


> > diff --git a/projects/activex/axvlc_rc.rc.in b/projects/activex/
> axvlc_rc.rc.in
> > index f3cf3a8..274aa47 100644
> > --- a/projects/activex/axvlc_rc.rc.in
> > +++ b/projects/activex/axvlc_rc.rc.in
> > @@ -25,6 +25,12 @@ BEGIN
> >  END
> >
> >  2 BITMAP DISCARDABLE "inplace.bmp"
> > +3 BITMAP DISCARDABLE "defullscreen.bmp"
> > +4 BITMAP DISCARDABLE "play.bmp"
> > +5 BITMAP DISCARDABLE "pause.bmp"
> > +6 BITMAP DISCARDABLE "volume.bmp"
> > +7 BITMAP DISCARDABLE "volume-muted.bmp"
> > +8 BITMAP DISCARDABLE "new_message.bmp"
> >
> >  1 TYPELIB "axvlc.tlb"
>
> Yep.
>
> > +        case WM_SIZE:{
> > +            int new_client_width = LOWORD(lParam);
> > +            int new_client_height = HIWORD(lParam);
> > +            //first child will be resized to client area
> > +            HWND hChildWnd = GetWindow(hWnd, GW_CHILD);
> > +            if(hChildWnd){
> > +                MoveWindow(hChildWnd, 0, 0, new_client_width,
> new_client_height, FALSE);
> > +            }
> > +            return 0L;
> > +        }
> > +        case WM_LBUTTONDBLCLK:{
> > +            p_instance->toggleFullscreen();
> > +            return 0L;
> > +        }
> Ok, I thnk.
>
> I must say that is addition is a really good thing and quite difficult
> to merge, I know.
> Thanks many times.
>
> Best Regards,
>
> --
> Jean-Baptiste Kempf
> http://www.jbkempf.com/ - +33 672 704 734
> Sent from my Electronic Device
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110601/d368c06f/attachment.html>