Created attachment 311666 [details] [review]
gtkglsink: Hide and cleaned the GtkWindow we might create

Created attachment 311667 [details] [review]
gtkglsink: Hide and clean the GtkWindow we might create

When stopping the sink we should always hide the window.

Review of attachment 311667 [details] [review]:

Looks good, please push after a small change

::: ext/gtk/gstgtkbasesink.c
@@ +287,3 @@
     /* User did not add widget its own UI, let's popup a new GtkWindow to
      * make gst-launch-1.0 work. */
+    gst_sink->window = gtk_window_new (GTK_WINDOW_TOPLEVEL);

This window creation and destruction is IMHO wrong. We're calling this from a random thread that is starting the sink, not necessarily the GTK main thread. Please create another bug about this as a reminder

@@ +304,3 @@
+  if (gst_sink->window) {
+    gtk_widget_hide (gst_sink->window);
+    gst_object_unref (gst_sink->window);

A GtkWindow is not a GstObject ;)

Attachment 311667 [details] pushed as a2bdce8 - gtkglsink: Hide and clean the GtkWindow we might create

Openned https://bugzilla.gnome.org/show_bug.cgi?id=755251

<ystreet00> thiblahute: your change segfaults now with videotestsrc ! glupload ! gtkglsink

I am getting

  (gst-launch-1.0:23903): Gtk-CRITICAL **: gtk_widget_destroy: assertion 'GTK_IS_WIDGET (widget)' failed

here. is that what you meant? Gonna investigate anyway.

Created attachment 311734 [details] [review]
gtksink: Do not re destroy the GtkWindow if destroyed by the user

Otherwise we will get an ASSERT.

Fixes

Review of attachment 311734 [details] [review]:

Looks fine.

::: ext/gtk/gstgtkbasesink.c
@@ +154,3 @@
+    g_clear_object (&gtk_sink->widget);
+  else
+    gtk_sink->window = NULL;

either make the else clause an else if (widget == gtk_sink->window) with an else g_assert_not_reached() or make a separate callback for the window destroy.

Added the check and assertion and pushed. Thanks for the review.

Attachment 311734 [details] pushed as 717f922 - gtksink: Do not re destroy the GtkWindow if destroyed by the user

doing a "gst-inspect-1.0 gtksink" will cause a segfault because it enters the g_assert_no_reached() code

Let's track this in bug #755249

I mean bug #755969