[Bug 736035] glimagesink: Regression: Gtk+ cannot get glimagesink input

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=736035
  GStreamer | gst-plugins-bad | unspecified

Matthew Waters <ystreet00> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #289580|none                        |needs-work
             status|                            |

--- Comment #12 from Matthew Waters <ystreet00 at gmail.com> 2014-10-29 12:34:50 UTC ---
Review of attachment 289580:
 --> (https://bugzilla.gnome.org/review?bug=736035&attachment=289580)

I have a couple of questions and comments below.

::: ext/gl/gstglimagesink.c
@@ +987,3 @@
+    GstGLWindow *window;
+    window = gst_gl_context_get_window (glimage_sink->context);
+    gst_gl_window_handle_events (window, handle_events);

Need to gst_object_unref the window when done.

::: ext/gl/gstglimagesink.h
@@ +68,3 @@
+    gboolean handle_events;
+
+    GMutex flow_lock;

Why the lock?

It only seems to protect multiple _handle_events() calls

::: gst-libs/gst/gl/gstglwindow.c
@@ +941,3 @@
+void
+gst_gl_window_handle_events (GstGLWindow * window, gboolean handle_events)
+{

This should be a vfunc on the GstWindowClass that is called when it exists. 
Like gst_gl_window_get_window_handle except the subclass should only be called
if it exists.

::: gst-libs/gst/gl/x11/gstglwindow_x11.h
@@ +104,3 @@
 gint gst_gl_window_x11_untrap_x_errors (void);
+void gst_gl_window_x11_handle_events (GstGLWindowX11 * window_x11,
+                                           gboolean handle_events);

There is no real reason to expose this after the move to the vfunc.

-- 
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.