<div dir="auto"><div>Hi Kieran,<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 8 Sep, 2021, 18:57 Kieran Bingham, <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 08/09/2021 14:20, Vedant Paranjape wrote:<br>
> Hi Kieran,<br>
> <br>
> On Wed, Sep 8, 2021 at 6:36 PM Kieran Bingham<br>
> <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a><br>
> <mailto:<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a>>> wrote:<br>
> <br>
>     On 08/09/2021 13:04, Vedant Paranjape wrote:<br>
>     > The destructor tried to check if pipeline_ is a parent of<br>
>     libcameraSrc_.<br>
>     > This was needed to be checked as if it is, cleanup of libcameraSrc_<br>
>     > would be handled by pipeline itself.<br>
>     ><br>
>     > Since, the destructor can be called anytime, even when pipeline_<br>
>     hasn't<br>
>     > been created, the use of pipeline_ to check if libcameraSrc_ has an<br>
>     > ancestor as pipeline_ caused a segmentation fault.<br>
> <br>
>     I presume the refcounting is what actually deals with the cleanup<br>
>     accordingly now?<br>
> <br>
> <br>
> Right !<br>
> <br>
>     Will the gst_object_unref(pipeline_); have any effect such as making<br>
>     libcameraSrc_ invalid? (because you said that the pipeline handler would<br>
> <br>
> <br>
> Yes, it will unref the libcameraSrc_, and I think gst_deinit will free<br>
> it. Pinging @Nicolas Dufresne <mailto:<a href="mailto:nicolas@ndufresne.ca" target="_blank" rel="noreferrer">nicolas@ndufresne.ca</a>> for clearity.<br>
<br>
The question is - have you made sure that the refcounting is correctly<br>
balanced, such that this final gst_object_unref(libcameraSrc_);  will be<br>
the one that does the release?<br>
<br>
I.e. to make sure it doesn't take the refcount below zero.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">It can't go below zero, and this is the only place where I do a unref, so Yes !</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> <br>
>     It looks reasonable otherwise:<br>
> <br>
>     Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a><br>
>     <mailto:<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a>>><br>
> <br>
>     ><br>
>     > Fixes: f58768092277 ("test: gstreamer: Fix the destructor of<br>
>     GstreamerTest base class")<br>
>     > Signed-off-by: Vedant Paranjape <<a href="mailto:vedantparanjape160201@gmail.com" target="_blank" rel="noreferrer">vedantparanjape160201@gmail.com</a><br>
>     <mailto:<a href="mailto:vedantparanjape160201@gmail.com" target="_blank" rel="noreferrer">vedantparanjape160201@gmail.com</a>>><br>
>     > ---<br>
>     >  test/gstreamer/gstreamer_test.cpp | 6 ++----<br>
>     >  1 file changed, 2 insertions(+), 4 deletions(-)<br>
>     ><br>
>     > diff --git a/test/gstreamer/gstreamer_test.cpp<br>
>     b/test/gstreamer/gstreamer_test.cpp<br>
>     > index dbdcaec0b111..46fa5abaea75 100644<br>
>     > --- a/test/gstreamer/gstreamer_test.cpp<br>
>     > +++ b/test/gstreamer/gstreamer_test.cpp<br>
>     > @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()<br>
>     > <br>
>     >  GstreamerTest::~GstreamerTest()<br>
>     >  {<br>
>     > -     if (libcameraSrc_ &&<br>
>     > -         !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),<br>
>     > -                                     GST_OBJECT(pipeline_)))<br>
>     > -             gst_object_unref(libcameraSrc_);<br>
>     >       if (pipeline_)<br>
>     >               gst_object_unref(pipeline_);<br>
>     > +     if (libcameraSrc_)<br>
>     > +             gst_object_unref(libcameraSrc_);<br>
>     > <br>
>     >       gst_deinit();<br>
>     >  }<br>
>     ><br>
> <br>
> <br>
> Regards,<br>
> /*Vedant Paranjape*/<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Regards,</div><div dir="auto">Vedant Paranjape</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote></div></div></div>