[{"id":17699,"web_url":"https://patchwork.libcamera.org/comment/17699/","msgid":"","date":"2021-06-22T13:48:21","subject":"Re: [libcamera-devel] [PATCH v2 1/2] ipa: raspberrypi: Use a\n\tunique_ptr for the metadata parser","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 your work!\n\nOn Tue, 22 Jun 2021 at 14:20, Naushir Patuck wrote:\n>\n> The derived CamHelper class now allocates a metadata parser object through a\n> unique_ptr that is passed to the base class constructor. This automates the\n> lifetime management of the parser object.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 5 ++---\n> src/ipa/raspberrypi/cam_helper.hpp | 5 +++--\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 ++--\n> src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +-\n> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-\n> 6 files changed, 10 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4fef3..90498c37af98 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 &cam_name)\n> return nullptr;\n> }\n>\n> -CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n> - : parser_(parser), initialized_(false),\n> +CamHelper::CamHelper(std::unique_ptr parser, unsigned int frameIntegrationDiff)\n> + : parser_(std::move(parser)), initialized_(false),\n\nI can't say it doesn't feel ever so slightly strange to require a\nunique_ptr to be passed in, which may even be a \"null\" one, which then\ngets \"moved\" so that we take it over. But I really can't get too\nworked up about it, and for all I know it may even be better in the\nwonderful world of C++ so don't change it on my account! Therefore:\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\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 b/src/ipa/raspberrypi/cam_helper.hpp\n> index f53f5c39b01c..7371b9d97f66 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -6,6 +6,7 @@\n> */\n> #pragma once\n>\n> +#include \n> #include \n>\n> #include \n> @@ -67,7 +68,7 @@ class CamHelper\n> {\n> public:\n> static CamHelper *Create(std::string const &cam_name);\n> - CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> + CamHelper(std::unique_ptr parser, unsigned int frameIntegrationDiff);\n> virtual ~CamHelper();\n> void SetCameraMode(const CameraMode &mode);\n> virtual void Prepare(libcamera::Span buffer,\n> @@ -92,7 +93,7 @@ protected:\n> void parseEmbeddedData(libcamera::Span buffer,\n> Metadata &metadata);\n>\n> - MdParser *parser_;\n> + std::unique_ptr parser_;\n> CameraMode mode_;\n>\n> private:\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index ec218dce5456..c85044a5fa6d 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -58,9 +58,9 @@ private:\n>\n> CamHelperImx219::CamHelperImx219()\n> #if ENABLE_EMBEDDED_DATA\n> - : CamHelper(new MdParserImx219(), frameIntegrationDiff)\n> + : CamHelper(std::make_unique(), frameIntegrationDiff)\n> #else\n> - : CamHelper(nullptr, frameIntegrationDiff)\n> + : CamHelper({}, frameIntegrationDiff)\n> #endif\n> {\n> }\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp\n> index 6f412e403f16..871c1f8eaec4 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 b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 25b36bce0dac..d72a9be0438e 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -50,7 +50,7 @@ private:\n> };\n>\n> CamHelperImx477::CamHelperImx477()\n> - : CamHelper(new MdParserImx477(), frameIntegrationDiff)\n> + : CamHelper(std::make_unique(), frameIntegrationDiff)\n> {\n> }\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> index 12be6bf931a8..702c2d07f73a 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>","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 7F6C1C321A\n\tfor ;\n\tTue, 22 Jun 2021 13:48:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E33068932;\n\tTue, 22 Jun 2021 15:48:34 +0200 (CEST)","from mail-wm1-x332.google.com (mail-wm1-x332.google.com\n\t[IPv6:2a00:1450:4864:20::332])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D478460292\n\tfor ;\n\tTue, 22 Jun 2021 15:48:32 +0200 (CEST)","by mail-wm1-x332.google.com with SMTP id k42so10988686wms.0\n\tfor ;\n\tTue, 22 Jun 2021 06:48:32 -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=\"gwCcyqsX\"; 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=p3rebrOJR8ohTnbA6HAy4n2EKq6lMLhCUe3L6+QnXNQ=;\n\tb=gwCcyqsXd6qpfGJ5fUlJMEQf7r94Ym02fKJnNopNTSf4Vsi2GkKDj7zZCox9ajOH2G\n\tvEC56sUJSmo/TUE9f+C3hKA+/5oGkEBObZvrmORuMmKJghqB6fshfWxraAcQdYlke/6J\n\tc+/fLQoVlQqiqjYKJ+vBxoMLa37GjdxD9zc1jkuwBsxE9MwcUAYtivb/vIK9JvHQBBtn\n\tuc9k55Ro8HIUDc2E7BzftCR+pWz/EMqvl0ek6TlT1ZP+2C/l0nrUJMaXnC/35jQEMkFd\n\t9r1hfYHDTbLHJT10xlX3G8RcsJN5aBcwVF79NNms7sKT+j8ps+v788TceMybunHCzXwT\n\tqcKw==","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=p3rebrOJR8ohTnbA6HAy4n2EKq6lMLhCUe3L6+QnXNQ=;\n\tb=sS7HOzC+hykRxGUJCRY5Eg66PYyQxMJQ/z+0ZE04H/eghaUv9qQTjgDs/8MaJbnPHE\n\t4UB7v/ECyAmSi+ZF0VSBv1mZ62Ob2q+t1IDH4bi4NHWCObunSELadXmS3KzC+6azeqon\n\tfd2z1S+RcKHYG1+BVfMJ87cWCrQto1dlA772P5GA2JlaWz2I8UJnPs2r6fALPAoAxL6y\n\tFT6vOE25ZxYrevO8z5tD6CNrpGkT26S1I1iuc/VPvPT/iSqcqkouiXXKo/QPps1ZwI7d\n\tsLQXk97Opis+JtPgjACM/Wj1jvCrVTFVAl7qRMVW4JpXppnZWTnOsCd7cuOjm7h5ko3K\n\tSvbg==","X-Gm-Message-State":"AOAM5316/2blWYBz85GdpmL8vJa7dsvjDwhiR7peFb+z371ZRsWJXZn3\n\t4U9bpRYIOfBMzzXK8eIZcp3pUwp2ivDAgGBhXMBpRg==","X-Google-Smtp-Source":"ABdhPJzchL5uHMfETxWn3ax7aWTn6eC0Y167Eoku7OxhkYncPxo8tHAYrUZEpTiPp/Nst5J+qW8NnRGyulPBU1hzuAU=","X-Received":"by 2002:a7b:c76a:: with SMTP id\n\tx10mr4462119wmk.135.1624369712621; \n\tTue, 22 Jun 2021 06:48:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622132014.949961-1-naush@raspberrypi.com>\n\t<20210622132014.949961-2-naush@raspberrypi.com>","In-Reply-To":"<20210622132014.949961-2-naush@raspberrypi.com>","From":"David Plowman ","Date":"Tue, 22 Jun 2021 14:48:21 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] ipa: raspberrypi: Use a\n\tunique_ptr for the metadata parser","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\" "}}]