GNOME Bugzilla – Bug 636241
Align XOverlay handling not to use deprecated methods
Last modified: 2010-12-22 23:53:50 UTC
Created attachment 175647 [details] [review] [PATCH] Removed access to deprecated methods The deprecated method gst_x_overlay_set_xwindow_id has been removed from GStreamer 0.10.31. This patch aligns the Gst::XOverlay interface with the gstreamer counterpart and chanfges the names of the methods to avoid ambiguity. For better conformance to Qt API, the WId type is also used for parameters to identify the window handle.
Review of attachment 175647 [details] [review]: In xoverlay.h: I'd prefer not to include <QtGui/QWidget>, but <QtGui/qwindowdefs.h> to keep the QtGui intrusion as minimal as possible. Note that this class is in the main QtGStreamer library which does not depend on QtGui (although we may want to reconsider that...) In xoverlay.cpp: The cast is plain wrong. The C method takes a gpointer, which means that: On X11 (where WId is ulong), it should be reinterpret_cast<gpointer>'ed On Mac (where WId is int or long), it should be reinterpret_cast<gpointer>'ed again. On Windows (where WId is HWND, which in turn is a void*), it should be passed without a cast (or maybe static_cast<HWND> for safety?) Btw, I'm not really sure if on X11 and mac you are supposed to cast the gpointer to ulong/long/int or pass a pointer to it. In videowidget.cpp: The #ifdef should be removed and the WId should be passed as is, without a cast.
Created attachment 175657 [details] [review] [PATCH] Removed access to deprecated methods
(In reply to comment #1) > Review of attachment 175647 [details] [review]: > > In xoverlay.h: > I'd prefer not to include <QtGui/QWidget>, but <QtGui/qwindowdefs.h> to keep > the QtGui intrusion as minimal as possible. Note that this class is in the main > QtGStreamer library which does not depend on QtGui (although we may want to > reconsider that...) Actually I was searching for that exact library but then I followed the example at: http://www.gstreamer.net/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-gstxoverlay.html btw I like much more the modification you suggested, so now it's there. > > In xoverlay.cpp: > The cast is plain wrong. The C method takes a gpointer, which means that: > On X11 (where WId is ulong), it should be reinterpret_cast<gpointer>'ed > On Mac (where WId is int or long), it should be reinterpret_cast<gpointer>'ed > again. > On Windows (where WId is HWND, which in turn is a void*), it should be passed > without a cast (or maybe static_cast<HWND> for safety?) > Actually, looking at the example of above we should not need any cast at all. If you're extra-sure we need it maybe we've a bug in the example documentation of GstXOverlay (I've never tried it outside Ubuntu so maybe we'll need some Windows/MacOS users to confirm). > Btw, I'm not really sure if on X11 and mac you are supposed to cast the > gpointer to ulong/long/int or pass a pointer to it. > > In videowidget.cpp: > The #ifdef should be removed and the WId should be passed as is, without a > cast. Done
> Btw, I'm not really sure if on X11 and mac you are supposed to cast the > gpointer to ulong/long/int or pass a pointer to it. For X11, you cast the XID into a pointer and the pointer back to an XID.
Any news here?
(In reply to comment #5) > Any news here? Yes, the patch is ok and I have it in a branch here: http://git.collabora.co.uk/?p=user/gkiagia/libqtgstreamer.git;a=shortlog;h=refs/heads/xoverlay I was just waiting a bit for distros to catch up with 0.10.31 so that we don't cause too much inconvenience to existing users. I think it should be ok to merge now.
Merged in master.