fixes: #514889

Original commit message from CVS:
patch by:  Wim Taymans  <wim.taymans@collabora.co.uk>
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

diff --git a/ChangeLog b/ChangeLog
index 9167d30..bda2cde 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2008-02-12  Jan Schmidt  <jan.schmidt@sun.com>
+
+       patch by:  Wim Taymans  <wim.taymans@collabora.co.uk>
+       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  <sebastien@moutte.net>
 
        * win32/MANIFEST:

diff --git a/gst/rtp/gstrtph264pay.c b/gst/rtp/gstrtph264pay.c
index 23c661d..c437c06 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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");
 }