[{"id":33443,"web_url":"https://patchwork.libcamera.org/comment/33443/","msgid":"<Z7xHDamTyvFEG80t@linux.intel.com>","date":"2025-02-24T10:16:45","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"Hi all,\n\nOn Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> A second use-after-free bug related to signals staying connected after\n> the receiver DelayedControls instance gets deleted has been found, this\n> time in the simple pipeline handler. Fix the issue once and for all by\n> making the DelayedControls class inherit from Object. This will\n> disconnect signals automatically upon deletion of the receiver.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Stan, would you be able to test this with the simple pipeline handler ?\n> It should work both with and without your series that deals with the\n> frame start signal, and should fix the crash that Kieran has reported.\n\nIt does not make assertion\n\"state_ == ProxyRunning\" failed in processStatsThread()\ngone for me. \n\nI tested this patch alone as well together with\n\n[PATCH v4 0/2] libcamera: start frame events changes\n[PATCH] pipeline: simple: Create DelayedControls instance once only\n\nSo far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\nmake the assertion gone.\n\nOne interesting thing is the issue is not reproducible when\nkilling qcam by sending TERM signal, like for example in\nbelow script:\n\n#!/bin/bash -x\nfor i in `seq 1 20` ; do\n        ls core &> /dev/null && break;\n\n        ./git/libcamera/build/src/apps/qcam/qcam &\n        pid=$!\n\n        sleep `expr $RANDOM % 10`\n        kill $pid\n        sleep `expr $RANDOM % 2`\ndone\n\nI have to manually press X on qcam window to trigger the assertion.\nUsually 2 or 3 times is enough.\n\nRegards\nStanislaw\n\n\n>  include/libcamera/internal/delayed_controls.h | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> index e8d3014d92cb..b64d8bba7cf7 100644\n> --- a/include/libcamera/internal/delayed_controls.h\n> +++ b/include/libcamera/internal/delayed_controls.h\n> @@ -10,13 +10,15 @@\n>  #include <stdint.h>\n>  #include <unordered_map>\n> \n> +#include <libcamera/base/object.h>\n> +\n>  #include <libcamera/controls.h>\n> \n>  namespace libcamera {\n> \n>  class V4L2Device;\n> \n> -class DelayedControls\n> +class DelayedControls : public Object\n>  {\n>  public:\n>  \tstruct ControlParams {\n> \n> base-commit: d476f8358be1536d4edd680c6024f784ff022f5d\n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D2FCFC324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 10:16:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8622686A6;\n\tMon, 24 Feb 2025 11:16:53 +0100 (CET)","from mgamail.intel.com (mgamail.intel.com [198.175.65.14])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0242686A6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 11:16:51 +0100 (CET)","from orviesa003.jf.intel.com ([10.64.159.143])\n\tby orvoesa106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t24 Feb 2025 02:16:50 -0800","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.245.97.15]) by ORVIESA003-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Feb 2025 02:16:49 -0800"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"bt8WJ4TS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1740392212; x=1771928212;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=JdsLayjBfKglDdRM9JOB22QlzZmoot+yaWx8dQxSGH4=;\n\tb=bt8WJ4TSjJkWiq6nWBnPyDo9uJ5QCh6xVQUFok+pxYl0vAs3Q9pRmh6k\n\tOaPC5rdPMLHG4NqTZ9jwiv646JpQGtcU85wD1GrQzMjzEmc0vpl5Fe361\n\tOkJ1u++m1bHLBogWsg9NioGP0XKz5/ZChuO5qDo0JS22PbZ4iCJDA/KSy\n\tizSwb4gcPbCK/bHgsFZNkqOmYqssHFtrRHO14SpmNLeRISnmnrIEkKueA\n\tl1y54B0XMmqFekFSsF2jnU+t3mA8LQvaq/dcV8sSFZjpW4p1nSGRIvRUr\n\tUzfVQs36Tb1a9YCjErIwbzAX3pbZQRv4fVoZSYT9fmpzKevmIl/U0j3l3 Q==;","X-CSE-ConnectionGUID":["fGjhNlWSR5mp/OVnk0y3DQ==","dOs9tjp+QlGrC4+2y2XyLg=="],"X-CSE-MsgGUID":["0Itk3FH7T1m1K+KKX5Dbvg==","ape3VBWNRoaG67UxRUlccQ=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11354\"; a=\"44917764\"","E=Sophos;i=\"6.13,309,1732608000\"; d=\"scan'208\";a=\"44917764\"","E=Sophos;i=\"6.13,309,1732608000\"; d=\"scan'208\";a=\"120932579\""],"X-ExtLoop1":"1","Date":"Mon, 24 Feb 2025 11:16:45 +0100","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tMilan Zamazal <mzamazal@redhat.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","Message-ID":"<Z7xHDamTyvFEG80t@linux.intel.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33469,"web_url":"https://patchwork.libcamera.org/comment/33469/","msgid":"<20250224212454.GO6778@pendragon.ideasonboard.com>","date":"2025-02-24T21:24:54","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stanislaw,\n\nOn Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > A second use-after-free bug related to signals staying connected after\n> > the receiver DelayedControls instance gets deleted has been found, this\n> > time in the simple pipeline handler. Fix the issue once and for all by\n> > making the DelayedControls class inherit from Object. This will\n> > disconnect signals automatically upon deletion of the receiver.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > Stan, would you be able to test this with the simple pipeline handler ?\n> > It should work both with and without your series that deals with the\n> > frame start signal, and should fix the crash that Kieran has reported.\n> \n> It does not make assertion\n> \"state_ == ProxyRunning\" failed in processStatsThread()\n> gone for me. \n\nRight, that one is addressed by Milan's \"PATCH v2 0/5] Fix occasional\nsoftware ISP assertion error on stop\" series.\n\nThis patch should fix a different crash, but now that I wrote this,\nKieran may not have reported it with the simple pipeline handler. He has\nreported it for the rkisp1 though.\n\nWould you be able to test this patch on top of Milan's series, without\nyour changes ? I'd like to make sure it introduces no regression. We can\nthen apply your series on top.\n\n> I tested this patch alone as well together with\n> \n> [PATCH v4 0/2] libcamera: start frame events changes\n> [PATCH] pipeline: simple: Create DelayedControls instance once only\n> \n> So far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\n> make the assertion gone.\n> \n> One interesting thing is the issue is not reproducible when\n> killing qcam by sending TERM signal, like for example in\n> below script:\n> \n> #!/bin/bash -x\n> for i in `seq 1 20` ; do\n>         ls core &> /dev/null && break;\n> \n>         ./git/libcamera/build/src/apps/qcam/qcam &\n>         pid=$!\n> \n>         sleep `expr $RANDOM % 10`\n>         kill $pid\n>         sleep `expr $RANDOM % 2`\n> done\n> \n> I have to manually press X on qcam window to trigger the assertion.\n> Usually 2 or 3 times is enough.\n> \n> >  include/libcamera/internal/delayed_controls.h | 4 +++-\n> >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> > index e8d3014d92cb..b64d8bba7cf7 100644\n> > --- a/include/libcamera/internal/delayed_controls.h\n> > +++ b/include/libcamera/internal/delayed_controls.h\n> > @@ -10,13 +10,15 @@\n> >  #include <stdint.h>\n> >  #include <unordered_map>\n> > \n> > +#include <libcamera/base/object.h>\n> > +\n> >  #include <libcamera/controls.h>\n> > \n> >  namespace libcamera {\n> > \n> >  class V4L2Device;\n> > \n> > -class DelayedControls\n> > +class DelayedControls : public Object\n> >  {\n> >  public:\n> >  \tstruct ControlParams {\n> > \n> > base-commit: d476f8358be1536d4edd680c6024f784ff022f5d","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6CAAEC324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 21:25:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C49E6870B;\n\tMon, 24 Feb 2025 22:25:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2669F61856\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 22:25:14 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 226D8220;\n\tMon, 24 Feb 2025 22:23:47 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ORbRuGZ9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740432227;\n\tbh=yh6aoUUNCrui0gSzvK/MmcYrT9ZPDaczaR4otwmKsGY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ORbRuGZ94FS3kzF2ZSXRun+KGBjbWBHF0VSdk8XVfH98Btc8ApfuNPO0keSOkZl3p\n\teXUBuB4Uw0YpxWWhgRmheIzBEMcdKKL2Np7E2karb39XW01E4KudagLUcuLKm7DLRT\n\tVnjV59966fJxJ1SFMoMsHH3W+8ym3eDR6iN0gtr0=","Date":"Mon, 24 Feb 2025 23:24:54 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tMilan Zamazal <mzamazal@redhat.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","Message-ID":"<20250224212454.GO6778@pendragon.ideasonboard.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Z7xHDamTyvFEG80t@linux.intel.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33470,"web_url":"https://patchwork.libcamera.org/comment/33470/","msgid":"<174043286288.2886126.13012776452543981091@ping.linuxembedded.co.uk>","date":"2025-02-24T21:34:22","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-02-24 21:24:54)\n> Hi Stanislaw,\n> \n> On Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> > On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > > A second use-after-free bug related to signals staying connected after\n> > > the receiver DelayedControls instance gets deleted has been found, this\n> > > time in the simple pipeline handler. Fix the issue once and for all by\n> > > making the DelayedControls class inherit from Object. This will\n> > > disconnect signals automatically upon deletion of the receiver.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > > Stan, would you be able to test this with the simple pipeline handler ?\n> > > It should work both with and without your series that deals with the\n> > > frame start signal, and should fix the crash that Kieran has reported.\n> > \n> > It does not make assertion\n> > \"state_ == ProxyRunning\" failed in processStatsThread()\n> > gone for me. \n> \n> Right, that one is addressed by Milan's \"PATCH v2 0/5] Fix occasional\n> software ISP assertion error on stop\" series.\n> \n> This patch should fix a different crash, but now that I wrote this,\n> Kieran may not have reported it with the simple pipeline handler. He has\n> reported it for the rkisp1 though.\n> \n\nYes, the delayed controls crash I hit was on rkisp1 with a video\nmultiplexor on the pipeline that lets me connect multiple cameras to a\nsingle ISP\n(https://www.arducam.com/product/multi-camera-v2-1-adapter-raspberry-pi/)\n\nWhen I get chance, I'll also test this patch in the rkisp1 context, but\nthat may take some time.\n\n--\nKieran\n\n> Would you be able to test this patch on top of Milan's series, without\n> your changes ? I'd like to make sure it introduces no regression. We can\n> then apply your series on top.\n> \n> > I tested this patch alone as well together with\n> > \n> > [PATCH v4 0/2] libcamera: start frame events changes\n> > [PATCH] pipeline: simple: Create DelayedControls instance once only\n> > \n> > So far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\n> > make the assertion gone.\n> > \n> > One interesting thing is the issue is not reproducible when\n> > killing qcam by sending TERM signal, like for example in\n> > below script:\n> > \n> > #!/bin/bash -x\n> > for i in `seq 1 20` ; do\n> >         ls core &> /dev/null && break;\n> > \n> >         ./git/libcamera/build/src/apps/qcam/qcam &\n> >         pid=$!\n> > \n> >         sleep `expr $RANDOM % 10`\n> >         kill $pid\n> >         sleep `expr $RANDOM % 2`\n> > done\n> > \n> > I have to manually press X on qcam window to trigger the assertion.\n> > Usually 2 or 3 times is enough.\n> > \n> > >  include/libcamera/internal/delayed_controls.h | 4 +++-\n> > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> > > index e8d3014d92cb..b64d8bba7cf7 100644\n> > > --- a/include/libcamera/internal/delayed_controls.h\n> > > +++ b/include/libcamera/internal/delayed_controls.h\n> > > @@ -10,13 +10,15 @@\n> > >  #include <stdint.h>\n> > >  #include <unordered_map>\n> > > \n> > > +#include <libcamera/base/object.h>\n> > > +\n> > >  #include <libcamera/controls.h>\n> > > \n> > >  namespace libcamera {\n> > > \n> > >  class V4L2Device;\n> > > \n> > > -class DelayedControls\n> > > +class DelayedControls : public Object\n> > >  {\n> > >  public:\n> > >     struct ControlParams {\n> > > \n> > > base-commit: d476f8358be1536d4edd680c6024f784ff022f5d\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5771EC32A9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 21:34:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F87D6870E;\n\tMon, 24 Feb 2025 22:34:27 +0100 (CET)","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 B279B68708\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 22:34:25 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFEAD220;\n\tMon, 24 Feb 2025 22:32:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sfyWRAbr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740432778;\n\tbh=Jkseab9SfOyM378m6wCStGakWB5JVD/92nfPXZjq/+g=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sfyWRAbr/1e9hFM4cGW/mzAqmkRcVSqvNN1EqE9/Eyk/CiRMQRZhB2cx8U4K5MTGb\n\tlsphjqaT6MdgyJhd2UOvI1i7U2Tnzm4nT6gOovlcQZIDlx/+Bq6l7iIMXRNsp/sAqi\n\twdhHN0LjDTqRTcO2yzVWfcOzN9uj4Iz/uf1CO3dY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250224212454.GO6778@pendragon.ideasonboard.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>\n\t<20250224212454.GO6778@pendragon.ideasonboard.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tStanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Date":"Mon, 24 Feb 2025 21:34:22 +0000","Message-ID":"<174043286288.2886126.13012776452543981091@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33476,"web_url":"https://patchwork.libcamera.org/comment/33476/","msgid":"<Z73i8XcuJfBRtg7q@linux.intel.com>","date":"2025-02-25T15:34:09","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"Hi Laurent,\n\nOn Mon, Feb 24, 2025 at 11:24:54PM +0200, Laurent Pinchart wrote:\n> Hi Stanislaw,\n> \n> On Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> > On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > > A second use-after-free bug related to signals staying connected after\n> > > the receiver DelayedControls instance gets deleted has been found, this\n> > > time in the simple pipeline handler. Fix the issue once and for all by\n> > > making the DelayedControls class inherit from Object. This will\n> > > disconnect signals automatically upon deletion of the receiver.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > > Stan, would you be able to test this with the simple pipeline handler ?\n> > > It should work both with and without your series that deals with the\n> > > frame start signal, and should fix the crash that Kieran has reported.\n> > \n> > It does not make assertion\n> > \"state_ == ProxyRunning\" failed in processStatsThread()\n> > gone for me. \n> \n> Right, that one is addressed by Milan's \"PATCH v2 0/5] Fix occasional\n> software ISP assertion error on stop\" series.\n> \n> This patch should fix a different crash, but now that I wrote this,\n> Kieran may not have reported it with the simple pipeline handler. He has\n> reported it for the rkisp1 though.\n> \n> Would you be able to test this patch on top of Milan's series, without\n> your changes ? I'd like to make sure it introduces no regression. We can\n> then apply your series on top.\n\nTested on master with Milan's \n[PATCH v3 0/6] Fix occasional software ISP assertion error on stop\n\n\"state_ == ProxyRunning\" assertion is gone and I did not encounter\nany regression with simple pipeline. Things looks good.\n\nTested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n\nRegards\nStanislaw","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4A3E3BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 25 Feb 2025 15:34:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B44C68720;\n\tTue, 25 Feb 2025 16:34:17 +0100 (CET)","from mgamail.intel.com (mgamail.intel.com [198.175.65.13])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B70A9686D6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Feb 2025 16:34:14 +0100 (CET)","from fmviesa009.fm.intel.com ([10.60.135.149])\n\tby orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t25 Feb 2025 07:34:13 -0800","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.245.115.185]) by fmviesa009-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Feb 2025 07:34:11 -0800"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"PASvI89G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1740497655; x=1772033655;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=E4Skafn+58ADyW1+kVT2JNAeZpRCgieQHqDVwKYWFEM=;\n\tb=PASvI89GEUg9BoTDr5+3v+qzKL2g+eVyR1sldSUnF7zUF1GtIKvGLx+F\n\tvgxlNYYeY0TNg8mPs5E1O8OoeN9HO6O3I5TGz5uer6Kvpxc7PXwBB335a\n\txz6YsyQV/MVqWzJEPVx8iM+6mYZ96YzITP3r53b4+o3kGRxTLtjOnEAw+\n\tpJW26LTGlaM7jLoeESdC+MP7T+C8LqXR8pDexKCglPucXJ/pwOO5y4eoT\n\tMkXtsFU1djJS5idzW1xzsZN45iS9ozXqOlQjxHncq8M+grBTUnd1en7zF\n\tyuNopyBTqo6vFmXgUl7DnNUDBu4rbPS1YMmaXWQ5n7H4CB/mAeLnHLr+O w==;","X-CSE-ConnectionGUID":["Q9gnAo/0Sou8+GH7HDkCyA==","jID6wQeEQh6H4tpLCX6VIg=="],"X-CSE-MsgGUID":["HT8WkLSRSk+eKX8G1qCgDg==","y4NzRvKcRTCz0+5XbIFIGQ=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11356\"; a=\"52306723\"","E=Sophos;i=\"6.13,314,1732608000\"; d=\"scan'208\";a=\"52306723\"","E=Sophos;i=\"6.13,314,1732608000\"; d=\"scan'208\";a=\"117033023\""],"X-ExtLoop1":"1","Date":"Tue, 25 Feb 2025 16:34:09 +0100","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tMilan Zamazal <mzamazal@redhat.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","Message-ID":"<Z73i8XcuJfBRtg7q@linux.intel.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>\n\t<20250224212454.GO6778@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250224212454.GO6778@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34856,"web_url":"https://patchwork.libcamera.org/comment/34856/","msgid":"<2dace631fd2d3d20272f05f2901d95bc00943817.camel@ideasonboard.com>","date":"2025-07-11T10:50:41","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch!\n\nOn Mon, 2025-02-24 at 23:24 +0200, Laurent Pinchart wrote:\n> Hi Stanislaw,\n> \n> On Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> > On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > > A second use-after-free bug related to signals staying connected\n> > > after\n> > > the receiver DelayedControls instance gets deleted has been\n> > > found, this\n> > > time in the simple pipeline handler. Fix the issue once and for\n> > > all by\n> > > making the DelayedControls class inherit from Object. This will\n> > > disconnect signals automatically upon deletion of the receiver.\n> > > \n> > > Signed-off-by: Laurent Pinchart\n> > > <laurent.pinchart@ideasonboard.com>\n\nThis fixes an issue when hotplugging cameras on the NXP i.MX 8M Plus,\nwhich it now works!\n\nTested-by: Isaac Scott <isaac.scott@ideasonboard.com>\n\n> > > ---\n> > > Stan, would you be able to test this with the simple pipeline\n> > > handler ?\n> > > It should work both with and without your series that deals with\n> > > the\n> > > frame start signal, and should fix the crash that Kieran has\n> > > reported.\n> > \n> > It does not make assertion\n> > \"state_ == ProxyRunning\" failed in processStatsThread()\n> > gone for me. \n> \n> Right, that one is addressed by Milan's \"PATCH v2 0/5] Fix occasional\n> software ISP assertion error on stop\" series.\n> \n> This patch should fix a different crash, but now that I wrote this,\n> Kieran may not have reported it with the simple pipeline handler. He\n> has\n> reported it for the rkisp1 though.\n> \n> Would you be able to test this patch on top of Milan's series,\n> without\n> your changes ? I'd like to make sure it introduces no regression. We\n> can\n> then apply your series on top.\n> \n> > I tested this patch alone as well together with\n> > \n> > [PATCH v4 0/2] libcamera: start frame events changes\n> > [PATCH] pipeline: simple: Create DelayedControls instance once only\n> > \n> > So far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\n> > make the assertion gone.\n> > \n> > One interesting thing is the issue is not reproducible when\n> > killing qcam by sending TERM signal, like for example in\n> > below script:\n> > \n> > #!/bin/bash -x\n> > for i in `seq 1 20` ; do\n> >         ls core &> /dev/null && break;\n> > \n> >         ./git/libcamera/build/src/apps/qcam/qcam &\n> >         pid=$!\n> > \n> >         sleep `expr $RANDOM % 10`\n> >         kill $pid\n> >         sleep `expr $RANDOM % 2`\n> > done\n> > \n> > I have to manually press X on qcam window to trigger the assertion.\n> > Usually 2 or 3 times is enough.\n> > \n> > >  include/libcamera/internal/delayed_controls.h | 4 +++-\n> > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/include/libcamera/internal/delayed_controls.h\n> > > b/include/libcamera/internal/delayed_controls.h\n> > > index e8d3014d92cb..b64d8bba7cf7 100644\n> > > --- a/include/libcamera/internal/delayed_controls.h\n> > > +++ b/include/libcamera/internal/delayed_controls.h\n> > > @@ -10,13 +10,15 @@\n> > >  #include <stdint.h>\n> > >  #include <unordered_map>\n> > > \n> > > +#include <libcamera/base/object.h>\n> > > +\n> > >  #include <libcamera/controls.h>\n> > > \n> > >  namespace libcamera {\n> > > \n> > >  class V4L2Device;\n> > > \n> > > -class DelayedControls\n> > > +class DelayedControls : public Object\n> > >  {\n> > >  public:\n> > >  \tstruct ControlParams {\n> > > \n> > > base-commit: d476f8358be1536d4edd680c6024f784ff022f5d","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B49EEC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Jul 2025 10:50:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E2C7C68F0C;\n\tFri, 11 Jul 2025 12:50:45 +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 1925A68E30\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Jul 2025 12:50:45 +0200 (CEST)","from isaac-ThinkPad-T16-Gen-2.lan\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4B69AC75;\n\tFri, 11 Jul 2025 12:50:15 +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=\"mAgfdzVn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752231015;\n\tbh=JalZefqi5d8gRRX1K39lgekHtPrwcLEhSMFUIJm/oXM=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=mAgfdzVnuwZIbElurMMO/wKGZw7XnpfKfL+360mlh3XIxjlkpdQDxBfW6P69qOJNH\n\tD/n3T+j3jdGka4on9l5NK/yXUS1SbprnIHJxD/52sniU8ESeUaADFzUe+yLwrw9+5U\n\tesH2E1sBxxmNkgEMng2RDQd2O9iBnMJcSzu8azTM=","Message-ID":"<2dace631fd2d3d20272f05f2901d95bc00943817.camel@ideasonboard.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","From":"Isaac Scott <isaac.scott@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Stanislaw Gruszka\n\t<stanislaw.gruszka@linux.intel.com>","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, Milan Zamazal <mzamazal@redhat.com>","Date":"Fri, 11 Jul 2025 11:50:41 +0100","In-Reply-To":"<20250224212454.GO6778@pendragon.ideasonboard.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>\n\t<20250224212454.GO6778@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.56.1 (by Flathub.org) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34857,"web_url":"https://patchwork.libcamera.org/comment/34857/","msgid":"<669d417ea88bffcfbdf233fda056f27dd5bfa5ce.camel@ideasonboard.com>","date":"2025-07-11T10:56:38","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, 2025-07-11 at 11:50 +0100, Isaac Scott wrote:\n> Hi Laurent,\n> \n> Thank you for the patch!\n> \n> On Mon, 2025-02-24 at 23:24 +0200, Laurent Pinchart wrote:\n> > Hi Stanislaw,\n> > \n> > On Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> > > On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > > > A second use-after-free bug related to signals staying\n> > > > connected\n> > > > after\n> > > > the receiver DelayedControls instance gets deleted has been\n> > > > found, this\n> > > > time in the simple pipeline handler. Fix the issue once and for\n> > > > all by\n> > > > making the DelayedControls class inherit from Object. This will\n> > > > disconnect signals automatically upon deletion of the receiver.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart\n> > > > <laurent.pinchart@ideasonboard.com>\n> \n> This fixes an issue when hotplugging cameras on the NXP i.MX 8M Plus,\n> which it now works!\n> \n> Tested-by: Isaac Scott <isaac.scott@ideasonboard.com>\n\nThe patch also looks good to me, so:\n\nReviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n\n> \n> > > > ---\n> > > > Stan, would you be able to test this with the simple pipeline\n> > > > handler ?\n> > > > It should work both with and without your series that deals\n> > > > with\n> > > > the\n> > > > frame start signal, and should fix the crash that Kieran has\n> > > > reported.\n> > > \n> > > It does not make assertion\n> > > \"state_ == ProxyRunning\" failed in processStatsThread()\n> > > gone for me. \n> > \n> > Right, that one is addressed by Milan's \"PATCH v2 0/5] Fix\n> > occasional\n> > software ISP assertion error on stop\" series.\n> > \n> > This patch should fix a different crash, but now that I wrote this,\n> > Kieran may not have reported it with the simple pipeline handler.\n> > He\n> > has\n> > reported it for the rkisp1 though.\n> > \n> > Would you be able to test this patch on top of Milan's series,\n> > without\n> > your changes ? I'd like to make sure it introduces no regression.\n> > We\n> > can\n> > then apply your series on top.\n> > \n> > > I tested this patch alone as well together with\n> > > \n> > > [PATCH v4 0/2] libcamera: start frame events changes\n> > > [PATCH] pipeline: simple: Create DelayedControls instance once\n> > > only\n> > > \n> > > So far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\n> > > make the assertion gone.\n> > > \n> > > One interesting thing is the issue is not reproducible when\n> > > killing qcam by sending TERM signal, like for example in\n> > > below script:\n> > > \n> > > #!/bin/bash -x\n> > > for i in `seq 1 20` ; do\n> > >         ls core &> /dev/null && break;\n> > > \n> > >         ./git/libcamera/build/src/apps/qcam/qcam &\n> > >         pid=$!\n> > > \n> > >         sleep `expr $RANDOM % 10`\n> > >         kill $pid\n> > >         sleep `expr $RANDOM % 2`\n> > > done\n> > > \n> > > I have to manually press X on qcam window to trigger the\n> > > assertion.\n> > > Usually 2 or 3 times is enough.\n> > > \n> > > >  include/libcamera/internal/delayed_controls.h | 4 +++-\n> > > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/delayed_controls.h\n> > > > b/include/libcamera/internal/delayed_controls.h\n> > > > index e8d3014d92cb..b64d8bba7cf7 100644\n> > > > --- a/include/libcamera/internal/delayed_controls.h\n> > > > +++ b/include/libcamera/internal/delayed_controls.h\n> > > > @@ -10,13 +10,15 @@\n> > > >  #include <stdint.h>\n> > > >  #include <unordered_map>\n> > > > \n> > > > +#include <libcamera/base/object.h>\n> > > > +\n> > > >  #include <libcamera/controls.h>\n> > > > \n> > > >  namespace libcamera {\n> > > > \n> > > >  class V4L2Device;\n> > > > \n> > > > -class DelayedControls\n> > > > +class DelayedControls : public Object\n> > > >  {\n> > > >  public:\n> > > >  \tstruct ControlParams {\n> > > > \n> > > > base-commit: d476f8358be1536d4edd680c6024f784ff022f5d\n\nBest wishes,\n\nIsaac","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 BE11CBE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Jul 2025 10:56:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0FC2C68F0C;\n\tFri, 11 Jul 2025 12:56:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C01968E30\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Jul 2025 12:56:41 +0200 (CEST)","from isaac-ThinkPad-T16-Gen-2.lan\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 994F2D77;\n\tFri, 11 Jul 2025 12:56:11 +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=\"HLbOv5r1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752231371;\n\tbh=ztN1byPRmb4zpAYgBF38nIeAWgt8k9VUc6K3NjlC5Q0=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=HLbOv5r1ZxXZ/WWK/9UbTLwQmekrMPS9oYGrwrHg9qShS0KABcxYLa1NFuLTdAvj2\n\tpsf+um7Fr+d89vzj1VuI1P7CkZA0lfofleHJd7lhWkBWUinzZ10MUD7Uv9n/7lCCUn\n\tsWVOwrv6NIkFSgVkRk1MEWjhOnUIhVgz8Mk+KEnw=","Message-ID":"<669d417ea88bffcfbdf233fda056f27dd5bfa5ce.camel@ideasonboard.com>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","From":"Isaac Scott <isaac.scott@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Stanislaw Gruszka\n\t<stanislaw.gruszka@linux.intel.com>","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, Milan Zamazal <mzamazal@redhat.com>","Date":"Fri, 11 Jul 2025 11:56:38 +0100","In-Reply-To":"<2dace631fd2d3d20272f05f2901d95bc00943817.camel@ideasonboard.com>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>\n\t<20250224212454.GO6778@pendragon.ideasonboard.com>\n\t<2dace631fd2d3d20272f05f2901d95bc00943817.camel@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.56.1 (by Flathub.org) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34858,"web_url":"https://patchwork.libcamera.org/comment/34858/","msgid":"<175223313296.74140.5100554418223625510@ping.linuxembedded.co.uk>","date":"2025-07-11T11:25:32","subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2025-02-24 21:34:22)\n> Quoting Laurent Pinchart (2025-02-24 21:24:54)\n> > Hi Stanislaw,\n> > \n> > On Mon, Feb 24, 2025 at 11:16:45AM +0100, Stanislaw Gruszka wrote:\n> > > On Mon, Feb 24, 2025 at 03:19:34AM +0200, Laurent Pinchart wrote:\n> > > > A second use-after-free bug related to signals staying connected after\n> > > > the receiver DelayedControls instance gets deleted has been found, this\n> > > > time in the simple pipeline handler. Fix the issue once and for all by\n> > > > making the DelayedControls class inherit from Object. This will\n> > > > disconnect signals automatically upon deletion of the receiver.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > ---\n> > > > Stan, would you be able to test this with the simple pipeline handler ?\n> > > > It should work both with and without your series that deals with the\n> > > > frame start signal, and should fix the crash that Kieran has reported.\n> > > \n> > > It does not make assertion\n> > > \"state_ == ProxyRunning\" failed in processStatsThread()\n> > > gone for me. \n> > \n> > Right, that one is addressed by Milan's \"PATCH v2 0/5] Fix occasional\n> > software ISP assertion error on stop\" series.\n> > \n> > This patch should fix a different crash, but now that I wrote this,\n> > Kieran may not have reported it with the simple pipeline handler. He has\n> > reported it for the rkisp1 though.\n> > \n> \n> Yes, the delayed controls crash I hit was on rkisp1 with a video\n> multiplexor on the pipeline that lets me connect multiple cameras to a\n> single ISP\n> (https://www.arducam.com/product/multi-camera-v2-1-adapter-raspberry-pi/)\n> \n> When I get chance, I'll also test this patch in the rkisp1 context, but\n> that may take some time.\n\nNow that Isaac has done the testing for me - I guess I should also add\nthis :\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> --\n> Kieran\n> \n> > Would you be able to test this patch on top of Milan's series, without\n> > your changes ? I'd like to make sure it introduces no regression. We can\n> > then apply your series on top.\n> > \n> > > I tested this patch alone as well together with\n> > > \n> > > [PATCH v4 0/2] libcamera: start frame events changes\n> > > [PATCH] pipeline: simple: Create DelayedControls instance once only\n> > > \n> > > So far, only revert of 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138\n> > > make the assertion gone.\n> > > \n> > > One interesting thing is the issue is not reproducible when\n> > > killing qcam by sending TERM signal, like for example in\n> > > below script:\n> > > \n> > > #!/bin/bash -x\n> > > for i in `seq 1 20` ; do\n> > >         ls core &> /dev/null && break;\n> > > \n> > >         ./git/libcamera/build/src/apps/qcam/qcam &\n> > >         pid=$!\n> > > \n> > >         sleep `expr $RANDOM % 10`\n> > >         kill $pid\n> > >         sleep `expr $RANDOM % 2`\n> > > done\n> > > \n> > > I have to manually press X on qcam window to trigger the assertion.\n> > > Usually 2 or 3 times is enough.\n> > > \n> > > >  include/libcamera/internal/delayed_controls.h | 4 +++-\n> > > >  1 file changed, 3 insertions(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> > > > index e8d3014d92cb..b64d8bba7cf7 100644\n> > > > --- a/include/libcamera/internal/delayed_controls.h\n> > > > +++ b/include/libcamera/internal/delayed_controls.h\n> > > > @@ -10,13 +10,15 @@\n> > > >  #include <stdint.h>\n> > > >  #include <unordered_map>\n> > > > \n> > > > +#include <libcamera/base/object.h>\n> > > > +\n> > > >  #include <libcamera/controls.h>\n> > > > \n> > > >  namespace libcamera {\n> > > > \n> > > >  class V4L2Device;\n> > > > \n> > > > -class DelayedControls\n> > > > +class DelayedControls : public Object\n> > > >  {\n> > > >  public:\n> > > >     struct ControlParams {\n> > > > \n> > > > base-commit: d476f8358be1536d4edd680c6024f784ff022f5d\n> > \n> > -- \n> > Regards,\n> > \n> > Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C375BC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Jul 2025 11:25:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84BEE68F0F;\n\tFri, 11 Jul 2025 13:25:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9643468E30\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Jul 2025 13:25:35 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF2FEC75;\n\tFri, 11 Jul 2025 13:25: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=\"CcfGkRoP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752233105;\n\tbh=ooijzWylJ+GPDX/Vcy6V5pbfC1giQjbkUDym4Dcfl6k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=CcfGkRoP55tNgucEpL8Tb6AP9zu4JyykcUOJDZdgmMGOg8ito2+xB6Gx8xzTxRtTj\n\trZ3y068oKqDowotVheLe+8A5+RiGSjxgjwCefubCfyZiWCkq0fHRb8ZZUDnK32ujmK\n\tw1dd6ybDEsIWAeJyu2lf7QiSL2WL83FhjP99q+DQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<174043286288.2886126.13012776452543981091@ping.linuxembedded.co.uk>","References":"<20250224011934.23818-1-laurent.pinchart@ideasonboard.com>\n\t<Z7xHDamTyvFEG80t@linux.intel.com>\n\t<20250224212454.GO6778@pendragon.ideasonboard.com>\n\t<174043286288.2886126.13012776452543981091@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH] libcamera: delayed_controls: Inherit from Object class","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tStanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Date":"Fri, 11 Jul 2025 12:25:32 +0100","Message-ID":"<175223313296.74140.5100554418223625510@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]