From f1976e0de509c919f33fbebd32fd37ee96bdf719 Mon Sep 17 00:00:00 2001 From: Ederson de Souza Date: Fri, 24 Jan 2020 15:47:56 -0800 Subject: [PATCH] avtp: Plug several leaks After finally running tests with valgrind enabled, some leaks were found - both on code and on tests themselves. This patch plugs them all! --- ext/avtp/gstavtpaafpay.c | 7 +++++-- ext/avtp/gstavtpcvfdepay.c | 28 ++++++++++++++++++++++++++++ ext/avtp/gstavtpcvfpay.c | 5 ++++- tests/check/elements/avtpaafdepay.c | 1 + tests/check/elements/avtpaafpay.c | 1 + tests/check/elements/avtpcvfdepay.c | 19 +++++++++++++++++++ tests/check/elements/avtpcvfpay.c | 6 ++++++ 7 files changed, 64 insertions(+), 3 deletions(-) diff --git a/ext/avtp/gstavtpaafpay.c b/ext/avtp/gstavtpaafpay.c index 1174f8e..fd6b1e6 100644 --- a/ext/avtp/gstavtpaafpay.c +++ b/ext/avtp/gstavtpaafpay.c @@ -196,7 +196,7 @@ gst_avtp_aaf_pay_change_state (GstElement * element, GstStateChange transition) GST_ERROR_OBJECT (avtpaafpay, "Failed to allocate GstMemory"); return GST_STATE_CHANGE_FAILURE; } - avtpaafpay->header = gst_memory_ref (mem); + avtpaafpay->header = mem; break; } case GST_STATE_CHANGE_READY_TO_PAUSED:{ @@ -366,13 +366,16 @@ gst_avtp_aaf_pay_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) { GstCaps *caps; GstAvtpAafPay *avtpaafpay = GST_AVTP_AAF_PAY (parent); + gboolean ret; GST_DEBUG_OBJECT (avtpaafpay, "event %s", GST_EVENT_TYPE_NAME (event)); switch (GST_EVENT_TYPE (event)) { case GST_EVENT_CAPS: gst_event_parse_caps (event, &caps); - return gst_avtp_aaf_pay_new_caps (avtpaafpay, caps); + ret = gst_avtp_aaf_pay_new_caps (avtpaafpay, caps); + gst_event_unref (event); + return ret; default: return GST_AVTP_BASE_PAYLOAD_CLASS (parent_class)->sink_event (pad, parent, event); diff --git a/ext/avtp/gstavtpcvfdepay.c b/ext/avtp/gstavtpcvfdepay.c index 6bb9650..eb3af0d 100644 --- a/ext/avtp/gstavtpcvfdepay.c +++ b/ext/avtp/gstavtpcvfdepay.c @@ -51,6 +51,8 @@ GST_DEBUG_CATEGORY_STATIC (avtpcvfdepay_debug); static GstFlowReturn gst_avtp_cvf_depay_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer); +static GstStateChangeReturn gst_avtp_cvf_depay_change_state (GstElement * + element, GstStateChange transition); #define AVTP_CVF_H264_HEADER_SIZE (sizeof(struct avtp_stream_pdu) + sizeof(guint32)) #define FU_A_HEADER_SIZE (sizeof(guint16)) @@ -78,6 +80,7 @@ static GstStaticPadTemplate src_template = GST_STATIC_PAD_TEMPLATE ("src", " stream-format = (string) avc, alignment = (string) au") ); +#define gst_avtp_cvf_depay_parent_class parent_class G_DEFINE_TYPE (GstAvtpCvfDepay, gst_avtp_cvf_depay, GST_TYPE_AVTP_BASE_DEPAYLOAD); @@ -96,6 +99,9 @@ gst_avtp_cvf_depay_class_init (GstAvtpCvfDepayClass * klass) "Extracts compressed video from CVF AVTPDUs", "Ederson de Souza "); + element_class->change_state = + GST_DEBUG_FUNCPTR (gst_avtp_cvf_depay_change_state); + avtpbasedepayload_class->chain = GST_DEBUG_FUNCPTR (gst_avtp_cvf_depay_chain); GST_DEBUG_CATEGORY_INIT (avtpcvfdepay_debug, "avtpcvfdepay", @@ -110,6 +116,28 @@ gst_avtp_cvf_depay_init (GstAvtpCvfDepay * avtpcvfdepay) avtpcvfdepay->seqnum = 0; } +static GstStateChangeReturn +gst_avtp_cvf_depay_change_state (GstElement * + element, GstStateChange transition) +{ + GstAvtpCvfDepay *avtpcvfdepay = GST_AVTP_CVF_DEPAY (element); + GstStateChangeReturn ret; + + ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); + if (ret == GST_STATE_CHANGE_FAILURE) { + return ret; + } + + if (transition == GST_STATE_CHANGE_READY_TO_NULL) { + if (avtpcvfdepay->out_buffer) { + gst_buffer_unref (avtpcvfdepay->out_buffer); + avtpcvfdepay->out_buffer = NULL; + } + } + + return ret; +} + static gboolean gst_avtp_cvf_depay_push_caps (GstAvtpCvfDepay * avtpcvfdepay) { diff --git a/ext/avtp/gstavtpcvfpay.c b/ext/avtp/gstavtpcvfpay.c index dcbdd9d..f60911b 100644 --- a/ext/avtp/gstavtpcvfpay.c +++ b/ext/avtp/gstavtpcvfpay.c @@ -584,13 +584,16 @@ gst_avtp_cvf_pay_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) GstCaps *caps; GstAvtpBasePayload *avtpbasepayload = GST_AVTP_BASE_PAYLOAD (parent); GstAvtpCvfPay *avtpcvfpay = GST_AVTP_CVF_PAY (avtpbasepayload); + gboolean ret; GST_DEBUG_OBJECT (avtpcvfpay, "Sink event %s", GST_EVENT_TYPE_NAME (event)); switch (GST_EVENT_TYPE (event)) { case GST_EVENT_CAPS: gst_event_parse_caps (event, &caps); - return gst_avtp_cvf_pay_new_caps (avtpcvfpay, caps); + ret = gst_avtp_cvf_pay_new_caps (avtpcvfpay, caps); + gst_event_unref (event); + return ret; default: break; } diff --git a/tests/check/elements/avtpaafdepay.c b/tests/check/elements/avtpaafdepay.c index 4441275..267461b 100644 --- a/tests/check/elements/avtpaafdepay.c +++ b/tests/check/elements/avtpaafdepay.c @@ -277,6 +277,7 @@ GST_START_TEST (test_property) g_object_get (G_OBJECT (element), "streamid", &val, NULL); fail_unless (val == streamid); + gst_object_unref (element); gst_harness_teardown (h); } diff --git a/tests/check/elements/avtpaafpay.c b/tests/check/elements/avtpaafpay.c index ff366ec..c09c6c6 100644 --- a/tests/check/elements/avtpaafpay.c +++ b/tests/check/elements/avtpaafpay.c @@ -110,6 +110,7 @@ GST_START_TEST (test_properties) g_object_get (G_OBJECT (element), "processing-deadline", &val_uint64, NULL); fail_unless (val_uint64 == processing_deadline); + gst_object_unref (element); gst_harness_teardown (h); } diff --git a/tests/check/elements/avtpcvfdepay.c b/tests/check/elements/avtpcvfdepay.c index c77f1c9..bcd0c03 100644 --- a/tests/check/elements/avtpcvfdepay.c +++ b/tests/check/elements/avtpcvfdepay.c @@ -45,6 +45,8 @@ check_nal_filling (GstBuffer * buffer, guint8 first) } } + gst_buffer_unmap (buffer, &map); + return result; } @@ -541,6 +543,7 @@ GST_START_TEST (test_depayloader_invalid_avtpdu) avtp_cvf_pdu_set (pdu, AVTP_CVF_FIELD_H264_TIMESTAMP, 2000000); avtp_cvf_pdu_set (pdu, AVTP_CVF_FIELD_STREAM_DATA_LEN, sizeof (guint32) + 1); map.data[AVTP_CVF_H264_HEADER_SIZE] = 28; + gst_buffer_unmap (small, &map); gst_harness_push (h, small); fail_unless_equals_uint64 (gst_harness_buffers_received (h), 0); @@ -672,6 +675,7 @@ GST_START_TEST (test_depayloader_lost_fragments) fail_unless_equals_uint64 (nal_size (nal), 17); fail_unless (check_nal_filling (nal, 40) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 4); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -756,10 +760,13 @@ GST_START_TEST (test_depayloader_lost_packet) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 7); + gst_buffer_unref (nal); + nal = fetch_nal (out, &offset); fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 7); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -833,6 +840,7 @@ GST_START_TEST (test_depayloader_single_and_messed_fragments) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless_equals_uint64 (nal_type (nal), 1); fail_unless (check_nal_filling (nal, 0) == TRUE); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -905,6 +913,7 @@ GST_START_TEST (test_depayloader_single_and_messed_fragments_2) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless_equals_uint64 (nal_type (nal), 2); fail_unless (check_nal_filling (nal, 5) == TRUE); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -1007,6 +1016,7 @@ GST_START_TEST (test_depayloader_single_and_messed_fragments_3) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless_equals_uint64 (nal_type (nal), 2); fail_unless (check_nal_filling (nal, 0) == TRUE); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -1022,6 +1032,7 @@ GST_START_TEST (test_depayloader_single_and_messed_fragments_3) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless_equals_uint64 (nal_type (nal), 3); fail_unless (check_nal_filling (nal, 7) == TRUE); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -1062,6 +1073,7 @@ GST_START_TEST (test_depayloader_property) g_object_get (G_OBJECT (element), "streamid", &streamid, NULL); fail_unless_equals_uint64 (streamid, 0xAABBCCDDEEFF0001); + gst_object_unref (element); gst_harness_teardown (h); } @@ -1137,10 +1149,12 @@ GST_START_TEST (test_depayloader_single_and_fragmented) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 1); + gst_buffer_unref (nal); nal = fetch_nal (out, &offset); fail_unless_equals_uint64 (nal_size (nal), 5); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 4); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); @@ -1223,6 +1237,7 @@ GST_START_TEST (test_depayloader_fragmented) fail_unless_equals_uint64 (nal_size (nal), 25); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 4); + gst_buffer_unref (nal); gst_buffer_unref (out); gst_buffer_unref (in); @@ -1310,6 +1325,7 @@ GST_START_TEST (test_depayloader_fragmented_big) fail_unless_equals_uint64 (nal_size (nal), (DATA_LEN - 2) * nal_count + 1); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 4); + gst_buffer_unref (nal); gst_buffer_unref (out); gst_buffer_unref (in); @@ -1386,14 +1402,17 @@ GST_START_TEST (test_depayloader_multiple_single) fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 7); + gst_buffer_unref (nal); nal = fetch_nal (out, &offset); fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 7); + gst_buffer_unref (nal); nal = fetch_nal (out, &offset); fail_unless_equals_uint64 (nal_size (nal), 4); fail_unless (check_nal_filling (nal, 0) == TRUE); fail_unless_equals_uint64 (nal_type (nal), 1); + gst_buffer_unref (nal); /* Ensure no other NAL units are present */ nal = fetch_nal (out, &offset); diff --git a/tests/check/elements/avtpcvfpay.c b/tests/check/elements/avtpcvfpay.c index 66f8d38..6273809 100644 --- a/tests/check/elements/avtpcvfpay.c +++ b/tests/check/elements/avtpcvfpay.c @@ -84,6 +84,8 @@ check_nal_filling (GstBuffer * buffer, guint8 first) } } + gst_buffer_unmap (buffer, &map); + return result; } @@ -271,6 +273,8 @@ GST_START_TEST (test_payloader_invalid_caps) fail_unless (gst_pad_get_current_caps (sinkpad) == NULL); gst_caps_unref (caps); + gst_object_unref (sinkpad); + gst_object_unref (element); gst_harness_teardown (h); } @@ -310,6 +314,7 @@ GST_START_TEST (test_payloader_incomplete_nal) map.data[4] = 1; /* Some dummy vcl NAL type */ map.data[5] = 0x0; map.data[6] = 0x1; + gst_buffer_unmap (in, &map); out = gst_harness_push_and_pull (h, in); @@ -352,6 +357,7 @@ GST_START_TEST (test_payloader_properties) NULL); fail_unless_equals_uint64 (processing_deadline, 5000); + gst_object_unref (element); gst_harness_teardown (h); } -- 2.7.4