[{"id":17637,"web_url":"https://patchwork.libcamera.org/comment/17637/","msgid":"<9345198e-6b95-5805-935d-32f22486b995@ideasonboard.com>","date":"2021-06-18T21:53:00","subject":"Re: [libcamera-devel] [PATCH 1/3] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 15/06/2021 15:42, 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\nReducing allocations always sounds like a win.\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 1 -\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 +++-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +++-\n> 3 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4fef3..1474464c9257 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -48,7 +48,6 @@ CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n> \n> CamHelper::~CamHelper()\n> {\n> -\tdelete parser_;\n> }\n> \n> void CamHelper::Prepare(Span buffer,\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index ec218dce5456..d951cd552a21 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -54,11 +54,13 @@ private:\n> \t * in units of lines.\n> \t */\n> \tstatic constexpr int frameIntegrationDiff = 4;\n> +\n> +\tMdParserImx219 imx219_parser;\n> };\n> \n> CamHelperImx219::CamHelperImx219()\n> #if ENABLE_EMBEDDED_DATA\n> -\t: CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> +\t: CamHelper(&imx219_parser, frameIntegrationDiff)\n> #else\n> \t: CamHelper(nullptr, frameIntegrationDiff)\n> #endif\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 25b36bce0dac..44f030ed7da9 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -47,10 +47,12 @@ private:\n> \t * in units of lines.\n> \t */\n> \tstatic constexpr int frameIntegrationDiff = 22;\n> +\n> +\tMdParserImx477 imx477_parser;\n> };\n> \n> CamHelperImx477::CamHelperImx477()\n> -\t: CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> +\t: CamHelper(&imx477_parser, frameIntegrationDiff)\n> {\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 6F8D3C3218\n\tfor ;\n\tFri, 18 Jun 2021 21:53:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6BAD68942;\n\tFri, 18 Jun 2021 23:53:04 +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 ED62260298\n\tfor ;\n\tFri, 18 Jun 2021 23:53:03 +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 716E83F0;\n\tFri, 18 Jun 2021 23:53:03 +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=\"DM7V0koR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624053183;\n\tbh=ogdjUTA3UYzzw8rsK43JToD2/1QGvjEypGns3odporc=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=DM7V0koRd0fGGAMWS4HJRdfBiLAlXW8ys5UR3LV//mEfx/Zw1hiZNyL8TpItOiXwW\n\trRrpVWNAt8FK1uABLAN6jaiMzJEn1wmlXWV4KHc0L5d3v205ID8aqUM0PpaMp9obcD\n\tCp9P6KvbOYuhNqoPMgE2+IO7ukmdC55ztkIQLhqA=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210615144211.173047-1-naush@raspberrypi.com>\n\t<20210615144211.173047-2-naush@raspberrypi.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<9345198e-6b95-5805-935d-32f22486b995@ideasonboard.com>","Date":"Fri, 18 Jun 2021 22:53:00 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210615144211.173047-2-naush@raspberrypi.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 1/3] 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","Reply-To":"kieran.bingham@ideasonboard.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17649,"web_url":"https://patchwork.libcamera.org/comment/17649/","msgid":"","date":"2021-06-21T08:02:35","subject":"Re: [libcamera-devel] [PATCH 1/3] ipa: raspberrypi: Embed the\n\tmetadata parser in the sensor CamHelper classes","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch.\n\nOn Tue, 15 Jun 2021 at 15:42, Naushir Patuck wrote:\n>\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\nAgree. no need for extra news and deletes.\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 1 -\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 +++-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +++-\n> 3 files changed, 6 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4fef3..1474464c9257 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -48,7 +48,6 @@ CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\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_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index ec218dce5456..d951cd552a21 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -54,11 +54,13 @@ private:\n> * in units of lines.\n> */\n> static constexpr int frameIntegrationDiff = 4;\n> +\n> + MdParserImx219 imx219_parser;\n> };\n>\n> CamHelperImx219::CamHelperImx219()\n> #if ENABLE_EMBEDDED_DATA\n> - : CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> + : CamHelper(&imx219_parser, frameIntegrationDiff)\n> #else\n> : CamHelper(nullptr, frameIntegrationDiff)\n> #endif\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 25b36bce0dac..44f030ed7da9 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -47,10 +47,12 @@ 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(&imx477_parser, frameIntegrationDiff)\n> {\n> }\n>\n> --\n> 2.25.1\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 21847BE58C\n\tfor ;\n\tMon, 21 Jun 2021 08:02:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5877668935;\n\tMon, 21 Jun 2021 10:02:47 +0200 (CEST)","from mail-wm1-x330.google.com (mail-wm1-x330.google.com\n\t[IPv6:2a00:1450:4864:20::330])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FB0460295\n\tfor ;\n\tMon, 21 Jun 2021 10:02:46 +0200 (CEST)","by mail-wm1-x330.google.com with SMTP id w13so3196330wmc.3\n\tfor ;\n\tMon, 21 Jun 2021 01:02:46 -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=\"nPdqZ+QB\"; 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=8GXbH/ze2xiGCllgktsf9Haj/9P6t8Do6+/q04Tx8eo=;\n\tb=nPdqZ+QBkJLBQvPMVhBJMX3HHrPgqXTFU/YvVcWS0xC5qRLMISFP0pDxS7u5d3j3a+\n\tNuVMtl5Ho8EoNveKJ15CeoGcnXIN1JTvtRtBXN77gPhCNY1Nt/AZbkxs8LeTBWqCySIN\n\tkmeExj5sGd1siK9AnZ8dhOXgStBby+MX0HMMAA7ZiRxSjivGIgo6zYesvn794UKkRFml\n\t+r0/Nb/Lh1VKoo30+ThTO5wGItRD+jm3AUx7c1isqbIAif1geHLahAdO/YSapoIjsgs0\n\tPZAUknUSDWtCOryN+1yTDkHgOUAikr3trPCAe2PeNlaZNsPxEdvzd6+MfPbitHFzUBE1\n\tuvBw==","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=8GXbH/ze2xiGCllgktsf9Haj/9P6t8Do6+/q04Tx8eo=;\n\tb=DuoiTxIzO4rrNqNz82HWUpB2cBMrAqkKqEugJtrDK2xIAOQ614WzYYuuab9Vf77HzX\n\tl8Rx6bgHeS6wF8R3KPZvHO+/qyeHbo5QhszaIslE2cD5VLtRL5nd3KSjjsrhDnTHtxrE\n\tamb2eW5YaxD/83DXKVioqXtp9MO3RePIMi4JmxKsNchihIqqGbx0a8GAKt9pC5OpqnqQ\n\tCZD7s1vE/j6UZZrMtK+5/M4rgrMFT9NL9a3ScJe6HQAbHufbxnQO2/DgmWQQsAL6aEKD\n\tkJfYzSebwp75RRNZRNX7HgOOpfza32NFTYBuFHRcl3dV91jN1/ystL02cb/bKa6EFEJq\n\ts+rQ==","X-Gm-Message-State":"AOAM5313D0V0ZhYwdRuqkvNeAPkl2fsheUA+n8eOtbsaPvDkuAB7R9N1\n\to1ZCRE42DdiYR+w/qYUPRK26gVuMBVXzaFOgMlwoGQ==","X-Google-Smtp-Source":"ABdhPJyXdgtZ5EsllnZFESeL8KCTifoYyUzSAFMxWWpDBZCX9sLe9NNHimlyQ6LcEgeXMje3B90299qieAFfOo3rsr0=","X-Received":"by 2002:a05:600c:4f09:: with SMTP id\n\tl9mr26244495wmq.114.1624262566072; \n\tMon, 21 Jun 2021 01:02:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20210615144211.173047-1-naush@raspberrypi.com>\n\t<20210615144211.173047-2-naush@raspberrypi.com>","In-Reply-To":"<20210615144211.173047-2-naush@raspberrypi.com>","From":"David Plowman ","Date":"Mon, 21 Jun 2021 09:02:35 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/3] 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":17689,"web_url":"https://patchwork.libcamera.org/comment/17689/","msgid":"","date":"2021-06-22T10:20:54","subject":"Re: [libcamera-devel] [PATCH 1/3] 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\nThank you for the patch.\n\nOn Tue, Jun 15, 2021 at 03:42:09PM +0100, 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\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 1 -\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 +++-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +++-\n> 3 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4fef3..1474464c9257 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -48,7 +48,6 @@ CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n> \n> CamHelper::~CamHelper()\n> {\n> -\tdelete parser_;\n> }\n> \n> void CamHelper::Prepare(Span buffer,\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index ec218dce5456..d951cd552a21 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -54,11 +54,13 @@ private:\n> \t * in units of lines.\n> \t */\n> \tstatic constexpr int frameIntegrationDiff = 4;\n> +\n> +\tMdParserImx219 imx219_parser;\n> };\n> \n> CamHelperImx219::CamHelperImx219()\n> #if ENABLE_EMBEDDED_DATA\n> -\t: CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> +\t: CamHelper(&imx219_parser, frameIntegrationDiff)\n> #else\n> \t: CamHelper(nullptr, frameIntegrationDiff)\n> #endif\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 25b36bce0dac..44f030ed7da9 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -47,10 +47,12 @@ private:\n> \t * in units of lines.\n> \t */\n> \tstatic constexpr int frameIntegrationDiff = 22;\n> +\n> +\tMdParserImx477 imx477_parser;\n> };\n> \n> CamHelperImx477::CamHelperImx477()\n> -\t: CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> +\t: CamHelper(&imx477_parser, 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 755D1C321B\n\tfor ;\n\tTue, 22 Jun 2021 10:21:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B9EE968932;\n\tTue, 22 Jun 2021 12:21:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ECAD60292\n\tfor ;\n\tTue, 22 Jun 2021 12:21:23 +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 B8B98A66;\n\tTue, 22 Jun 2021 12:21:22 +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=\"taePuJNn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624357282;\n\tbh=oh8ctfJQQFoSDwQwMuHoCwLnpbgnvkaiarD1t3lQQy8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=taePuJNnHAAfJ7Ay+sjMkuAr9itqIOGlluvylkRmLsLYMyhoBCf/pVjkXNaKinC90\n\t+MwgHHoXkKURvYr+uO9iQkDgu+PFAUw4IyXCz8rd0jitRCInwW7BX2MPZ2sHDbTK3v\n\t5zRv640q3OX0g04QCV1NXqzqDPzwTUXxeWwVX34Q=","Date":"Tue, 22 Jun 2021 13:20:54 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210615144211.173047-1-naush@raspberrypi.com>\n\t<20210615144211.173047-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210615144211.173047-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] 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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]