[{"id":32683,"web_url":"https://patchwork.libcamera.org/comment/32683/","msgid":"","date":"2024-12-11T16:55:22","subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch :-)\n\nOn Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:\n> From: Jacopo Mondi \n> \n> Add to the Converter interface two functions used by pipeline handlers\n> to validate and adjust the converter input and output configurations\n> by specifying the desired alignment for the adjustment.\n> \n> Add the adjustInputSize() and adjustOutputSize() functions that allows\n> to adjust the converter input/output sizes with the desired alignment.\n> \n> Add a validateOutput() function meant to be used by the pipeline\n> handler implementations of validate(). The function adjusts a\n> StreamConfiguration to a valid configuration produced by the Converter.\n> \n> Signed-off-by: Jacopo Mondi \n> \n> ---\n> Changes in v3:\n> - Added this patch\n> ---\n> include/libcamera/internal/converter.h | 17 ++\n> .../internal/converter/converter_v4l2_m2m.h | 11 ++\n> src/libcamera/converter.cpp | 43 +++++\n> .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++\n> 4 files changed, 240 insertions(+)\n> \n> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> index ffbb6f345cd5..9213ae5b9c33 100644\n> --- a/include/libcamera/internal/converter.h\n> +++ b/include/libcamera/internal/converter.h\n> @@ -41,6 +41,13 @@ public:\n> \n> \tusing Features = Flags;\n> \n> +\tenum class Alignment {\n> +\t\tDown = 0,\n> +\t\tUp = (1 << 0),\n> +\t};\n> +\n> +\tusing Alignments = Flags;\n> +\n> \tConverter(MediaDevice *media, Features features = Feature::None);\n> \tvirtual ~Converter();\n> \n> @@ -51,9 +58,19 @@ public:\n> \tvirtual std::vector formats(PixelFormat input) = 0;\n> \tvirtual SizeRange sizes(const Size &input) = 0;\n> \n> +\tvirtual Size adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> +\tvirtual Size adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> +\n> \tvirtual std::tuple\n> \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> \n> +\tvirtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> +\n\nI'm wondering when to use non-const references and when a pointer. Is\nthere a reason cfg is a pointer?\n\n> \tvirtual int configure(const StreamConfiguration &inputCfg,\n> \t\t\t const std::vector> &outputCfgs) = 0;\n> \tvirtual int exportBuffers(const Stream *stream, unsigned int count,\n> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> index a5286166f574..89bd2878b190 100644\n> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> @@ -47,6 +47,11 @@ public:\n> \tstd::tuple\n> \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> \n> +\tSize adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> +\tSize adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> +\n> \tint configure(const StreamConfiguration &inputCfg,\n> \t\t const std::vector> &outputCfg);\n> \tint exportBuffers(const Stream *stream, unsigned int count,\n> @@ -55,6 +60,9 @@ public:\n> \tint start();\n> \tvoid stop();\n> \n> +\tint validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t Alignments align = Alignment::Down) override;\n> +\n> \tint queueBuffers(FrameBuffer *input,\n> \t\t\t const std::map &outputs);\n> \n> @@ -101,6 +109,9 @@ private:\n> \t\tstd::pair inputCropBounds_;\n> \t};\n> \n> +\tSize adjustSizes(const Size &size, const std::vector &ranges,\n> +\t\t\t Alignments align);\n> +\n> \tstd::unique_ptr m2m_;\n> \n> \tstd::map> streams_;\n> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> index aa16970cd446..c3da162b7de7 100644\n> --- a/src/libcamera/converter.cpp\n> +++ b/src/libcamera/converter.cpp\n> @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)\n> * \\brief A bitwise combination of features supported by the converter\n> */\n> \n> +/**\n> + * \\enum Converter::Alignment\n> + * \\brief The alignment mode specified when adjusting the converter input or\n> + * output sizes\n> + * \\var Converter::Alignment::Down\n> + * \\brief Adjust the Converter sizes to a smaller valid size\n> + * \\var Converter::Alignment::Up\n> + * \\brief Adjust the Converter sizes to a larger valid size\n> + */\n> +\n> +/**\n> + * \\typedef Converter::Alignments\n> + * \\brief A bitwise combination of alignments supported by the converter\n> + */\n> +\n> /**\n> * \\brief Construct a Converter instance\n> * \\param[in] media The media device implementing the converter\n> @@ -110,6 +125,24 @@ Converter::~Converter()\n> * \\return A range of output image sizes\n> */\n> \n> +/**\n> + * \\fn Converter::adjustInputSize()\n> + * \\brief Adjust the converter input \\a size to a valid value\n> + * \\param[in] pixFmt The pixel format of the converter input stream\n> + * \\param[in] size The converter input size to adjust to a valid value\n> + * \\param[in] align The desired alignment\n> + * \\return The adjusted converter input size\n> + */\n\nWhat gets returned if there is no valid side for the request?\n\nMy comments where all nits. So\n\nReviewed-by: Stefan Klug \n\n> +\n> +/**\n> + * \\fn Converter::adjustOutputSize()\n> + * \\brief Adjust the converter output \\a size to a valid value\n> + * \\param[in] pixFmt The pixel format of the converter output stream\n> + * \\param[in] size The converter output size to adjust to a valid value\n> + * \\param[in] align The desired alignment\n> + * \\return The adjusted converter output size\n> + */\n> +\n> /**\n> * \\fn Converter::strideAndFrameSize()\n> * \\brief Retrieve the output stride and frame size for an input configutation\n> @@ -118,6 +151,16 @@ Converter::~Converter()\n> * \\return A tuple indicating the stride and frame size or an empty tuple on error\n> */\n> \n> +/**\n> + * \\fn Converter::validateOutput()\n> + * \\brief Validate and possibily adjust \\a cfg to a valid converter output\n> + * \\param[inout] cfg The StreamConfiguration to validate and adjust\n> + * \\param[out] adjusted Set to true if \\a cfg has been adjusted\n> + * \\param[in] align The desired alignment\n> + * \\return 0 if \\a cfg is valid or has been adjusted, a negative error code\n> + * otherwise if \\a cfg cannot be adjusted\n> + */\n> +\n> /**\n> * \\fn Converter::configure()\n> * \\brief Configure a set of output stream conversion from an input stream\n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index bd7e5cce600d..6857472b29f2 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -8,6 +8,7 @@\n> \n> #include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> \treturn std::make_tuple(format.planes[0].bpl, format.planes[0].size);\n> }\n> \n> +/**\n> + * \\copydoc libcamera::Converter::adjustInputSize\n> + */\n> +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size, Alignments align)\n> +{\n> +\tauto formats = m2m_->output()->formats();\n> +\tV4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);\n> +\n> +\tauto it = formats.find(v4l2PixFmt);\n> +\tif (it == formats.end()) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn adjustSizes(size, it->second, align);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::Converter::adjustOutputSize\n> + */\n> +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t\tconst Size &size, Alignments align)\n> +{\n> +\tauto formats = m2m_->capture()->formats();\n> +\tV4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);\n> +\n> +\tauto it = formats.find(v4l2PixFmt);\n> +\tif (it == formats.end()) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn adjustSizes(size, it->second, align);\n> +}\n> +\n> +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,\n> +\t\t\t\t const std::vector &ranges,\n> +\t\t\t\t Alignments align)\n> +{\n> +\tSize size = cfgSize;\n> +\n> +\tif (ranges.size() == 1) {\n> +\t\t/*\n> +\t\t * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or\n> +\t\t * V4L2_FRMSIZE_TYPE_STEPWISE.\n> +\t\t */\n> +\t\tconst SizeRange &range = *ranges.begin();\n> +\n> +\t\tsize.width = std::clamp(size.width, range.min.width,\n> +\t\t\t\t\trange.max.width);\n> +\t\tsize.height = std::clamp(size.height, range.min.height,\n> +\t\t\t\t\t range.max.height);\n> +\n> +\t\t/*\n> +\t\t * Check if any alignment is needed. If the sizes are already\n> +\t\t * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS\n> +\t\t * with hStep and vStep equal to 1, we're done here.\n> +\t\t */\n> +\t\tint widthR = size.width % range.hStep;\n> +\t\tint heightR = size.height % range.vStep;\n> +\n> +\t\t/* Align up or down according to the caller request. */\n> +\n> +\t\tif (widthR != 0)\n> +\t\t\tsize.width = size.width - widthR\n> +\t\t\t\t + ((align == Alignment::Up) ? range.hStep : 0);\n> +\n> +\t\tif (heightR != 0)\n> +\t\t\tsize.height = size.height - heightR\n> +\t\t\t\t + ((align == Alignment::Up) ? range.vStep : 0);\n> +\t} else {\n> +\t\t/*\n> +\t\t * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the\n> +\t\t * size closer to the requested output configuration.\n> +\t\t *\n> +\t\t * The size ranges vector is not ordered, so we sort it first.\n> +\t\t * If we align up, start from the larger element.\n> +\t\t */\n> +\t\tstd::vector sizes(ranges.size());\n> +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),\n> +\t\t\t [](const SizeRange &range) { return range.max; });\n> +\t\tstd::sort(sizes.begin(), sizes.end());\n> +\n> +\t\tif (align == Alignment::Up)\n> +\t\t\tstd::reverse(sizes.begin(), sizes.end());\n> +\n> +\t\t/*\n> +\t\t * Return true if s2 is valid according to the desired\n> +\t\t * alignment: smaller than s1 if we align down, larger than s1\n> +\t\t * if we align up.\n> +\t\t */\n> +\t\tauto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {\n> +\t\t\treturn a == Alignment::Down\n> +\t\t\t\t? (s1.width > s2.width && s1.height > s2.height)\n> +\t\t\t\t: (s1.width < s2.width && s1.height < s2.height);\n> +\t\t};\n> +\n> +\t\tSize newSize;\n> +\t\tfor (const Size &sz : sizes) {\n> +\t\t\tif (!nextSizeValid(size, sz, align))\n> +\t\t\t\tbreak;\n> +\n> +\t\t\tnewSize = sz;\n> +\t\t}\n> +\n> +\t\tif (newSize.isNull()) {\n> +\t\t\tLOG(Converter, Error)\n> +\t\t\t\t<< \"Cannot adjust \" << cfgSize\n> +\t\t\t\t<< \" to a supported converter size\";\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tsize = newSize;\n> +\t}\n> +\n> +\treturn size;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::Converter::configure\n> */\n> @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()\n> \t\titer.second->stop();\n> }\n> \n> +/**\n> + * \\copydoc libcamera::Converter::validateOutput\n> + */\n> +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t\t Alignments align)\n> +{\n> +\tV4L2VideoDevice *capture = m2m_->capture();\n> +\tV4L2VideoDevice::Formats fmts = capture->formats();\n> +\n> +\tif (adjusted)\n> +\t\t*adjusted = false;\n> +\n> +\tPixelFormat fmt = cfg->pixelFormat;\n> +\tV4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);\n> +\n> +\tauto it = fmts.find(v4l2PixFmt);\n> +\tif (it == fmts.end()) {\n> +\t\tit = fmts.begin();\n> +\t\tv4l2PixFmt = it->first;\n> +\t\tcfg->pixelFormat = v4l2PixFmt.toPixelFormat();\n> +\n> +\t\tif (adjusted)\n> +\t\t\t*adjusted = true;\n> +\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Converter output pixel format adjusted to \"\n> +\t\t\t<< cfg->pixelFormat;\n> +\t}\n> +\n> +\tconst Size cfgSize = cfg->size;\n> +\tcfg->size = adjustSizes(cfgSize, it->second, align);\n> +\n> +\tif (cfg->size.isNull())\n> +\t\treturn -EINVAL;\n> +\n> +\tif (cfg->size.width != cfgSize.width ||\n> +\t cfg->size.height != cfgSize.height) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Converter size adjusted to \"\n> +\t\t\t<< cfg->size;\n> +\t\tif (adjusted)\n> +\t\t\t*adjusted = true;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::Converter::queueBuffers\n> */\n> -- \n> 2.43.0\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 6841CC32DD\n\tfor ;\n\tWed, 11 Dec 2024 16:55:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 905A967EBB;\n\tWed, 11 Dec 2024 17:55:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C7DD67EAF\n\tfor ;\n\tWed, 11 Dec 2024 17:55:25 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:af9:9d8e:d17e:723])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3509621C;\n\tWed, 11 Dec 2024 17:54:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ffGu28x0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733936092;\n\tbh=S7vHTuR7R7XzH/QNxqxN9Kg6bhypc7mv36yLem1BxJk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ffGu28x0m7S0t4D2DUIhPeLszsu2EIGCISecx5itMChzzvHVds72tYY12WYyhgDrV\n\t+p3LPJtielC5ErGIdv0++RgfzLX60SYXXwpJrSQxsxUk/FuYsIC4dE+nyMc6CMhd0b\n\tc8Lf9qBRYcslmhn/K98x2o2ES3TlAglbQUOXSfIM=","Date":"Wed, 11 Dec 2024 17:55:22 +0100","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org","Cc":"Jacopo Mondi ","Subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","Message-ID":"","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-10-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241206101344.767170-10-stefan.klug@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\" "}},{"id":32697,"web_url":"https://patchwork.libcamera.org/comment/32697/","msgid":"","date":"2024-12-12T11:43:15","subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:\n> From: Jacopo Mondi \n> \n> Add to the Converter interface two functions used by pipeline handlers\n> to validate and adjust the converter input and output configurations\n> by specifying the desired alignment for the adjustment.\n> \n> Add the adjustInputSize() and adjustOutputSize() functions that allows\n> to adjust the converter input/output sizes with the desired alignment.\n> \n> Add a validateOutput() function meant to be used by the pipeline\n> handler implementations of validate(). The function adjusts a\n> StreamConfiguration to a valid configuration produced by the Converter.\n> \n> Signed-off-by: Jacopo Mondi \n> \n> ---\n> Changes in v3:\n> - Added this patch\n> ---\n> include/libcamera/internal/converter.h | 17 ++\n> .../internal/converter/converter_v4l2_m2m.h | 11 ++\n> src/libcamera/converter.cpp | 43 +++++\n> .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++\n> 4 files changed, 240 insertions(+)\n> \n> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> index ffbb6f345cd5..9213ae5b9c33 100644\n> --- a/include/libcamera/internal/converter.h\n> +++ b/include/libcamera/internal/converter.h\n> @@ -41,6 +41,13 @@ public:\n> \n> \tusing Features = Flags;\n> \n> +\tenum class Alignment {\n> +\t\tDown = 0,\n> +\t\tUp = (1 << 0),\n> +\t};\n> +\n> +\tusing Alignments = Flags;\n> +\n> \tConverter(MediaDevice *media, Features features = Feature::None);\n> \tvirtual ~Converter();\n> \n> @@ -51,9 +58,19 @@ public:\n> \tvirtual std::vector formats(PixelFormat input) = 0;\n> \tvirtual SizeRange sizes(const Size &input) = 0;\n> \n> +\tvirtual Size adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> +\tvirtual Size adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> +\n> \tvirtual std::tuple\n> \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> \n> +\tvirtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t\t Alignments align = Alignment::Down) = 0;\n\nI'm confused. Here (and everywhere else it's used), since the type is\nAlignments (Flags), doesn't that mean that\nAlignment::Down | Alignment::Up is a valid input? Wouldn't that cause\nunexpected behavior?\n\nAlthough I suppose if Alignment::Down | Alignment::Up was actually\npassed then it wouldn't be equal Alignment::Down nor Alignment::Up so\nnothing would happen? Except maybe the ternary operators ig, those would\njust activate the false clause.\n\nAm I missing something here?\n\n\nPaul\n\n\n> +\n> \tvirtual int configure(const StreamConfiguration &inputCfg,\n> \t\t\t const std::vector> &outputCfgs) = 0;\n> \tvirtual int exportBuffers(const Stream *stream, unsigned int count,\n> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> index a5286166f574..89bd2878b190 100644\n> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> @@ -47,6 +47,11 @@ public:\n> \tstd::tuple\n> \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> \n> +\tSize adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> +\tSize adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> +\n> \tint configure(const StreamConfiguration &inputCfg,\n> \t\t const std::vector> &outputCfg);\n> \tint exportBuffers(const Stream *stream, unsigned int count,\n> @@ -55,6 +60,9 @@ public:\n> \tint start();\n> \tvoid stop();\n> \n> +\tint validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t Alignments align = Alignment::Down) override;\n> +\n> \tint queueBuffers(FrameBuffer *input,\n> \t\t\t const std::map &outputs);\n> \n> @@ -101,6 +109,9 @@ private:\n> \t\tstd::pair inputCropBounds_;\n> \t};\n> \n> +\tSize adjustSizes(const Size &size, const std::vector &ranges,\n> +\t\t\t Alignments align);\n> +\n> \tstd::unique_ptr m2m_;\n> \n> \tstd::map> streams_;\n> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> index aa16970cd446..c3da162b7de7 100644\n> --- a/src/libcamera/converter.cpp\n> +++ b/src/libcamera/converter.cpp\n> @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)\n> * \\brief A bitwise combination of features supported by the converter\n> */\n> \n> +/**\n> + * \\enum Converter::Alignment\n> + * \\brief The alignment mode specified when adjusting the converter input or\n> + * output sizes\n> + * \\var Converter::Alignment::Down\n> + * \\brief Adjust the Converter sizes to a smaller valid size\n> + * \\var Converter::Alignment::Up\n> + * \\brief Adjust the Converter sizes to a larger valid size\n> + */\n> +\n> +/**\n> + * \\typedef Converter::Alignments\n> + * \\brief A bitwise combination of alignments supported by the converter\n> + */\n> +\n> /**\n> * \\brief Construct a Converter instance\n> * \\param[in] media The media device implementing the converter\n> @@ -110,6 +125,24 @@ Converter::~Converter()\n> * \\return A range of output image sizes\n> */\n> \n> +/**\n> + * \\fn Converter::adjustInputSize()\n> + * \\brief Adjust the converter input \\a size to a valid value\n> + * \\param[in] pixFmt The pixel format of the converter input stream\n> + * \\param[in] size The converter input size to adjust to a valid value\n> + * \\param[in] align The desired alignment\n> + * \\return The adjusted converter input size\n> + */\n> +\n> +/**\n> + * \\fn Converter::adjustOutputSize()\n> + * \\brief Adjust the converter output \\a size to a valid value\n> + * \\param[in] pixFmt The pixel format of the converter output stream\n> + * \\param[in] size The converter output size to adjust to a valid value\n> + * \\param[in] align The desired alignment\n> + * \\return The adjusted converter output size\n> + */\n> +\n> /**\n> * \\fn Converter::strideAndFrameSize()\n> * \\brief Retrieve the output stride and frame size for an input configutation\n> @@ -118,6 +151,16 @@ Converter::~Converter()\n> * \\return A tuple indicating the stride and frame size or an empty tuple on error\n> */\n> \n> +/**\n> + * \\fn Converter::validateOutput()\n> + * \\brief Validate and possibily adjust \\a cfg to a valid converter output\n> + * \\param[inout] cfg The StreamConfiguration to validate and adjust\n> + * \\param[out] adjusted Set to true if \\a cfg has been adjusted\n> + * \\param[in] align The desired alignment\n> + * \\return 0 if \\a cfg is valid or has been adjusted, a negative error code\n> + * otherwise if \\a cfg cannot be adjusted\n> + */\n> +\n> /**\n> * \\fn Converter::configure()\n> * \\brief Configure a set of output stream conversion from an input stream\n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index bd7e5cce600d..6857472b29f2 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -8,6 +8,7 @@\n> \n> #include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> \n> +#include \n> #include \n> \n> #include \n> @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> \treturn std::make_tuple(format.planes[0].bpl, format.planes[0].size);\n> }\n> \n> +/**\n> + * \\copydoc libcamera::Converter::adjustInputSize\n> + */\n> +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t const Size &size, Alignments align)\n> +{\n> +\tauto formats = m2m_->output()->formats();\n> +\tV4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);\n> +\n> +\tauto it = formats.find(v4l2PixFmt);\n> +\tif (it == formats.end()) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn adjustSizes(size, it->second, align);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::Converter::adjustOutputSize\n> + */\n> +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,\n> +\t\t\t\t\tconst Size &size, Alignments align)\n> +{\n> +\tauto formats = m2m_->capture()->formats();\n> +\tV4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);\n> +\n> +\tauto it = formats.find(v4l2PixFmt);\n> +\tif (it == formats.end()) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn adjustSizes(size, it->second, align);\n> +}\n> +\n> +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,\n> +\t\t\t\t const std::vector &ranges,\n> +\t\t\t\t Alignments align)\n> +{\n> +\tSize size = cfgSize;\n> +\n> +\tif (ranges.size() == 1) {\n> +\t\t/*\n> +\t\t * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or\n> +\t\t * V4L2_FRMSIZE_TYPE_STEPWISE.\n> +\t\t */\n> +\t\tconst SizeRange &range = *ranges.begin();\n> +\n> +\t\tsize.width = std::clamp(size.width, range.min.width,\n> +\t\t\t\t\trange.max.width);\n> +\t\tsize.height = std::clamp(size.height, range.min.height,\n> +\t\t\t\t\t range.max.height);\n> +\n> +\t\t/*\n> +\t\t * Check if any alignment is needed. If the sizes are already\n> +\t\t * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS\n> +\t\t * with hStep and vStep equal to 1, we're done here.\n> +\t\t */\n> +\t\tint widthR = size.width % range.hStep;\n> +\t\tint heightR = size.height % range.vStep;\n> +\n> +\t\t/* Align up or down according to the caller request. */\n> +\n> +\t\tif (widthR != 0)\n> +\t\t\tsize.width = size.width - widthR\n> +\t\t\t\t + ((align == Alignment::Up) ? range.hStep : 0);\n> +\n> +\t\tif (heightR != 0)\n> +\t\t\tsize.height = size.height - heightR\n> +\t\t\t\t + ((align == Alignment::Up) ? range.vStep : 0);\n> +\t} else {\n> +\t\t/*\n> +\t\t * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the\n> +\t\t * size closer to the requested output configuration.\n> +\t\t *\n> +\t\t * The size ranges vector is not ordered, so we sort it first.\n> +\t\t * If we align up, start from the larger element.\n> +\t\t */\n> +\t\tstd::vector sizes(ranges.size());\n> +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),\n> +\t\t\t [](const SizeRange &range) { return range.max; });\n> +\t\tstd::sort(sizes.begin(), sizes.end());\n> +\n> +\t\tif (align == Alignment::Up)\n> +\t\t\tstd::reverse(sizes.begin(), sizes.end());\n> +\n> +\t\t/*\n> +\t\t * Return true if s2 is valid according to the desired\n> +\t\t * alignment: smaller than s1 if we align down, larger than s1\n> +\t\t * if we align up.\n> +\t\t */\n> +\t\tauto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {\n> +\t\t\treturn a == Alignment::Down\n> +\t\t\t\t? (s1.width > s2.width && s1.height > s2.height)\n> +\t\t\t\t: (s1.width < s2.width && s1.height < s2.height);\n> +\t\t};\n> +\n> +\t\tSize newSize;\n> +\t\tfor (const Size &sz : sizes) {\n> +\t\t\tif (!nextSizeValid(size, sz, align))\n> +\t\t\t\tbreak;\n> +\n> +\t\t\tnewSize = sz;\n> +\t\t}\n> +\n> +\t\tif (newSize.isNull()) {\n> +\t\t\tLOG(Converter, Error)\n> +\t\t\t\t<< \"Cannot adjust \" << cfgSize\n> +\t\t\t\t<< \" to a supported converter size\";\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tsize = newSize;\n> +\t}\n> +\n> +\treturn size;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::Converter::configure\n> */\n> @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()\n> \t\titer.second->stop();\n> }\n> \n> +/**\n> + * \\copydoc libcamera::Converter::validateOutput\n> + */\n> +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> +\t\t\t\t Alignments align)\n> +{\n> +\tV4L2VideoDevice *capture = m2m_->capture();\n> +\tV4L2VideoDevice::Formats fmts = capture->formats();\n> +\n> +\tif (adjusted)\n> +\t\t*adjusted = false;\n> +\n> +\tPixelFormat fmt = cfg->pixelFormat;\n> +\tV4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);\n> +\n> +\tauto it = fmts.find(v4l2PixFmt);\n> +\tif (it == fmts.end()) {\n> +\t\tit = fmts.begin();\n> +\t\tv4l2PixFmt = it->first;\n> +\t\tcfg->pixelFormat = v4l2PixFmt.toPixelFormat();\n> +\n> +\t\tif (adjusted)\n> +\t\t\t*adjusted = true;\n> +\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Converter output pixel format adjusted to \"\n> +\t\t\t<< cfg->pixelFormat;\n> +\t}\n> +\n> +\tconst Size cfgSize = cfg->size;\n> +\tcfg->size = adjustSizes(cfgSize, it->second, align);\n> +\n> +\tif (cfg->size.isNull())\n> +\t\treturn -EINVAL;\n> +\n> +\tif (cfg->size.width != cfgSize.width ||\n> +\t cfg->size.height != cfgSize.height) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Converter size adjusted to \"\n> +\t\t\t<< cfg->size;\n> +\t\tif (adjusted)\n> +\t\t\t*adjusted = true;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::Converter::queueBuffers\n> */\n> -- \n> 2.43.0\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 C09F3C3260\n\tfor ;\n\tThu, 12 Dec 2024 11:43:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1DBA67ECD;\n\tThu, 12 Dec 2024 12:43:25 +0100 (CET)","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 BF7CA608B6\n\tfor ;\n\tThu, 12 Dec 2024 12:43:23 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:39eb:989c:b016:217b])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D393F229;\n\tThu, 12 Dec 2024 12:42:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tixTXvoz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734003770;\n\tbh=3PmYHohUFkLNDBnClk7q/wpQezv7tMzLlqBJ4jQckKw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tixTXvoz6Gbebb3GdtgwPnc+yY/O4fAM6esTnGZ/m8iRT88EgrjJ37vqVXti07vvZ\n\tt5GwhuonC0fsEw4WNVvRhJX9BeFjhQ5QhZSpNnehQQDQygu7I7poRSXbJ6pwpoovzi\n\t+64rtRPhuuo+Jw8FIdrSQSxBBpMh5kz2oVk3el14=","Date":"Thu, 12 Dec 2024 20:43:15 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","Subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","Message-ID":"","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-10-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241206101344.767170-10-stefan.klug@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\" "}},{"id":32707,"web_url":"https://patchwork.libcamera.org/comment/32707/","msgid":"<4e45hgp675d72xveouujzuyw6kula4jpyk2giqz4trxdhwzyuk@wxiugljpsbfy>","date":"2024-12-12T17:23:16","subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Wed, Dec 11, 2024 at 05:55:22PM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch :-)\n>\n> On Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:\n> > From: Jacopo Mondi \n> >\n> > Add to the Converter interface two functions used by pipeline handlers\n> > to validate and adjust the converter input and output configurations\n> > by specifying the desired alignment for the adjustment.\n> >\n> > Add the adjustInputSize() and adjustOutputSize() functions that allows\n> > to adjust the converter input/output sizes with the desired alignment.\n> >\n> > Add a validateOutput() function meant to be used by the pipeline\n> > handler implementations of validate(). The function adjusts a\n> > StreamConfiguration to a valid configuration produced by the Converter.\n> >\n> > Signed-off-by: Jacopo Mondi \n> >\n> > ---\n> > Changes in v3:\n> > - Added this patch\n> > ---\n> > include/libcamera/internal/converter.h | 17 ++\n> > .../internal/converter/converter_v4l2_m2m.h | 11 ++\n> > src/libcamera/converter.cpp | 43 +++++\n> > .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++\n> > 4 files changed, 240 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> > index ffbb6f345cd5..9213ae5b9c33 100644\n> > --- a/include/libcamera/internal/converter.h\n> > +++ b/include/libcamera/internal/converter.h\n> > @@ -41,6 +41,13 @@ public:\n> >\n> > \tusing Features = Flags;\n> >\n> > +\tenum class Alignment {\n> > +\t\tDown = 0,\n> > +\t\tUp = (1 << 0),\n> > +\t};\n> > +\n> > +\tusing Alignments = Flags;\n> > +\n> > \tConverter(MediaDevice *media, Features features = Feature::None);\n> > \tvirtual ~Converter();\n> >\n> > @@ -51,9 +58,19 @@ public:\n> > \tvirtual std::vector formats(PixelFormat input) = 0;\n> > \tvirtual SizeRange sizes(const Size &input) = 0;\n> >\n> > +\tvirtual Size adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > +\tvirtual Size adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > +\n> > \tvirtual std::tuple\n> > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> >\n> > +\tvirtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > +\n>\n> I'm wondering when to use non-const references and when a pointer. Is\n> there a reason cfg is a pointer?\n>\n\nI presume the idea is to use * if the parameter can be modified and\nconst & for non-modifiable ones.\n\nI presume the idea was that being used to the pass-by-value syntax from C,\nit felt natural to convey the mutable nature of an argument using pointers.\n\nHowever it's more of an habit than something we enforce constantly\nthroughout the code base.\n\n\n> > \tvirtual int configure(const StreamConfiguration &inputCfg,\n> > \t\t\t const std::vector> &outputCfgs) = 0;\n> > \tvirtual int exportBuffers(const Stream *stream, unsigned int count,\n> > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > index a5286166f574..89bd2878b190 100644\n> > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > @@ -47,6 +47,11 @@ public:\n> > \tstd::tuple\n> > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> >\n> > +\tSize adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > +\tSize adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > +\n> > \tint configure(const StreamConfiguration &inputCfg,\n> > \t\t const std::vector> &outputCfg);\n> > \tint exportBuffers(const Stream *stream, unsigned int count,\n> > @@ -55,6 +60,9 @@ public:\n> > \tint start();\n> > \tvoid stop();\n> >\n> > +\tint validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t Alignments align = Alignment::Down) override;\n> > +\n> > \tint queueBuffers(FrameBuffer *input,\n> > \t\t\t const std::map &outputs);\n> >\n> > @@ -101,6 +109,9 @@ private:\n> > \t\tstd::pair inputCropBounds_;\n> > \t};\n> >\n> > +\tSize adjustSizes(const Size &size, const std::vector &ranges,\n> > +\t\t\t Alignments align);\n> > +\n> > \tstd::unique_ptr m2m_;\n> >\n> > \tstd::map> streams_;\n> > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> > index aa16970cd446..c3da162b7de7 100644\n> > --- a/src/libcamera/converter.cpp\n> > +++ b/src/libcamera/converter.cpp\n> > @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)\n> > * \\brief A bitwise combination of features supported by the converter\n> > */\n> >\n> > +/**\n> > + * \\enum Converter::Alignment\n> > + * \\brief The alignment mode specified when adjusting the converter input or\n> > + * output sizes\n> > + * \\var Converter::Alignment::Down\n> > + * \\brief Adjust the Converter sizes to a smaller valid size\n> > + * \\var Converter::Alignment::Up\n> > + * \\brief Adjust the Converter sizes to a larger valid size\n> > + */\n> > +\n> > +/**\n> > + * \\typedef Converter::Alignments\n> > + * \\brief A bitwise combination of alignments supported by the converter\n> > + */\n> > +\n> > /**\n> > * \\brief Construct a Converter instance\n> > * \\param[in] media The media device implementing the converter\n> > @@ -110,6 +125,24 @@ Converter::~Converter()\n> > * \\return A range of output image sizes\n> > */\n> >\n> > +/**\n> > + * \\fn Converter::adjustInputSize()\n> > + * \\brief Adjust the converter input \\a size to a valid value\n> > + * \\param[in] pixFmt The pixel format of the converter input stream\n> > + * \\param[in] size The converter input size to adjust to a valid value\n> > + * \\param[in] align The desired alignment\n> > + * \\return The adjusted converter input size\n> > + */\n>\n> What gets returned if there is no valid side for the request?\n\nYou're right, I should probably add\n\n * \\return The adjusted converter input size or a null Size if \\a\n * size cannot be adjusted\n\nI would use \"null Size\" because of Size::isNull().\n\n>\n> My comments where all nits. So\n>\n> Reviewed-by: Stefan Klug \n>\n\nThanks, I'll send a fixup on top.\n\n> > +\n> > +/**\n> > + * \\fn Converter::adjustOutputSize()\n> > + * \\brief Adjust the converter output \\a size to a valid value\n> > + * \\param[in] pixFmt The pixel format of the converter output stream\n> > + * \\param[in] size The converter output size to adjust to a valid value\n> > + * \\param[in] align The desired alignment\n> > + * \\return The adjusted converter output size\n> > + */\n> > +\n> > /**\n> > * \\fn Converter::strideAndFrameSize()\n> > * \\brief Retrieve the output stride and frame size for an input configutation\n> > @@ -118,6 +151,16 @@ Converter::~Converter()\n> > * \\return A tuple indicating the stride and frame size or an empty tuple on error\n> > */\n> >\n> > +/**\n> > + * \\fn Converter::validateOutput()\n> > + * \\brief Validate and possibily adjust \\a cfg to a valid converter output\n> > + * \\param[inout] cfg The StreamConfiguration to validate and adjust\n> > + * \\param[out] adjusted Set to true if \\a cfg has been adjusted\n> > + * \\param[in] align The desired alignment\n> > + * \\return 0 if \\a cfg is valid or has been adjusted, a negative error code\n> > + * otherwise if \\a cfg cannot be adjusted\n> > + */\n> > +\n> > /**\n> > * \\fn Converter::configure()\n> > * \\brief Configure a set of output stream conversion from an input stream\n> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > index bd7e5cce600d..6857472b29f2 100644\n> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > @@ -8,6 +8,7 @@\n> >\n> > #include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > \treturn std::make_tuple(format.planes[0].bpl, format.planes[0].size);\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::Converter::adjustInputSize\n> > + */\n> > +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size, Alignments align)\n> > +{\n> > +\tauto formats = m2m_->output()->formats();\n> > +\tV4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);\n> > +\n> > +\tauto it = formats.find(v4l2PixFmt);\n> > +\tif (it == formats.end()) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\treturn adjustSizes(size, it->second, align);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::Converter::adjustOutputSize\n> > + */\n> > +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t\tconst Size &size, Alignments align)\n> > +{\n> > +\tauto formats = m2m_->capture()->formats();\n> > +\tV4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);\n> > +\n> > +\tauto it = formats.find(v4l2PixFmt);\n> > +\tif (it == formats.end()) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\treturn adjustSizes(size, it->second, align);\n> > +}\n> > +\n> > +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,\n> > +\t\t\t\t const std::vector &ranges,\n> > +\t\t\t\t Alignments align)\n> > +{\n> > +\tSize size = cfgSize;\n> > +\n> > +\tif (ranges.size() == 1) {\n> > +\t\t/*\n> > +\t\t * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or\n> > +\t\t * V4L2_FRMSIZE_TYPE_STEPWISE.\n> > +\t\t */\n> > +\t\tconst SizeRange &range = *ranges.begin();\n> > +\n> > +\t\tsize.width = std::clamp(size.width, range.min.width,\n> > +\t\t\t\t\trange.max.width);\n> > +\t\tsize.height = std::clamp(size.height, range.min.height,\n> > +\t\t\t\t\t range.max.height);\n> > +\n> > +\t\t/*\n> > +\t\t * Check if any alignment is needed. If the sizes are already\n> > +\t\t * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS\n> > +\t\t * with hStep and vStep equal to 1, we're done here.\n> > +\t\t */\n> > +\t\tint widthR = size.width % range.hStep;\n> > +\t\tint heightR = size.height % range.vStep;\n> > +\n> > +\t\t/* Align up or down according to the caller request. */\n> > +\n> > +\t\tif (widthR != 0)\n> > +\t\t\tsize.width = size.width - widthR\n> > +\t\t\t\t + ((align == Alignment::Up) ? range.hStep : 0);\n> > +\n> > +\t\tif (heightR != 0)\n> > +\t\t\tsize.height = size.height - heightR\n> > +\t\t\t\t + ((align == Alignment::Up) ? range.vStep : 0);\n> > +\t} else {\n> > +\t\t/*\n> > +\t\t * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the\n> > +\t\t * size closer to the requested output configuration.\n> > +\t\t *\n> > +\t\t * The size ranges vector is not ordered, so we sort it first.\n> > +\t\t * If we align up, start from the larger element.\n> > +\t\t */\n> > +\t\tstd::vector sizes(ranges.size());\n> > +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),\n> > +\t\t\t [](const SizeRange &range) { return range.max; });\n> > +\t\tstd::sort(sizes.begin(), sizes.end());\n> > +\n> > +\t\tif (align == Alignment::Up)\n> > +\t\t\tstd::reverse(sizes.begin(), sizes.end());\n> > +\n> > +\t\t/*\n> > +\t\t * Return true if s2 is valid according to the desired\n> > +\t\t * alignment: smaller than s1 if we align down, larger than s1\n> > +\t\t * if we align up.\n> > +\t\t */\n> > +\t\tauto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {\n> > +\t\t\treturn a == Alignment::Down\n> > +\t\t\t\t? (s1.width > s2.width && s1.height > s2.height)\n> > +\t\t\t\t: (s1.width < s2.width && s1.height < s2.height);\n> > +\t\t};\n> > +\n> > +\t\tSize newSize;\n> > +\t\tfor (const Size &sz : sizes) {\n> > +\t\t\tif (!nextSizeValid(size, sz, align))\n> > +\t\t\t\tbreak;\n> > +\n> > +\t\t\tnewSize = sz;\n> > +\t\t}\n> > +\n> > +\t\tif (newSize.isNull()) {\n> > +\t\t\tLOG(Converter, Error)\n> > +\t\t\t\t<< \"Cannot adjust \" << cfgSize\n> > +\t\t\t\t<< \" to a supported converter size\";\n> > +\t\t\treturn {};\n> > +\t\t}\n> > +\n> > +\t\tsize = newSize;\n> > +\t}\n> > +\n> > +\treturn size;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::Converter::configure\n> > */\n> > @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()\n> > \t\titer.second->stop();\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::Converter::validateOutput\n> > + */\n> > +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t\t Alignments align)\n> > +{\n> > +\tV4L2VideoDevice *capture = m2m_->capture();\n> > +\tV4L2VideoDevice::Formats fmts = capture->formats();\n> > +\n> > +\tif (adjusted)\n> > +\t\t*adjusted = false;\n> > +\n> > +\tPixelFormat fmt = cfg->pixelFormat;\n> > +\tV4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);\n> > +\n> > +\tauto it = fmts.find(v4l2PixFmt);\n> > +\tif (it == fmts.end()) {\n> > +\t\tit = fmts.begin();\n> > +\t\tv4l2PixFmt = it->first;\n> > +\t\tcfg->pixelFormat = v4l2PixFmt.toPixelFormat();\n> > +\n> > +\t\tif (adjusted)\n> > +\t\t\t*adjusted = true;\n> > +\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Converter output pixel format adjusted to \"\n> > +\t\t\t<< cfg->pixelFormat;\n> > +\t}\n> > +\n> > +\tconst Size cfgSize = cfg->size;\n> > +\tcfg->size = adjustSizes(cfgSize, it->second, align);\n> > +\n> > +\tif (cfg->size.isNull())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tif (cfg->size.width != cfgSize.width ||\n> > +\t cfg->size.height != cfgSize.height) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Converter size adjusted to \"\n> > +\t\t\t<< cfg->size;\n> > +\t\tif (adjusted)\n> > +\t\t\t*adjusted = true;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::Converter::queueBuffers\n> > */\n> > --\n> > 2.43.0\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 9675DC3260\n\tfor ;\n\tThu, 12 Dec 2024 17:23:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 859F067ED9;\n\tThu, 12 Dec 2024 18:23:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31C70608B6\n\tfor ;\n\tThu, 12 Dec 2024 18:23:20 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFE9E18D;\n\tThu, 12 Dec 2024 18:22:45 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ssXXuSDL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734024165;\n\tbh=nS+mnQ13Nljc6wYA8NKz+fEdU9aAolWDCWCf2pyYWGQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ssXXuSDLtV8RtpNNsPMego2lQSTgEoVxQOQkb6x4VUGtjTdE7/RzusA5U1bhYOdZU\n\ty+h/p6XwXvaQCewlXfWdU5l0h2Bb1s5uvqCGPdtxnoWOSpSwziEsrXkJpGx2uLMWUV\n\t1nvuWVO2999h2+NbouqHRz7UyzIzNWwNuTqb6ir8=","Date":"Thu, 12 Dec 2024 18:23:16 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org, \n\tJacopo Mondi ","Subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","Message-ID":"<4e45hgp675d72xveouujzuyw6kula4jpyk2giqz4trxdhwzyuk@wxiugljpsbfy>","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-10-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":32708,"web_url":"https://patchwork.libcamera.org/comment/32708/","msgid":"","date":"2024-12-12T17:28:16","subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Paul\n\nOn Thu, Dec 12, 2024 at 08:43:15PM +0900, Paul Elder wrote:\n> On Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:\n> > From: Jacopo Mondi \n> >\n> > Add to the Converter interface two functions used by pipeline handlers\n> > to validate and adjust the converter input and output configurations\n> > by specifying the desired alignment for the adjustment.\n> >\n> > Add the adjustInputSize() and adjustOutputSize() functions that allows\n> > to adjust the converter input/output sizes with the desired alignment.\n> >\n> > Add a validateOutput() function meant to be used by the pipeline\n> > handler implementations of validate(). The function adjusts a\n> > StreamConfiguration to a valid configuration produced by the Converter.\n> >\n> > Signed-off-by: Jacopo Mondi \n> >\n> > ---\n> > Changes in v3:\n> > - Added this patch\n> > ---\n> > include/libcamera/internal/converter.h | 17 ++\n> > .../internal/converter/converter_v4l2_m2m.h | 11 ++\n> > src/libcamera/converter.cpp | 43 +++++\n> > .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++\n> > 4 files changed, 240 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> > index ffbb6f345cd5..9213ae5b9c33 100644\n> > --- a/include/libcamera/internal/converter.h\n> > +++ b/include/libcamera/internal/converter.h\n> > @@ -41,6 +41,13 @@ public:\n> >\n> > \tusing Features = Flags;\n> >\n> > +\tenum class Alignment {\n> > +\t\tDown = 0,\n> > +\t\tUp = (1 << 0),\n> > +\t};\n> > +\n> > +\tusing Alignments = Flags;\n> > +\n> > \tConverter(MediaDevice *media, Features features = Feature::None);\n> > \tvirtual ~Converter();\n> >\n> > @@ -51,9 +58,19 @@ public:\n> > \tvirtual std::vector formats(PixelFormat input) = 0;\n> > \tvirtual SizeRange sizes(const Size &input) = 0;\n> >\n> > +\tvirtual Size adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > +\tvirtual Size adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > +\n> > \tvirtual std::tuple\n> > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> >\n> > +\tvirtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n>\n> I'm confused. Here (and everywhere else it's used), since the type is\n> Alignments (Flags), doesn't that mean that\n> Alignment::Down | Alignment::Up is a valid input? Wouldn't that cause\n> unexpected behavior?\n>\n> Although I suppose if Alignment::Down | Alignment::Up was actually\n> passed then it wouldn't be equal Alignment::Down nor Alignment::Up so\n> nothing would happen? Except maybe the ternary operators ig, those would\n> just activate the false clause.\n>\n> Am I missing something here?\n>\n\nnah, you're absolutely right, Alignments should be a Flag<>, it's fine\nas an enum!\n\nI'll send fixups\n\n>\n> Paul\n>\n>\n> > +\n> > \tvirtual int configure(const StreamConfiguration &inputCfg,\n> > \t\t\t const std::vector> &outputCfgs) = 0;\n> > \tvirtual int exportBuffers(const Stream *stream, unsigned int count,\n> > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > index a5286166f574..89bd2878b190 100644\n> > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > @@ -47,6 +47,11 @@ public:\n> > \tstd::tuple\n> > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> >\n> > +\tSize adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > +\tSize adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > +\n> > \tint configure(const StreamConfiguration &inputCfg,\n> > \t\t const std::vector> &outputCfg);\n> > \tint exportBuffers(const Stream *stream, unsigned int count,\n> > @@ -55,6 +60,9 @@ public:\n> > \tint start();\n> > \tvoid stop();\n> >\n> > +\tint validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t Alignments align = Alignment::Down) override;\n> > +\n> > \tint queueBuffers(FrameBuffer *input,\n> > \t\t\t const std::map &outputs);\n> >\n> > @@ -101,6 +109,9 @@ private:\n> > \t\tstd::pair inputCropBounds_;\n> > \t};\n> >\n> > +\tSize adjustSizes(const Size &size, const std::vector &ranges,\n> > +\t\t\t Alignments align);\n> > +\n> > \tstd::unique_ptr m2m_;\n> >\n> > \tstd::map> streams_;\n> > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> > index aa16970cd446..c3da162b7de7 100644\n> > --- a/src/libcamera/converter.cpp\n> > +++ b/src/libcamera/converter.cpp\n> > @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)\n> > * \\brief A bitwise combination of features supported by the converter\n> > */\n> >\n> > +/**\n> > + * \\enum Converter::Alignment\n> > + * \\brief The alignment mode specified when adjusting the converter input or\n> > + * output sizes\n> > + * \\var Converter::Alignment::Down\n> > + * \\brief Adjust the Converter sizes to a smaller valid size\n> > + * \\var Converter::Alignment::Up\n> > + * \\brief Adjust the Converter sizes to a larger valid size\n> > + */\n> > +\n> > +/**\n> > + * \\typedef Converter::Alignments\n> > + * \\brief A bitwise combination of alignments supported by the converter\n> > + */\n> > +\n> > /**\n> > * \\brief Construct a Converter instance\n> > * \\param[in] media The media device implementing the converter\n> > @@ -110,6 +125,24 @@ Converter::~Converter()\n> > * \\return A range of output image sizes\n> > */\n> >\n> > +/**\n> > + * \\fn Converter::adjustInputSize()\n> > + * \\brief Adjust the converter input \\a size to a valid value\n> > + * \\param[in] pixFmt The pixel format of the converter input stream\n> > + * \\param[in] size The converter input size to adjust to a valid value\n> > + * \\param[in] align The desired alignment\n> > + * \\return The adjusted converter input size\n> > + */\n> > +\n> > +/**\n> > + * \\fn Converter::adjustOutputSize()\n> > + * \\brief Adjust the converter output \\a size to a valid value\n> > + * \\param[in] pixFmt The pixel format of the converter output stream\n> > + * \\param[in] size The converter output size to adjust to a valid value\n> > + * \\param[in] align The desired alignment\n> > + * \\return The adjusted converter output size\n> > + */\n> > +\n> > /**\n> > * \\fn Converter::strideAndFrameSize()\n> > * \\brief Retrieve the output stride and frame size for an input configutation\n> > @@ -118,6 +151,16 @@ Converter::~Converter()\n> > * \\return A tuple indicating the stride and frame size or an empty tuple on error\n> > */\n> >\n> > +/**\n> > + * \\fn Converter::validateOutput()\n> > + * \\brief Validate and possibily adjust \\a cfg to a valid converter output\n> > + * \\param[inout] cfg The StreamConfiguration to validate and adjust\n> > + * \\param[out] adjusted Set to true if \\a cfg has been adjusted\n> > + * \\param[in] align The desired alignment\n> > + * \\return 0 if \\a cfg is valid or has been adjusted, a negative error code\n> > + * otherwise if \\a cfg cannot be adjusted\n> > + */\n> > +\n> > /**\n> > * \\fn Converter::configure()\n> > * \\brief Configure a set of output stream conversion from an input stream\n> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > index bd7e5cce600d..6857472b29f2 100644\n> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > @@ -8,6 +8,7 @@\n> >\n> > #include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > \treturn std::make_tuple(format.planes[0].bpl, format.planes[0].size);\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::Converter::adjustInputSize\n> > + */\n> > +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t const Size &size, Alignments align)\n> > +{\n> > +\tauto formats = m2m_->output()->formats();\n> > +\tV4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);\n> > +\n> > +\tauto it = formats.find(v4l2PixFmt);\n> > +\tif (it == formats.end()) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\treturn adjustSizes(size, it->second, align);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::Converter::adjustOutputSize\n> > + */\n> > +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,\n> > +\t\t\t\t\tconst Size &size, Alignments align)\n> > +{\n> > +\tauto formats = m2m_->capture()->formats();\n> > +\tV4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);\n> > +\n> > +\tauto it = formats.find(v4l2PixFmt);\n> > +\tif (it == formats.end()) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\treturn adjustSizes(size, it->second, align);\n> > +}\n> > +\n> > +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,\n> > +\t\t\t\t const std::vector &ranges,\n> > +\t\t\t\t Alignments align)\n> > +{\n> > +\tSize size = cfgSize;\n> > +\n> > +\tif (ranges.size() == 1) {\n> > +\t\t/*\n> > +\t\t * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or\n> > +\t\t * V4L2_FRMSIZE_TYPE_STEPWISE.\n> > +\t\t */\n> > +\t\tconst SizeRange &range = *ranges.begin();\n> > +\n> > +\t\tsize.width = std::clamp(size.width, range.min.width,\n> > +\t\t\t\t\trange.max.width);\n> > +\t\tsize.height = std::clamp(size.height, range.min.height,\n> > +\t\t\t\t\t range.max.height);\n> > +\n> > +\t\t/*\n> > +\t\t * Check if any alignment is needed. If the sizes are already\n> > +\t\t * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS\n> > +\t\t * with hStep and vStep equal to 1, we're done here.\n> > +\t\t */\n> > +\t\tint widthR = size.width % range.hStep;\n> > +\t\tint heightR = size.height % range.vStep;\n> > +\n> > +\t\t/* Align up or down according to the caller request. */\n> > +\n> > +\t\tif (widthR != 0)\n> > +\t\t\tsize.width = size.width - widthR\n> > +\t\t\t\t + ((align == Alignment::Up) ? range.hStep : 0);\n> > +\n> > +\t\tif (heightR != 0)\n> > +\t\t\tsize.height = size.height - heightR\n> > +\t\t\t\t + ((align == Alignment::Up) ? range.vStep : 0);\n> > +\t} else {\n> > +\t\t/*\n> > +\t\t * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the\n> > +\t\t * size closer to the requested output configuration.\n> > +\t\t *\n> > +\t\t * The size ranges vector is not ordered, so we sort it first.\n> > +\t\t * If we align up, start from the larger element.\n> > +\t\t */\n> > +\t\tstd::vector sizes(ranges.size());\n> > +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),\n> > +\t\t\t [](const SizeRange &range) { return range.max; });\n> > +\t\tstd::sort(sizes.begin(), sizes.end());\n> > +\n> > +\t\tif (align == Alignment::Up)\n> > +\t\t\tstd::reverse(sizes.begin(), sizes.end());\n> > +\n> > +\t\t/*\n> > +\t\t * Return true if s2 is valid according to the desired\n> > +\t\t * alignment: smaller than s1 if we align down, larger than s1\n> > +\t\t * if we align up.\n> > +\t\t */\n> > +\t\tauto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {\n> > +\t\t\treturn a == Alignment::Down\n> > +\t\t\t\t? (s1.width > s2.width && s1.height > s2.height)\n> > +\t\t\t\t: (s1.width < s2.width && s1.height < s2.height);\n> > +\t\t};\n> > +\n> > +\t\tSize newSize;\n> > +\t\tfor (const Size &sz : sizes) {\n> > +\t\t\tif (!nextSizeValid(size, sz, align))\n> > +\t\t\t\tbreak;\n> > +\n> > +\t\t\tnewSize = sz;\n> > +\t\t}\n> > +\n> > +\t\tif (newSize.isNull()) {\n> > +\t\t\tLOG(Converter, Error)\n> > +\t\t\t\t<< \"Cannot adjust \" << cfgSize\n> > +\t\t\t\t<< \" to a supported converter size\";\n> > +\t\t\treturn {};\n> > +\t\t}\n> > +\n> > +\t\tsize = newSize;\n> > +\t}\n> > +\n> > +\treturn size;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::Converter::configure\n> > */\n> > @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()\n> > \t\titer.second->stop();\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::Converter::validateOutput\n> > + */\n> > +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > +\t\t\t\t Alignments align)\n> > +{\n> > +\tV4L2VideoDevice *capture = m2m_->capture();\n> > +\tV4L2VideoDevice::Formats fmts = capture->formats();\n> > +\n> > +\tif (adjusted)\n> > +\t\t*adjusted = false;\n> > +\n> > +\tPixelFormat fmt = cfg->pixelFormat;\n> > +\tV4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);\n> > +\n> > +\tauto it = fmts.find(v4l2PixFmt);\n> > +\tif (it == fmts.end()) {\n> > +\t\tit = fmts.begin();\n> > +\t\tv4l2PixFmt = it->first;\n> > +\t\tcfg->pixelFormat = v4l2PixFmt.toPixelFormat();\n> > +\n> > +\t\tif (adjusted)\n> > +\t\t\t*adjusted = true;\n> > +\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Converter output pixel format adjusted to \"\n> > +\t\t\t<< cfg->pixelFormat;\n> > +\t}\n> > +\n> > +\tconst Size cfgSize = cfg->size;\n> > +\tcfg->size = adjustSizes(cfgSize, it->second, align);\n> > +\n> > +\tif (cfg->size.isNull())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tif (cfg->size.width != cfgSize.width ||\n> > +\t cfg->size.height != cfgSize.height) {\n> > +\t\tLOG(Converter, Info)\n> > +\t\t\t<< \"Converter size adjusted to \"\n> > +\t\t\t<< cfg->size;\n> > +\t\tif (adjusted)\n> > +\t\t\t*adjusted = true;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /**\n> > * \\copydoc libcamera::Converter::queueBuffers\n> > */\n> > --\n> > 2.43.0\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 007C5C32EA\n\tfor ;\n\tThu, 12 Dec 2024 17:28:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1087667ED9;\n\tThu, 12 Dec 2024 18:28:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A9FB4608B6\n\tfor ;\n\tThu, 12 Dec 2024 18:28:20 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9500518D;\n\tThu, 12 Dec 2024 18:27:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fZCR9X/9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734024466;\n\tbh=Rho8VhI/+L63OzRmRS+lhrWkInVvEjxyZi1tfrCvhxg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fZCR9X/9EXNxoWIYbRhmTF7SgD+li+u5JiqDrldjA42jbbdmC5c86XyiVx+G72btv\n\tGVHOxzEwgd1UEMFOUydXhWrCpu/0f0vip9CtMKUZzDxn9dFruqcVKLFiJR+Ab+nOi6\n\tdZMjjdnpCjhECkHm5ubtBPrkm2loaLFU8LIcU41Y=","Date":"Thu, 12 Dec 2024 18:28:16 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Cc":"Stefan Klug , \n\tlibcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","Subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","Message-ID":"","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-10-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":32712,"web_url":"https://patchwork.libcamera.org/comment/32712/","msgid":"","date":"2024-12-13T08:37:20","subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Dec 12, 2024 at 06:28:16PM +0100, Jacopo Mondi wrote:\n> Hi Paul\n> \n> On Thu, Dec 12, 2024 at 08:43:15PM +0900, Paul Elder wrote:\n> > On Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:\n> > > From: Jacopo Mondi \n> > >\n> > > Add to the Converter interface two functions used by pipeline handlers\n> > > to validate and adjust the converter input and output configurations\n> > > by specifying the desired alignment for the adjustment.\n> > >\n> > > Add the adjustInputSize() and adjustOutputSize() functions that allows\n> > > to adjust the converter input/output sizes with the desired alignment.\n> > >\n> > > Add a validateOutput() function meant to be used by the pipeline\n> > > handler implementations of validate(). The function adjusts a\n> > > StreamConfiguration to a valid configuration produced by the Converter.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > >\n> > > ---\n> > > Changes in v3:\n> > > - Added this patch\n> > > ---\n> > > include/libcamera/internal/converter.h | 17 ++\n> > > .../internal/converter/converter_v4l2_m2m.h | 11 ++\n> > > src/libcamera/converter.cpp | 43 +++++\n> > > .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++\n> > > 4 files changed, 240 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> > > index ffbb6f345cd5..9213ae5b9c33 100644\n> > > --- a/include/libcamera/internal/converter.h\n> > > +++ b/include/libcamera/internal/converter.h\n> > > @@ -41,6 +41,13 @@ public:\n> > >\n> > > \tusing Features = Flags;\n> > >\n> > > +\tenum class Alignment {\n> > > +\t\tDown = 0,\n> > > +\t\tUp = (1 << 0),\n> > > +\t};\n> > > +\n> > > +\tusing Alignments = Flags;\n> > > +\n> > > \tConverter(MediaDevice *media, Features features = Feature::None);\n> > > \tvirtual ~Converter();\n> > >\n> > > @@ -51,9 +58,19 @@ public:\n> > > \tvirtual std::vector formats(PixelFormat input) = 0;\n> > > \tvirtual SizeRange sizes(const Size &input) = 0;\n> > >\n> > > +\tvirtual Size adjustInputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t\t const Size &size,\n> > > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > > +\tvirtual Size adjustOutputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t\t const Size &size,\n> > > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> > > +\n> > > \tvirtual std::tuple\n> > > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> > >\n> > > +\tvirtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > > +\t\t\t\t Alignments align = Alignment::Down) = 0;\n> >\n> > I'm confused. Here (and everywhere else it's used), since the type is\n> > Alignments (Flags), doesn't that mean that\n> > Alignment::Down | Alignment::Up is a valid input? Wouldn't that cause\n> > unexpected behavior?\n> >\n> > Although I suppose if Alignment::Down | Alignment::Up was actually\n> > passed then it wouldn't be equal Alignment::Down nor Alignment::Up so\n> > nothing would happen? Except maybe the ternary operators ig, those would\n> > just activate the false clause.\n> >\n> > Am I missing something here?\n> >\n> \n> nah, you're absolutely right, Alignments should be a Flag<>, it's fine\n> as an enum!\n> \n> I'll send fixups\n\nFixup looks good! With that,\n\nReviewed-by: Paul Elder \n\n> >\n> >\n> > > +\n> > > \tvirtual int configure(const StreamConfiguration &inputCfg,\n> > > \t\t\t const std::vector> &outputCfgs) = 0;\n> > > \tvirtual int exportBuffers(const Stream *stream, unsigned int count,\n> > > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > index a5286166f574..89bd2878b190 100644\n> > > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > @@ -47,6 +47,11 @@ public:\n> > > \tstd::tuple\n> > > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> > >\n> > > +\tSize adjustInputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > > +\tSize adjustOutputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t const Size &size, Alignments align = Alignment::Down) override;\n> > > +\n> > > \tint configure(const StreamConfiguration &inputCfg,\n> > > \t\t const std::vector> &outputCfg);\n> > > \tint exportBuffers(const Stream *stream, unsigned int count,\n> > > @@ -55,6 +60,9 @@ public:\n> > > \tint start();\n> > > \tvoid stop();\n> > >\n> > > +\tint validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > > +\t\t\t Alignments align = Alignment::Down) override;\n> > > +\n> > > \tint queueBuffers(FrameBuffer *input,\n> > > \t\t\t const std::map &outputs);\n> > >\n> > > @@ -101,6 +109,9 @@ private:\n> > > \t\tstd::pair inputCropBounds_;\n> > > \t};\n> > >\n> > > +\tSize adjustSizes(const Size &size, const std::vector &ranges,\n> > > +\t\t\t Alignments align);\n> > > +\n> > > \tstd::unique_ptr m2m_;\n> > >\n> > > \tstd::map> streams_;\n> > > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> > > index aa16970cd446..c3da162b7de7 100644\n> > > --- a/src/libcamera/converter.cpp\n> > > +++ b/src/libcamera/converter.cpp\n> > > @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)\n> > > * \\brief A bitwise combination of features supported by the converter\n> > > */\n> > >\n> > > +/**\n> > > + * \\enum Converter::Alignment\n> > > + * \\brief The alignment mode specified when adjusting the converter input or\n> > > + * output sizes\n> > > + * \\var Converter::Alignment::Down\n> > > + * \\brief Adjust the Converter sizes to a smaller valid size\n> > > + * \\var Converter::Alignment::Up\n> > > + * \\brief Adjust the Converter sizes to a larger valid size\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\typedef Converter::Alignments\n> > > + * \\brief A bitwise combination of alignments supported by the converter\n> > > + */\n> > > +\n> > > /**\n> > > * \\brief Construct a Converter instance\n> > > * \\param[in] media The media device implementing the converter\n> > > @@ -110,6 +125,24 @@ Converter::~Converter()\n> > > * \\return A range of output image sizes\n> > > */\n> > >\n> > > +/**\n> > > + * \\fn Converter::adjustInputSize()\n> > > + * \\brief Adjust the converter input \\a size to a valid value\n> > > + * \\param[in] pixFmt The pixel format of the converter input stream\n> > > + * \\param[in] size The converter input size to adjust to a valid value\n> > > + * \\param[in] align The desired alignment\n> > > + * \\return The adjusted converter input size\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Converter::adjustOutputSize()\n> > > + * \\brief Adjust the converter output \\a size to a valid value\n> > > + * \\param[in] pixFmt The pixel format of the converter output stream\n> > > + * \\param[in] size The converter output size to adjust to a valid value\n> > > + * \\param[in] align The desired alignment\n> > > + * \\return The adjusted converter output size\n> > > + */\n> > > +\n> > > /**\n> > > * \\fn Converter::strideAndFrameSize()\n> > > * \\brief Retrieve the output stride and frame size for an input configutation\n> > > @@ -118,6 +151,16 @@ Converter::~Converter()\n> > > * \\return A tuple indicating the stride and frame size or an empty tuple on error\n> > > */\n> > >\n> > > +/**\n> > > + * \\fn Converter::validateOutput()\n> > > + * \\brief Validate and possibily adjust \\a cfg to a valid converter output\n> > > + * \\param[inout] cfg The StreamConfiguration to validate and adjust\n> > > + * \\param[out] adjusted Set to true if \\a cfg has been adjusted\n> > > + * \\param[in] align The desired alignment\n> > > + * \\return 0 if \\a cfg is valid or has been adjusted, a negative error code\n> > > + * otherwise if \\a cfg cannot be adjusted\n> > > + */\n> > > +\n> > > /**\n> > > * \\fn Converter::configure()\n> > > * \\brief Configure a set of output stream conversion from an input stream\n> > > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > index bd7e5cce600d..6857472b29f2 100644\n> > > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > @@ -8,6 +8,7 @@\n> > >\n> > > #include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \n> > > @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > > \treturn std::make_tuple(format.planes[0].bpl, format.planes[0].size);\n> > > }\n> > >\n> > > +/**\n> > > + * \\copydoc libcamera::Converter::adjustInputSize\n> > > + */\n> > > +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t\t const Size &size, Alignments align)\n> > > +{\n> > > +\tauto formats = m2m_->output()->formats();\n> > > +\tV4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);\n> > > +\n> > > +\tauto it = formats.find(v4l2PixFmt);\n> > > +\tif (it == formats.end()) {\n> > > +\t\tLOG(Converter, Info)\n> > > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > > +\t\treturn {};\n> > > +\t}\n> > > +\n> > > +\treturn adjustSizes(size, it->second, align);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::Converter::adjustOutputSize\n> > > + */\n> > > +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,\n> > > +\t\t\t\t\tconst Size &size, Alignments align)\n> > > +{\n> > > +\tauto formats = m2m_->capture()->formats();\n> > > +\tV4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);\n> > > +\n> > > +\tauto it = formats.find(v4l2PixFmt);\n> > > +\tif (it == formats.end()) {\n> > > +\t\tLOG(Converter, Info)\n> > > +\t\t\t<< \"Unsupported pixel format \" << pixFmt;\n> > > +\t\treturn {};\n> > > +\t}\n> > > +\n> > > +\treturn adjustSizes(size, it->second, align);\n> > > +}\n> > > +\n> > > +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,\n> > > +\t\t\t\t const std::vector &ranges,\n> > > +\t\t\t\t Alignments align)\n> > > +{\n> > > +\tSize size = cfgSize;\n> > > +\n> > > +\tif (ranges.size() == 1) {\n> > > +\t\t/*\n> > > +\t\t * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or\n> > > +\t\t * V4L2_FRMSIZE_TYPE_STEPWISE.\n> > > +\t\t */\n> > > +\t\tconst SizeRange &range = *ranges.begin();\n> > > +\n> > > +\t\tsize.width = std::clamp(size.width, range.min.width,\n> > > +\t\t\t\t\trange.max.width);\n> > > +\t\tsize.height = std::clamp(size.height, range.min.height,\n> > > +\t\t\t\t\t range.max.height);\n> > > +\n> > > +\t\t/*\n> > > +\t\t * Check if any alignment is needed. If the sizes are already\n> > > +\t\t * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS\n> > > +\t\t * with hStep and vStep equal to 1, we're done here.\n> > > +\t\t */\n> > > +\t\tint widthR = size.width % range.hStep;\n> > > +\t\tint heightR = size.height % range.vStep;\n> > > +\n> > > +\t\t/* Align up or down according to the caller request. */\n> > > +\n> > > +\t\tif (widthR != 0)\n> > > +\t\t\tsize.width = size.width - widthR\n> > > +\t\t\t\t + ((align == Alignment::Up) ? range.hStep : 0);\n> > > +\n> > > +\t\tif (heightR != 0)\n> > > +\t\t\tsize.height = size.height - heightR\n> > > +\t\t\t\t + ((align == Alignment::Up) ? range.vStep : 0);\n> > > +\t} else {\n> > > +\t\t/*\n> > > +\t\t * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the\n> > > +\t\t * size closer to the requested output configuration.\n> > > +\t\t *\n> > > +\t\t * The size ranges vector is not ordered, so we sort it first.\n> > > +\t\t * If we align up, start from the larger element.\n> > > +\t\t */\n> > > +\t\tstd::vector sizes(ranges.size());\n> > > +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),\n> > > +\t\t\t [](const SizeRange &range) { return range.max; });\n> > > +\t\tstd::sort(sizes.begin(), sizes.end());\n> > > +\n> > > +\t\tif (align == Alignment::Up)\n> > > +\t\t\tstd::reverse(sizes.begin(), sizes.end());\n> > > +\n> > > +\t\t/*\n> > > +\t\t * Return true if s2 is valid according to the desired\n> > > +\t\t * alignment: smaller than s1 if we align down, larger than s1\n> > > +\t\t * if we align up.\n> > > +\t\t */\n> > > +\t\tauto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {\n> > > +\t\t\treturn a == Alignment::Down\n> > > +\t\t\t\t? (s1.width > s2.width && s1.height > s2.height)\n> > > +\t\t\t\t: (s1.width < s2.width && s1.height < s2.height);\n> > > +\t\t};\n> > > +\n> > > +\t\tSize newSize;\n> > > +\t\tfor (const Size &sz : sizes) {\n> > > +\t\t\tif (!nextSizeValid(size, sz, align))\n> > > +\t\t\t\tbreak;\n> > > +\n> > > +\t\t\tnewSize = sz;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (newSize.isNull()) {\n> > > +\t\t\tLOG(Converter, Error)\n> > > +\t\t\t\t<< \"Cannot adjust \" << cfgSize\n> > > +\t\t\t\t<< \" to a supported converter size\";\n> > > +\t\t\treturn {};\n> > > +\t\t}\n> > > +\n> > > +\t\tsize = newSize;\n> > > +\t}\n> > > +\n> > > +\treturn size;\n> > > +}\n> > > +\n> > > /**\n> > > * \\copydoc libcamera::Converter::configure\n> > > */\n> > > @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()\n> > > \t\titer.second->stop();\n> > > }\n> > >\n> > > +/**\n> > > + * \\copydoc libcamera::Converter::validateOutput\n> > > + */\n> > > +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,\n> > > +\t\t\t\t Alignments align)\n> > > +{\n> > > +\tV4L2VideoDevice *capture = m2m_->capture();\n> > > +\tV4L2VideoDevice::Formats fmts = capture->formats();\n> > > +\n> > > +\tif (adjusted)\n> > > +\t\t*adjusted = false;\n> > > +\n> > > +\tPixelFormat fmt = cfg->pixelFormat;\n> > > +\tV4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);\n> > > +\n> > > +\tauto it = fmts.find(v4l2PixFmt);\n> > > +\tif (it == fmts.end()) {\n> > > +\t\tit = fmts.begin();\n> > > +\t\tv4l2PixFmt = it->first;\n> > > +\t\tcfg->pixelFormat = v4l2PixFmt.toPixelFormat();\n> > > +\n> > > +\t\tif (adjusted)\n> > > +\t\t\t*adjusted = true;\n> > > +\n> > > +\t\tLOG(Converter, Info)\n> > > +\t\t\t<< \"Converter output pixel format adjusted to \"\n> > > +\t\t\t<< cfg->pixelFormat;\n> > > +\t}\n> > > +\n> > > +\tconst Size cfgSize = cfg->size;\n> > > +\tcfg->size = adjustSizes(cfgSize, it->second, align);\n> > > +\n> > > +\tif (cfg->size.isNull())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tif (cfg->size.width != cfgSize.width ||\n> > > +\t cfg->size.height != cfgSize.height) {\n> > > +\t\tLOG(Converter, Info)\n> > > +\t\t\t<< \"Converter size adjusted to \"\n> > > +\t\t\t<< cfg->size;\n> > > +\t\tif (adjusted)\n> > > +\t\t\t*adjusted = true;\n> > > +\t}\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > /**\n> > > * \\copydoc libcamera::Converter::queueBuffers\n> > > */\n> > > --\n> > > 2.43.0\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 AD60AC32EA\n\tfor ;\n\tFri, 13 Dec 2024 08:37:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9278D67EE1;\n\tFri, 13 Dec 2024 09:37:34 +0100 (CET)","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 14AB3618AD\n\tfor ;\n\tFri, 13 Dec 2024 09:37:28 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:ea43:38b1:35c3:8c30])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BD6F752;\n\tFri, 13 Dec 2024 09:36:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"I28qMLfS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734079013;\n\tbh=rDRJ4y0T1/yVJlM5IuslJmg6ZrSL2CrtmJEa6N7gsIM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I28qMLfSTALwgq92L12li5Akwk1B0QnLHhdlRQi2rWamm6aIAnUBs7GYPTbJzgIhE\n\tVDtsjo1bBk6oguNNkFvVCpnQXVDSNbGuxFze2NGJOMtX23g9iSbMItrjjuc1xWMueV\n\tKZljbMzG6n9wc6IRhixInSsKFZUWBQzb2noieerc=","Date":"Fri, 13 Dec 2024 17:37:20 +0900","From":"Paul Elder ","To":"Jacopo Mondi ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 09/17] libcamera: converter: Add functions to adjust\n\tconfig","Message-ID":"","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-10-stefan.klug@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","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\" "}}]