sys/: Move some cleanup stuff from the state change handler into a _reset() function...

Original commit message from CVS:
* sys/ximage/ximagesink.c: (gst_ximagesink_xcontext_clear),
(gst_ximagesink_change_state), (gst_ximagesink_reset),
(gst_ximagesink_finalize):
* sys/xvimage/xvimagesink.c: (gst_xvimagesink_change_state),
(gst_xvimagesink_reset), (gst_xvimagesink_finalize):
Move some cleanup stuff from the state change handler into a _reset()
function that can be called from _finalize(). This ensures that things
get freed even if (for some reason) the NULL->READY state transition
fails in the parent class.
Even if a parent state change fails, process our downward state change
logic instead of bailing out early.
Free the correct xcontext pointer in ximagesink's xcontext_clear.

diff --git a/ChangeLog b/ChangeLog
index b608ba6..4133d29 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2007-03-08  Jan Schmidt  <thaytan@mad.scientist.com>
 
+       * sys/ximage/ximagesink.c: (gst_ximagesink_xcontext_clear),
+       (gst_ximagesink_change_state), (gst_ximagesink_reset),
+       (gst_ximagesink_finalize):
+       * sys/xvimage/xvimagesink.c: (gst_xvimagesink_change_state),
+       (gst_xvimagesink_reset), (gst_xvimagesink_finalize):
+       Move some cleanup stuff from the state change handler into a _reset()
+       function that can be called from _finalize(). This ensures that things
+       get freed even if (for some reason) the NULL->READY state transition
+       fails in the parent class.
+       Even if a parent state change fails, process our downward state change
+       logic instead of bailing out early.
+       Free the correct xcontext pointer in ximagesink's xcontext_clear.
+
+2007-03-08  Jan Schmidt  <thaytan@mad.scientist.com>
+
        * ext/alsa/gstalsasink.c: (gst_alsasink_open):
        Extra log line.
 

diff --git a/sys/ximage/ximagesink.c b/sys/ximage/ximagesink.c
index 38f367b..3ecd805 100644 (file)
--- a/sys/ximage/ximagesink.c
+++ b/sys/ximage/ximagesink.c
@@ -136,6 +136,7 @@ MotifWmHints, MwmHints;
 
 #define MWM_HINTS_DECORATIONS   (1L << 1)
 
+static void gst_ximagesink_reset (GstXImageSink * ximagesink);
 static void gst_ximagesink_ximage_destroy (GstXImageSink * ximagesink,
     GstXImageBuffer * ximage);
 static void gst_ximagesink_xwindow_update_geometry (GstXImageSink * ximagesink,
@@ -1260,7 +1261,7 @@ gst_ximagesink_xcontext_clear (GstXImageSink * ximagesink)
 
   g_mutex_unlock (ximagesink->x_lock);
 
-  g_free (ximagesink->xcontext);
+  g_free (xcontext);
 }
 
 static void
@@ -1465,8 +1466,6 @@ gst_ximagesink_change_state (GstElement * element, GstStateChange transition)
   }
 
   ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
-  if (ret == GST_STATE_CHANGE_FAILURE)
-    return ret;
 
   switch (transition) {
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
@@ -1478,30 +1477,7 @@ gst_ximagesink_change_state (GstElement * element, GstStateChange transition)
       GST_VIDEO_SINK_HEIGHT (ximagesink) = 0;
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
-      GST_OBJECT_LOCK (ximagesink);
-      ximagesink->running = FALSE;
-      GST_OBJECT_UNLOCK (ximagesink);
-
-      if (ximagesink->ximage) {
-        gst_buffer_unref (ximagesink->ximage);
-        ximagesink->ximage = NULL;
-      }
-      if (ximagesink->cur_image) {
-        gst_buffer_unref (ximagesink->cur_image);
-        ximagesink->cur_image = NULL;
-      }
-
-      gst_ximagesink_bufferpool_clear (ximagesink);
-
-      g_mutex_lock (ximagesink->flow_lock);
-      if (ximagesink->xwindow) {
-        gst_ximagesink_xwindow_clear (ximagesink, ximagesink->xwindow);
-        gst_ximagesink_xwindow_destroy (ximagesink, ximagesink->xwindow);
-        ximagesink->xwindow = NULL;
-      }
-      g_mutex_unlock (ximagesink->flow_lock);
-
-      gst_ximagesink_xcontext_clear (ximagesink);
+      gst_ximagesink_reset (ximagesink);
       break;
     default:
       break;
@@ -2041,12 +2017,43 @@ gst_ximagesink_get_property (GObject * object, guint prop_id,
 }
 
 static void
+gst_ximagesink_reset (GstXImageSink * ximagesink)
+{
+  GST_OBJECT_LOCK (ximagesink);
+  ximagesink->running = FALSE;
+  GST_OBJECT_UNLOCK (ximagesink);
+
+  if (ximagesink->ximage) {
+    gst_buffer_unref (ximagesink->ximage);
+    ximagesink->ximage = NULL;
+  }
+  if (ximagesink->cur_image) {
+    gst_buffer_unref (ximagesink->cur_image);
+    ximagesink->cur_image = NULL;
+  }
+
+  gst_ximagesink_bufferpool_clear (ximagesink);
+
+  g_mutex_lock (ximagesink->flow_lock);
+  if (ximagesink->xwindow) {
+    gst_ximagesink_xwindow_clear (ximagesink, ximagesink->xwindow);
+    gst_ximagesink_xwindow_destroy (ximagesink, ximagesink->xwindow);
+    ximagesink->xwindow = NULL;
+  }
+  g_mutex_unlock (ximagesink->flow_lock);
+
+  gst_ximagesink_xcontext_clear (ximagesink);
+}
+
+static void
 gst_ximagesink_finalize (GObject * object)
 {
   GstXImageSink *ximagesink;
 
   ximagesink = GST_XIMAGESINK (object);
 
+  gst_ximagesink_reset (ximagesink);
+
   if (ximagesink->display_name) {
     g_free (ximagesink->display_name);
     ximagesink->display_name = NULL;

diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c
index 8b59f34..1d2bcda 100644 (file)
--- a/sys/xvimage/xvimagesink.c
+++ b/sys/xvimage/xvimagesink.c
@@ -149,6 +149,8 @@ MotifWmHints, MwmHints;
 
 #define MWM_HINTS_DECORATIONS   (1L << 1)
 
+static void gst_xvimagesink_reset (GstXvImageSink * xvimagesink);
+
 static void gst_xvimage_buffer_finalize (GstXvImageBuffer * xvimage);
 
 static void gst_xvimagesink_xwindow_update_geometry (GstXvImageSink *
@@ -2032,8 +2034,6 @@ gst_xvimagesink_change_state (GstElement * element, GstStateChange transition)
   }
 
   ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
-  if (ret == GST_STATE_CHANGE_FAILURE)
-    return ret;
 
   switch (transition) {
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
@@ -2045,27 +2045,7 @@ gst_xvimagesink_change_state (GstElement * element, GstStateChange transition)
       GST_VIDEO_SINK_HEIGHT (xvimagesink) = 0;
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
-      GST_OBJECT_LOCK (xvimagesink);
-      xvimagesink->running = FALSE;
-      GST_OBJECT_UNLOCK (xvimagesink);
-      if (xvimagesink->cur_image) {
-        gst_buffer_unref (xvimagesink->cur_image);
-        xvimagesink->cur_image = NULL;
-      }
-      if (xvimagesink->xvimage) {
-        gst_buffer_unref (xvimagesink->xvimage);
-        xvimagesink->xvimage = NULL;
-      }
-
-      gst_xvimagesink_imagepool_clear (xvimagesink);
-
-      if (xvimagesink->xwindow) {
-        gst_xvimagesink_xwindow_clear (xvimagesink, xvimagesink->xwindow);
-        gst_xvimagesink_xwindow_destroy (xvimagesink, xvimagesink->xwindow);
-        xvimagesink->xwindow = NULL;
-      }
-
-      gst_xvimagesink_xcontext_clear (xvimagesink);
+      gst_xvimagesink_reset (xvimagesink);
       break;
     default:
       break;
@@ -2862,6 +2842,32 @@ gst_xvimagesink_get_property (GObject * object, guint prop_id,
   }
 }
 
+static void
+gst_xvimagesink_reset (GstXvImageSink * xvimagesink)
+{
+  GST_OBJECT_LOCK (xvimagesink);
+  xvimagesink->running = FALSE;
+  GST_OBJECT_UNLOCK (xvimagesink);
+  if (xvimagesink->cur_image) {
+    gst_buffer_unref (xvimagesink->cur_image);
+    xvimagesink->cur_image = NULL;
+  }
+  if (xvimagesink->xvimage) {
+    gst_buffer_unref (xvimagesink->xvimage);
+    xvimagesink->xvimage = NULL;
+  }
+
+  gst_xvimagesink_imagepool_clear (xvimagesink);
+
+  if (xvimagesink->xwindow) {
+    gst_xvimagesink_xwindow_clear (xvimagesink, xvimagesink->xwindow);
+    gst_xvimagesink_xwindow_destroy (xvimagesink, xvimagesink->xwindow);
+    xvimagesink->xwindow = NULL;
+  }
+
+  gst_xvimagesink_xcontext_clear (xvimagesink);
+}
+
 /* Finalize is called only once, dispose can be called multiple times.
  * We use mutexes and don't reset stuff to NULL here so let's register
  * as a finalize. */
@@ -2872,6 +2878,8 @@ gst_xvimagesink_finalize (GObject * object)
 
   xvimagesink = GST_XVIMAGESINK (object);
 
+  gst_xvimagesink_reset (xvimagesink);
+
   if (xvimagesink->display_name) {
     g_free (xvimagesink->display_name);
     xvimagesink->display_name = NULL;