From bc93eeb57503ec11e72ffc20458c6681bf86adb1 Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Mon, 22 May 2006 15:53:07 +0000 Subject: [PATCH] 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 | 27 +++++++++++-- gst/gdp/gstgdpdepay.h | 4 +- gst/gdp/gstgdppay.c | 70 +++++++++++++++++++++++++-------- gst/gdp/gstgdppay.h | 4 +- tests/check/elements/gdpdepay.c | 9 +++++ tests/check/elements/gdppay.c | 12 ++++++ 6 files changed, 102 insertions(+), 24 deletions(-) diff --git a/gst/gdp/gstgdpdepay.c b/gst/gdp/gstgdpdepay.c index 5a37d98430..89de9b4f9a 100644 --- a/gst/gdp/gstgdpdepay.c +++ b/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 "); @@ -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; } diff --git a/gst/gdp/gstgdpdepay.h b/gst/gdp/gstgdpdepay.h index d39ec5a5bd..8345c8b646 100644 --- a/gst/gdp/gstgdpdepay.h +++ b/gst/gdp/gstgdpdepay.h @@ -1,5 +1,5 @@ -/* Gnome-Streamer - * Copyright (C) <2005> Wim Taymans +/* GStreamer + * Copyright (C) 2006 Thomas Vander Stichele * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public diff --git a/gst/gdp/gstgdppay.c b/gst/gdp/gstgdppay.c index e40d2f1ba0..f498477481 100644 --- a/gst/gdp/gstgdppay.c +++ b/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 "); @@ -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; } diff --git a/gst/gdp/gstgdppay.h b/gst/gdp/gstgdppay.h index 255d6c772a..2dc02d91af 100644 --- a/gst/gdp/gstgdppay.h +++ b/gst/gdp/gstgdppay.h @@ -1,5 +1,5 @@ -/* Gnome-Streamer - * Copyright (C) <2005> Wim Taymans +/* GStreamer + * Copyright (C) 2006 Thomas Vander Stichele * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public diff --git a/tests/check/elements/gdpdepay.c b/tests/check/elements/gdpdepay.c index 4e8c9d5066..ff24ffdb19 100644 --- a/tests/check/elements/gdpdepay.c +++ b/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); } diff --git a/tests/check/elements/gdppay.c b/tests/check/elements/gdppay.c index 5915ef32c1..6c11759893 100644 --- a/tests/check/elements/gdppay.c +++ b/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); }