From 11302ea42a8c232aa04cf852f00e0a1cdbcb4dca Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Wed, 6 Sep 2023 16:57:10 +0900 Subject: [PATCH] memleak fix: gst_tensor_meta_info_parse_memory usage The input GstMemory given to gst_tensor_meta_info_parse_memory should be deallocated (unref) by the caller. There are a few elements that do not call unref it, which often create a huge memory leak with converter/filter. Leave a Doxygen note that the caller should unref it, too. Signed-off-by: MyungJoo Ham --- gst/nnstreamer/elements/gsttensor_converter.c | 41 ++++++++----------- gst/nnstreamer/elements/gsttensor_mux.c | 24 +++++------ gst/nnstreamer/elements/gsttensor_transform.c | 5 ++- gst/nnstreamer/nnstreamer_plugin_api_impl.c | 1 + gst/nnstreamer/tensor_filter/tensor_filter.c | 3 ++ 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/gst/nnstreamer/elements/gsttensor_converter.c b/gst/nnstreamer/elements/gsttensor_converter.c index c142cfcc9e..7f97a9d3ff 100644 --- a/gst/nnstreamer/elements/gsttensor_converter.c +++ b/gst/nnstreamer/elements/gsttensor_converter.c @@ -839,12 +839,12 @@ _gst_tensor_converter_chain_timestamp (GstTensorConverter * self, } /** @brief Chain function's private routine to process octet stream */ -static GstBuffer * +static void _gst_tensor_converter_chain_octet (GstTensorConverter * self, GstBuffer * buf) { - GstBuffer *buffer = buf; GstTensorsInfo *_info = &self->tensors_config.info; gboolean multi = (_info->num_tensors > 1); + guint nummem = gst_buffer_n_memory (buf); /* configure multi tensors */ if (multi || gst_buffer_n_memory (buf) > 1) { @@ -855,42 +855,38 @@ _gst_tensor_converter_chain_octet (GstTensorConverter * self, GstBuffer * buf) g_assert (self->frames_per_tensor == 1); offset = 0; - buffer = gst_buffer_new (); mem = gst_buffer_get_all_memory (buf); if (multi) { for (i = 0; i < _info->num_tensors; ++i) { size = gst_tensors_info_get_size (_info, i); - gst_buffer_append_memory (buffer, gst_memory_share (mem, offset, size)); + if (nummem > i) { + gst_buffer_replace_memory (buf, i, + gst_memory_share (mem, offset, size)); + } else { + gst_buffer_append_memory (buf, gst_memory_share (mem, offset, size)); + } offset += size; } gst_memory_unref (mem); } else { - gst_buffer_append_memory (buffer, mem); + gst_buffer_replace_all_memory (buf, mem); } - - /* copy timestamps */ - gst_buffer_copy_into (buffer, buf, GST_BUFFER_COPY_METADATA, 0, -1); - gst_buffer_unref (buf); } - - return buffer; } /** @brief Chain function's private routine to process flex tensor */ -static GstBuffer * +static void _gst_tensor_converter_chain_flex_tensor (GstTensorConverter * self, GstBuffer * buf) { - GstBuffer *buffer; - GstMemory *mem; + GstMemory *mem, *newmem; GstTensorsInfo *info; GstTensorMetaInfo meta; guint i; info = &self->tensors_config.info; - buffer = gst_buffer_new (); for (i = 0; i < info->num_tensors; i++) { gst_tensor_info_convert_to_meta (&info->info[i], &meta); @@ -910,14 +906,11 @@ _gst_tensor_converter_chain_flex_tensor (GstTensorConverter * self, } mem = gst_buffer_peek_memory (buf, i); - mem = gst_tensor_meta_info_append_header (&meta, mem); + newmem = gst_tensor_meta_info_append_header (&meta, mem); + /** The old mem is unref'ed by gst_buffer_unref (buf) a few lines below */ - gst_buffer_append_memory (buffer, mem); + gst_buffer_replace_memory (buf, i, newmem); } - - gst_buffer_copy_into (buffer, buf, GST_BUFFER_COPY_METADATA, 0, -1); - gst_buffer_unref (buf); - return buffer; } /** @brief Chain function's private routine to push buffer into src pad */ @@ -926,15 +919,16 @@ _gst_tensor_converter_chain_push (GstTensorConverter * self, GstBuffer * buf) { GstBuffer *buffer = buf; + if (self->in_media_type == _NNS_OCTET) { /* configure multi tensors */ - buffer = _gst_tensor_converter_chain_octet (self, buffer); + _gst_tensor_converter_chain_octet (self, buffer); } /* if output is flexible, add header. */ if (!self->do_not_append_header && gst_tensor_pad_caps_is_flexible (self->srcpad)) { - buffer = _gst_tensor_converter_chain_flex_tensor (self, buffer); + _gst_tensor_converter_chain_flex_tensor (self, buffer); } silent_debug_timestamp (self, buffer); @@ -1298,6 +1292,7 @@ gst_tensor_converter_chain (GstPad * pad, GstObject * parent, GstBuffer * buf) if (frames_in == frames_out) { /** do nothing, push the incoming buffer */ + inbuf = gst_buffer_make_writable (inbuf); return _gst_tensor_converter_chain_push (self, inbuf); } diff --git a/gst/nnstreamer/elements/gsttensor_mux.c b/gst/nnstreamer/elements/gsttensor_mux.c index 0d234cf0c2..fb85bbeea5 100644 --- a/gst/nnstreamer/elements/gsttensor_mux.c +++ b/gst/nnstreamer/elements/gsttensor_mux.c @@ -442,36 +442,36 @@ gst_tensor_mux_send_segment_event (GstTensorMux * tensor_mux, /** * @brief Process flex tensor. + * @param buf The buffer to be updated with flexible-tensor header info */ -static GstBuffer * +static void gst_tensor_mux_chain_flex_tensor (GstTensorMux * tensor_mux, GstBuffer * buf) { - GstBuffer *buffer; - GstMemory *mem; + GstMemory *mem, *mem_temp; GstTensorsInfo *info; GstTensorMetaInfo meta; guint i; /* If input is flexible, do nothing. It is already flexible tensor. */ if (gst_tensors_config_is_flexible (&tensor_mux->tensors_config)) - return buf; + return; info = &tensor_mux->tensors_config.info; - buffer = gst_buffer_new (); for (i = 0; i < info->num_tensors; i++) { mem = gst_buffer_peek_memory (buf, i); /* append header */ gst_tensor_info_convert_to_meta (&info->info[i], &meta); - mem = gst_tensor_meta_info_append_header (&meta, mem); + mem_temp = gst_tensor_meta_info_append_header (&meta, mem); + gst_buffer_replace_memory (buf, i, mem_temp); - gst_buffer_append_memory (buffer, mem); } - - gst_buffer_copy_into (buffer, buf, GST_BUFFER_COPY_METADATA, 0, -1); - gst_buffer_unref (buf); - return buffer; + /** Don't replace buf with another instance of GstBuffer + * because the original buf instance is in tensor_mux->collect_pad + * which becomes dangling after the replacement. + * So, replace GstMemory of the GstBuffer instance (buf)! + */ } /** @@ -531,7 +531,7 @@ gst_tensor_mux_collected (GstCollectPads * pads, GstTensorMux * tensor_mux) /* add header if output is flexible */ if (gst_tensor_pad_caps_is_flexible (tensor_mux->srcpad)) - tensors_buf = gst_tensor_mux_chain_flex_tensor (tensor_mux, tensors_buf); + gst_tensor_mux_chain_flex_tensor (tensor_mux, tensors_buf); ret = gst_pad_push (tensor_mux->srcpad, tensors_buf); tensor_mux->need_set_time = TRUE; diff --git a/gst/nnstreamer/elements/gsttensor_transform.c b/gst/nnstreamer/elements/gsttensor_transform.c index 4416cfba96..eca0df2d9a 100644 --- a/gst/nnstreamer/elements/gsttensor_transform.c +++ b/gst/nnstreamer/elements/gsttensor_transform.c @@ -1723,11 +1723,14 @@ gst_tensor_transform_transform (GstBaseTransform * trans, if (filter->apply && !g_list_find (filter->apply, GINT_TO_POINTER (i))) { GstMemory *mem = gst_buffer_peek_memory (inbuf, i); + GstMemory *mem_temp; if (!in_flexible && out_flexible) { /* append meta */ gst_tensor_info_convert_to_meta (out_info, &meta); - mem = gst_tensor_meta_info_append_header (&meta, mem); + mem_temp = gst_tensor_meta_info_append_header (&meta, mem); + gst_memory_unref (mem); + mem = mem_temp; } else { mem = gst_memory_ref (mem); } diff --git a/gst/nnstreamer/nnstreamer_plugin_api_impl.c b/gst/nnstreamer/nnstreamer_plugin_api_impl.c index 376ed8c6d9..50e2cd8a55 100644 --- a/gst/nnstreamer/nnstreamer_plugin_api_impl.c +++ b/gst/nnstreamer/nnstreamer_plugin_api_impl.c @@ -1495,6 +1495,7 @@ gst_tensor_meta_info_parse_memory (GstTensorMetaInfo * meta, GstMemory * mem) * @param[in] meta tensor meta structure * @param[in] mem pointer to GstMemory * @return Newly allocated GstMemory (Caller should free returned memory using gst_memory_unref()) + * @note The caller has both original GstMemory (mem) and returned GstMemory objects. The caller should unref both. */ GstMemory * gst_tensor_meta_info_append_header (GstTensorMetaInfo * meta, GstMemory * mem) diff --git a/gst/nnstreamer/tensor_filter/tensor_filter.c b/gst/nnstreamer/tensor_filter/tensor_filter.c index f07334a73c..f94068e33d 100644 --- a/gst/nnstreamer/tensor_filter/tensor_filter.c +++ b/gst/nnstreamer/tensor_filter/tensor_filter.c @@ -865,6 +865,7 @@ gst_tensor_filter_transform (GstBaseTransform * trans, } gst_buffer_append_memory (outbuf, mem); + gst_memory_unref (in_mem[i]); } } @@ -903,6 +904,8 @@ gst_tensor_filter_transform (GstBaseTransform * trans, out_tensors[i].data, out_tensors[i].size, 0, out_tensors[i].size, out_tensors[i].data, g_free); + if (!allocate_in_invoke) + gst_allocator_free (out_mem[i]->allocator, out_mem[i]); out_mem[i] = gst_tensor_meta_info_append_header (&meta, flex_mem); gst_memory_unref (flex_mem); } else if (allocate_in_invoke) {