From 86350ff8b7aee835ea43022e203b52dce7f0c53b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim-Philipp=20M=C3=BCller?= Date: Fri, 11 Dec 2015 10:25:00 +0000 Subject: [PATCH] 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. --- gst-libs/gst/rtp/gstrtpbasedepayload.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gst-libs/gst/rtp/gstrtpbasedepayload.c b/gst-libs/gst/rtp/gstrtpbasedepayload.c index 4c5637d..a790c0f 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? */ -- 2.7.4