[{"id":17531,"web_url":"https://patchwork.libcamera.org/comment/17531/","msgid":"","date":"2021-06-14T17:07:41","subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Mon, 14 Jun 2021, 10:53 am Naushir Patuck, wrote:\n\n> This avoids the need for any dynamic allocations and lifetime management.\n> The\n> base CamHelper class still accesses the parser through a pointer that is\n> setup\n> by the derived class constructor.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 5 ++---\n> src/ipa/raspberrypi/cam_helper.hpp | 2 +-\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 9 +++++----\n> src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-\n> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-\n> 6 files changed, 14 insertions(+), 11 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4fef3..ab66e2ddef3e 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -40,15 +40,14 @@ CamHelper *CamHelper::Create(std::string const\n> &cam_name)\n> return nullptr;\n> }\n>\n> -CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n> - : parser_(parser), initialized_(false),\n> +CamHelper::CamHelper(unsigned int frameIntegrationDiff)\n> + : parser_(nullptr), initialized_(false),\n> frameIntegrationDiff_(frameIntegrationDiff)\n> {\n> }\n>\n> CamHelper::~CamHelper()\n> {\n> - delete parser_;\n> }\n>\n> void CamHelper::Prepare(Span buffer,\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> b/src/ipa/raspberrypi/cam_helper.hpp\n> index f53f5c39b01c..460839079741 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -67,7 +67,7 @@ class CamHelper\n> {\n> public:\n> static CamHelper *Create(std::string const &cam_name);\n> - CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> + CamHelper(unsigned int frameIntegrationDiff);\n> virtual ~CamHelper();\n> void SetCameraMode(const CameraMode &mode);\n> virtual void Prepare(libcamera::Span buffer,\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index ec218dce5456..36dbe8cd941a 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -54,15 +54,16 @@ private:\n> * in units of lines.\n> */\n> static constexpr int frameIntegrationDiff = 4;\n> +\n> + MdParserImx219 imx219_parser;\n> };\n>\n\n> CamHelperImx219::CamHelperImx219()\n> + : CamHelper(frameIntegrationDiff)\n> +{\n> #if ENABLE_EMBEDDED_DATA\n> - : CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> -#else\n> - : CamHelper(nullptr, frameIntegrationDiff)\n> + parser_ = &imx219_parser;\n> #endif\n> -{\n> }\n>\n\n\nLooking at this again, I think it might be better to pass the parser\npointer thorough the CamHelper constructor as before. I'll change that back\nfor v2.\n\n\n\n>\n>\n> uint32_t CamHelperImx219::GainCode(double gain) const\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> index 6f412e403f16..dea57b84bf0b 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> @@ -30,7 +30,7 @@ private:\n> };\n>\n> CamHelperImx290::CamHelperImx290()\n> - : CamHelper(nullptr, frameIntegrationDiff)\n> + : CamHelper(frameIntegrationDiff)\n> {\n> }\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 25b36bce0dac..038a8583d311 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -47,11 +47,14 @@ private:\n> * in units of lines.\n> */\n> static constexpr int frameIntegrationDiff = 22;\n> +\n> + MdParserImx477 imx477_parser;\n> };\n>\n> CamHelperImx477::CamHelperImx477()\n> - : CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> + : CamHelper(frameIntegrationDiff)\n> {\n> + parser_ = &imx477_parser;\n> }\n>\n> uint32_t CamHelperImx477::GainCode(double gain) const\n> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> index 12be6bf931a8..fff648279d2a 100644\n> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> @@ -38,7 +38,7 @@ private:\n> */\n>\n> CamHelperOv5647::CamHelperOv5647()\n> - : CamHelper(nullptr, frameIntegrationDiff)\n> + : CamHelper(frameIntegrationDiff)\n> {\n> }\n>\n> --\n> 2.25.1\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 D4E99BD78E\n\tfor ;\n\tMon, 14 Jun 2021 17:07:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D9F768934;\n\tMon, 14 Jun 2021 19:07:55 +0200 (CEST)","from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87EBA602A1\n\tfor ;\n\tMon, 14 Jun 2021 19:07:53 +0200 (CEST)","by mail-lf1-x12c.google.com with SMTP id k40so22263215lfv.7\n\tfor ;\n\tMon, 14 Jun 2021 10:07:53 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"C59U7y8V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=3aGT48Nnm8PBSWjQySRUDhkguqVMm5ipUSud/U0yBFE=;\n\tb=C59U7y8VhEJdfLXRaXwVcXQxOTPsckdpbs/l113bZRWMoETq4VUrOOWoqaM9J+Lb2/\n\tVipiBsPJa0+2x/BgmUOF+ijCn6qzflPDVsv0vi0tDR8xJ1mON675Skxhnnv2oWqweQTP\n\to/v6aPzVxmQ4TM+kLDNY4d6BuiCaMcoDi98biISD48HvnxQX6LPWmTJMOzCmZgZEVt7C\n\tVztUeaQkvIZiw3oQp8dG/tNMYdCIIUOVOE0BMRL1pR0K1rsx4GcWY+Hr39YTlGpL5fO2\n\tAZb7K2uGRR0asP/uRXbNKAV1WlxFAimF/O2+tHfzJa5ypQNjRCagXZ3YhnEAGzlQd6kS\n\tX1Uw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=3aGT48Nnm8PBSWjQySRUDhkguqVMm5ipUSud/U0yBFE=;\n\tb=F/6I8er/wS6KNvEiSFPsb0fhuqNWzNZf9T0QWufjRYoJl/Ri2Wxqul3UT4+2AuoWy1\n\tW8mqnr52TJe479wv4oSTZqg1nSX/mH+d1+ezVF3Z0Rgj6/214rwEuRTcxGIFbxF9f8iF\n\tFhX2fI9hU4F4asRBBdk2gv558haPplI32yfauVRRa6xGGVFs9X7VeJqOkfIS1dbv90ug\n\tz6w7nwJ8db5h814/TUWyCZCYXzJfPGsN4PcqRGn8/kWKkWmKvNL0jEX+lpH6RhrlTLDy\n\t35myGdRZ32fXBP5HxOvtm2l1EpU4un+aQ2HIrBBOfeQNuSQctOUSgDriIo5deSZT+APr\n\tURxw==","X-Gm-Message-State":"AOAM53006353acN9/8XxRSyvAwfYOtjOIsBfdOoIgi6relewNuOzLvoy\n\tLuy5cd2BXINgjiRNU1wbFG9Q/CzxbUGhSi5rddg2hyAyGiA=","X-Google-Smtp-Source":"ABdhPJywS8U3WoQO3Wl6u6DDL9ZxX7yze41FcQei7Gs798jLuww79wWNTLh9MnCi+/NFUw2GucMV2iq2X8wFGUZxPWU=","X-Received":"by 2002:a05:6512:3ee:: with SMTP id\n\tn14mr12343690lfq.567.1623690472329; \n\tMon, 14 Jun 2021 10:07:52 -0700 (PDT)","MIME-Version":"1.0","References":"<20210614095340.3051816-1-naush@raspberrypi.com>\n\t<20210614095340.3051816-6-naush@raspberrypi.com>","In-Reply-To":"<20210614095340.3051816-6-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Mon, 14 Jun 2021 18:07:41 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"multipart/alternative; boundary=\"000000000000d422a205c4bce5cf\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","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":17546,"web_url":"https://patchwork.libcamera.org/comment/17546/","msgid":"","date":"2021-06-15T02:52:24","subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Jun 14, 2021 at 06:07:41PM +0100, Naushir Patuck wrote:\n> On Mon, 14 Jun 2021, 10:53 am Naushir Patuck wrote:\n> > This avoids the need for any dynamic allocations and lifetime management. The\n> > base CamHelper class still accesses the parser through a pointer that is setup\n> > by the derived class constructor.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/cam_helper.cpp | 5 ++---\n> > src/ipa/raspberrypi/cam_helper.hpp | 2 +-\n> > src/ipa/raspberrypi/cam_helper_imx219.cpp | 9 +++++----\n> > src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-\n> > src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-\n> > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-\n> > 6 files changed, 14 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> > b/src/ipa/raspberrypi/cam_helper.cpp\n> > index 062e94c4fef3..ab66e2ddef3e 100644\n> > --- a/src/ipa/raspberrypi/cam_helper.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> > @@ -40,15 +40,14 @@ CamHelper *CamHelper::Create(std::string const\n> > &cam_name)\n> > return nullptr;\n> > }\n> >\n> > -CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n> > - : parser_(parser), initialized_(false),\n> > +CamHelper::CamHelper(unsigned int frameIntegrationDiff)\n> > + : parser_(nullptr), initialized_(false),\n> > frameIntegrationDiff_(frameIntegrationDiff)\n> > {\n> > }\n> >\n> > CamHelper::~CamHelper()\n> > {\n> > - delete parser_;\n> > }\n> >\n> > void CamHelper::Prepare(Span buffer,\n> > diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> > b/src/ipa/raspberrypi/cam_helper.hpp\n> > index f53f5c39b01c..460839079741 100644\n> > --- a/src/ipa/raspberrypi/cam_helper.hpp\n> > +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> > @@ -67,7 +67,7 @@ class CamHelper\n> > {\n> > public:\n> > static CamHelper *Create(std::string const &cam_name);\n> > - CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> > + CamHelper(unsigned int frameIntegrationDiff);\n> > virtual ~CamHelper();\n> > void SetCameraMode(const CameraMode &mode);\n> > virtual void Prepare(libcamera::Span buffer,\n> > diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > index ec218dce5456..36dbe8cd941a 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > @@ -54,15 +54,16 @@ private:\n> > * in units of lines.\n> > */\n> > static constexpr int frameIntegrationDiff = 4;\n> > +\n> > + MdParserImx219 imx219_parser;\n> > };\n> >\n> \n> > CamHelperImx219::CamHelperImx219()\n> > + : CamHelper(frameIntegrationDiff)\n> > +{\n> > #if ENABLE_EMBEDDED_DATA\n> > - : CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> > -#else\n> > - : CamHelper(nullptr, frameIntegrationDiff)\n> > + parser_ = &imx219_parser;\n> > #endif\n> > -{\n> > }\n> \n> Looking at this again, I think it might be better to pass the parser\n> pointer thorough the CamHelper constructor as before. I'll change that back\n> for v2.\n\nThe rest of the patch looks fine to me.\n\nShould I push patch 1/6 to 4/6 already, or will you include them in a v2\n? I have them ready in a branch :-)\n\n> > uint32_t CamHelperImx219::GainCode(double gain) const\n> > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > index 6f412e403f16..dea57b84bf0b 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > @@ -30,7 +30,7 @@ private:\n> > };\n> >\n> > CamHelperImx290::CamHelperImx290()\n> > - : CamHelper(nullptr, frameIntegrationDiff)\n> > + : CamHelper(frameIntegrationDiff)\n> > {\n> > }\n> >\n> > diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > index 25b36bce0dac..038a8583d311 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > @@ -47,11 +47,14 @@ private:\n> > * in units of lines.\n> > */\n> > static constexpr int frameIntegrationDiff = 22;\n> > +\n> > + MdParserImx477 imx477_parser;\n> > };\n> >\n> > CamHelperImx477::CamHelperImx477()\n> > - : CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> > + : CamHelper(frameIntegrationDiff)\n> > {\n> > + parser_ = &imx477_parser;\n> > }\n> >\n> > uint32_t CamHelperImx477::GainCode(double gain) const\n> > diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > index 12be6bf931a8..fff648279d2a 100644\n> > --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > @@ -38,7 +38,7 @@ private:\n> > */\n> >\n> > CamHelperOv5647::CamHelperOv5647()\n> > - : CamHelper(nullptr, frameIntegrationDiff)\n> > + : CamHelper(frameIntegrationDiff)\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 4FBC4C3218\n\tfor ;\n\tTue, 15 Jun 2021 02:52:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C65D668942;\n\tTue, 15 Jun 2021 04:52:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA9986050D\n\tfor ;\n\tTue, 15 Jun 2021 04:52:44 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 46C4BE9;\n\tTue, 15 Jun 2021 04:52:44 +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=\"GTwLleMg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623725564;\n\tbh=ydiPbb/dg+FOZF1UePUYcP3BHrou5+w5kjMgKJX3FGA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GTwLleMg3Yf2U3QGTPSLle1/wx7/8I7FoogI9V1wi9AzI2b1xOxyEDlRWLTyuhkni\n\tKcmf1Sw0OnRXx3O3aUCdEWPW158jRqYQY0pRQWSWAlJgNk03vF8hnOafAzBL76Ky5c\n\tpFNbQ9tLZ5bmp0q/0kRoJR5SwbxylJv716gZPW4A=","Date":"Tue, 15 Jun 2021 05:52:24 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210614095340.3051816-1-naush@raspberrypi.com>\n\t<20210614095340.3051816-6-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17558,"web_url":"https://patchwork.libcamera.org/comment/17558/","msgid":"","date":"2021-06-15T08:56:15","subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review on this series.\n\nOn Tue, 15 Jun 2021 at 03:52, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Mon, Jun 14, 2021 at 06:07:41PM +0100, Naushir Patuck wrote:\n> > On Mon, 14 Jun 2021, 10:53 am Naushir Patuck wrote:\n> > > This avoids the need for any dynamic allocations and lifetime\n> management. The\n> > > base CamHelper class still accesses the parser through a pointer that\n> is setup\n> > > by the derived class constructor.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/ipa/raspberrypi/cam_helper.cpp | 5 ++---\n> > > src/ipa/raspberrypi/cam_helper.hpp | 2 +-\n> > > src/ipa/raspberrypi/cam_helper_imx219.cpp | 9 +++++----\n> > > src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-\n> > > src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-\n> > > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-\n> > > 6 files changed, 14 insertions(+), 11 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> > > b/src/ipa/raspberrypi/cam_helper.cpp\n> > > index 062e94c4fef3..ab66e2ddef3e 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper.cpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> > > @@ -40,15 +40,14 @@ CamHelper *CamHelper::Create(std::string const\n> > > &cam_name)\n> > > return nullptr;\n> > > }\n> > >\n> > > -CamHelper::CamHelper(MdParser *parser, unsigned int\n> frameIntegrationDiff)\n> > > - : parser_(parser), initialized_(false),\n> > > +CamHelper::CamHelper(unsigned int frameIntegrationDiff)\n> > > + : parser_(nullptr), initialized_(false),\n> > > frameIntegrationDiff_(frameIntegrationDiff)\n> > > {\n> > > }\n> > >\n> > > CamHelper::~CamHelper()\n> > > {\n> > > - delete parser_;\n> > > }\n> > >\n> > > void CamHelper::Prepare(Span buffer,\n> > > diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> > > b/src/ipa/raspberrypi/cam_helper.hpp\n> > > index f53f5c39b01c..460839079741 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper.hpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> > > @@ -67,7 +67,7 @@ class CamHelper\n> > > {\n> > > public:\n> > > static CamHelper *Create(std::string const &cam_name);\n> > > - CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> > > + CamHelper(unsigned int frameIntegrationDiff);\n> > > virtual ~CamHelper();\n> > > void SetCameraMode(const CameraMode &mode);\n> > > virtual void Prepare(libcamera::Span buffer,\n> > > diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > > b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > > index ec218dce5456..36dbe8cd941a 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> > > @@ -54,15 +54,16 @@ private:\n> > > * in units of lines.\n> > > */\n> > > static constexpr int frameIntegrationDiff = 4;\n> > > +\n> > > + MdParserImx219 imx219_parser;\n> > > };\n> > >\n> >\n> > > CamHelperImx219::CamHelperImx219()\n> > > + : CamHelper(frameIntegrationDiff)\n> > > +{\n> > > #if ENABLE_EMBEDDED_DATA\n> > > - : CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> > > -#else\n> > > - : CamHelper(nullptr, frameIntegrationDiff)\n> > > + parser_ = &imx219_parser;\n> > > #endif\n> > > -{\n> > > }\n> >\n> > Looking at this again, I think it might be better to pass the parser\n> > pointer thorough the CamHelper constructor as before. I'll change that\n> back\n> > for v2.\n>\n> The rest of the patch looks fine to me.\n>\n> Should I push patch 1/6 to 4/6 already, or will you include them in a v2\n> ? I have them ready in a branch :-)\n>\n\nAck all your suggestions, and happy for you to merge patches 1-4 straight\naway\nwhile I rework 5/6 and 6/6 (will discuss that in the other email thread).\n\nRegards,\nNaush\n\n\n\n>\n> > > uint32_t CamHelperImx219::GainCode(double gain) const\n> > > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > > b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > > index 6f412e403f16..dea57b84bf0b 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> > > @@ -30,7 +30,7 @@ private:\n> > > };\n> > >\n> > > CamHelperImx290::CamHelperImx290()\n> > > - : CamHelper(nullptr, frameIntegrationDiff)\n> > > + : CamHelper(frameIntegrationDiff)\n> > > {\n> > > }\n> > >\n> > > diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > > b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > > index 25b36bce0dac..038a8583d311 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> > > @@ -47,11 +47,14 @@ private:\n> > > * in units of lines.\n> > > */\n> > > static constexpr int frameIntegrationDiff = 22;\n> > > +\n> > > + MdParserImx477 imx477_parser;\n> > > };\n> > >\n> > > CamHelperImx477::CamHelperImx477()\n> > > - : CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> > > + : CamHelper(frameIntegrationDiff)\n> > > {\n> > > + parser_ = &imx477_parser;\n> > > }\n> > >\n> > > uint32_t CamHelperImx477::GainCode(double gain) const\n> > > diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > > b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > > index 12be6bf931a8..fff648279d2a 100644\n> > > --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > > +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> > > @@ -38,7 +38,7 @@ private:\n> > > */\n> > >\n> > > CamHelperOv5647::CamHelperOv5647()\n> > > - : CamHelper(nullptr, frameIntegrationDiff)\n> > > + : CamHelper(frameIntegrationDiff)\n> > > {\n> > > }\n> > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 78271C3218\n\tfor ;\n\tTue, 15 Jun 2021 08:56:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36B3268944;\n\tTue, 15 Jun 2021 10:56:32 +0200 (CEST)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ABEBA6892D\n\tfor ;\n\tTue, 15 Jun 2021 10:56:31 +0200 (CEST)","by mail-lf1-x133.google.com with SMTP id v22so25785217lfa.3\n\tfor ;\n\tTue, 15 Jun 2021 01:56:31 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"PYW3iDfH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=gNg/KKJ//NXvOk0E0mh4r+kLZ2q4J5+4v+tAdYzyjhs=;\n\tb=PYW3iDfHaqVhIgRj3NXwg4glO3eL0m5RtuTkAH8/D5RjTDGsoalmWWKoetgfgHA2y7\n\t3LCvoDeeivBp2uW8x04FFr1j8Dqv6tMp6sSOkqkdJjOaqzjZUUkG/SrcJsr3UqH/sVpK\n\tSW7n+g28L/niZYFOMmEUMohw+FOxuoSVY0UtcMTdlj6hK0MwZrjvWvdm18+aG2Vq9V3m\n\tPMAS1WsA42REdyIlIKzEDyijb9ymhBTlnpBReiykk2FDYdQSKdDuHERCQRoAE3bYBDoQ\n\tduSNk7zavYsWxybOvwDG/qsNGElaf6T0wSDZoJwAJywkh0jp0zv8zvKMfrzEZgRFHzle\n\t3PwA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=gNg/KKJ//NXvOk0E0mh4r+kLZ2q4J5+4v+tAdYzyjhs=;\n\tb=H/Z+J7hozgNieBVtKEwswyGykC2Z9VCe1oSppVbCwjo4347Y0FSzcI830vcwbRQMtm\n\tbJRRZnoO/8H9ntMkx1/ojxlaAnpBrqpsSOdFLUfTXD5HDdJ3SZjnaF8zeAkIucmz9HUN\n\tH5iKrBY2jz/hgHx97c5rYHO1L2fOi2bI4VmpjgWsOiVj2b1f0dRudRHsAMClGQVxq100\n\tyjEd0GU0Dn7HSSyfbyVioQDn/AOHjo7B1BNcUSxrEZkhxP9owqcjX/Za/1mWMV6RWEhM\n\tlz0dPQ3fg8cJzhMpcW+xZbDbkRH0/aX28JIl899Fwixn0ZQE7RVHj3qzVdNSAbYu6T8j\n\t8IjQ==","X-Gm-Message-State":"AOAM532D6crTl/tzj1kGbvOflKLhqSmka7rkVkFLZ4S/DmpkGc8o5aUu\n\tGRfMQZ8rXtwcQ0tkFSxJ/I5dJMkbX4L2O7lNU805EwHZMSE=","X-Google-Smtp-Source":"ABdhPJzARKMy9QyzzB5XdkZEnvwI1wwObEIerJy27r3oEckXRzbx/HcPYNUml2yxWxEsvQHbHjSHMDDJTnFviYAyCpQ=","X-Received":"by 2002:a05:6512:2096:: with SMTP id\n\tt22mr15183067lfr.272.1623747391163; \n\tTue, 15 Jun 2021 01:56:31 -0700 (PDT)","MIME-Version":"1.0","References":"<20210614095340.3051816-1-naush@raspberrypi.com>\n\t<20210614095340.3051816-6-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 15 Jun 2021 09:56:15 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000749f3805c4ca26a7\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]