sys/: Make sure that before we clean up the X resources, we shutdown and join the...

Original commit message from CVS:
* sys/ximage/ximagesink.c: (gst_ximagesink_event_thread),
(gst_ximagesink_xcontext_get), (gst_ximagesink_xcontext_clear),
(gst_ximagesink_change_state), (gst_ximagesink_reset):
* sys/xvimage/xvimagesink.c: (gst_xvimagesink_event_thread),
(gst_xvimagesink_xcontext_get), (gst_xvimagesink_xcontext_clear),
(gst_xvimagesink_change_state), (gst_xvimagesink_reset):
Make sure that before we clean up the X resources, we shutdown and join
the event thread.
Also make sure the event thread does not shut down immediatly after
startup because the running variable is not yet correctly set.
Fixes #378770.

diff --git a/ChangeLog b/ChangeLog
index e50b4ef..bc08e11 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2007-10-22  Wim Taymans  <wim.taymans@gmail.com>
+
+       * sys/ximage/ximagesink.c: (gst_ximagesink_event_thread),
+       (gst_ximagesink_xcontext_get), (gst_ximagesink_xcontext_clear),
+       (gst_ximagesink_change_state), (gst_ximagesink_reset):
+       * sys/xvimage/xvimagesink.c: (gst_xvimagesink_event_thread),
+       (gst_xvimagesink_xcontext_get), (gst_xvimagesink_xcontext_clear),
+       (gst_xvimagesink_change_state), (gst_xvimagesink_reset):
+       Make sure that before we clean up the X resources, we shutdown and join
+       the event thread.
+       Also make sure the event thread does not shut down immediatly after
+       startup because the running variable is not yet correctly set.
+       Fixes #378770. 
+
 2007-10-16  Wim Taymans  <wim.taymans@gmail.com>
 
        * gst/playback/gstdecodebin.c: (new_pad), (type_found):

diff --git a/sys/ximage/ximagesink.c b/sys/ximage/ximagesink.c
index 4a6f095..1bed764 100644 (file)
--- a/sys/ximage/ximagesink.c
+++ b/sys/ximage/ximagesink.c
@@ -1089,12 +1089,18 @@ gst_ximagesink_event_thread (GstXImageSink * ximagesink)
 {
   g_return_val_if_fail (GST_IS_XIMAGESINK (ximagesink), NULL);
 
+  GST_OBJECT_LOCK (ximagesink);
   while (ximagesink->running) {
+    GST_OBJECT_UNLOCK (ximagesink);
+
     if (ximagesink->xwindow) {
       gst_ximagesink_handle_xevents (ximagesink);
     }
     g_usleep (100000);
+
+    GST_OBJECT_LOCK (ximagesink);
   }
+  GST_OBJECT_UNLOCK (ximagesink);
 
   return NULL;
 }
@@ -1279,8 +1285,11 @@ gst_ximagesink_xcontext_get (GstXImageSink * ximagesink)
   g_mutex_unlock (ximagesink->x_lock);
 
   /* Setup our event listening thread */
+  GST_OBJECT_LOCK (ximagesink);
+  ximagesink->running = TRUE;
   ximagesink->event_thread = g_thread_create (
       (GThreadFunc) gst_ximagesink_event_thread, ximagesink, TRUE, NULL);
+  GST_OBJECT_UNLOCK (ximagesink);
 
   return xcontext;
 }
@@ -1308,12 +1317,6 @@ gst_ximagesink_xcontext_clear (GstXImageSink * ximagesink)
 
   GST_OBJECT_UNLOCK (ximagesink);
 
-  /* Wait for our event thread */
-  if (ximagesink->event_thread) {
-    g_thread_join (ximagesink->event_thread);
-    ximagesink->event_thread = NULL;
-  }
-
   gst_caps_unref (xcontext->caps);
   g_free (xcontext->par);
   g_free (ximagesink->par);
@@ -1497,19 +1500,18 @@ gst_ximagesink_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_NULL_TO_READY:
 
       /* Initializing the XContext */
-      if (!ximagesink->xcontext)
+      if (ximagesink->xcontext == NULL) {
         xcontext = gst_ximagesink_xcontext_get (ximagesink);
-
-      GST_OBJECT_LOCK (ximagesink);
-      ximagesink->running = TRUE;
-      if (xcontext)
-        ximagesink->xcontext = xcontext;
-      GST_OBJECT_UNLOCK (ximagesink);
-
-      if (!ximagesink->xcontext) {
-        ret = GST_STATE_CHANGE_FAILURE;
-        goto beach;
+        if (xcontext == NULL) {
+          ret = GST_STATE_CHANGE_FAILURE;
+          goto beach;
+        }
+        GST_OBJECT_LOCK (ximagesink);
+        if (xcontext)
+          ximagesink->xcontext = xcontext;
+        GST_OBJECT_UNLOCK (ximagesink);
       }
+
       /* call XSynchronize with the current value of synchronous */
       GST_DEBUG_OBJECT (ximagesink, "XSynchronize called with %s",
           ximagesink->synchronous ? "TRUE" : "FALSE");
@@ -2097,10 +2099,19 @@ gst_ximagesink_get_property (GObject * object, guint prop_id,
 static void
 gst_ximagesink_reset (GstXImageSink * ximagesink)
 {
+  GThread *thread;
+
   GST_OBJECT_LOCK (ximagesink);
   ximagesink->running = FALSE;
+  /* grab thread and mark it as NULL */
+  thread = ximagesink->event_thread;
+  ximagesink->event_thread = NULL;
   GST_OBJECT_UNLOCK (ximagesink);
 
+  /* Wait for our event thread to finish before we clean up our stuff. */
+  if (thread)
+    g_thread_join (thread);
+
   if (ximagesink->ximage) {
     gst_buffer_unref (ximagesink->ximage);
     ximagesink->ximage = NULL;

diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c
index 0bb1612..af5b8a7 100644 (file)
--- a/sys/xvimage/xvimagesink.c
+++ b/sys/xvimage/xvimagesink.c
@@ -1523,12 +1523,18 @@ gst_xvimagesink_event_thread (GstXvImageSink * xvimagesink)
 {
   g_return_val_if_fail (GST_IS_XVIMAGESINK (xvimagesink), NULL);
 
+  GST_OBJECT_LOCK (xvimagesink);
   while (xvimagesink->running) {
+    GST_OBJECT_UNLOCK (xvimagesink);
+
     if (xvimagesink->xwindow) {
       gst_xvimagesink_handle_xevents (xvimagesink);
     }
     g_usleep (50000);
+
+    GST_OBJECT_LOCK (xvimagesink);
   }
+  GST_OBJECT_UNLOCK (xvimagesink);
 
   return NULL;
 }
@@ -1761,8 +1767,11 @@ gst_xvimagesink_xcontext_get (GstXvImageSink * xvimagesink)
   g_mutex_unlock (xvimagesink->x_lock);
 
   /* Setup our event listening thread */
+  GST_OBJECT_LOCK (xvimagesink);
+  xvimagesink->running = TRUE;
   xvimagesink->event_thread = g_thread_create (
       (GThreadFunc) gst_xvimagesink_event_thread, xvimagesink, TRUE, NULL);
+  GST_OBJECT_UNLOCK (xvimagesink);
 
   return xcontext;
 }
@@ -1790,11 +1799,6 @@ gst_xvimagesink_xcontext_clear (GstXvImageSink * xvimagesink)
 
   GST_OBJECT_UNLOCK (xvimagesink);
 
-  /* Wait for our event thread */
-  if (xvimagesink->event_thread) {
-    g_thread_join (xvimagesink->event_thread);
-    xvimagesink->event_thread = NULL;
-  }
 
   formats_list = xcontext->formats_list;
 
@@ -2088,14 +2092,12 @@ gst_xvimagesink_change_state (GstElement * element, GstStateChange transition)
         xcontext = gst_xvimagesink_xcontext_get (xvimagesink);
         if (xcontext == NULL)
           return GST_STATE_CHANGE_FAILURE;
+        GST_OBJECT_LOCK (xvimagesink);
+        if (xcontext)
+          xvimagesink->xcontext = xcontext;
+        GST_OBJECT_UNLOCK (xvimagesink);
       }
 
-      GST_OBJECT_LOCK (xvimagesink);
-      xvimagesink->running = TRUE;
-      if (xcontext)
-        xvimagesink->xcontext = xcontext;
-      GST_OBJECT_UNLOCK (xvimagesink);
-
       /* update object's par with calculated one if not set yet */
       if (!xvimagesink->par) {
         xvimagesink->par = g_new0 (GValue, 1);
@@ -2952,9 +2954,19 @@ gst_xvimagesink_get_property (GObject * object, guint prop_id,
 static void
 gst_xvimagesink_reset (GstXvImageSink * xvimagesink)
 {
+  GThread *thread;
+
   GST_OBJECT_LOCK (xvimagesink);
   xvimagesink->running = FALSE;
+  /* grab thread and mark it as NULL */
+  thread = xvimagesink->event_thread;
+  xvimagesink->event_thread = NULL;
   GST_OBJECT_UNLOCK (xvimagesink);
+
+  /* Wait for our event thread to finish before we clean up our stuff. */
+  if (thread)
+    g_thread_join (thread);
+
   if (xvimagesink->cur_image) {
     gst_buffer_unref (xvimagesink->cur_image);
     xvimagesink->cur_image = NULL;