rtpbasedepay: fix possible refcounting issue when detecting a discont

When we detect a discont and the input buffer isn't already flagged
as discont, handle_buffer() does a gst_buffer_make_writable() on the
input buffer in order to set the flag. This assumed it had ownership
of the input buffer though, which it didn't. This would still work
fine in most scenarios, but could lead to crashes or mini object
unref criticals in some cases when a discont is detected, e.g. when
using pcapparse in front of a depayloader. This problem was
introduced in bc14cdf529e.

1 files changed, 11 insertions, 2 deletions

diff --git a/gst-libs/gst/rtp/gstrtpbasedepayload.c b/gst-libs/gst/rtp/gstrtpbasedepayload.c
index 4c5637dce..a790c0f58 100644
--- a/gst-libs/gst/rtp/gstrtpbasedepayload.c
+++ b/gst-libs/gst/rtp/gstrtpbasedepayload.c
@@ -345,6 +345,7 @@ caps_not_changed:
   }
 }
 
+/* takes ownership of the input buffer */
 static GstFlowReturn
 gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
     GstRTPBaseDepayloadClass * bclass, GstBuffer * in)
@@ -455,6 +456,8 @@ gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
     ret = gst_rtp_base_depayload_push (filter, out_buf);
   }
 
+  gst_buffer_unref (in);
+
   return ret;
 
   /* ERRORS */
@@ -469,6 +472,7 @@ not_negotiated:
             "element before the depayloader and setting the 'caps' property "
             "on that. Also see http://cgit.freedesktop.org/gstreamer/"
             "gst-plugins-good/tree/gst/rtp/README"));
+    gst_buffer_unref (in);
     return GST_FLOW_NOT_NEGOTIATED;
   }
 invalid_buffer:
@@ -476,12 +480,14 @@ invalid_buffer:
     /* this is not fatal but should be filtered earlier */
     GST_ELEMENT_WARNING (filter, STREAM, DECODE, (NULL),
         ("Received invalid RTP payload, dropping"));
+    gst_buffer_unref (in);
     return GST_FLOW_OK;
   }
 dropping:
   {
     gst_rtp_buffer_unmap (&rtp);
     GST_WARNING_OBJECT (filter, "%d <= 100, dropping old packet", gap);
+    gst_buffer_unref (in);
     return GST_FLOW_OK;
   }
 no_process:
@@ -490,6 +496,7 @@ no_process:
     /* this is not fatal but should be filtered earlier */
     GST_ELEMENT_ERROR (filter, STREAM, NOT_IMPLEMENTED, (NULL),
         ("The subclass does not have a process or process_rtp_packet method"));
+    gst_buffer_unref (in);
     return GST_FLOW_ERROR;
   }
 }
@@ -507,8 +514,6 @@ gst_rtp_base_depayload_chain (GstPad * pad, GstObject * parent, GstBuffer * in)
 
   flow_ret = gst_rtp_base_depayload_handle_buffer (basedepay, bclass, in);
 
-  gst_buffer_unref (in);
-
   return flow_ret;
 }
 
@@ -537,6 +542,10 @@ gst_rtp_base_depayload_chain_list (GstPad * pad, GstObject * parent,
   for (i = 0; i < len; i++) {
     buffer = gst_buffer_list_get (list, i);
 
+    /* handle_buffer takes ownership of input buffer */
+    /* FIXME: add a way to steal buffers from list as we will unref it anyway */
+    gst_buffer_ref (buffer);
+
     /* Should we fix up any missing timestamps for list buffers here
      * (e.g. set to first or previous timestamp in list) or just assume
      * the's a jitterbuffer that will have done that for us? */