[{"id":19550,"web_url":"https://patchwork.libcamera.org/comment/19550/","msgid":"","date":"2021-09-08T13:06:03","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 08/09/2021 13:04, Vedant Paranjape wrote:\n> The destructor tried to check if pipeline_ is a parent of libcameraSrc_.\n> This was needed to be checked as if it is, cleanup of libcameraSrc_\n> would be handled by pipeline itself.\n> \n> Since, the destructor can be called anytime, even when pipeline_ hasn't\n> been created, the use of pipeline_ to check if libcameraSrc_ has an\n> ancestor as pipeline_ caused a segmentation fault.\n\nI presume the refcounting is what actually deals with the cleanup\naccordingly now?\n\nWill the gst_object_unref(pipeline_); have any effect such as making\nlibcameraSrc_ invalid? (because you said that the pipeline handler would\n\nIt looks reasonable otherwise:\n\nReviewed-by: Kieran Bingham \n\n> \n> Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of GstreamerTest base class\")\n> Signed-off-by: Vedant Paranjape \n> ---\n> test/gstreamer/gstreamer_test.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp\n> index dbdcaec0b111..46fa5abaea75 100644\n> --- a/test/gstreamer/gstreamer_test.cpp\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()\n> \n> GstreamerTest::~GstreamerTest()\n> {\n> -\tif (libcameraSrc_ &&\n> -\t !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> -\t\t\t\t\tGST_OBJECT(pipeline_)))\n> -\t\tgst_object_unref(libcameraSrc_);\n> \tif (pipeline_)\n> \t\tgst_object_unref(pipeline_);\n> +\tif (libcameraSrc_)\n> +\t\tgst_object_unref(libcameraSrc_);\n> \n> \tgst_deinit();\n> }\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5002DBDC71\n\tfor ;\n\tWed, 8 Sep 2021 13:06:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEF2F69167;\n\tWed, 8 Sep 2021 15:06:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D97160503\n\tfor ;\n\tWed, 8 Sep 2021 15:06:06 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3BBC993;\n\tWed, 8 Sep 2021 15:06:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jc6ARx1D\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631106366;\n\tbh=VSxZGM48RTHTCPqklBg2f+UNWoq+uWybeM0xUjiOB6A=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=jc6ARx1DRlWFoDnIjLNxSUfC6RvlgeDc3fYk1GSbIuc6G7egXYI9KSoobI6YJaAOF\n\tsQjycZobDxuAG9wbvTqvxA42zqqCDSVk0Af/egSI/IxzMwBRXGt73ZolJm7UzuVv4m\n\txFjcohnuG/auV8/oeRCuI3vvqQwwf1zRXm+2DD1w=","To":"Vedant Paranjape ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>","From":"Kieran Bingham ","Message-ID":"","Date":"Wed, 8 Sep 2021 14:06:03 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19552,"web_url":"https://patchwork.libcamera.org/comment/19552/","msgid":"","date":"2021-09-08T13:20:00","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Kieran,\n\nOn Wed, Sep 8, 2021 at 6:36 PM Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> On 08/09/2021 13:04, Vedant Paranjape wrote:\n> > The destructor tried to check if pipeline_ is a parent of libcameraSrc_.\n> > This was needed to be checked as if it is, cleanup of libcameraSrc_\n> > would be handled by pipeline itself.\n> >\n> > Since, the destructor can be called anytime, even when pipeline_ hasn't\n> > been created, the use of pipeline_ to check if libcameraSrc_ has an\n> > ancestor as pipeline_ caused a segmentation fault.\n>\n> I presume the refcounting is what actually deals with the cleanup\n> accordingly now?\n>\n\nRight !\n\nWill the gst_object_unref(pipeline_); have any effect such as making\n> libcameraSrc_ invalid? (because you said that the pipeline handler would\n>\n\nYes, it will unref the libcameraSrc_, and I think gst_deinit will free it.\nPinging @Nicolas Dufresne for clearity.\n\nIt looks reasonable otherwise:\n>\n> Reviewed-by: Kieran Bingham \n>\n> >\n> > Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of\n> GstreamerTest base class\")\n> > Signed-off-by: Vedant Paranjape \n> > ---\n> > test/gstreamer/gstreamer_test.cpp | 6 ++----\n> > 1 file changed, 2 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/test/gstreamer/gstreamer_test.cpp\n> b/test/gstreamer/gstreamer_test.cpp\n> > index dbdcaec0b111..46fa5abaea75 100644\n> > --- a/test/gstreamer/gstreamer_test.cpp\n> > +++ b/test/gstreamer/gstreamer_test.cpp\n> > @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()\n> >\n> > GstreamerTest::~GstreamerTest()\n> > {\n> > - if (libcameraSrc_ &&\n> > - !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> > - GST_OBJECT(pipeline_)))\n> > - gst_object_unref(libcameraSrc_);\n> > if (pipeline_)\n> > gst_object_unref(pipeline_);\n> > + if (libcameraSrc_)\n> > + gst_object_unref(libcameraSrc_);\n> >\n> > gst_deinit();\n> > }\n> >\n>\n\nRegards,\n*Vedant Paranjape*","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A9F78BDB1D\n\tfor ;\n\tWed, 8 Sep 2021 13:20:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 794BB60503;\n\tWed, 8 Sep 2021 15:20:14 +0200 (CEST)","from mail-yb1-xb33.google.com (mail-yb1-xb33.google.com\n\t[IPv6:2607:f8b0:4864:20::b33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 30D8C60503\n\tfor ;\n\tWed, 8 Sep 2021 15:20:13 +0200 (CEST)","by mail-yb1-xb33.google.com with SMTP id a93so4250805ybi.1\n\tfor ;\n\tWed, 08 Sep 2021 06:20:13 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"blEtKrQ2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Coxw5bdGmVK5V6jUsCcoYmfIsYSiNpxv/aJoy2K+a0o=;\n\tb=blEtKrQ2xtY59nmgftPbMrdsXbduDDDK/NsxBROnEllf0tfidzvStjDlOXX7halqL+\n\trFi4yClwKlohSNqTBgW8o4AKPilLnVegdBGyNnpvGsN03Q1U/Rm+fZnFPs2JkWwBz8H5\n\t0mUVZbhcb8oTuSEjpXGe/c72J6sV+OI/brZ0voIs+Lon+nkmZZMoMpB3QHjBEs6TFMWc\n\tyWFFHsL3teFD3SnC8HGUW1WwiEJ8ieoWSxgMid9xCxQSqCfmcP/wKMUczro/exNMgDIN\n\tnUtctZNZrFw3QxiV1Frq3Itak7+jF2lclw0hJwROF7Tx2SrqQrjJ6QWvv8z9mllMa6Ps\n\th8XQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Coxw5bdGmVK5V6jUsCcoYmfIsYSiNpxv/aJoy2K+a0o=;\n\tb=fwEIQ957U/eqwxCmM+577iefnbw53Yk/M0lDz17V6UGcRYE5Rcg4YGZVs0AiduNeYX\n\tzZDkpiQbZxug+VwlTnxECULab8ndGfYldACpRkVsjVESZ9CntYKbiqpFKoo8EiEZgJhN\n\tde6p8n3IYKIFbV0X5Xxv7exki/JWU5JXLA1WLnHD//6enH2M2VPCleJsQayx2Z5g3iYq\n\tN9sI6MPG4mEIs8RMZciby+6A02NlVRo7YXmcD2vrtf1NwQ8h/f5SGixlqyGZ9alix51T\n\tkk7IsHrctJM+31mGXW48pn0m3WTqn5RfVVHPwM/9ZvaAj06tEWaSaZRxTypW8qOp4JtA\n\tlmkA==","X-Gm-Message-State":"AOAM531/pd+uNubS11xV7YnfN/dvuPo6IQaOubbTT6WDqgCv8+coafT5\n\tGfCNSpO8XdmYmxdOEWJj9P4a7MRSM8H+r6uLw2E=","X-Google-Smtp-Source":"ABdhPJzMQ4GlInfPceokUDSdt+aCvaUWu42D4z/EfXPJYIa9e+P8nk9wf7OHuRyHFseLObt7Mfedo5kf/AUlN/PWoVM=","X-Received":"by 2002:a25:b5ce:: with SMTP id\n\td14mr4841176ybg.415.1631107211703; \n\tWed, 08 Sep 2021 06:20:11 -0700 (PDT)","MIME-Version":"1.0","References":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Wed, 8 Sep 2021 18:50:00 +0530","Message-ID":"","To":"Kieran Bingham , \n\tNicolas Dufresne ","Content-Type":"multipart/alternative; boundary=\"000000000000f1b8d905cb7bbde0\"","Subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19553,"web_url":"https://patchwork.libcamera.org/comment/19553/","msgid":"","date":"2021-09-08T13:27:21","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 08/09/2021 14:20, Vedant Paranjape wrote:\n> Hi Kieran,\n> \n> On Wed, Sep 8, 2021 at 6:36 PM Kieran Bingham\n> > wrote:\n> \n> On 08/09/2021 13:04, Vedant Paranjape wrote:\n> > The destructor tried to check if pipeline_ is a parent of\n> libcameraSrc_.\n> > This was needed to be checked as if it is, cleanup of libcameraSrc_\n> > would be handled by pipeline itself.\n> >\n> > Since, the destructor can be called anytime, even when pipeline_\n> hasn't\n> > been created, the use of pipeline_ to check if libcameraSrc_ has an\n> > ancestor as pipeline_ caused a segmentation fault.\n> \n> I presume the refcounting is what actually deals with the cleanup\n> accordingly now?\n> \n> \n> Right !\n> \n> Will the gst_object_unref(pipeline_); have any effect such as making\n> libcameraSrc_ invalid? (because you said that the pipeline handler would\n> \n> \n> Yes, it will unref the libcameraSrc_, and I think gst_deinit will free\n> it. Pinging @Nicolas Dufresne for clearity.\n\nThe question is - have you made sure that the refcounting is correctly\nbalanced, such that this final gst_object_unref(libcameraSrc_); will be\nthe one that does the release?\n\nI.e. to make sure it doesn't take the refcount below zero.\n\n\n> \n> It looks reasonable otherwise:\n> \n> Reviewed-by: Kieran Bingham >\n> \n> >\n> > Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of\n> GstreamerTest base class\")\n> > Signed-off-by: Vedant Paranjape >\n> > ---\n> >  test/gstreamer/gstreamer_test.cpp | 6 ++----\n> >  1 file changed, 2 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/test/gstreamer/gstreamer_test.cpp\n> b/test/gstreamer/gstreamer_test.cpp\n> > index dbdcaec0b111..46fa5abaea75 100644\n> > --- a/test/gstreamer/gstreamer_test.cpp\n> > +++ b/test/gstreamer/gstreamer_test.cpp\n> > @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()\n> > \n> >  GstreamerTest::~GstreamerTest()\n> >  {\n> > -     if (libcameraSrc_ &&\n> > -         !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> > -                                     GST_OBJECT(pipeline_)))\n> > -             gst_object_unref(libcameraSrc_);\n> >       if (pipeline_)\n> >               gst_object_unref(pipeline_);\n> > +     if (libcameraSrc_)\n> > +             gst_object_unref(libcameraSrc_);\n> > \n> >       gst_deinit();\n> >  }\n> >\n> \n> \n> Regards,\n> /*Vedant Paranjape*/","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BE727BDC71\n\tfor ;\n\tWed, 8 Sep 2021 13:27:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4695A69169;\n\tWed, 8 Sep 2021 15:27:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C354760503\n\tfor ;\n\tWed, 8 Sep 2021 15:27:24 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C1ADC993;\n\tWed, 8 Sep 2021 15:27:23 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rYQqDeRp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631107644;\n\tbh=TsXkRARtVu9wKVslG/UiS3yyGPQKihG1ym2vUsnpEbw=;\n\th=From:Subject:To:Cc:References:Date:In-Reply-To:From;\n\tb=rYQqDeRpGxiB7T9yp7OR30g2sBq08QKaPBBBw9hMCWC2TkwcFjMI5bULOzCkMU30L\n\t2fhjebJNocen1EDMNJ6hOOIpRUNhoKyWoq6912TMPRT2jZQ/8pzuKhFSJfoW1u/qXo\n\tEvELPR1Eqv5YlaqB+2YufFimMt7QM9bH8yUt/Iw0=","From":"Kieran Bingham ","To":"Vedant Paranjape ,\n\tNicolas Dufresne ","References":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>\n\t\n\t","Message-ID":"","Date":"Wed, 8 Sep 2021 14:27:21 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19554,"web_url":"https://patchwork.libcamera.org/comment/19554/","msgid":"","date":"2021-09-08T13:30:54","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Kieran,\n\nOn Wed, 8 Sep, 2021, 18:57 Kieran Bingham, \nwrote:\n\n> On 08/09/2021 14:20, Vedant Paranjape wrote:\n> > Hi Kieran,\n> >\n> > On Wed, Sep 8, 2021 at 6:36 PM Kieran Bingham\n> > > > wrote:\n> >\n> > On 08/09/2021 13:04, Vedant Paranjape wrote:\n> > > The destructor tried to check if pipeline_ is a parent of\n> > libcameraSrc_.\n> > > This was needed to be checked as if it is, cleanup of libcameraSrc_\n> > > would be handled by pipeline itself.\n> > >\n> > > Since, the destructor can be called anytime, even when pipeline_\n> > hasn't\n> > > been created, the use of pipeline_ to check if libcameraSrc_ has an\n> > > ancestor as pipeline_ caused a segmentation fault.\n> >\n> > I presume the refcounting is what actually deals with the cleanup\n> > accordingly now?\n> >\n> >\n> > Right !\n> >\n> > Will the gst_object_unref(pipeline_); have any effect such as making\n> > libcameraSrc_ invalid? (because you said that the pipeline handler\n> would\n> >\n> >\n> > Yes, it will unref the libcameraSrc_, and I think gst_deinit will free\n> > it. Pinging @Nicolas Dufresne for\n> clearity.\n>\n> The question is - have you made sure that the refcounting is correctly\n> balanced, such that this final gst_object_unref(libcameraSrc_); will be\n> the one that does the release?\n>\n> I.e. to make sure it doesn't take the refcount below zero.\n>\n\nIt can't go below zero, and this is the only place where I do a unref, so\nYes !\n\n\n> >\n> > It looks reasonable otherwise:\n> >\n> > Reviewed-by: Kieran Bingham > >\n> >\n> > >\n> > > Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of\n> > GstreamerTest base class\")\n> > > Signed-off-by: Vedant Paranjape > >\n> > > ---\n> > > test/gstreamer/gstreamer_test.cpp | 6 ++----\n> > > 1 file changed, 2 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/test/gstreamer/gstreamer_test.cpp\n> > b/test/gstreamer/gstreamer_test.cpp\n> > > index dbdcaec0b111..46fa5abaea75 100644\n> > > --- a/test/gstreamer/gstreamer_test.cpp\n> > > +++ b/test/gstreamer/gstreamer_test.cpp\n> > > @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()\n> > >\n> > > GstreamerTest::~GstreamerTest()\n> > > {\n> > > - if (libcameraSrc_ &&\n> > > - !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> > > - GST_OBJECT(pipeline_)))\n> > > - gst_object_unref(libcameraSrc_);\n> > > if (pipeline_)\n> > > gst_object_unref(pipeline_);\n> > > + if (libcameraSrc_)\n> > > + gst_object_unref(libcameraSrc_);\n> > >\n> > > gst_deinit();\n> > > }\n> > >\n> >\n> >\n> > Regards,\n> > /*Vedant Paranjape*/\n>\n\nRegards,\nVedant Paranjape\n\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 86729BDB1D\n\tfor ;\n\tWed, 8 Sep 2021 13:31:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1EBA69169;\n\tWed, 8 Sep 2021 15:31:08 +0200 (CEST)","from mail-yb1-xb33.google.com (mail-yb1-xb33.google.com\n\t[IPv6:2607:f8b0:4864:20::b33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C90760503\n\tfor ;\n\tWed, 8 Sep 2021 15:31:07 +0200 (CEST)","by mail-yb1-xb33.google.com with SMTP id c206so4229503ybb.12\n\tfor ;\n\tWed, 08 Sep 2021 06:31:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"JzHxojyP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Bbq6xocmC2qPicJ7lqbENNv9aZm4UNKM1ZF18SQl68I=;\n\tb=JzHxojyPm8S/Fly3Ujm0kl1nzH2tE3Kmy+jR4vCLJ0FF6HHVGbo9NtliQsjug5RyWo\n\t/gr1EXAbzC8/4VKpZmj+wnSEAIp9fN6t3eXndJl2sgxBuI43hNk+hIkLeM3AxH3bnDUY\n\tDPJMyJnFk7lMvZOcSd3SJDErcdeDZ0+FhVNgR0DiShJOymJbbCNsyG7RiDXebLc4fhnN\n\tmXC2U2Q9YbtQErI4nzrRcXgq0jWKZaiSL7TPT/fJ3/k7Co5fEZC10ya8K1fWyX+Qbxkn\n\tX+WuTjZ243SRKTCy3v2J3IY6Zq/6pnjlgZUMEe285DPXpg5xNGPu/Ihf7EWa7Bm5OvSP\n\tQyyg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Bbq6xocmC2qPicJ7lqbENNv9aZm4UNKM1ZF18SQl68I=;\n\tb=X5nQa06v9sCk13LHc2Qi1//L20enkWc28DSKPJivlUwAXfvf2EoebX3hmkLS0yGoWR\n\tKnSCd7PCn22iBEILhRbLec/8yJQFWkEAdjrATcQPYawB27Dk+8MxYrub7E/q83GvgxGa\n\tyesMWArP3lEGHKsSoKv6K7sKxC7gvlA5pfUa0oRxRzIaj8b3cdMMY62jp9tVYwBNaDmb\n\tIXRLnFHMsla9o33W+YIupKUm+ocuF6gl8/pr84c4vdZcpTot92HdziCRFbsaL768Rr7S\n\tm/urnQwC9laGGtO0MB7IUKA5NPWLGeNFXSStWd92fEoxlWEOp+2SElTQnhIy7ElFz5Uh\n\t23uw==","X-Gm-Message-State":"AOAM532Y9kiMCgLvcEW4DJDa5Q2/hpghfJtlfiuqTFag07KyiM4810g6\n\tvpSTwtoTRDWtq0+zGQvz8EYd//fSqeDlxNTNWEw=","X-Google-Smtp-Source":"ABdhPJxaEQ53WtW0i+PHkzBvGXcq++88Q8lBVG9sAcjMvC9bnUJMfyKoVmeQJzsxI9TrUebL86NZcbxtb+FG85e+x5Y=","X-Received":"by 2002:a05:6902:154e:: with SMTP id\n\tr14mr5011790ybu.308.1631107866074; \n\tWed, 08 Sep 2021 06:31:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Wed, 8 Sep 2021 19:00:54 +0530","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"000000000000f2a3b305cb7be447\"","Subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19556,"web_url":"https://patchwork.libcamera.org/comment/19556/","msgid":"<2aa7c93394a1b392538111f914dce72d7c00fc4e.camel@ndufresne.ca>","date":"2021-09-08T13:42:24","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mercredi 08 septembre 2021 à 17:34 +0530, Vedant Paranjape a écrit :\n> The destructor tried to check if pipeline_ is a parent of libcameraSrc_.\n> This was needed to be checked as if it is, cleanup of libcameraSrc_\n> would be handled by pipeline itself.\n> \n> Since, the destructor can be called anytime, even when pipeline_ hasn't\n> been created, the use of pipeline_ to check if libcameraSrc_ has an\n> ancestor as pipeline_ caused a segmentation fault.\n\nThis fix i correct since you do call gst_object_ref_sink() after creating the\nlibcamerasrc element. Element are created with a floating ref and this floating\nref is taken by gst_bin_add() normally, unless you have called ref_sink(). So\nfor the correctness:\n\nReviewed-by: Nicolas Dufresne \n\nThough, I still believe the implementation is messy and error prone. I would\nrather drop the creation of pipeline_ and libcameraSrc_ in the base class, and\nuse gst_parse_launch() in the tests themself. This would be much more readable,\nfaster to implement, and less error prone.\n\n> \n> Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of GstreamerTest base class\")\n> Signed-off-by: Vedant Paranjape \n> ---\n> test/gstreamer/gstreamer_test.cpp | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp\n> index dbdcaec0b111..46fa5abaea75 100644\n> --- a/test/gstreamer/gstreamer_test.cpp\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -69,12 +69,10 @@ GstreamerTest::GstreamerTest()\n> \n> GstreamerTest::~GstreamerTest()\n> {\n> -\tif (libcameraSrc_ &&\n> -\t !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> -\t\t\t\t\tGST_OBJECT(pipeline_)))\n> -\t\tgst_object_unref(libcameraSrc_);\n> \tif (pipeline_)\n> \t\tgst_object_unref(pipeline_);\n> +\tif (libcameraSrc_)\n> +\t\tgst_object_unref(libcameraSrc_);\n> \n> \tgst_deinit();\n> }","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6DA68BDB1D\n\tfor ;\n\tWed, 8 Sep 2021 13:42:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6AB36916E;\n\tWed, 8 Sep 2021 15:42:28 +0200 (CEST)","from mail-il1-x134.google.com (mail-il1-x134.google.com\n\t[IPv6:2607:f8b0:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0842560503\n\tfor ;\n\tWed, 8 Sep 2021 15:42:27 +0200 (CEST)","by mail-il1-x134.google.com with SMTP id i13so2359466ilm.4\n\tfor ;\n\tWed, 08 Sep 2021 06:42:26 -0700 (PDT)","from nicolas-tpx395.localdomain (mtl.collabora.ca. [66.171.169.34])\n\tby smtp.gmail.com with ESMTPSA id\n\th11sm1180618ilc.7.2021.09.08.06.42.24\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 08 Sep 2021 06:42:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"QkgY/Hv+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=7t/fDKlzL47EByZ306h262viJLoTgnccELxls6MWz2I=;\n\tb=QkgY/Hv+9VQwQJA6WC9vuFNYrkNda8IUvt4j43+QXaMRN5i8Q4W+zBxKHMq99iMdDW\n\tLO0qvEn5+iq95lLQi0AaDQOk3USmP6EDWaXa23omg8HD6B62BY0jM8h/bJOXg2LxVhe/\n\tCkcX9XwK9to5DiOdwLcjtmF/7ziGZ4rCl5tJ9PuU8bCzhxPexbLb4ZNQYBRAxV5y3Fz8\n\tBu3lql5W8CfKTpeazMmSl08M4K8H61d7VcZubWnlEy6lXjTVfS1fo//4NaAprEK2iXTR\n\tg2KO7qXlx3dyJH7COVrCdKNmzcilsWuvsQMd+pP23by7W7udY4veDJscwNYYXKvcCha8\n\tY6iw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=7t/fDKlzL47EByZ306h262viJLoTgnccELxls6MWz2I=;\n\tb=MiOC3Nx0agjIwc6TZwQUCADPGFo82YiMH0mpIOgtYYSjuCbCqnvK1Pb6+HDnxkwPFF\n\t6vF064xbxA9x7LuwYphD4eWz/dMy6BqjfBM9k7/SpM6JO8n+OyFw5/wpY6w1zcSGUYbU\n\tGu9CMMEBDD2VvznHi4/EH2G7jMj04cw4rn4K7IJ4zy3HnB1wL/aYPMMSGnfFVXXUNt1d\n\tVGb5Hiw/8SruQPdqb8SXXZJpcHjNhKMdIKIFkPuosts1q0EhjXIuCBYf9lMAYstHSZrV\n\toxiURPsCXTfhFNrQmrHyKkfQEZyeFHTV8iJjFK5RbrW4M3BLlBORXzqs8NBxIULOGPCB\n\tgkQg==","X-Gm-Message-State":"AOAM533N6xNPr4YtcWSxs3p2KGN0WF4g/dZcykn7cHny8osE85XH0fZ+\n\tSh3GWDthpLenmfHFHj7uWNvQTQ==","X-Google-Smtp-Source":"ABdhPJyVzzQSTHDdcTAOooNzaPV1As1ajwFLMB7dRgRQTF2f5aLY7Vz9tiEhteWKH5ac153WRUFORg==","X-Received":"by 2002:a92:611:: with SMTP id x17mr2803107ilg.41.1631108545635; \n\tWed, 08 Sep 2021 06:42:25 -0700 (PDT)","Message-ID":"<2aa7c93394a1b392538111f914dce72d7c00fc4e.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Vedant Paranjape , \n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 08 Sep 2021 09:42:24 -0400","In-Reply-To":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>","References":"<20210908120420.312959-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]