memory leak on custom meta data insert

[Michael Gruner]

Hey Tom

> or should I completely skip the 
>   ret = GST_BASE_TRANSFORM_CLASS(gst_mtdata_meta_inject_parent_class)->prepare_output_buffer(trans, input, outbuf);
> part ?

No, I believe chaining to the parent’s prepare_output_buffer is the smart thing to do. You actually need to create an output buffer in the prepare_output_buffer method, and you don’t actually want to deal with the buffer allocation yourself if possible, so it’s fine that you are relying on the default implementation. The default implementation does not simply allocate the memory for the buffer, it may also takes into account buffer pools and allocators proposed by downstream elements. 

Michael

> On 8 Mar 2022, at 06:18, tom <tomschuring at gmail.com> wrote:
> 
> Hello Michael, (sorry for the re-send as I forgot to include the mail-list)
> 
> Thank you for picking up on that !  In my mind I was taking the input buffer and turning it into an output buffer, but that obviously is the wrong way to do it. I've changed it to:
> 
> static GstFlowReturn
> gst_mtdata_meta_inject_prepare_output_buffer(GstBaseTransform *trans, GstBuffer *input, GstBuffer **outbuf)
> {
>   GstMtdataMetaInject *self = GST_MTDMETAINJECT(trans);
>   GstFlowReturn ret;
>   ret = GST_BASE_TRANSFORM_CLASS(gst_mtdata_meta_inject_parent_class)->prepare_output_buffer(trans, input, outbuf);
>   if (ret != GST_FLOW_OK)
>     return ret;
> 
>   *outbuf = gst_buffer_make_writable(*outbuf);
>   //  Then you can add the meta to the output buffer.
> 
>   // add extra meta data
>   if (*outbuf != NULL)
>   if (ret != GST_FLOW_OK)
>     return ret;
> 
>   *outbuf = gst_buffer_make_writable(*outbuf);
>   if (*outbuf != NULL)
> .....
> 
> or should I completely skip the 
>   ret = GST_BASE_TRANSFORM_CLASS(gst_mtdata_meta_inject_parent_class)->prepare_output_buffer(trans, input, outbuf);
> part ?
> 
> Kind regards,
> Tom
> 
> On Tue, 8 Mar 2022 at 04:04, Michael Gruner <michael.gruner at ridgerun.com <mailto:michael.gruner at ridgerun.com>> wrote:
> Hey Tom
> 
> Is this line ok?
> 
>   *outbuf = gst_buffer_make_writable(input);
> 
> Shouldn’t it be:
> 
>   *outbuf = gst_buffer_make_writable(*outbuf);
> 
> The way it is right now you are leaking the outbuf creating when calling:
> 
>  ret = GST_BASE_TRANSFORM_CLASS(gst_mtdata_meta_inject_parent_class)->prepare_output_buffer(trans, input, outbuf);
> 
> Michael 
> 
>> On 6 Mar 2022, at 21:11, tom via gstreamer-devel <gstreamer-devel at lists.freedesktop.org <mailto:gstreamer-devel at lists.freedesktop.org>> wrote:
>> 
>> Hello,
>> 
>> I'm trying to write a simple plugin that only inserts data as meta data to the stream.
>> My plugin is derived from GstBaseTransform
>> 
>> typedef struct GstMtdataMetaInject
>> {
>>   GstBaseTransform base_mtdataplugins;
>> 
>> and i've created a simple meta data like: (see attached files)
>> 
>> struct _MtdataMeta
>> {
>>     GstMeta meta;
>>     gchar *jsonstring;
>> };
>> 
>> 
>> and to insert the meta data (on every frame for now)
>> 
>> 
>> static GstFlowReturn
>> gst_mtdata_meta_inject_prepare_output_buffer(GstBaseTransform *trans, GstBuffer *input, GstBuffer **outbuf)
>> {
>>   GstMtdataMetaInject *self = GST_MTDMETAINJECT(trans);
>>   GstFlowReturn ret;
>>   ret = GST_BASE_TRANSFORM_CLASS(gst_mtdata_meta_inject_parent_class)->prepare_output_buffer(trans, input, outbuf);
>>   if (ret != GST_FLOW_OK)
>>     return ret;
>>   *outbuf = gst_buffer_make_writable(input);
>>   if (*outbuf != NULL)
>>   {
>>     // string hello = StringUtils::current_date_time_string();
>>     // string hello = "12345678901234";
>>     string hello = StringUtils::random_string(14);
>> 
>>     MtdataMeta *in = gst_buffer_add_mtdata_meta(*outbuf, (gchar *)hello.c_str());
>> 
>>     GST_WARNING("Added metdata: %s", in->jsonstring);
>>   }
>>   return ret;
>> }
>> however i'm getting a quite fast memory leak that i can see in gst-launch-1.0 where it goes from 30% memory up to 60% in a matter of a minute.
>> the funny think is that if i change the meta data string to be something constant like "12345678901234" with the same length, there is no memory leak and it stays on 28% forever.
>> 
>> Do I need to go to the last element in the stream and 'free' the meta data so it doesn't add up ?
>> Kind regards,
>> Tom
>> 
>> <mtdata_meta.h><mtdata_meta.cpp><gstmtdmetainject.h><gstmtdmetainject.cpp>
> 
> 
> 
> -- 
> ☕𝓉𝓈

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/gstreamer-devel/attachments/20220308/d8a53f69/attachment-0001.htm>