Skip to content

Commit

Permalink
memleak fix: gst_tensor_meta_info_parse_memory usage
Browse files Browse the repository at this point in the history
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 <myungjoo.ham@samsung.com>
  • Loading branch information
myungjoo committed Oct 11, 2023
1 parent 19193d9 commit 3ad9acd
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 36 deletions.
40 changes: 17 additions & 23 deletions gst/nnstreamer/elements/gsttensor_converter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -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 */
Expand All @@ -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);
Expand Down
24 changes: 12 additions & 12 deletions gst/nnstreamer/elements/gsttensor_mux.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)!
*/
}

/**
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion gst/nnstreamer/elements/gsttensor_transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions gst/nnstreamer/nnstreamer_plugin_api_impl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions gst/nnstreamer/tensor_filter/tensor_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,7 @@ gst_tensor_filter_transform (GstBaseTransform * trans,
}

gst_buffer_append_memory (outbuf, mem);
gst_memory_unref (in_mem[i]);
}
}

Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 3ad9acd

Please sign in to comment.