[{"id":30079,"web_url":"https://patchwork.libcamera.org/comment/30079/","msgid":"<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>","date":"2024-06-25T20:56:52","subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-06-25 07:23:26)\n> DW100 Dewarp engine is present on i.MX8MP SoC. This patch\n> provides a converter class (inherited from V4L2M2MConverter).\n> \n> The DW100 converter will be used for scaling capabilites hence, has\n> a helper to set scaler crop rectangles. Plumbing in the RkISP1\n> pipeline-handler is done in the subsequent patch.\n> \n> The ConverterDW100 class can be extended later (as additional\n> developement) to support the dewarping by setting a dewarp vertex map.\n> \n> Signed-off-by: Umang Jain \n> ---\n> .../internal/converter/converter_dw100.h | 26 +++++++++\n> .../libcamera/internal/converter/meson.build | 1 +\n> src/libcamera/converter/converter_dw100.cpp | 56 +++++++++++++++++++\n> src/libcamera/converter/meson.build | 1 +\n> 4 files changed, 84 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..ae6d0732\n> --- /dev/null\n> +++ b/include/libcamera/internal/converter/converter_dw100.h\n> @@ -0,0 +1,26 @@\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\ns/*i.MX8MP/* i.MX8MP/\n\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> +\n> +class ConverterDW100 : public V4L2M2MConverter\n> +{\n> +public:\n> + ConverterDW100(std::shared_ptr media);\n> +\n> + int setScalerCrop(const Stream *stream, Rectangle rect);\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..04e6942f\n> --- /dev/null\n> +++ b/src/libcamera/converter/converter_dw100.cpp\n> @@ -0,0 +1,56 @@\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> + : V4L2M2MConverter(media.get())\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Set the scaler crop rectangle \\a rect on \\a stream\n> + * \\param[in] stream Pointer to output stream\n> + * \\param[inout] rect The selection rectangle to be applied\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ConverterDW100::setScalerCrop(const Stream *stream, Rectangle rect)\n> +{\n> + int ret;\n> +\n> + ret = setSelection(stream, V4L2_SEL_TGT_CROP, &rect);\n> + if (ret < 0)\n> + LOG(Converter, Error) << \"Failed to set scaler crop on dewarper \"\n> + << strerror(-ret);\n> +\n\nDoes calling setSelection modify the rect? Do we need to check that at\nall or is it up to the caller to verify?\n\n> + return ret;\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> ])\n> -- \n> 2.44.0\n> \n\nOh - wow, that's a small implementation! I guess that's the benefit of\ninheriting from M2MConvertor.\n\nThat makes me wonder if ScalerCrop should be handled in the\nV4L2M2MConvertor generically instead ... but ... as we expect this class\nto be expanded for the dewarp, I think it's fine to be specialised here.\n\nReviewed-by: Kieran Bingham ","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 72AEFBDB1D\n\tfor ;\n\tTue, 25 Jun 2024 20:56:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 97F92654A9;\n\tTue, 25 Jun 2024 22:56:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE723654A1\n\tfor ;\n\tTue, 25 Jun 2024 22:56:55 +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 E64CD7E0;\n\tTue, 25 Jun 2024 22:56:32 +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=\"ez1FSwY+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719348993;\n\tbh=0ckWJmWeFAPVK3DPLehPQJhnceBgFiFNup9GYV4iBeQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ez1FSwY+oalkAwge3q7KrqdyhkUFFrX5+lnjUPZGRQc10KAf25fDUUTCuCv9Y9M8V\n\t0hV2PhkM5Qw8R2tbi879GDex+CV8lP2SBqtLpAoo8PG0SKIuM6k/I5Dazq/gRrbiEv\n\tsexDm6P/FNL2p7V6xDqjVCZ0U7jX+AHeDn3vRYi0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240625062327.50940-4-umang.jain@ideasonboard.com>","References":"<20240625062327.50940-1-umang.jain@ideasonboard.com>\n\t<20240625062327.50940-4-umang.jain@ideasonboard.com>","Subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","From":"Kieran Bingham ","Cc":"Umang Jain ","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 25 Jun 2024 21:56:52 +0100","Message-ID":"<171934901275.739659.17444278821138209102@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":"","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":30083,"web_url":"https://patchwork.libcamera.org/comment/30083/","msgid":"<3903ab6b-d18b-42e3-b90e-2a94d67c552a@ideasonboard.com>","date":"2024-06-26T04:04:09","subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 26/06/24 2:26 am, Kieran Bingham wrote:\n> Quoting Umang Jain (2024-06-25 07:23:26)\n>> DW100 Dewarp engine is present on i.MX8MP SoC. This patch\n>> provides a converter class (inherited from V4L2M2MConverter).\n>>\n>> The DW100 converter will be used for scaling capabilites hence, has\n>> a helper to set scaler crop rectangles. Plumbing in the RkISP1\n>> pipeline-handler is done in the subsequent patch.\n>>\n>> The ConverterDW100 class can be extended later (as additional\n>> developement) to support the dewarping by setting a dewarp vertex map.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> .../internal/converter/converter_dw100.h | 26 +++++++++\n>> .../libcamera/internal/converter/meson.build | 1 +\n>> src/libcamera/converter/converter_dw100.cpp | 56 +++++++++++++++++++\n>> src/libcamera/converter/meson.build | 1 +\n>> 4 files changed, 84 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..ae6d0732\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/converter/converter_dw100.h\n>> @@ -0,0 +1,26 @@\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> s/*i.MX8MP/* i.MX8MP/\n>\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>> +\n>> +class ConverterDW100 : public V4L2M2MConverter\n>> +{\n>> +public:\n>> + ConverterDW100(std::shared_ptr media);\n>> +\n>> + int setScalerCrop(const Stream *stream, Rectangle rect);\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..04e6942f\n>> --- /dev/null\n>> +++ b/src/libcamera/converter/converter_dw100.cpp\n>> @@ -0,0 +1,56 @@\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>> + : V4L2M2MConverter(media.get())\n>> +{\n>> +}\n>> +\n>> +/**\n>> + * \\brief Set the scaler crop rectangle \\a rect on \\a stream\n>> + * \\param[in] stream Pointer to output stream\n>> + * \\param[inout] rect The selection rectangle to be applied\n>> + *\n>> + * \\return 0 on success or a negative error code otherwise\n>> + */\n>> +int ConverterDW100::setScalerCrop(const Stream *stream, Rectangle rect)\n>> +{\n>> + int ret;\n>> +\n>> + ret = setSelection(stream, V4L2_SEL_TGT_CROP, &rect);\n>> + if (ret < 0)\n>> + LOG(Converter, Error) << \"Failed to set scaler crop on dewarper \"\n>> + << strerror(-ret);\n>> +\n> Does calling setSelection modify the rect? Do we need to check that at\n> all or is it up to the caller to verify?\n\nI am not sure here to be honest, If the selection rectangles get \nmodified while applying, it's probably the caller's job to verify it.\n\nLooking up the V4L2_SEL_FLAG_* flags, it seems there are ways to control \nthe modification / constraints of the rectangles that get applied. So \nyes, a verification mechanism of what gets applied vs what was \nrequested, needs to be present. If the rectangle is modified, we need to \nalso look at  what are the  acceptable % of margins which we should \nallow - or print a error/warning otherwise?\n\n>> + return ret;\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>> ])\n>> -- \n>> 2.44.0\n>>\n> Oh - wow, that's a small implementation! I guess that's the benefit of\n> inheriting from M2MConvertor.\n>\n> That makes me wonder if ScalerCrop should be handled in the\n> V4L2M2MConvertor generically instead ... but ... as we expect this class\n> to be expanded for the dewarp, I think it's fine to be specialised here.\n>\n> Reviewed-by: Kieran Bingham ","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 B2673BD87C\n\tfor ;\n\tWed, 26 Jun 2024 04:04:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC2F4654A3;\n\tWed, 26 Jun 2024 06:04:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69588619CC\n\tfor ;\n\tWed, 26 Jun 2024 06:04:14 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E10F34CF;\n\tWed, 26 Jun 2024 06:03:50 +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=\"WCb3lX5z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719374631;\n\tbh=L7hRY/2tqN1nXNvoV4rjLtBWn/9i+zfFfFDdBV0goNQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=WCb3lX5zwmIiI6Zk+mNU+MP4Je0CoKBB+EvhdZCrLm5umROe9aCF+elB/x8sP4MNQ\n\tnuA867W/q7HpWtEzJ0rtiWryj+u6b97C8OXatqLMuoBWExHUDAcRFtI2GSO2vJcBxK\n\t3Z7UDvitgeRefHAhos7iFjlWUIbsdUZEfI5LTlxM=","Message-ID":"<3903ab6b-d18b-42e3-b90e-2a94d67c552a@ideasonboard.com>","Date":"Wed, 26 Jun 2024 09:34:09 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240625062327.50940-1-umang.jain@ideasonboard.com>\n\t<20240625062327.50940-4-umang.jain@ideasonboard.com>\n\t<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>","From":"Umang Jain ","In-Reply-To":"<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":30088,"web_url":"https://patchwork.libcamera.org/comment/30088/","msgid":"<20240626074103.GE10453@pendragon.ideasonboard.com>","date":"2024-06-26T07:41:03","subject":"Re: [PATCH v3 3/4] 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":"On Tue, Jun 25, 2024 at 09:56:52PM +0100, Kieran Bingham wrote:\n> Quoting Umang Jain (2024-06-25 07:23:26)\n> > DW100 Dewarp engine is present on i.MX8MP SoC. This patch\n> > provides a converter class (inherited from V4L2M2MConverter).\n> > \n> > The DW100 converter will be used for scaling capabilites hence, has\n> > a helper to set scaler crop rectangles. Plumbing in the RkISP1\n> > pipeline-handler is done in the subsequent patch.\n> > \n> > The ConverterDW100 class can be extended later (as additional\n> > developement) to support the dewarping by setting a dewarp vertex map.\n> > \n> > Signed-off-by: Umang Jain \n> > ---\n> > .../internal/converter/converter_dw100.h | 26 +++++++++\n> > .../libcamera/internal/converter/meson.build | 1 +\n> > src/libcamera/converter/converter_dw100.cpp | 56 +++++++++++++++++++\n> > src/libcamera/converter/meson.build | 1 +\n> > 4 files changed, 84 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..ae6d0732\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/converter/converter_dw100.h\n> > @@ -0,0 +1,26 @@\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> s/*i.MX8MP/* i.MX8MP/\n> \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> > +\n> > +class ConverterDW100 : public V4L2M2MConverter\n> > +{\n> > +public:\n> > + ConverterDW100(std::shared_ptr media);\n> > +\n> > + int setScalerCrop(const Stream *stream, Rectangle rect);\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..04e6942f\n> > --- /dev/null\n> > +++ b/src/libcamera/converter/converter_dw100.cpp\n> > @@ -0,0 +1,56 @@\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> > + : V4L2M2MConverter(media.get())\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Set the scaler crop rectangle \\a rect on \\a stream\n> > + * \\param[in] stream Pointer to output stream\n> > + * \\param[inout] rect The selection rectangle to be applied\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int ConverterDW100::setScalerCrop(const Stream *stream, Rectangle rect)\n> > +{\n> > + int ret;\n> > +\n> > + ret = setSelection(stream, V4L2_SEL_TGT_CROP, &rect);\n> > + if (ret < 0)\n> > + LOG(Converter, Error) << \"Failed to set scaler crop on dewarper \"\n> > + << strerror(-ret);\n> > +\n> \n> Does calling setSelection modify the rect? Do we need to check that at\n> all or is it up to the caller to verify?\n> \n> > + return ret;\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> > ])\n> > -- \n> > 2.44.0\n> > \n> \n> Oh - wow, that's a small implementation! I guess that's the benefit of\n> inheriting from M2MConvertor.\n> \n> That makes me wonder if ScalerCrop should be handled in the\n> V4L2M2MConvertor generically instead\n\nSee the reply I just sent for 2/4 :-)\n\n> ... but ... as we expect this class\n> to be expanded for the dewarp, I think it's fine to be specialised here.\n> \n> Reviewed-by: Kieran Bingham ","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 DC567BD87C\n\tfor ;\n\tWed, 26 Jun 2024 07:41:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D61E4654AD;\n\tWed, 26 Jun 2024 09:41:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45F2C654A8\n\tfor ;\n\tWed, 26 Jun 2024 09:41:25 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3A61FD6;\n\tWed, 26 Jun 2024 09:41:01 +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=\"JpZEDVxb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719387662;\n\tbh=YaGLLoCWvPpywNh8ryVYdzuxn237jCtAurGAFh6ekrM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JpZEDVxbMWwur7Rq8P+LcipdFGWAWU4J4N+7Wmo7yw26AOqTfqX3ONHpYG6aOtGOC\n\tKjgYA4R6UG8KJtVuQBFKSiVVYmrDjYRHWpTeWoKfDEaVzmFglnXGx3qrkWuS0E18RL\n\tH+o4hgZmRzhYQElaYV9a39i7Ju7jcHlCUzSj8MsU=","Date":"Wed, 26 Jun 2024 10:41:03 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","Message-ID":"<20240626074103.GE10453@pendragon.ideasonboard.com>","References":"<20240625062327.50940-1-umang.jain@ideasonboard.com>\n\t<20240625062327.50940-4-umang.jain@ideasonboard.com>\n\t<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>","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":30089,"web_url":"https://patchwork.libcamera.org/comment/30089/","msgid":"<20240626082858.GF10453@pendragon.ideasonboard.com>","date":"2024-06-26T08:28:58","subject":"Re: [PATCH v3 3/4] 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\nOn Wed, Jun 26, 2024 at 09:34:09AM +0530, Umang Jain wrote:\n> On 26/06/24 2:26 am, Kieran Bingham wrote:\n> > Quoting Umang Jain (2024-06-25 07:23:26)\n> >> DW100 Dewarp engine is present on i.MX8MP SoC. This patch\n> >> provides a converter class (inherited from V4L2M2MConverter).\n> >>\n> >> The DW100 converter will be used for scaling capabilites hence, has\n> >> a helper to set scaler crop rectangles. Plumbing in the RkISP1\n> >> pipeline-handler is done in the subsequent patch.\n> >>\n> >> The ConverterDW100 class can be extended later (as additional\n> >> developement) to support the dewarping by setting a dewarp vertex map.\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> .../internal/converter/converter_dw100.h | 26 +++++++++\n> >> .../libcamera/internal/converter/meson.build | 1 +\n> >> src/libcamera/converter/converter_dw100.cpp | 56 +++++++++++++++++++\n> >> src/libcamera/converter/meson.build | 1 +\n> >> 4 files changed, 84 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..ae6d0732\n> >> --- /dev/null\n> >> +++ b/include/libcamera/internal/converter/converter_dw100.h\n> >> @@ -0,0 +1,26 @@\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> > s/*i.MX8MP/* i.MX8MP/\n> >\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> >> +\n> >> +class ConverterDW100 : public V4L2M2MConverter\n> >> +{\n> >> +public:\n> >> + ConverterDW100(std::shared_ptr media);\n> >> +\n> >> + int setScalerCrop(const Stream *stream, Rectangle rect);\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..04e6942f\n> >> --- /dev/null\n> >> +++ b/src/libcamera/converter/converter_dw100.cpp\n> >> @@ -0,0 +1,56 @@\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> >> + : V4L2M2MConverter(media.get())\n> >> +{\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Set the scaler crop rectangle \\a rect on \\a stream\n> >> + * \\param[in] stream Pointer to output stream\n> >> + * \\param[inout] rect The selection rectangle to be applied\n> >> + *\n> >> + * \\return 0 on success or a negative error code otherwise\n> >> + */\n> >> +int ConverterDW100::setScalerCrop(const Stream *stream, Rectangle rect)\n> >> +{\n> >> + int ret;\n> >> +\n> >> + ret = setSelection(stream, V4L2_SEL_TGT_CROP, &rect);\n> >> + if (ret < 0)\n> >> + LOG(Converter, Error) << \"Failed to set scaler crop on dewarper \"\n> >> + << strerror(-ret);\n> >> +\n> >\n> > Does calling setSelection modify the rect? Do we need to check that at\n> > all or is it up to the caller to verify?\n> \n> I am not sure here to be honest, If the selection rectangles get \n> modified while applying, it's probably the caller's job to verify it.\n\nYou're passing the rectangle by value to setScalerCrop(), so the caller\ncan't check that. Unless we mandate the converter to support arbitrary\ncropping with 1-pixel granularity (and I think that's an option we can\nconsider), this is problematic.\n\nWhen it comes to crop bounds (and thus scaling ratio bounds), I think\nthey need to be reported explicitly by the converter.\n\n> Looking up the V4L2_SEL_FLAG_* flags, it seems there are ways to control \n> the modification / constraints of the rectangles that get applied. So \n> yes, a verification mechanism of what gets applied vs what was \n> requested, needs to be present. If the rectangle is modified, we need to \n> also look at  what are the  acceptable % of margins which we should \n> allow - or print a error/warning otherwise?\n\nI don't think just printing a message will solve the problem.\n\n> >> + return ret;\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> >> ])\n> >\n> > Oh - wow, that's a small implementation! I guess that's the benefit of\n> > inheriting from M2MConvertor.\n> >\n> > That makes me wonder if ScalerCrop should be handled in the\n> > V4L2M2MConvertor generically instead ... but ... as we expect this class\n> > to be expanded for the dewarp, I think it's fine to be specialised here.\n> >\n> > Reviewed-by: Kieran Bingham ","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 13B58BDB1D\n\tfor ;\n\tWed, 26 Jun 2024 08:29:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D29A654A9;\n\tWed, 26 Jun 2024 10:29:21 +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 9ED37619E5\n\tfor ;\n\tWed, 26 Jun 2024 10:29:19 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A66E2C5;\n\tWed, 26 Jun 2024 10:28:56 +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=\"GzUjG6AY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719390536;\n\tbh=8ucXBX+k1AHShWWDIN/kmOf88o9FJnLEPgZ5LjF6s44=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GzUjG6AY5P8SjhS60vQFKlP00oEIXXKMBx8BCVssbpnc2eMWB7auiTIXk+D3Xhmyc\n\tZAS/Llt2A1oGusA1y7Nh8NXFdTReZc7k3gfxXc0+d3y/A/nO2anc4SmSBp31DKa9Ag\n\t7g7Htxgr5EDnSteNRW6UMli+BvJIRcfZILREjE7o=","Date":"Wed, 26 Jun 2024 11:28:58 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 3/4] libcamera: converter: Add DW100 converter class","Message-ID":"<20240626082858.GF10453@pendragon.ideasonboard.com>","References":"<20240625062327.50940-1-umang.jain@ideasonboard.com>\n\t<20240625062327.50940-4-umang.jain@ideasonboard.com>\n\t<171934901275.739659.17444278821138209102@ping.linuxembedded.co.uk>\n\t<3903ab6b-d18b-42e3-b90e-2a94d67c552a@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<3903ab6b-d18b-42e3-b90e-2a94d67c552a@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\" "}}]