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
This commit is contained in:
Nicolas Dufresne 2015-07-15 14:32:42 -04:00
parent 8d9fbc5e49
commit 1a7c9b82f4
5 changed files with 89 additions and 30 deletions

View file

@ -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;
}

View file

@ -53,7 +53,6 @@ struct _GstGtkGLSink
GstVideoSink parent;
GtkGstGLWidget *widget;
gboolean widget_destroyed;
GstVideoInfo v_info;
GstBufferPool *pool;

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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;
if (!widget->priv->resize_id) {
widget->priv->resize_id = g_idle_add_full (G_PRIORITY_DEFAULT,
(GSourceFunc) _queue_resize, widget, NULL);
}
g_mutex_unlock (&widget->priv->lock);
gtk_widget_queue_resize (GTK_WIDGET (widget));
g_main_context_invoke (main_context, (GSourceFunc) _queue_resize, widget);
return TRUE;
}