gtk: Fix race between queue_draw and destroy

In GTK dispose can be called before the last ref is reached. This
happens when you close the container window. The dispose will be
explicitly called, and destroyed notify will be fired. This patch
fixes this race by properly tracking the widget state.

In the sink, we now set the widget pointer to NULL, so the widget
will properly get created again if you set your pipeline to NULL
state after the widget was destroy, and set it back to PLAYING.

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

diff --git a/ext/gtk/gstgtkglsink.c b/ext/gtk/gstgtkglsink.c
index 1a98c02213243fc0bf8a72e0ffd3c205a07993cf..cbb0e948bbc81b1614750a8b64d1cb2f1aa826c8 100644 (file)
--- a/ext/gtk/gstgtkglsink.c
+++ b/ext/gtk/gstgtkglsink.c
@@ -164,7 +164,9 @@ gst_gtk_gl_sink_finalize (GObject * object)
 static void
 widget_destroy_cb (GtkWidget * widget, GstGtkGLSink * gtk_sink)
 {
-  g_atomic_int_set (&gtk_sink->widget_destroyed, 1);
+  GST_OBJECT_LOCK (gtk_sink);
+  g_clear_object (&gtk_sink->widget);
+  GST_OBJECT_UNLOCK (gtk_sink);
 }
 
 static GtkGstGLWidget *
@@ -367,7 +369,10 @@ gst_gtk_gl_sink_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       break;
     case GST_STATE_CHANGE_PAUSED_TO_READY:
-      gtk_gst_gl_widget_set_buffer (gtk_sink->widget, NULL);
+      GST_OBJECT_LOCK (gtk_sink);
+      if (gtk_sink->widget)
+        gtk_gst_gl_widget_set_buffer (gtk_sink->widget, NULL);
+      GST_OBJECT_UNLOCK (gtk_sink);
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
       if (gtk_sink->display) {
@@ -440,14 +445,19 @@ gst_gtk_gl_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
 
   gtk_sink = GST_GTK_GL_SINK (vsink);
 
-  gtk_gst_gl_widget_set_buffer (gtk_sink->widget, buf);
+  GST_OBJECT_LOCK (vsink);
 
-  if (g_atomic_int_get (&gtk_sink->widget_destroyed)) {
+  if (gtk_sink->widget == NULL) {
+    GST_OBJECT_UNLOCK (vsink);
     GST_ELEMENT_ERROR (gtk_sink, RESOURCE, NOT_FOUND,
         ("%s", "Output widget was destroyed"), (NULL));
     return GST_FLOW_ERROR;
   }
 
+  gtk_gst_gl_widget_set_buffer (gtk_sink->widget, buf);
+
+  GST_OBJECT_UNLOCK (vsink);
+
   return GST_FLOW_OK;
 }
 

diff --git a/ext/gtk/gstgtkglsink.h b/ext/gtk/gstgtkglsink.h
index d1c65c00a8ad56561aac42aed2444f28374ea22d..985486fb24b93864945ce21b2adcd2a613dc1ee4 100644 (file)
--- a/ext/gtk/gstgtkglsink.h
+++ b/ext/gtk/gstgtkglsink.h
@@ -53,7 +53,6 @@ struct _GstGtkGLSink
   GstVideoSink          parent;
 
   GtkGstGLWidget       *widget;
-  gboolean              widget_destroyed;
 
   GstVideoInfo          v_info;
   GstBufferPool        *pool;

diff --git a/ext/gtk/gstgtksink.c b/ext/gtk/gstgtksink.c
index 2c5d3ed3ebf993a2701ae71d5aacf6ddbd8a9aca..524ff5ab6c703dafdcc9ebf127283d11c61b20f3 100644 (file)
--- a/ext/gtk/gstgtksink.c
+++ b/ext/gtk/gstgtksink.c
@@ -294,7 +294,10 @@ gst_gtk_sink_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       break;
     case GST_STATE_CHANGE_PAUSED_TO_READY:
-      gtk_gst_widget_set_buffer (gtk_sink->widget, NULL);
+      GST_OBJECT_LOCK (gtk_sink);
+      if (gtk_sink->widget)
+        gtk_gst_widget_set_buffer (gtk_sink->widget, NULL);
+      GST_OBJECT_UNLOCK (gtk_sink);
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
       break;
@@ -353,8 +356,18 @@ gst_gtk_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
 
   gtk_sink = GST_GTK_SINK (vsink);
 
-  if (gtk_sink->widget)
-    gtk_gst_widget_set_buffer (gtk_sink->widget, buf);
+  GST_OBJECT_LOCK (vsink);
+
+  if (gtk_sink->widget == NULL) {
+    GST_OBJECT_UNLOCK (vsink);
+    GST_ELEMENT_ERROR (gtk_sink, RESOURCE, NOT_FOUND,
+        ("%s", "Output widget was destroyed"), (NULL));
+    return GST_FLOW_ERROR;
+  }
+
+  gtk_gst_widget_set_buffer (gtk_sink->widget, buf);
+
+  GST_OBJECT_UNLOCK (vsink);
 
   return GST_FLOW_OK;
 }

diff --git a/ext/gtk/gtkgstglwidget.c b/ext/gtk/gtkgstglwidget.c
index 2e108c9f935a69850e9686984121c67810a7f0c4..3d05d37b7d757d41ffb97add0e6d722bdd30e418 100644 (file)
--- a/ext/gtk/gtkgstglwidget.c
+++ b/ext/gtk/gtkgstglwidget.c
@@ -100,6 +100,10 @@ struct _GtkGstGLWidgetPrivate
   GLint attr_position;
   GLint attr_texture;
   GLuint current_tex;
+
+  /* Pending queued idles callback */
+  guint draw_id;
+  guint resize_id;
 };
 
 static void
@@ -430,15 +434,17 @@ gtk_gst_gl_widget_finalize (GObject * object)
     _invoke_on_main ((ThreadFunc) _reset_gl, widget);
   }
 
-  if (widget->priv->context) {
+  if (widget->priv->context)
     gst_object_unref (widget->priv->context);
-    widget->priv->context = NULL;
-  }
 
-  if (widget->priv->display) {
+  if (widget->priv->display)
     gst_object_unref (widget->priv->display);
-    widget->priv->display = NULL;
-  }
+
+  if (widget->priv->draw_id)
+    g_source_remove (widget->priv->draw_id);
+
+  if (widget->priv->resize_id)
+    g_source_remove (widget->priv->resize_id);
 
   G_OBJECT_CLASS (gtk_gst_gl_widget_parent_class)->finalize (object);
 }
@@ -575,6 +581,10 @@ gtk_gst_gl_widget_new (void)
 static gboolean
 _queue_draw (GtkGstGLWidget * widget)
 {
+  g_mutex_lock (&widget->priv->lock);
+  widget->priv->draw_id = 0;
+  g_mutex_unlock (&widget->priv->lock);
+
   gtk_widget_queue_draw (GTK_WIDGET (widget));
 
   return G_SOURCE_REMOVE;
@@ -583,8 +593,6 @@ _queue_draw (GtkGstGLWidget * widget)
 void
 gtk_gst_gl_widget_set_buffer (GtkGstGLWidget * widget, GstBuffer * buffer)
 {
-  GMainContext *main_context = g_main_context_default ();
-
   g_return_if_fail (GTK_IS_GST_GL_WIDGET (widget));
   g_return_if_fail (buffer == NULL || widget->priv->negotiated);
 
@@ -593,9 +601,12 @@ gtk_gst_gl_widget_set_buffer (GtkGstGLWidget * widget, GstBuffer * buffer)
   gst_buffer_replace (&widget->priv->buffer, buffer);
   widget->priv->new_buffer = TRUE;
 
-  g_mutex_unlock (&widget->priv->lock);
+  if (!widget->priv->draw_id) {
+    widget->priv->draw_id = g_idle_add_full (G_PRIORITY_DEFAULT,
+        (GSourceFunc) _queue_draw, widget, NULL);
+  }
 
-  g_main_context_invoke (main_context, (GSourceFunc) _queue_draw, widget);
+  g_mutex_unlock (&widget->priv->lock);
 }
 
 static void
@@ -664,6 +675,10 @@ _get_gl_context (GtkGstGLWidget * gst_widget)
 static gboolean
 _queue_resize (GtkGstGLWidget * widget)
 {
+  g_mutex_lock (&widget->priv->lock);
+  widget->priv->resize_id = 0;
+  g_mutex_unlock (&widget->priv->lock);
+
   gtk_widget_queue_resize (GTK_WIDGET (widget));
 
   return G_SOURCE_REMOVE;
@@ -770,7 +785,6 @@ _calculate_par (GtkGstGLWidget * widget, GstVideoInfo * info)
 gboolean
 gtk_gst_gl_widget_set_caps (GtkGstGLWidget * widget, GstCaps * caps)
 {
-  GMainContext *main_context = g_main_context_default ();
   GstVideoInfo v_info;
 
   g_return_val_if_fail (GTK_IS_GST_GL_WIDGET (widget), FALSE);
@@ -801,9 +815,12 @@ gtk_gst_gl_widget_set_caps (GtkGstGLWidget * widget, GstCaps * caps)
   widget->priv->v_info = v_info;
   widget->priv->negotiated = TRUE;
 
-  g_mutex_unlock (&widget->priv->lock);
+  if (!widget->priv->resize_id) {
+    widget->priv->resize_id = g_idle_add_full (G_PRIORITY_DEFAULT,
+        (GSourceFunc) _queue_resize, widget, NULL);
+  }
 
-  g_main_context_invoke (main_context, (GSourceFunc) _queue_resize, widget);
+  g_mutex_unlock (&widget->priv->lock);
 
   return TRUE;
 }

diff --git a/ext/gtk/gtkgstwidget.c b/ext/gtk/gtkgstwidget.c
index 3e2f9bb286d3ffb555f6a2ecaa89a784c6e04263..3fbb0ca4edc2f7260d630991091997f17004956f 100644 (file)
--- a/ext/gtk/gtkgstwidget.c
+++ b/ext/gtk/gtkgstwidget.c
@@ -69,6 +69,10 @@ struct _GtkGstWidgetPrivate
   GstBuffer *buffer;
   GstCaps *caps;
   GstVideoInfo v_info;
+
+  /* Pending queued idles callback */
+  guint draw_id;
+  guint resize_id;
 };
 
 static void
@@ -226,6 +230,12 @@ gtk_gst_widget_finalize (GObject * object)
   gst_buffer_replace (&widget->priv->buffer, NULL);
   g_mutex_clear (&widget->priv->lock);
 
+  if (widget->priv->draw_id)
+    g_source_remove (widget->priv->draw_id);
+
+  if (widget->priv->resize_id)
+    g_source_remove (widget->priv->resize_id);
+
   G_OBJECT_CLASS (gtk_gst_widget_parent_class)->finalize (object);
 }
 
@@ -331,6 +341,10 @@ gtk_gst_widget_new (void)
 static gboolean
 _queue_draw (GtkGstWidget * widget)
 {
+  g_mutex_lock (&widget->priv->lock);
+  widget->priv->draw_id = 0;
+  g_mutex_unlock (&widget->priv->lock);
+
   gtk_widget_queue_draw (GTK_WIDGET (widget));
 
   return G_SOURCE_REMOVE;
@@ -339,8 +353,6 @@ _queue_draw (GtkGstWidget * widget)
 void
 gtk_gst_widget_set_buffer (GtkGstWidget * widget, GstBuffer * buffer)
 {
-  GMainContext *main_context = g_main_context_default ();
-
   g_return_if_fail (GTK_IS_GST_WIDGET (widget));
   g_return_if_fail (buffer == NULL || widget->priv->negotiated);
 
@@ -348,14 +360,21 @@ gtk_gst_widget_set_buffer (GtkGstWidget * widget, GstBuffer * buffer)
 
   gst_buffer_replace (&widget->priv->buffer, buffer);
 
-  g_mutex_unlock (&widget->priv->lock);
+  if (!widget->priv->draw_id) {
+    widget->priv->draw_id = g_idle_add_full (G_PRIORITY_DEFAULT,
+        (GSourceFunc) _queue_draw, widget, NULL);
+  }
 
-  g_main_context_invoke (main_context, (GSourceFunc) _queue_draw, widget);
+  g_mutex_unlock (&widget->priv->lock);
 }
 
 static gboolean
 _queue_resize (GtkGstWidget * widget)
 {
+  g_mutex_lock (&widget->priv->lock);
+  widget->priv->resize_id = 0;
+  g_mutex_unlock (&widget->priv->lock);
+
   gtk_widget_queue_resize (GTK_WIDGET (widget));
 
   return G_SOURCE_REMOVE;
@@ -424,7 +443,6 @@ _calculate_par (GtkGstWidget * widget, GstVideoInfo * info)
 gboolean
 gtk_gst_widget_set_caps (GtkGstWidget * widget, GstCaps * caps)
 {
-  GMainContext *main_context = g_main_context_default ();
   GstVideoInfo v_info;
 
   g_return_val_if_fail (GTK_IS_GST_WIDGET (widget), FALSE);
@@ -455,15 +473,17 @@ gtk_gst_widget_set_caps (GtkGstWidget * widget, GstCaps * caps)
     return FALSE;
   }
 
+  gst_buffer_replace (&widget->priv->buffer, NULL);
   gst_caps_replace (&widget->priv->caps, caps);
   widget->priv->v_info = v_info;
   widget->priv->negotiated = TRUE;
 
-  g_mutex_unlock (&widget->priv->lock);
-
-  gtk_widget_queue_resize (GTK_WIDGET (widget));
+  if (!widget->priv->resize_id) {
+    widget->priv->resize_id = g_idle_add_full (G_PRIORITY_DEFAULT,
+        (GSourceFunc) _queue_resize, widget, NULL);
+  }
 
-  g_main_context_invoke (main_context, (GSourceFunc) _queue_resize, widget);
+  g_mutex_unlock (&widget->priv->lock);
 
   return TRUE;
 }