gst/gdp/: Handle error cases when calling functions do downwards state change after parent's change_state

Original commit message from CVS:
* gst/gdp/gstgdpdepay.c: (gst_gdp_depay_chain),
(gst_gdp_depay_change_state):
* gst/gdp/gstgdpdepay.h:
* gst/gdp/gstgdppay.c: (gst_gdp_pay_reset_streamheader),
(gst_gdp_pay_chain), (gst_gdp_pay_sink_event),
(gst_gdp_pay_change_state):
* gst/gdp/gstgdppay.h:
Handle error cases when calling functions
do downwards state change after parent's change_state
* tests/check/elements/gdpdepay.c: (GST_START_TEST):
* tests/check/elements/gdppay.c: (GST_START_TEST):
clean up more

gst/gdp/gstgdpdepay.c

@@ -42,7 +42,7 @@
 /* elementfactory information */
 static const GstElementDetails gdp_depay_details =
 GST_ELEMENT_DETAILS ("GDP Depayloader",
-    "Filter/Effect/Video",
+    "GDP/Depayloader",
     "Depayloads GStreamer Data Protocol buffers",
     "Thomas Vander Stichele <thomas at apestaart dot org>");
 
@@ -221,6 +221,13 @@ gst_gdp_depay_chain (GstPad * pad, GstBuffer * buffer)
 
         GST_LOG_OBJECT (this, "reading GDP buffer from adapter");
         buf = gst_dp_buffer_from_header (GST_DP_HEADER_LENGTH, this->header);
+        if (!buf) {
+          GST_ELEMENT_ERROR (this, STREAM, DECODE, (NULL),
+              ("could not create buffer from GDP packet"));
+          ret = GST_FLOW_ERROR;
+          goto done;
+        }
+
         payload = gst_adapter_take (this->adapter, this->payload_length);
         memcpy (GST_BUFFER_DATA (buf), payload, this->payload_length);
         g_free (payload);
@@ -243,6 +250,12 @@ gst_gdp_depay_chain (GstPad * pad, GstBuffer * buffer)
         caps = gst_dp_caps_from_packet (GST_DP_HEADER_LENGTH, this->header,
             payload);
         g_free (payload);
+        if (!caps) {
+          GST_ELEMENT_ERROR (this, STREAM, DECODE, (NULL),
+              ("could not create caps from GDP packet"));
+          ret = GST_FLOW_ERROR;
+          goto done;
+        }
         GST_DEBUG_OBJECT (this, "read caps %" GST_PTR_FORMAT, caps);
         gst_caps_replace (&(this->caps), caps);
         gst_pad_set_caps (this->srcpad, caps);
@@ -260,6 +273,14 @@ gst_gdp_depay_chain (GstPad * pad, GstBuffer * buffer)
           payload = gst_adapter_take (this->adapter, this->payload_length);
         event = gst_dp_event_from_packet (GST_DP_HEADER_LENGTH, this->header,
             payload);
+        if (payload)
+          g_free (payload);
+        if (!event) {
+          GST_ELEMENT_ERROR (this, STREAM, DECODE, (NULL),
+              ("could not create event from GDP packet"));
+          ret = GST_FLOW_ERROR;
+          goto done;
+        }
         /* FIXME: set me as source ? */
         gst_pad_push_event (this->srcpad, event);
 
@@ -295,6 +316,8 @@ gst_gdp_depay_change_state (GstElement * element, GstStateChange transition)
   GstStateChangeReturn ret;
   GstGDPDepay *this = GST_GDP_DEPAY (element);
 
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
+
   switch (transition) {
     case GST_STATE_CHANGE_READY_TO_NULL:
       if (this->caps) {
@@ -306,8 +329,6 @@ gst_gdp_depay_change_state (GstElement * element, GstStateChange transition)
       break;
   }
 
-  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
-
   return ret;
 }
 

gst/gdp/gstgdpdepay.h

@@ -1,5 +1,5 @@
-/* Gnome-Streamer
- * Copyright (C) <2005> Wim Taymans <wim@fluendo.com>
+/* GStreamer
+ * Copyright (C) 2006 Thomas Vander Stichele <thomas at apestaart dot org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public

gst/gdp/gstgdppay.c

@@ -39,7 +39,7 @@
 /* elementfactory information */
 static const GstElementDetails gdp_pay_details =
 GST_ELEMENT_DETAILS ("GDP Payloader",
-    "Filter/Effect/Video",
+    "GDP/Payloader",
     "Payloads GStreamer Data Protocol buffers",
     "Thomas Vander Stichele <thomas at apestaart dot org>");
 
@@ -260,8 +260,6 @@ gst_gdp_pay_reset_streamheader (GstGDPPay * this)
 
   /* we also need to add GDP serializations of the streamheaders of the
    * incoming caps */
-  /* FIXME: HEREIAM */
-
   structure = gst_caps_get_structure (this->caps, 0);
   if (gst_structure_has_field (structure, "streamheader")) {
     const GValue *sh;
@@ -281,10 +279,14 @@ gst_gdp_pay_reset_streamheader (GstGDPPay * this)
       bufval = &g_array_index (buffers, GValue, i);
       buffer = g_value_peek_pointer (bufval);
       outbuffer = gst_gdp_pay_buffer_from_buffer (this, buffer);
-      g_value_init (&value, GST_TYPE_BUFFER);
-      gst_value_set_buffer (&value, outbuffer);
-      gst_value_array_append_value (&array, &value);
-      g_value_unset (&value);
+      if (outbuffer) {
+        g_value_init (&value, GST_TYPE_BUFFER);
+        gst_value_set_buffer (&value, outbuffer);
+        gst_value_array_append_value (&array, &value);
+        g_value_unset (&value);
+      }
+      /* FIXME: if one or more in this loop fail to produce and outbuffer,
+       * should we error out ? Once ? Every time ? */
     }
   }
 
@@ -373,11 +375,10 @@ gst_gdp_pay_chain (GstPad * pad, GstBuffer * buffer)
   GstGDPPay *this;
   GstCaps *caps;
   GstBuffer *outbuffer;
+  GstFlowReturn ret;
 
   this = GST_GDP_PAY (gst_pad_get_parent (pad));
 
-  caps = gst_buffer_get_caps (buffer);
-
   /* we should have received a new_segment before, otherwise it's a bug.
    * fake one in that case */
   if (!this->new_segment_buf) {
@@ -387,25 +388,44 @@ gst_gdp_pay_chain (GstPad * pad, GstBuffer * buffer)
         "did not receive new-segment before first buffer");
     event = gst_event_new_new_segment (TRUE, 1.0, GST_FORMAT_BYTES, 0, -1, 0);
     outbuffer = gst_gdp_buffer_from_event (this, event);
+    gst_event_unref (event);
+
+    if (!outbuffer) {
+      GST_ELEMENT_ERROR (this, STREAM, ENCODE, (NULL),
+          ("Could not create GDP buffer from new segment event"));
+      ret = GST_FLOW_ERROR;
+      goto done;
+    }
+
     gst_gdp_stamp_buffer (this, outbuffer);
     GST_BUFFER_TIMESTAMP (outbuffer) = GST_BUFFER_TIMESTAMP (buffer);
     GST_BUFFER_DURATION (outbuffer) = 0;
     this->new_segment_buf = outbuffer;
-    gst_event_unref (event);
   }
 
   /* make sure we've received caps before */
+  caps = gst_buffer_get_caps (buffer);
   if (!this->caps && !caps) {
     GST_WARNING_OBJECT (this, "first received buffer does not have caps set");
-    gst_buffer_unref (buffer);
-    gst_object_unref (this);
-    return GST_FLOW_NOT_NEGOTIATED;
+    if (caps)
+      gst_caps_unref (caps);
+    ret = GST_FLOW_NOT_NEGOTIATED;
+    goto done;
   }
+
   /* if the caps have changed, process caps first */
   if (caps && !gst_caps_is_equal (this->caps, caps)) {
     GST_LOG_OBJECT (this, "caps changed to %p, %" GST_PTR_FORMAT, caps, caps);
     gst_caps_replace (&(this->caps), caps);
     outbuffer = gst_gdp_buffer_from_caps (this, caps);
+    if (!outbuffer) {
+      GST_ELEMENT_ERROR (this, STREAM, ENCODE, (NULL),
+          ("Could not create GDP buffer from caps %" GST_PTR_FORMAT, caps));
+      gst_caps_unref (caps);
+      ret = GST_FLOW_ERROR;
+      goto done;
+    }
+
     gst_gdp_stamp_buffer (this, outbuffer);
     GST_BUFFER_TIMESTAMP (outbuffer) = GST_BUFFER_TIMESTAMP (buffer);
     GST_BUFFER_DURATION (outbuffer) = 0;
@@ -417,13 +437,23 @@ gst_gdp_pay_chain (GstPad * pad, GstBuffer * buffer)
   /* create a GDP header packet,
    * then create a GST buffer of the header packet and the buffer contents */
   outbuffer = gst_gdp_pay_buffer_from_buffer (this, buffer);
+  if (!outbuffer) {
+    GST_ELEMENT_ERROR (this, STREAM, ENCODE, (NULL),
+        ("Could not create GDP buffer from buffer"));
+    ret = GST_FLOW_ERROR;
+    goto done;
+  }
+
   gst_gdp_stamp_buffer (this, outbuffer);
   GST_BUFFER_TIMESTAMP (outbuffer) = GST_BUFFER_TIMESTAMP (buffer);
   GST_BUFFER_DURATION (outbuffer) = GST_BUFFER_DURATION (buffer);
-  gst_buffer_unref (buffer);
 
+  ret = gst_gdp_queue_buffer (this, outbuffer);
+
+done:
+  gst_buffer_unref (buffer);
   gst_object_unref (this);
-  return gst_gdp_queue_buffer (this, outbuffer);
+  return ret;
 }
 
 static gboolean
@@ -436,6 +466,12 @@ gst_gdp_pay_sink_event (GstPad * pad, GstEvent * event)
 
   /* now turn the event into a buffer */
   outbuffer = gst_gdp_buffer_from_event (this, event);
+  if (!outbuffer) {
+    GST_ELEMENT_ERROR (this, STREAM, ENCODE, (NULL),
+        ("Could not create GDP buffer from event"));
+    ret = FALSE;
+    goto done;
+  }
   gst_gdp_stamp_buffer (this, outbuffer);
   GST_BUFFER_TIMESTAMP (outbuffer) = GST_EVENT_TIMESTAMP (event);
   GST_BUFFER_DURATION (outbuffer) = 0;
@@ -476,6 +512,8 @@ gst_gdp_pay_change_state (GstElement * element, GstStateChange transition)
   GstStateChangeReturn ret;
   GstGDPPay *this = GST_GDP_PAY (element);
 
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
+
   switch (transition) {
     case GST_STATE_CHANGE_READY_TO_NULL:
       if (this->caps) {
@@ -487,8 +525,6 @@ gst_gdp_pay_change_state (GstElement * element, GstStateChange transition)
       break;
   }
 
-  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
-
   return ret;
 }
 

gst/gdp/gstgdppay.h

@@ -1,5 +1,5 @@
-/* Gnome-Streamer
- * Copyright (C) <2005> Wim Taymans <wim@fluendo.com>
+/* GStreamer
+ * Copyright (C) 2006 Thomas Vander Stichele <thomas at apestaart dot org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public

tests/check/elements/gdpdepay.c

@@ -163,6 +163,9 @@ GST_START_TEST (test_audio_per_byte)
           GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
 
   ASSERT_OBJECT_REFCOUNT (gdpdepay, "gdpdepay", 1);
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   gst_object_unref (srcpad);
   cleanup_gdpdepay (gdpdepay);
 }
@@ -232,6 +235,9 @@ GST_START_TEST (test_audio_in_one_buffer)
           GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
 
   gst_object_unref (srcpad);
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdpdepay, "gdpdepay", 1);
   cleanup_gdpdepay (gdpdepay);
 }
@@ -361,6 +367,9 @@ GST_START_TEST (test_streamheader)
           GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
 
   gst_object_unref (srcpad);
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdpdepay, "gdpdepay", 1);
   cleanup_gdpdepay (gdpdepay);
 }

tests/check/elements/gdppay.c

@@ -193,6 +193,9 @@ GST_START_TEST (test_audio)
 
   gst_caps_unref (caps);
   g_free (caps_string);
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdppay, "gdppay", 1);
   gst_object_unref (gdppay);
 }
@@ -362,6 +365,9 @@ GST_START_TEST (test_streamheader)
 
   gst_caps_unref (caps);
   g_free (caps_string);
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdppay, "gdppay", 1);
   gst_object_unref (gdppay);
 }
@@ -392,6 +398,9 @@ GST_START_TEST (test_first_no_caps)
   fail_unless (gst_element_set_state (gdppay,
           GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
 
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdppay, "gdppay", 1);
   gst_object_unref (gdppay);
 }
@@ -430,6 +439,9 @@ GST_START_TEST (test_first_no_new_segment)
   fail_unless (gst_element_set_state (gdppay,
           GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
 
+  g_list_foreach (buffers, (GFunc) gst_mini_object_unref, NULL);
+  g_list_free (buffers);
+  buffers = NULL;
   ASSERT_OBJECT_REFCOUNT (gdppay, "gdppay", 1);
   gst_object_unref (gdppay);
 }