[{"id":30550,"web_url":"https://patchwork.libcamera.org/comment/30550/","msgid":"<20240802204212.GC3295@pendragon.ideasonboard.com>","date":"2024-08-02T20:42:12","subject":"Re: [PATCH v6 4/5] libcamera: converter: Add DW100 converter class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Jul 26, 2024 at 05:17:14PM +0530, Umang Jain wrote:\n> DW100 Dewarp engine is present on i.MX8MP SoC. This patch\n> provides a converter class (inherited from V4L2M2MConverter).\n> \n> The DW100 dewarp engine has scaling capabilites and can be used\n\nSounds like you should expose a scaling feature flag then, it could come\nhandy later.\n\n> to set crop rectangles in the rkisp1 pipeline handler. Plumbing\n> in the RkISP1 pipeline-handler is done in the subsequent patch.\n> \n> The ConverterDW100 class can be extended later (as additional\n> development) to support the dewarping by setting a dewarp vertex\n> map as well.\n> \n> Signed-off-by: Umang Jain \n> Reviewed-by: Paul Elder \n> ---\n> .../internal/converter/converter_dw100.h | 24 ++++++++++++\n> .../libcamera/internal/converter/meson.build | 1 +\n> src/libcamera/converter/converter_dw100.cpp | 37 +++++++++++++++++++\n> src/libcamera/converter/meson.build | 1 +\n> 4 files changed, 63 insertions(+)\n> create mode 100644 include/libcamera/internal/converter/converter_dw100.h\n> create mode 100644 src/libcamera/converter/converter_dw100.cpp\n> \n> diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h\n> new file mode 100644\n> index 00000000..dc41f365\n> --- /dev/null\n> +++ b/include/libcamera/internal/converter/converter_dw100.h\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * i.MX8MP Dewarp Engine integration\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> +\n> +namespace libcamera {\n> +\n> +class MediaDevice;\n> +class Rectangle;\n> +class Stream;\n\nThe last two are not needed.\n\n> +\n> +class ConverterDW100 : public V4L2M2MConverter\n> +{\n> +public:\n> +\tConverterDW100(std::shared_ptr media);\n\nWhy a shared pointer ?\n\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build\n> index 891e79e7..85007a4b 100644\n> --- a/include/libcamera/internal/converter/meson.build\n> +++ b/include/libcamera/internal/converter/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_headers += files([\n> + 'converter_dw100.h',\n> 'converter_v4l2_m2m.h',\n> ])\n> diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp\n> new file mode 100644\n> index 00000000..eb9906f0\n> --- /dev/null\n> +++ b/src/libcamera/converter/converter_dw100.cpp\n> @@ -0,0 +1,37 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * i.MX8MP Dewarp Engine integration\n> + */\n> +\n> +#include \"libcamera/internal/converter/converter_dw100.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Converter)\n> +\n> +/**\n> + * \\class libcamera::ConverterDW100\n> + * \\brief The i.MX8MP dewarp converter implements the converter interface based\n> + * on V4L2 M2M device.\n> +*/\n> +\n> +/**\n> + * \\fn ConverterDW100::ConverterDW100\n> + * \\brief Construct a ConverterDW100 instance\n> + * \\param[in] media The media device implementing the converter\n> + */\n> +ConverterDW100::ConverterDW100(std::shared_ptr media)\n> +\t: V4L2M2MConverter(media.get(), Feature::InputCrop)\n\nCan't you query support for the input crop feature in the\nV4L2M2MConverter class and avoid subclassing ? I know we should add\nsupport for dewarping later, I think the subclass should be introduced\nat that point. Dynamic discovery of features will be useful for other\nconverters.\n\n> +{\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build\n> index 2aa72fe4..175581b8 100644\n> --- a/src/libcamera/converter/meson.build\n> +++ b/src/libcamera/converter/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_sources += files([\n> + 'converter_dw100.cpp',\n> 'converter_v4l2_m2m.cpp'\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 CFEE5C323E\n\tfor ;\n\tFri, 2 Aug 2024 20:42:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECAF563384;\n\tFri, 2 Aug 2024 22:42:34 +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 2F5386336B\n\tfor ;\n\tFri, 2 Aug 2024 22:42:34 +0200 (CEST)","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 4DEEC2D5;\n\tFri, 2 Aug 2024 22:41: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=\"Kg3XOMpm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722631304;\n\tbh=zXrMVjecKHmC3o999X76sHHasP/SJNbdQUDUmx4m2Zc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kg3XOMpmWeW/cOR7s/p0VPxj+LQpR15Rok3yHtY6uavrP416+ye5kvFrw2miNstuW\n\tVOHVfjReXtneb9hISpX6jDCQL34j0caAAXm2xbFCEAPsaHXiG31m9XAWY5Eg0xmcOY\n\t2S4/ssuTp1yH12nFgs4w73wI5X6m5McpuEDUD/NE=","Date":"Fri, 2 Aug 2024 23:42:12 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ","Subject":"Re: [PATCH v6 4/5] libcamera: converter: Add DW100 converter class","Message-ID":"<20240802204212.GC3295@pendragon.ideasonboard.com>","References":"<20240726114715.226468-1-umang.jain@ideasonboard.com>\n\t<20240726114715.226468-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240726114715.226468-5-umang.jain@ideasonboard.com>","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\" "}}]