<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>