[{"id":19557,"web_url":"https://patchwork.libcamera.org/comment/19557/","msgid":"","date":"2021-09-08T14:12:06","subject":"Re: [libcamera-devel] [PATCH v3] test: gstreamer: Fix the\n\tdestructor of GstreamerTest base class","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Vedant,\n\nOn 08/09/2021 15:57, 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> Fixes: f58768092277 (\"test: gstreamer: Fix the destructor of GstreamerTest base class\")\n> Signed-off-by: Vedant Paranjape \n> ---\n> test/gstreamer/gstreamer_test.cpp | 8 ++------\n> 1 file changed, 2 insertions(+), 6 deletions(-)\n> \n> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp\n> index dbdcaec0b111..e0f8de29cbd2 100644\n> --- a/test/gstreamer/gstreamer_test.cpp\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -69,12 +69,8 @@ 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> +\tgst_clear_object(&pipeline_);\n> +\tgst_clear_object(&libcameraSrc_);\n\nAccording to the GStreamer documentation[0], gst_clear_object is only\nsupported since 1.16 and libcamera currently requires 1.14.\n\nYou can use g_clear_object, the effect will be the same.\n\nReviewed-by: Jean-Michel Hautbois \n\n[0]\nhttps://gstreamer.freedesktop.org/documentation/gstreamer/gstobject.html?gi-language=c#gst_clear_object\n\n\n\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 92BC5BDC71\n\tfor ;\n\tWed, 8 Sep 2021 14:12:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1C196916E;\n\tWed, 8 Sep 2021 16:12:09 +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 DF20160503\n\tfor ;\n\tWed, 8 Sep 2021 16:12:08 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:1ff4:3000:cbce:eabb])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7CD77993;\n\tWed, 8 Sep 2021 16:12:08 +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=\"r6THf9wK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631110328;\n\tbh=S0IENAF9PUO2eKfDY2H/Pk67AlF1cJ+Kb+1JmmGCa0s=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=r6THf9wK+4V0x8LFloHxz3AzcKEBn9rRZpez+N6+lJkksEPGw889ibICckapDYuuV\n\t7rN+JmeuaSTupynp1yrnej8Bdn42SL7UiRxaCmYxdTGTzwvsaVe+dpOfw0KKz6BSTW\n\tiw7H2tdEYPQwxiIyWcN3TD/Y40rZPQi0ExTofmbY=","To":"Vedant Paranjape ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210908135706.419622-1-vedantparanjape160201@gmail.com>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Wed, 8 Sep 2021 16:12:06 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.13.0","MIME-Version":"1.0","In-Reply-To":"<20210908135706.419622-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3] 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":19558,"web_url":"https://patchwork.libcamera.org/comment/19558/","msgid":"","date":"2021-09-08T14:15:21","subject":"Re: [libcamera-devel] [PATCH v3] 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 Jean-Michel,\nI am sending v4 with a fix.\n\nOn Wed, 8 Sep, 2021, 19:42 Jean-Michel Hautbois, <\njeanmichel.hautbois@ideasonboard.com> wrote:\n\n> Hi Vedant,\n>\n> On 08/09/2021 15:57, 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> > 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 | 8 ++------\n> > 1 file changed, 2 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/test/gstreamer/gstreamer_test.cpp\n> b/test/gstreamer/gstreamer_test.cpp\n> > index dbdcaec0b111..e0f8de29cbd2 100644\n> > --- a/test/gstreamer/gstreamer_test.cpp\n> > +++ b/test/gstreamer/gstreamer_test.cpp\n> > @@ -69,12 +69,8 @@ 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> > + gst_clear_object(&pipeline_);\n> > + gst_clear_object(&libcameraSrc_);\n>\n> According to the GStreamer documentation[0], gst_clear_object is only\n> supported since 1.16 and libcamera currently requires 1.14.\n>\n> You can use g_clear_object, the effect will be the same.\n>\n> Reviewed-by: Jean-Michel Hautbois \n>\n> [0]\n>\n> https://gstreamer.freedesktop.org/documentation/gstreamer/gstobject.html?gi-language=c#gst_clear_object\n>\n>\n>\n> >\n> > gst_deinit();\n> > }\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 57C1EBDB1D\n\tfor ;\n\tWed, 8 Sep 2021 14:15:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE1DD69167;\n\tWed, 8 Sep 2021 16:15:36 +0200 (CEST)","from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com\n\t[IPv6:2607:f8b0:4864:20::b2d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EE75760503\n\tfor ;\n\tWed, 8 Sep 2021 16:15:34 +0200 (CEST)","by mail-yb1-xb2d.google.com with SMTP id s16so3669178ybe.0\n\tfor ;\n\tWed, 08 Sep 2021 07:15:34 -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=\"joIsSpIu\"; 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=Bk1oMeNM09IXc0MBao+r5qW4XI/mrgPelJp8pKKR70g=;\n\tb=joIsSpIujIs0t9RPrPt0ASfOxMXPMbyMTlQUKb/xSXfPlstDKdi0xGYwHa3SbGYsws\n\t+/LEKuyCuYycPHGg9BMCI/r/G9SBE3rZMZcVt55VuBKUHImG0AD9j8ona+evqyoSBPZK\n\t2UojzHHiAC63GmcPqdcdwyCZx3hqBw092qtZ104qoJS7ECkN6NB0hhLdT1HIkLrBtIOI\n\tVgNQyET+4WK7XkjGlze9Sg9ldb2ij9h5CC9rmw2l1LDxxZnG/9FQ49JkIX0r2hqJBQpb\n\tloF2cBfk2KkbswsX71+cenTD7eMMHDXuw1QiXCmsNO8szhT+2o8gum30n2UEXbYGfQse\n\t5cYQ==","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=Bk1oMeNM09IXc0MBao+r5qW4XI/mrgPelJp8pKKR70g=;\n\tb=K8h4vigicYE0cE0crK+CMF8iRguH1q19EIQbbCb1KCC7eYnhH7JwVBHcnf9J9fYIfC\n\tkq2fSG5c6NiSm/bXH1tayTjBRmi3+RFd92fZNFqNfecELU221xNBCZ72IU9nxzJyhk4u\n\tGHLklqD6tP6Yq1nDGYtgDmFiWpXVjW4DYwIpd0Yc+LJFqLvPYErLnejc/QAKP+BcW5of\n\tdXMJa/kBpvuxDTfjx9kd2XjjFH/l4p6M4iIJb0aXueX8UqekyKmbf7MB08UD+PXd3tfo\n\tEM07dwfezwYl/4hczxw5HV+okYjjczBGwtdWZ3PS1R42vYvKLoGOq/EvpxSy7ueWNO49\n\ts2uQ==","X-Gm-Message-State":"AOAM5329nPs85lqzJ8b0ehQ/2O5B7DN5nE2M/JxTqdvXOrnPeA6IeJiI\n\tVLfqcCJuqffDRmswzRgoQ9WMTXO0CplEz629+7VFpNES6oU=","X-Google-Smtp-Source":"ABdhPJz2LL2pdi+un/6Feb6eYhSzTP3+ihYjYM9hTaVEdO+KgB0+HTcw+k6gq074OqICf4ZKqwDjHYhxg8BsFxOrsDY=","X-Received":"by 2002:a05:6902:154e:: with SMTP id\n\tr14mr5267622ybu.308.1631110532952; \n\tWed, 08 Sep 2021 07:15:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20210908135706.419622-1-vedantparanjape160201@gmail.com>\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Wed, 8 Sep 2021 19:45:21 +0530","Message-ID":"","To":"Jean-Michel Hautbois ","Content-Type":"multipart/alternative; boundary=\"000000000000e7f46405cb7c8397\"","Subject":"Re: [libcamera-devel] [PATCH v3] 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\" "}}]