[gstreamer-bugs] [Bug 627565] [xoverlay][win64] gulong can't hold a HANDLE

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=627565
  GStreamer | gst-plugins-base | 0.10.x

--- Comment #6 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2010-09-14 23:17:01 UTC ---
(From update of attachment 169888)
>@@ -353,8 +376,19 @@ gst_x_overlay_set_xwindow_id (GstXOverlay * overlay, gulong xwindow_id)
> 
>   klass = GST_X_OVERLAY_GET_CLASS (overlay);
> 
>-  if (klass->set_xwindow_id) {
>-    klass->set_xwindow_id (overlay, xwindow_id);
>+  if (klass->set_window_handle) {
>+    klass->set_window_handle (overlay, handle);
>+  } else {
>+#ifndef GST_REMOVE_DEPRECATED
>+#ifdef GST_DISABLE_DEPRECATED
>+#define set_xwindow_id set_xwindow_id_disabled
>+#endif
>+    if (sizeof (guintptr) <= sizeof (gulong) && klass->set_xwindow_id) {
>+      klass->set_xwindow_id (overlay, handle);
>+    } else {
>+      g_warning ("Refusing to cast guintptr to smaller gulong");
>+    }
>+#endif
>   }
> }

I wonder if the size check should be skipped here if the actual handle value to
be set fits into a gulong, to preserve maximum backwards compatibility (but
maybe this only affects win64 anyway which was broken before so we don't really
care too much?)?


>@@ -366,19 +400,43 @@ gst_x_overlay_set_xwindow_id (GstXOverlay * overlay, gulong xwindow_id)
>  * This will post a "have-xwindow-id" element message on the bus.
>  *
>  * This function should only be used by video overlay plugin developers.
>+ *
>+ * Deprecated: Use gst_x_overlay_got_xwindow_id_intptr() instead.
>  */
>+#ifndef GST_REMOVE_DEPRECATED
>+#ifdef GST_DISABLE_DEPRECATED
>+void gst_x_overlay_got_xwindow_id (GstXOverlay * overlay, gulong xwindow_id);
>+#endif
> void
> gst_x_overlay_got_xwindow_id (GstXOverlay * overlay, gulong xwindow_id)
> {
>+  gst_x_overlay_got_xwindow_id (overlay, xwindow_id);
>+}
>+#endif
>+
>+/**
>+ * gst_x_overlay_got_window_handle:
>+ * @overlay: a #GstXOverlay which got a window
>+ * @handle: a platform-specific handle referencing the window
>+ *
>+ * This will post a "have-xwindow-id" element message on the bus.
>+ *
>+ * This function should only be used by video overlay plugin developers.
>+ */
>+void
>+gst_x_overlay_got_window_handle (GstXOverlay * overlay, guintptr handle)
>+{
>   GstStructure *s;
>   GstMessage *msg;
> 
>   g_return_if_fail (overlay != NULL);
>   g_return_if_fail (GST_IS_X_OVERLAY (overlay));
> 
>-  GST_LOG_OBJECT (GST_OBJECT (overlay), "xwindow_id = %lu", xwindow_id);
>-  s = gst_structure_new ("have-xwindow-id", "xwindow-id", G_TYPE_ULONG,
>-      xwindow_id, NULL);
>+  GST_LOG_OBJECT (GST_OBJECT (overlay), "xwindow_id = %" G_GUINTPTR_FORMAT,
>+      handle);
>+  s = gst_structure_new ("have-xwindow-id",
>+      "xwindow-id", G_TYPE_ULONG, (unsigned long) handle,
>+      "xwindow-id-uintptr", G_TYPE_UINT64, (guint64) handle, NULL);

Should we rename this field to match the new function names? (We might also
want to provide API to detect this kind of message and parse the info later).

Looks good to me otherwise. Might want to sprinkle in some more GST_WARNINGs if
elements don't implement the new vfunc yet or call the old methods.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.