<div dir="auto">Hi Umang, I'll take ownership of these patches.<div dir="auto">Thanks for the fix. I was missing a key info about floating refs, so I assumed that pipeline will take care of lifetime of other elements.</div><div dir="auto"><br></div><div dir="auto">Regards,</div><div dir="auto">Vedant Paranjape</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 14 Sep, 2021, 23:24 Umang Jain, <<a href="mailto:umang.jain@ideasonboard.com">umang.jain@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nicolas,<br>
<br>
On 9/14/21 11:15 PM, Nicolas Dufresne wrote:<br>
> Le mardi 14 septembre 2021 à 17:47 +0530, Umang Jain a écrit :<br>
>> The test hold a valid reference to convert0_ and sink0_ but<br>
>> not released. This results in a memory leak and can be checked<br>
>> via valgrind. Drop the references with test cleanup() virtual<br>
>> function.<br>
>><br>
>> Valgrind log:<br>
>> ==95352==    definitely lost: 1,688 bytes in 2 blocks<br>
>> ==95352==    indirectly lost: 11,901 bytes in 37 blocks<br>
>><br>
>> The patch fixes the leaks reported by valgrind above to:<br>
>> ==120397==    definitely lost: 0 bytes in 0 blocks<br>
>> ==120397==    indirectly lost: 0 bytes in 0 blocks<br>
>><br>
>> Signed-off-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank" rel="noreferrer">umang.jain@ideasonboard.com</a>><br>
>> ---<br>
>>   test/gstreamer/gstreamer_single_stream_test.cpp | 6 ++++++<br>
>>   1 file changed, 6 insertions(+)<br>
>><br>
>> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp<br>
>> index d992455c..763a77b1 100644<br>
>> --- a/test/gstreamer/gstreamer_single_stream_test.cpp<br>
>> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp<br>
>> @@ -70,6 +70,12 @@ protected:<br>
>>              return TestPass;<br>
>>      }<br>
>>   <br>
>> +    void cleanup() override<br>
>> +    {<br>
>> +            gst_object_unref(convert0_);<br>
>> +            gst_object_unref(sink0_);<br>
>> +    }<br>
> This is correct of course, but I think we could cleanup more the test. With a<br>
> run/cleanup() set of function, I would expect to be able to loop over and run<br>
> the test multiple times, but it does not seem to be the case due to the fact the<br>
<br>
<br>
Indeed, we can and should. I went in a rush to fix the leak, to see if <br>
that's all we leak or more!<br>
<br>
+cc Vedant,<br>
<br>
Vedant, could you assume ownership of these patches/series and work upon <br>
it? I won't have more time to allocate to this and it will  generally <br>
help you as well to fix the multi stream test on top of this as well and <br>
make sure we don't have leaks anywhere.<br>
<br>
> pipeline is not fully constructed in run().<br>
><br>
> For the leak being fixed:<br>
> Reviewed-by: Nicolas Dufresne <<a href="mailto:nicolas.dufresne@collabora.com" target="_blank" rel="noreferrer">nicolas.dufresne@collabora.com</a>><br>
<br>
<br>
Thanks for taking a look Nicolas.<br>
<br>
><br>
>> +<br>
>>   private:<br>
>>      GstElement *convert0_;<br>
>>      GstElement *sink0_;<br>
><br>
</blockquote></div>