[{"id":32776,"web_url":"https://patchwork.libcamera.org/comment/32776/","msgid":"<nzqsyuqaa57nisujetn2haqt7zr7gkzkudwyjrfvomcpzbqsjk@v2ghc6i5vx5g>","date":"2024-12-16T18:03:44","subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Mon, Dec 16, 2024 at 04:40:47PM +0100, Stefan Klug wrote:\n> In an upcoming patch it is necessary to get the crop bounds on the\n> converter itself. The V4L2M2MConverter contains code that is very\n> similar to the get crop bounds code in the V4L2M2MStream. Merge these\n> code blocks into a static function to be used from both classes.  This\n> patch contains no functional changes.\n>\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>\n> ---\n>\n> Changes in v4:\n> - Split patch from libcamera: converter_v4l2_m2m: Improve crop bounds\n>   support\n> ---\n>  .../converter/converter_v4l2_m2m.cpp          | 106 +++++++++---------\n>  1 file changed, 52 insertions(+), 54 deletions(-)\n>\n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index d63ef2f8028f..8c341fe199f6 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -30,6 +30,52 @@ namespace libcamera {\n>\n>  LOG_DECLARE_CATEGORY(Converter)\n>\n> +namespace {\n> +\n> +int getCropBounds(V4L2VideoDevice *device, Rectangle &minCrop,\n> +\t\t  Rectangle &maxCrop)\n\nNot thrilled by static functions, but we discussed it already :)\n\n> +{\n> +\tRectangle minC;\n> +\tRectangle maxC;\n\nYou could use minCrop and maxCrop maybe\n\nfor this patch\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\n\n> +\n> +\t/* Find crop bounds */\n> +\tminC.width = 1;\n> +\tminC.height = 1;\n> +\tmaxC.width = UINT_MAX;\n> +\tmaxC.height = UINT_MAX;\n> +\n> +\tint ret = device->setSelection(V4L2_SEL_TGT_CROP, &minC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not query minimum selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = device->getSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not query maximum selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Reset the crop to its maximum */\n> +\tret = device->setSelection(V4L2_SEL_TGT_CROP, &maxC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not reset selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tminCrop = minC;\n> +\tmaxCrop = maxC;\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace */\n> +\n>  /* -----------------------------------------------------------------------------\n>   * V4L2M2MConverter::V4L2M2MStream\n>   */\n> @@ -98,41 +144,10 @@ int V4L2M2MConverter::V4L2M2MStream::configure(const StreamConfiguration &inputC\n>  \toutputBufferCount_ = outputCfg.bufferCount;\n>\n>  \tif (converter_->features() & Feature::InputCrop) {\n> -\t\tRectangle minCrop;\n> -\t\tRectangle maxCrop;\n> -\n> -\t\t/* Find crop bounds */\n> -\t\tminCrop.width = 1;\n> -\t\tminCrop.height = 1;\n> -\t\tmaxCrop.width = UINT_MAX;\n> -\t\tmaxCrop.height = UINT_MAX;\n> -\n> -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &minCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not query minimum selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> -\t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\tret = getInputSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not query maximum selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> +\t\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> +\t\t\t\t    inputCropBounds_.second);\n> +\t\tif (ret)\n>  \t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\t/* Reset the crop to its maximum */\n> -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not reset selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> -\t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\tinputCropBounds_ = { minCrop, maxCrop };\n>  \t}\n>\n>  \treturn 0;\n> @@ -258,27 +273,10 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n>  \t\treturn;\n>  \t}\n>\n> -\t/* Discover Feature::InputCrop */\n> +\tRectangle minCrop;\n>  \tRectangle maxCrop;\n> -\tmaxCrop.width = UINT_MAX;\n> -\tmaxCrop.height = UINT_MAX;\n> -\n> -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> -\tif (ret)\n> -\t\treturn;\n> -\n> -\t/*\n> -\t * Rectangles for cropping targets are defined even if the device\n> -\t * does not support cropping. Their sizes and positions will be\n> -\t * fixed in such cases.\n> -\t *\n> -\t * Set and inspect a crop equivalent to half of the maximum crop\n> -\t * returned earlier. Use this to determine whether the crop on\n> -\t * input is really supported.\n> -\t */\n> -\tRectangle halfCrop(maxCrop.size() / 2);\n> -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &halfCrop);\n> -\tif (!ret && halfCrop != maxCrop) {\n> +\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> +\tif (!ret && minCrop != maxCrop) {\n>  \t\tfeatures_ |= Feature::InputCrop;\n>\n>  \t\tLOG(Converter, Info)\n> --\n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 F2098C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Dec 2024 18:03:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3985567F97;\n\tMon, 16 Dec 2024 19:03:49 +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 CAAE267F7F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 19:03:47 +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 0F445160;\n\tMon, 16 Dec 2024 19:03:10 +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=\"t9sbiWZX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734372191;\n\tbh=Jd/uS1ft6eF96RMYrw4QAlY6K6GZgttDUwDXlxOS2UU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t9sbiWZXxx5dBpOuNKYhMsNs6AmyTQSdGFxvuRKEYUgw2iTFc+ZDurQs1z1c86usy\n\t5FoJj60SWjFPzT6L5gdfJjFF1Q31u5DIePHPNraVFknASfuWy8hMhG8v4dRwUyycZV\n\tjjtrmhxphDq8KyoGGMf/0PTgKftZTNTJ+9LOrrSQ=","Date":"Mon, 16 Dec 2024 19:03:44 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","Message-ID":"<nzqsyuqaa57nisujetn2haqt7zr7gkzkudwyjrfvomcpzbqsjk@v2ghc6i5vx5g>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-8-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-8-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32796,"web_url":"https://patchwork.libcamera.org/comment/32796/","msgid":"<Z2Dx6M5L9mrkFF56@pyrite.rasen.tech>","date":"2024-12-17T03:37:12","subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Dec 16, 2024 at 04:40:47PM +0100, Stefan Klug wrote:\n> In an upcoming patch it is necessary to get the crop bounds on the\n> converter itself. The V4L2M2MConverter contains code that is very\n> similar to the get crop bounds code in the V4L2M2MStream. Merge these\n> code blocks into a static function to be used from both classes.  This\n> patch contains no functional changes.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> \n> ---\n> \n> Changes in v4:\n> - Split patch from libcamera: converter_v4l2_m2m: Improve crop bounds\n>   support\n> ---\n>  .../converter/converter_v4l2_m2m.cpp          | 106 +++++++++---------\n>  1 file changed, 52 insertions(+), 54 deletions(-)\n> \n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index d63ef2f8028f..8c341fe199f6 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -30,6 +30,52 @@ namespace libcamera {\n>  \n>  LOG_DECLARE_CATEGORY(Converter)\n>  \n> +namespace {\n> +\n> +int getCropBounds(V4L2VideoDevice *device, Rectangle &minCrop,\n> +\t\t  Rectangle &maxCrop)\n> +{\n> +\tRectangle minC;\n> +\tRectangle maxC;\n> +\n> +\t/* Find crop bounds */\n> +\tminC.width = 1;\n> +\tminC.height = 1;\n> +\tmaxC.width = UINT_MAX;\n> +\tmaxC.height = UINT_MAX;\n> +\n> +\tint ret = device->setSelection(V4L2_SEL_TGT_CROP, &minC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not query minimum selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = device->getSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not query maximum selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Reset the crop to its maximum */\n> +\tret = device->setSelection(V4L2_SEL_TGT_CROP, &maxC);\n> +\tif (ret) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Could not reset selection crop: \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tminCrop = minC;\n> +\tmaxCrop = maxC;\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace */\n> +\n>  /* -----------------------------------------------------------------------------\n>   * V4L2M2MConverter::V4L2M2MStream\n>   */\n> @@ -98,41 +144,10 @@ int V4L2M2MConverter::V4L2M2MStream::configure(const StreamConfiguration &inputC\n>  \toutputBufferCount_ = outputCfg.bufferCount;\n>  \n>  \tif (converter_->features() & Feature::InputCrop) {\n> -\t\tRectangle minCrop;\n> -\t\tRectangle maxCrop;\n> -\n> -\t\t/* Find crop bounds */\n> -\t\tminCrop.width = 1;\n> -\t\tminCrop.height = 1;\n> -\t\tmaxCrop.width = UINT_MAX;\n> -\t\tmaxCrop.height = UINT_MAX;\n> -\n> -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &minCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not query minimum selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> -\t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\tret = getInputSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not query maximum selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> +\t\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> +\t\t\t\t    inputCropBounds_.second);\n> +\t\tif (ret)\n>  \t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\t/* Reset the crop to its maximum */\n> -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> -\t\tif (ret) {\n> -\t\t\tLOG(Converter, Error)\n> -\t\t\t\t<< \"Could not reset selection crop: \"\n> -\t\t\t\t<< strerror(-ret);\n> -\t\t\treturn ret;\n> -\t\t}\n> -\n> -\t\tinputCropBounds_ = { minCrop, maxCrop };\n>  \t}\n>  \n>  \treturn 0;\n> @@ -258,27 +273,10 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n>  \t\treturn;\n>  \t}\n>  \n> -\t/* Discover Feature::InputCrop */\n> +\tRectangle minCrop;\n>  \tRectangle maxCrop;\n> -\tmaxCrop.width = UINT_MAX;\n> -\tmaxCrop.height = UINT_MAX;\n> -\n> -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> -\tif (ret)\n> -\t\treturn;\n> -\n> -\t/*\n> -\t * Rectangles for cropping targets are defined even if the device\n> -\t * does not support cropping. Their sizes and positions will be\n> -\t * fixed in such cases.\n> -\t *\n> -\t * Set and inspect a crop equivalent to half of the maximum crop\n> -\t * returned earlier. Use this to determine whether the crop on\n> -\t * input is really supported.\n> -\t */\n> -\tRectangle halfCrop(maxCrop.size() / 2);\n> -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &halfCrop);\n> -\tif (!ret && halfCrop != maxCrop) {\n> +\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> +\tif (!ret && minCrop != maxCrop) {\n>  \t\tfeatures_ |= Feature::InputCrop;\n>  \n>  \t\tLOG(Converter, Info)\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D61C0C32AF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 03:37:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 229D367FB5;\n\tTue, 17 Dec 2024 04:37: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 C53CB61899\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 04:37:19 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:1a78:c005:412c:c675])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B179A3E;\n\tTue, 17 Dec 2024 04:36:41 +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=\"d9v64Jrd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734406602;\n\tbh=UeKzQPdg0JuuSQmYPzjM2GSAWl76o/iLDM2ZOq7bmHs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=d9v64JrdntfkOXF+HSshY3bRnHaDEcBgU5izK/BnKRqLkeMZHKtLtyKQN1MDLV2uI\n\tCR17vp+6LV0tJ/OjeZ7QdAGWxLlqMXfVcLJ/iVXLfAk3poE7KHfwmVeVtvHHK5rEes\n\t8MZOqLGcKR+PBXtKeEAGrvgP0NzDbg54O2Z2Lqn4=","Date":"Tue, 17 Dec 2024 12:37:12 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","Message-ID":"<Z2Dx6M5L9mrkFF56@pyrite.rasen.tech>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-8-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-8-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32798,"web_url":"https://patchwork.libcamera.org/comment/32798/","msgid":"<Z2D0c8ZwsoLswrb-@pyrite.rasen.tech>","date":"2024-12-17T03:48:03","subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Dec 16, 2024 at 07:03:44PM +0100, Jacopo Mondi wrote:\n> Hi Stefan\n> \n> On Mon, Dec 16, 2024 at 04:40:47PM +0100, Stefan Klug wrote:\n> > In an upcoming patch it is necessary to get the crop bounds on the\n> > converter itself. The V4L2M2MConverter contains code that is very\n> > similar to the get crop bounds code in the V4L2M2MStream. Merge these\n> > code blocks into a static function to be used from both classes.  This\n> > patch contains no functional changes.\n> >\n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> >\n> > ---\n> >\n> > Changes in v4:\n> > - Split patch from libcamera: converter_v4l2_m2m: Improve crop bounds\n> >   support\n> > ---\n> >  .../converter/converter_v4l2_m2m.cpp          | 106 +++++++++---------\n> >  1 file changed, 52 insertions(+), 54 deletions(-)\n> >\n> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > index d63ef2f8028f..8c341fe199f6 100644\n> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > @@ -30,6 +30,52 @@ namespace libcamera {\n> >\n> >  LOG_DECLARE_CATEGORY(Converter)\n> >\n> > +namespace {\n> > +\n> > +int getCropBounds(V4L2VideoDevice *device, Rectangle &minCrop,\n> > +\t\t  Rectangle &maxCrop)\n> \n> Not thrilled by static functions, but we discussed it already :)\n> \n> > +{\n> > +\tRectangle minC;\n> > +\tRectangle maxC;\n> \n> You could use minCrop and maxCrop maybe\n\nThat would shadow the output parameters which you would only want to\nwrite to on success, I think.\n\n\nPaul\n\n> \n> for this patch\n> \n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> \n> \n> > +\n> > +\t/* Find crop bounds */\n> > +\tminC.width = 1;\n> > +\tminC.height = 1;\n> > +\tmaxC.width = UINT_MAX;\n> > +\tmaxC.height = UINT_MAX;\n> > +\n> > +\tint ret = device->setSelection(V4L2_SEL_TGT_CROP, &minC);\n> > +\tif (ret) {\n> > +\t\tLOG(Converter, Error)\n> > +\t\t\t<< \"Could not query minimum selection crop: \"\n> > +\t\t\t<< strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tret = device->getSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxC);\n> > +\tif (ret) {\n> > +\t\tLOG(Converter, Error)\n> > +\t\t\t<< \"Could not query maximum selection crop: \"\n> > +\t\t\t<< strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\t/* Reset the crop to its maximum */\n> > +\tret = device->setSelection(V4L2_SEL_TGT_CROP, &maxC);\n> > +\tif (ret) {\n> > +\t\tLOG(Converter, Error)\n> > +\t\t\t<< \"Could not reset selection crop: \"\n> > +\t\t\t<< strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tminCrop = minC;\n> > +\tmaxCrop = maxC;\n> > +\treturn 0;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> >  /* -----------------------------------------------------------------------------\n> >   * V4L2M2MConverter::V4L2M2MStream\n> >   */\n> > @@ -98,41 +144,10 @@ int V4L2M2MConverter::V4L2M2MStream::configure(const StreamConfiguration &inputC\n> >  \toutputBufferCount_ = outputCfg.bufferCount;\n> >\n> >  \tif (converter_->features() & Feature::InputCrop) {\n> > -\t\tRectangle minCrop;\n> > -\t\tRectangle maxCrop;\n> > -\n> > -\t\t/* Find crop bounds */\n> > -\t\tminCrop.width = 1;\n> > -\t\tminCrop.height = 1;\n> > -\t\tmaxCrop.width = UINT_MAX;\n> > -\t\tmaxCrop.height = UINT_MAX;\n> > -\n> > -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &minCrop);\n> > -\t\tif (ret) {\n> > -\t\t\tLOG(Converter, Error)\n> > -\t\t\t\t<< \"Could not query minimum selection crop: \"\n> > -\t\t\t\t<< strerror(-ret);\n> > -\t\t\treturn ret;\n> > -\t\t}\n> > -\n> > -\t\tret = getInputSelection(V4L2_SEL_TGT_CROP_BOUNDS, &maxCrop);\n> > -\t\tif (ret) {\n> > -\t\t\tLOG(Converter, Error)\n> > -\t\t\t\t<< \"Could not query maximum selection crop: \"\n> > -\t\t\t\t<< strerror(-ret);\n> > +\t\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> > +\t\t\t\t    inputCropBounds_.second);\n> > +\t\tif (ret)\n> >  \t\t\treturn ret;\n> > -\t\t}\n> > -\n> > -\t\t/* Reset the crop to its maximum */\n> > -\t\tret = setInputSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> > -\t\tif (ret) {\n> > -\t\t\tLOG(Converter, Error)\n> > -\t\t\t\t<< \"Could not reset selection crop: \"\n> > -\t\t\t\t<< strerror(-ret);\n> > -\t\t\treturn ret;\n> > -\t\t}\n> > -\n> > -\t\tinputCropBounds_ = { minCrop, maxCrop };\n> >  \t}\n> >\n> >  \treturn 0;\n> > @@ -258,27 +273,10 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n> >  \t\treturn;\n> >  \t}\n> >\n> > -\t/* Discover Feature::InputCrop */\n> > +\tRectangle minCrop;\n> >  \tRectangle maxCrop;\n> > -\tmaxCrop.width = UINT_MAX;\n> > -\tmaxCrop.height = UINT_MAX;\n> > -\n> > -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &maxCrop);\n> > -\tif (ret)\n> > -\t\treturn;\n> > -\n> > -\t/*\n> > -\t * Rectangles for cropping targets are defined even if the device\n> > -\t * does not support cropping. Their sizes and positions will be\n> > -\t * fixed in such cases.\n> > -\t *\n> > -\t * Set and inspect a crop equivalent to half of the maximum crop\n> > -\t * returned earlier. Use this to determine whether the crop on\n> > -\t * input is really supported.\n> > -\t */\n> > -\tRectangle halfCrop(maxCrop.size() / 2);\n> > -\tret = m2m_->output()->setSelection(V4L2_SEL_TGT_CROP, &halfCrop);\n> > -\tif (!ret && halfCrop != maxCrop) {\n> > +\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> > +\tif (!ret && minCrop != maxCrop) {\n> >  \t\tfeatures_ |= Feature::InputCrop;\n> >\n> >  \t\tLOG(Converter, Info)\n> > --\n> > 2.43.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 62502C32AF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 03:48:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7738667FBB;\n\tTue, 17 Dec 2024 04:48:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5061061899\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 04:48:10 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:1a78:c005:412c:c675])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0D2943E;\n\tTue, 17 Dec 2024 04:47:31 +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=\"YrIEptsX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734407253;\n\tbh=d1x4otkHVgfRxsIruYTnSPX0i21w44cauXD7M7vBLkA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YrIEptsXGID6F/NmedUjK2SvZMCjtPKUrLfSBlMrzlcAN68jwYR7OzexzDtMjILTq\n\tWoMXhr1wL7OnY+TFrSwjfeMfbnKo1OGY9VK7415e9R7klr2E6W9ixylkyGTsN/Mvoo\n\tybpb7aLRGubXc5cdU9M8qb2uVzoloEz1wf1aaYr8=","Date":"Tue, 17 Dec 2024 12:48:03 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 07/20] libcamera: converter_v4l2_m2m: Refactor get\n\tcrop bounds code","Message-ID":"<Z2D0c8ZwsoLswrb-@pyrite.rasen.tech>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-8-stefan.klug@ideasonboard.com>\n\t<nzqsyuqaa57nisujetn2haqt7zr7gkzkudwyjrfvomcpzbqsjk@v2ghc6i5vx5g>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<nzqsyuqaa57nisujetn2haqt7zr7gkzkudwyjrfvomcpzbqsjk@v2ghc6i5vx5g>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]