From 4a7cbe84897d94309c860140d70940a9632d969b Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 12 Feb 2008 23:38:19 +0000 Subject: [PATCH] fixes: #514889 Original commit message from CVS: patch by: Wim Taymans fixes: #514889 * gst/rtp/gstrtph264pay.c: * gst/rtp/gstrtpmp4gdepay.c: * gst/rtp/gstrtpmp4gpay.c: * gst/rtp/gstrtpmp4gpay.h: * gst/rtp/gstrtptheorapay.c: * gst/rtp/gstrtpvorbispay.c: Fix various leaks shown up in valgrind - free sprops and buffer in error cases in H264 payloader - fix leak in mp4g depayloader when construction the caps - don't leak config string in the mp4g payloader - don't leak buffers and headers in theora and vorbis payloaders * tests/check/elements/rtp-payloading.c: Fix the RTP data test - Actually send valid amr data to the payloader instead of 20 zero-bytes - The mp4g payloader expects codec_data on the caps --- ChangeLog | 25 ++++++++++++++++++ gst/rtp/gstrtph264pay.c | 2 ++ gst/rtp/gstrtpmp4gdepay.c | 4 +-- gst/rtp/gstrtpmp4gpay.c | 37 +++++++++++++++++++++++++-- gst/rtp/gstrtpmp4gpay.h | 3 ++- gst/rtp/gstrtptheorapay.c | 48 +++++++++++++++++++++++++++++++++++ gst/rtp/gstrtpvorbispay.c | 48 +++++++++++++++++++++++++++++++++++ tests/check/elements/rtp-payloading.c | 10 +++++--- 8 files changed, 167 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9167d30..bda2cde 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2008-02-12 Jan Schmidt + + patch by: Wim Taymans + fixes: #514889 + + * gst/rtp/gstrtph264pay.c: + * gst/rtp/gstrtpmp4gdepay.c: + * gst/rtp/gstrtpmp4gpay.c: + * gst/rtp/gstrtpmp4gpay.h: + * gst/rtp/gstrtptheorapay.c: + * gst/rtp/gstrtpvorbispay.c: + + Fix various leaks shown up in valgrind + - free sprops and buffer in error cases in H264 payloader + - fix leak in mp4g depayloader when construction the caps + - don't leak config string in the mp4g payloader + - don't leak buffers and headers in theora and vorbis payloaders + + * tests/check/elements/rtp-payloading.c: + + Fix the RTP data test + - Actually send valid amr data to the payloader instead of 20 + zero-bytes + - The mp4g payloader expects codec_data on the caps + 2008-02-12 Sebastien Moutte * win32/MANIFEST: diff --git a/gst/rtp/gstrtph264pay.c b/gst/rtp/gstrtph264pay.c index 23c661d..c437c06 100644 --- a/gst/rtp/gstrtph264pay.c +++ b/gst/rtp/gstrtph264pay.c @@ -383,6 +383,7 @@ gst_rtp_h264_pay_parse_sps_pps (GstBaseRTPPayload * basepayload, GST_DEBUG ("outcaps udpate: profile=%s, sps=%s, pps=%s\n", profile, sps, pps); + g_free (sprops); g_free (profile); g_free (sps); g_free (pps); @@ -431,6 +432,7 @@ gst_rtp_h264_pay_handle_buffer (GstBaseRTPPayload * basepayload, if (idxdata < 5) { GST_DEBUG_OBJECT (basepayload, "Returning GST_FLOW_OK without creating RTP packet"); + gst_buffer_unref (buffer); return GST_FLOW_OK; } diff --git a/gst/rtp/gstrtpmp4gdepay.c b/gst/rtp/gstrtpmp4gdepay.c index 7e5d5b1..3512265 100644 --- a/gst/rtp/gstrtpmp4gdepay.c +++ b/gst/rtp/gstrtpmp4gdepay.c @@ -223,11 +223,9 @@ gst_rtp_mp4g_depay_setcaps (GstBaseRTPDepayload * depayload, GstCaps * caps) GstBuffer *buffer; buffer = gst_value_get_buffer (&v); - gst_buffer_ref (buffer); - g_value_unset (&v); - gst_caps_set_simple (srccaps, "codec_data", GST_TYPE_BUFFER, buffer, NULL); + g_value_unset (&v); } else { g_warning ("cannot convert config to buffer"); } diff --git a/gst/rtp/gstrtpmp4gpay.c b/gst/rtp/gstrtpmp4gpay.c index 2b8b865..92d7224 100644 --- a/gst/rtp/gstrtpmp4gpay.c +++ b/gst/rtp/gstrtpmp4gpay.c @@ -85,6 +85,8 @@ static void gst_rtp_mp4g_pay_finalize (GObject * object); static gboolean gst_rtp_mp4g_pay_setcaps (GstBaseRTPPayload * payload, GstCaps * caps); +static GstStateChangeReturn gst_rtp_mp4g_pay_change_state (GstElement * element, + GstStateChange transition); static GstFlowReturn gst_rtp_mp4g_pay_handle_buffer (GstBaseRTPPayload * payload, GstBuffer * buffer); @@ -143,6 +145,8 @@ gst_rtp_mp4g_pay_class_init (GstRtpMP4GPayClass * klass) gobject_class->finalize = gst_rtp_mp4g_pay_finalize; + gstelement_class->change_state = gst_rtp_mp4g_pay_change_state; + gstbasertppayload_class->set_caps = gst_rtp_mp4g_pay_setcaps; gstbasertppayload_class->handle_buffer = gst_rtp_mp4g_pay_handle_buffer; @@ -171,6 +175,10 @@ gst_rtp_mp4g_pay_finalize (GObject * object) g_free (rtpmp4gpay->params); rtpmp4gpay->params = NULL; + if (rtpmp4gpay->config) + gst_buffer_unref (rtpmp4gpay->config); + rtpmp4gpay->config = NULL; + g_free (rtpmp4gpay->profile); rtpmp4gpay->profile = NULL; @@ -504,7 +512,8 @@ gst_rtp_mp4g_pay_flush (GstRtpMP4GPay * rtpmp4gpay) /* marker only if the packet is complete */ gst_rtp_buffer_set_marker (outbuf, avail <= payload_len); - GST_BUFFER_TIMESTAMP (outbuf) = rtpmp4gpay->first_ts; + GST_BUFFER_TIMESTAMP (outbuf) = rtpmp4gpay->first_timestamp; + GST_BUFFER_DURATION (outbuf) = rtpmp4gpay->first_duration; ret = gst_basertppayload_push (GST_BASE_RTP_PAYLOAD (rtpmp4gpay), outbuf); @@ -528,7 +537,8 @@ gst_rtp_mp4g_pay_handle_buffer (GstBaseRTPPayload * basepayload, rtpmp4gpay = GST_RTP_MP4G_PAY (basepayload); - rtpmp4gpay->first_ts = GST_BUFFER_TIMESTAMP (buffer); + rtpmp4gpay->first_timestamp = GST_BUFFER_TIMESTAMP (buffer); + rtpmp4gpay->first_duration = GST_BUFFER_DURATION (buffer); /* we always encode and flush a full AU */ gst_adapter_push (rtpmp4gpay->adapter, buffer); @@ -537,6 +547,29 @@ gst_rtp_mp4g_pay_handle_buffer (GstBaseRTPPayload * basepayload, return ret; } +static GstStateChangeReturn +gst_rtp_mp4g_pay_change_state (GstElement * element, GstStateChange transition) +{ + GstRtpMP4GPay *rtpmp4gpay; + GstStateChangeReturn ret; + + rtpmp4gpay = GST_RTP_MP4G_PAY (element); + + switch (transition) { + default: + break; + } + + ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); + + switch (transition) { + default: + break; + } + return ret; +} + + gboolean gst_rtp_mp4g_pay_plugin_init (GstPlugin * plugin) { diff --git a/gst/rtp/gstrtpmp4gpay.h b/gst/rtp/gstrtpmp4gpay.h index b1557f5..586b4e0 100644 --- a/gst/rtp/gstrtpmp4gpay.h +++ b/gst/rtp/gstrtpmp4gpay.h @@ -45,7 +45,8 @@ struct _GstRtpMP4GPay GstBaseRTPPayload payload; GstAdapter *adapter; - GstClockTime first_ts; + GstClockTime first_timestamp; + GstClockTime first_duration; GstClockTime duration; gint rate; diff --git a/gst/rtp/gstrtptheorapay.c b/gst/rtp/gstrtptheorapay.c index a6c22d9..23485ca 100644 --- a/gst/rtp/gstrtptheorapay.c +++ b/gst/rtp/gstrtptheorapay.c @@ -79,6 +79,8 @@ GST_BOILERPLATE (GstRtpTheoraPay, gst_rtp_theora_pay, GstBaseRTPPayload, static gboolean gst_rtp_theora_pay_setcaps (GstBaseRTPPayload * basepayload, GstCaps * caps); +static GstStateChangeReturn gst_rtp_theora_pay_change_state (GstElement * + element, GstStateChange transition); static GstFlowReturn gst_rtp_theora_pay_handle_buffer (GstBaseRTPPayload * pad, GstBuffer * buffer); @@ -108,6 +110,8 @@ gst_rtp_theora_pay_class_init (GstRtpTheoraPayClass * klass) parent_class = g_type_class_peek_parent (klass); + gstelement_class->change_state = gst_rtp_theora_pay_change_state; + gstbasertppayload_class->set_caps = gst_rtp_theora_pay_setcaps; gstbasertppayload_class->handle_buffer = gst_rtp_theora_pay_handle_buffer; @@ -122,6 +126,18 @@ gst_rtp_theora_pay_init (GstRtpTheoraPay * rtptheorapay, /* needed because of GST_BOILERPLATE */ } +static void +gst_rtp_theora_pay_cleanup (GstRtpTheoraPay * rtptheorapay) +{ + g_list_foreach (rtptheorapay->headers, (GFunc) gst_mini_object_unref, NULL); + g_list_free (rtptheorapay->headers); + rtptheorapay->headers = NULL; + + if (rtptheorapay->packet) + gst_buffer_unref (rtptheorapay->packet); + rtptheorapay->packet = NULL; +} + static gboolean gst_rtp_theora_pay_setcaps (GstBaseRTPPayload * basepayload, GstCaps * caps) { @@ -624,6 +640,8 @@ gst_rtp_theora_pay_handle_buffer (GstBaseRTPPayload * basepayload, rtptheorapay->payload_duration += duration; } } + gst_buffer_unref (buffer); + done: return ret; @@ -632,26 +650,56 @@ wrong_size: { GST_ELEMENT_WARNING (rtptheorapay, STREAM, DECODE, ("Invalid packet size (1 < %d <= 0xffff)", size), (NULL)); + gst_buffer_unref (buffer); return GST_FLOW_OK; } parse_id_failed: { + gst_buffer_unref (buffer); return GST_FLOW_ERROR; } unknown_header: { GST_ELEMENT_WARNING (rtptheorapay, STREAM, DECODE, (NULL), ("Ignoring unknown header received")); + gst_buffer_unref (buffer); return GST_FLOW_OK; } header_error: { GST_ELEMENT_WARNING (rtptheorapay, STREAM, DECODE, (NULL), ("Error initializing header config")); + gst_buffer_unref (buffer); return GST_FLOW_OK; } } +static GstStateChangeReturn +gst_rtp_theora_pay_change_state (GstElement * element, + GstStateChange transition) +{ + GstRtpTheoraPay *rtptheorapay; + GstStateChangeReturn ret; + + rtptheorapay = GST_RTP_THEORA_PAY (element); + + switch (transition) { + default: + break; + } + + ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); + + switch (transition) { + case GST_STATE_CHANGE_PAUSED_TO_READY: + gst_rtp_theora_pay_cleanup (rtptheorapay); + break; + default: + break; + } + return ret; +} + gboolean gst_rtp_theora_pay_plugin_init (GstPlugin * plugin) { diff --git a/gst/rtp/gstrtpvorbispay.c b/gst/rtp/gstrtpvorbispay.c index 30f484b..f9fa15b 100644 --- a/gst/rtp/gstrtpvorbispay.c +++ b/gst/rtp/gstrtpvorbispay.c @@ -75,6 +75,8 @@ GST_BOILERPLATE (GstRtpVorbisPay, gst_rtp_vorbis_pay, GstBaseRTPPayload, static gboolean gst_rtp_vorbis_pay_setcaps (GstBaseRTPPayload * basepayload, GstCaps * caps); +static GstStateChangeReturn gst_rtp_vorbis_pay_change_state (GstElement * + element, GstStateChange transition); static GstFlowReturn gst_rtp_vorbis_pay_handle_buffer (GstBaseRTPPayload * pad, GstBuffer * buffer); @@ -104,6 +106,8 @@ gst_rtp_vorbis_pay_class_init (GstRtpVorbisPayClass * klass) parent_class = g_type_class_peek_parent (klass); + gstelement_class->change_state = gst_rtp_vorbis_pay_change_state; + gstbasertppayload_class->set_caps = gst_rtp_vorbis_pay_setcaps; gstbasertppayload_class->handle_buffer = gst_rtp_vorbis_pay_handle_buffer; @@ -118,6 +122,18 @@ gst_rtp_vorbis_pay_init (GstRtpVorbisPay * rtpvorbispay, /* needed because of GST_BOILERPLATE */ } +static void +gst_rtp_vorbis_pay_cleanup (GstRtpVorbisPay * rtpvorbispay) +{ + g_list_foreach (rtpvorbispay->headers, (GFunc) gst_mini_object_unref, NULL); + g_list_free (rtpvorbispay->headers); + rtpvorbispay->headers = NULL; + + if (rtpvorbispay->packet) + gst_buffer_unref (rtpvorbispay->packet); + rtpvorbispay->packet = NULL; +} + static gboolean gst_rtp_vorbis_pay_setcaps (GstBaseRTPPayload * basepayload, GstCaps * caps) { @@ -623,6 +639,8 @@ gst_rtp_vorbis_pay_handle_buffer (GstBaseRTPPayload * basepayload, rtpvorbispay->payload_duration += duration; } } + gst_buffer_unref (buffer); + done: return ret; @@ -631,26 +649,56 @@ wrong_size: { GST_ELEMENT_WARNING (rtpvorbispay, STREAM, DECODE, ("Invalid packet size (1 < %d <= 0xffff)", size), (NULL)); + gst_buffer_unref (buffer); return GST_FLOW_OK; } parse_id_failed: { + gst_buffer_unref (buffer); return GST_FLOW_ERROR; } unknown_header: { GST_ELEMENT_WARNING (rtpvorbispay, STREAM, DECODE, (NULL), ("Ignoring unknown header received")); + gst_buffer_unref (buffer); return GST_FLOW_OK; } header_error: { GST_ELEMENT_WARNING (rtpvorbispay, STREAM, DECODE, (NULL), ("Error initializing header config")); + gst_buffer_unref (buffer); return GST_FLOW_OK; } } +static GstStateChangeReturn +gst_rtp_vorbis_pay_change_state (GstElement * element, + GstStateChange transition) +{ + GstRtpVorbisPay *rtpvorbispay; + GstStateChangeReturn ret; + + rtpvorbispay = GST_RTP_VORBIS_PAY (element); + + switch (transition) { + default: + break; + } + + ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); + + switch (transition) { + case GST_STATE_CHANGE_PAUSED_TO_READY: + gst_rtp_vorbis_pay_cleanup (rtpvorbispay); + break; + default: + break; + } + return ret; +} + gboolean gst_rtp_vorbis_pay_plugin_init (GstPlugin * plugin) { diff --git a/tests/check/elements/rtp-payloading.c b/tests/check/elements/rtp-payloading.c index 3d0acd0..13da5e8 100644 --- a/tests/check/elements/rtp-payloading.c +++ b/tests/check/elements/rtp-payloading.c @@ -323,10 +323,11 @@ GST_START_TEST (rtp_gsm) GST_END_TEST; static char rtp_amr_frame_data[] = - { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + { 0x3c, 0x24, 0x03, 0xb3, 0x48, 0x10, 0x68, 0x46, 0x6c, 0xec, 0x03, + 0x7a, 0x37, 0x16, 0x41, 0x41, 0xc0, 0x00, 0x0d, 0xcd, 0x12, 0xed, + 0xad, 0x80, 0x00, 0x00, 0x11, 0x31, 0x00, 0x00, 0x0d, 0xa0 }; -static int rtp_amr_frame_data_size = 20; +static int rtp_amr_frame_data_size = 32; static int rtp_amr_frame_count = 1; GST_START_TEST (rtp_amr) @@ -481,7 +482,8 @@ static int rtp_mp4g_frame_count = 1; GST_START_TEST (rtp_mp4g) { rtp_pipeline_test (rtp_mp4g_frame_data, rtp_mp4g_frame_data_size, - rtp_mp4g_frame_count, "video/mpeg,mpegversion=4", "rtpmp4gpay", + rtp_mp4g_frame_count, + "video/mpeg,mpegversion=4,codec_data=(buffer)000001b001", "rtpmp4gpay", "rtpmp4gdepay"); } -- 2.7.4