[{"id":32777,"web_url":"https://patchwork.libcamera.org/comment/32777/","msgid":"","date":"2024-12-16T18:08:02","subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","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:48PM +0100, Stefan Klug wrote:\n> The inputCropBounds_ member of the V4L2M2MConverter::Stream class is\n> only initialized after a V4L2M2MConverter::configure() call, when the\n> streams are initialized.\n>\n> However, the converter has crop limits that do not depend on the\n> configured Streams, and which are currently not accessible from the\n> class interface.\n\nHowever you seem to store the absolute limts computed at class\ncreation in inputCropBounds_ which gets updated at each configure()\ntime.\n\n>\n> Add a new inputCropBounds() function to the V4L2M2MConverter class\n> that allows to retrieve the converter crop limits before any stream\n> is configured.\n\nBut in this way, after a configuration, calling\n\n inputCropBounds()\n inputCropBounds(stream)\n\nreturn the same data.\n\nOr have I missed something ?\nWhen we discussed it I know I suggested to split the two functions\napart, but my thinking was about being able to return the default crop\nrectangle despite the currently configured format. Sorry if I wasn't\nclear. Or do you actually prefer this version ?\n\n>\n> This is particularly useful for pipelines to initialize controls and\n> properties and to implement validation before the Camera gets\n> configured.\n>\n> Signed-off-by: Stefan Klug \n>\n> ---\n>\n> Changes in v4:\n> - Split off from libcamera: converter_v4l2_m2m: Improve crop bounds\n> support\n> - Added separate inputCropBounds() function to the dewarper class\n>\n> Fuxup inputCrop()\n> ---\n> include/libcamera/internal/converter.h | 1 +\n> .../internal/converter/converter_v4l2_m2m.h | 2 ++\n> src/libcamera/converter.cpp | 13 +++++++++++++\n> src/libcamera/converter/converter_v4l2_m2m.cpp | 18 ++++++++++++------\n> 4 files changed, 28 insertions(+), 6 deletions(-)\n>\n> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> index ffbb6f345cd5..04187a2a96e8 100644\n> --- a/include/libcamera/internal/converter.h\n> +++ b/include/libcamera/internal/converter.h\n> @@ -66,6 +66,7 @@ public:\n> \t\t\t\t const std::map &outputs) = 0;\n>\n> \tvirtual int setInputCrop(const Stream *stream, Rectangle *rect) = 0;\n> +\tvirtual std::pair inputCropBounds() = 0;\n> \tvirtual std::pair inputCropBounds(const Stream *stream) = 0;\n>\n> \tSignal inputBufferReady;\n> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> index 9b8e43ff0b91..402a803959b9 100644\n> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> @@ -60,6 +60,7 @@ public:\n> \t\t\t const std::map &outputs) override;\n>\n> \tint setInputCrop(const Stream *stream, Rectangle *rect) override;\n> +\tstd::pair inputCropBounds() override { return inputCropBounds_; }\n> \tstd::pair inputCropBounds(const Stream *stream) override;\n>\n> private:\n> @@ -106,6 +107,7 @@ private:\n>\n> \tstd::map> streams_;\n> \tstd::map queue_;\n> +\tstd::pair inputCropBounds_;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> index 3a3f84344c5e..73c02fdcf4bb 100644\n> --- a/src/libcamera/converter.cpp\n> +++ b/src/libcamera/converter.cpp\n> @@ -185,6 +185,16 @@ Converter::~Converter()\n>\n> /**\n> * \\fn Converter::inputCropBounds()\n> + * \\brief Retrieve the crop bounds of the converter\n> + *\n> + * Retrieve the minimum and maximum crop bounds of the converter. This can be\n> + * used to query the crop bounds before configuring a stream.\n> + *\n> + * \\return A pair containing the minimum and maximum crop bound in that order\n> + */\n> +\n> +/**\n> + * \\fn Converter::inputCropBounds(const Stream *stream)\n> * \\brief Retrieve the crop bounds for \\a stream\n> * \\param[in] stream The output stream\n> *\n> @@ -195,6 +205,9 @@ Converter::~Converter()\n> * this function should be called after the \\a stream has been configured using\n> * configure().\n> *\n> + * When called with an unconfigured \\a stream, this function returns a pair of\n> + * null rectangles.\n> + *\n> * \\return A pair containing the minimum and maximum crop bound in that order\n> */\n>\n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index 8c341fe199f6..342aa32dab52 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -273,10 +273,9 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n> \t\treturn;\n> \t}\n>\n> -\tRectangle minCrop;\n> -\tRectangle maxCrop;\n> -\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> -\tif (!ret && minCrop != maxCrop) {\n> +\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> +\t\t\t inputCropBounds_.second);\n> +\tif (!ret && inputCropBounds_.first != inputCropBounds_.second) {\n> \t\tfeatures_ |= Feature::InputCrop;\n>\n> \t\tLOG(Converter, Info)\n> @@ -469,14 +468,21 @@ int V4L2M2MConverter::setInputCrop(const Stream *stream, Rectangle *rect)\n> }\n>\n> /**\n> - * \\copydoc libcamera::Converter::inputCropBounds\n> + * \\fn libcamera::V4L2M2MConverter::inputCropBounds()\n> + * \\copydoc libcamera::Converter::inputCropBounds()\n> + */\n> +\n> +/**\n> + * \\copydoc libcamera::Converter::inputCropBounds(const Stream *stream)\n> */\n> std::pair\n> V4L2M2MConverter::inputCropBounds(const Stream *stream)\n> {\n> \tauto iter = streams_.find(stream);\n> -\tif (iter == streams_.end())\n> +\tif (iter == streams_.end()) {\n> +\t\tLOG(Converter, Error) << \"Invalid output stream\";\n> \t\treturn {};\n> +\t}\n>\n> \treturn iter->second->inputCropBounds();\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 A4B21C326C\n\tfor ;\n\tMon, 16 Dec 2024 18:08:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 31A3967F98;\n\tMon, 16 Dec 2024 19:08:07 +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 7869A67F7F\n\tfor ;\n\tMon, 16 Dec 2024 19:08:05 +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 A34E9160;\n\tMon, 16 Dec 2024 19:07:28 +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=\"Vcu6GudN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734372448;\n\tbh=wXQ2dfqtFz3WZI5wkp7glGQlhoI3XHH4Q3wAB/dG5bg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vcu6GudN4OzXEGasA/pbqGejeq/6JbBOuMRK98nkFEbSPbYbXjHpk69yCqdLX59IO\n\t2NQ5uG4Ko4RM8tnKJ69ez3vQgTvWlC+15h134CMj8S2F3kJo7DmJGGR8WmCvJ1rCSu\n\txCzj5GwoG4X2cEXYjWmXrTgkrNhfK5mILDbthmDU=","Date":"Mon, 16 Dec 2024 19:08:02 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","Message-ID":"","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-9-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-9-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":32785,"web_url":"https://patchwork.libcamera.org/comment/32785/","msgid":"","date":"2024-12-16T19:53:08","subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","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 review. \n\nOn Mon, Dec 16, 2024 at 07:08:02PM +0100, Jacopo Mondi wrote:\n> Hi Stefan\n> \n> On Mon, Dec 16, 2024 at 04:40:48PM +0100, Stefan Klug wrote:\n> > The inputCropBounds_ member of the V4L2M2MConverter::Stream class is\n> > only initialized after a V4L2M2MConverter::configure() call, when the\n> > streams are initialized.\n> >\n> > However, the converter has crop limits that do not depend on the\n> > configured Streams, and which are currently not accessible from the\n> > class interface.\n> \n> However you seem to store the absolute limts computed at class\n> creation in inputCropBounds_ which gets updated at each configure()\n> time.\n> \n> >\n> > Add a new inputCropBounds() function to the V4L2M2MConverter class\n> > that allows to retrieve the converter crop limits before any stream\n> > is configured.\n> \n> But in this way, after a configuration, calling\n> \n> inputCropBounds()\n> inputCropBounds(stream)\n> \n> return the same data.\n> \n> Or have I missed something ?\n\nMaybe, let's see. The V4L2M2MConverter and the V4L2M2MStream both have a\ninputCropBounds_ member. The one on the converter is initialized at\nconverter construction time. The one on the stream is written when the\nstream is configured. So the above two line should/could return\ndifferent things. Does that explain your concern?\n\n> When we discussed it I know I suggested to split the two functions\n> apart, but my thinking was about being able to return the default crop\n> rectangle despite the currently configured format. Sorry if I wasn't\n> clear. Or do you actually prefer this version ?\n\nI think you were clear and I think it behaves the way you said :-)\n\nCheers,\nStefan\n\n> \n> >\n> > This is particularly useful for pipelines to initialize controls and\n> > properties and to implement validation before the Camera gets\n> > configured.\n> >\n> > Signed-off-by: Stefan Klug \n> >\n> > ---\n> >\n> > Changes in v4:\n> > - Split off from libcamera: converter_v4l2_m2m: Improve crop bounds\n> > support\n> > - Added separate inputCropBounds() function to the dewarper class\n> >\n> > Fuxup inputCrop()\n> > ---\n> > include/libcamera/internal/converter.h | 1 +\n> > .../internal/converter/converter_v4l2_m2m.h | 2 ++\n> > src/libcamera/converter.cpp | 13 +++++++++++++\n> > src/libcamera/converter/converter_v4l2_m2m.cpp | 18 ++++++++++++------\n> > 4 files changed, 28 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> > index ffbb6f345cd5..04187a2a96e8 100644\n> > --- a/include/libcamera/internal/converter.h\n> > +++ b/include/libcamera/internal/converter.h\n> > @@ -66,6 +66,7 @@ public:\n> > \t\t\t\t const std::map &outputs) = 0;\n> >\n> > \tvirtual int setInputCrop(const Stream *stream, Rectangle *rect) = 0;\n> > +\tvirtual std::pair inputCropBounds() = 0;\n> > \tvirtual std::pair inputCropBounds(const Stream *stream) = 0;\n> >\n> > \tSignal inputBufferReady;\n> > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > index 9b8e43ff0b91..402a803959b9 100644\n> > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > @@ -60,6 +60,7 @@ public:\n> > \t\t\t const std::map &outputs) override;\n> >\n> > \tint setInputCrop(const Stream *stream, Rectangle *rect) override;\n> > +\tstd::pair inputCropBounds() override { return inputCropBounds_; }\n> > \tstd::pair inputCropBounds(const Stream *stream) override;\n> >\n> > private:\n> > @@ -106,6 +107,7 @@ private:\n> >\n> > \tstd::map> streams_;\n> > \tstd::map queue_;\n> > +\tstd::pair inputCropBounds_;\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> > index 3a3f84344c5e..73c02fdcf4bb 100644\n> > --- a/src/libcamera/converter.cpp\n> > +++ b/src/libcamera/converter.cpp\n> > @@ -185,6 +185,16 @@ Converter::~Converter()\n> >\n> > /**\n> > * \\fn Converter::inputCropBounds()\n> > + * \\brief Retrieve the crop bounds of the converter\n> > + *\n> > + * Retrieve the minimum and maximum crop bounds of the converter. This can be\n> > + * used to query the crop bounds before configuring a stream.\n> > + *\n> > + * \\return A pair containing the minimum and maximum crop bound in that order\n> > + */\n> > +\n> > +/**\n> > + * \\fn Converter::inputCropBounds(const Stream *stream)\n> > * \\brief Retrieve the crop bounds for \\a stream\n> > * \\param[in] stream The output stream\n> > *\n> > @@ -195,6 +205,9 @@ Converter::~Converter()\n> > * this function should be called after the \\a stream has been configured using\n> > * configure().\n> > *\n> > + * When called with an unconfigured \\a stream, this function returns a pair of\n> > + * null rectangles.\n> > + *\n> > * \\return A pair containing the minimum and maximum crop bound in that order\n> > */\n> >\n> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > index 8c341fe199f6..342aa32dab52 100644\n> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > @@ -273,10 +273,9 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n> > \t\treturn;\n> > \t}\n> >\n> > -\tRectangle minCrop;\n> > -\tRectangle maxCrop;\n> > -\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> > -\tif (!ret && minCrop != maxCrop) {\n> > +\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> > +\t\t\t inputCropBounds_.second);\n> > +\tif (!ret && inputCropBounds_.first != inputCropBounds_.second) {\n> > \t\tfeatures_ |= Feature::InputCrop;\n> >\n> > \t\tLOG(Converter, Info)\n> > @@ -469,14 +468,21 @@ int V4L2M2MConverter::setInputCrop(const Stream *stream, Rectangle *rect)\n> > }\n> >\n> > /**\n> > - * \\copydoc libcamera::Converter::inputCropBounds\n> > + * \\fn libcamera::V4L2M2MConverter::inputCropBounds()\n> > + * \\copydoc libcamera::Converter::inputCropBounds()\n> > + */\n> > +\n> > +/**\n> > + * \\copydoc libcamera::Converter::inputCropBounds(const Stream *stream)\n> > */\n> > std::pair\n> > V4L2M2MConverter::inputCropBounds(const Stream *stream)\n> > {\n> > \tauto iter = streams_.find(stream);\n> > -\tif (iter == streams_.end())\n> > +\tif (iter == streams_.end()) {\n> > +\t\tLOG(Converter, Error) << \"Invalid output stream\";\n> > \t\treturn {};\n> > +\t}\n> >\n> > \treturn iter->second->inputCropBounds();\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 023C1C326C\n\tfor ;\n\tMon, 16 Dec 2024 19:53:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 34F0267FAB;\n\tMon, 16 Dec 2024 20:53:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFF5362C8B\n\tfor ;\n\tMon, 16 Dec 2024 20:53:11 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:15e9:bfc7:2fd9:3f8a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1670F160;\n\tMon, 16 Dec 2024 20:52:35 +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=\"nkkI8G4b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734378755;\n\tbh=Yc5l1RtSKtJm3xvC3TaoZ2i2LA4hNxDK6j04/POkaBE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nkkI8G4brc6V4W5DqLAWTQBS2CAHtdq6JxcolGAXlCbCbnM1b1KKwa10nhkZbEXUs\n\teMgib/O7pNV/W+80qSFJSDGddMAGlR9Pigs+B5IzR3Qb5C+OgDLbMGEjsbx7sZgXbW\n\t9RXmR2ae53J6hpARrI35Vlb8Yud3kbuV9gSL9vcE=","Date":"Mon, 16 Dec 2024 20:53:08 +0100","From":"Stefan Klug ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","Message-ID":"","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-9-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":32799,"web_url":"https://patchwork.libcamera.org/comment/32799/","msgid":"","date":"2024-12-17T03:49:06","subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","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:48PM +0100, Stefan Klug wrote:\n> The inputCropBounds_ member of the V4L2M2MConverter::Stream class is\n> only initialized after a V4L2M2MConverter::configure() call, when the\n> streams are initialized.\n> \n> However, the converter has crop limits that do not depend on the\n> configured Streams, and which are currently not accessible from the\n> class interface.\n> \n> Add a new inputCropBounds() function to the V4L2M2MConverter class\n> that allows to retrieve the converter crop limits before any stream\n> is configured.\n> \n> This is particularly useful for pipelines to initialize controls and\n> properties and to implement validation before the Camera gets\n> configured.\n> \n> Signed-off-by: Stefan Klug \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v4:\n> - Split off from libcamera: converter_v4l2_m2m: Improve crop bounds\n> support\n> - Added separate inputCropBounds() function to the dewarper class\n> \n> Fuxup inputCrop()\n> ---\n> include/libcamera/internal/converter.h | 1 +\n> .../internal/converter/converter_v4l2_m2m.h | 2 ++\n> src/libcamera/converter.cpp | 13 +++++++++++++\n> src/libcamera/converter/converter_v4l2_m2m.cpp | 18 ++++++++++++------\n> 4 files changed, 28 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> index ffbb6f345cd5..04187a2a96e8 100644\n> --- a/include/libcamera/internal/converter.h\n> +++ b/include/libcamera/internal/converter.h\n> @@ -66,6 +66,7 @@ public:\n> \t\t\t\t const std::map &outputs) = 0;\n> \n> \tvirtual int setInputCrop(const Stream *stream, Rectangle *rect) = 0;\n> +\tvirtual std::pair inputCropBounds() = 0;\n> \tvirtual std::pair inputCropBounds(const Stream *stream) = 0;\n> \n> \tSignal inputBufferReady;\n> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> index 9b8e43ff0b91..402a803959b9 100644\n> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> @@ -60,6 +60,7 @@ public:\n> \t\t\t const std::map &outputs) override;\n> \n> \tint setInputCrop(const Stream *stream, Rectangle *rect) override;\n> +\tstd::pair inputCropBounds() override { return inputCropBounds_; }\n> \tstd::pair inputCropBounds(const Stream *stream) override;\n> \n> private:\n> @@ -106,6 +107,7 @@ private:\n> \n> \tstd::map> streams_;\n> \tstd::map queue_;\n> +\tstd::pair inputCropBounds_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> index 3a3f84344c5e..73c02fdcf4bb 100644\n> --- a/src/libcamera/converter.cpp\n> +++ b/src/libcamera/converter.cpp\n> @@ -185,6 +185,16 @@ Converter::~Converter()\n> \n> /**\n> * \\fn Converter::inputCropBounds()\n> + * \\brief Retrieve the crop bounds of the converter\n> + *\n> + * Retrieve the minimum and maximum crop bounds of the converter. This can be\n> + * used to query the crop bounds before configuring a stream.\n> + *\n> + * \\return A pair containing the minimum and maximum crop bound in that order\n> + */\n> +\n> +/**\n> + * \\fn Converter::inputCropBounds(const Stream *stream)\n> * \\brief Retrieve the crop bounds for \\a stream\n> * \\param[in] stream The output stream\n> *\n> @@ -195,6 +205,9 @@ Converter::~Converter()\n> * this function should be called after the \\a stream has been configured using\n> * configure().\n> *\n> + * When called with an unconfigured \\a stream, this function returns a pair of\n> + * null rectangles.\n> + *\n> * \\return A pair containing the minimum and maximum crop bound in that order\n> */\n> \n> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> index 8c341fe199f6..342aa32dab52 100644\n> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> @@ -273,10 +273,9 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n> \t\treturn;\n> \t}\n> \n> -\tRectangle minCrop;\n> -\tRectangle maxCrop;\n> -\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> -\tif (!ret && minCrop != maxCrop) {\n> +\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> +\t\t\t inputCropBounds_.second);\n> +\tif (!ret && inputCropBounds_.first != inputCropBounds_.second) {\n> \t\tfeatures_ |= Feature::InputCrop;\n> \n> \t\tLOG(Converter, Info)\n> @@ -469,14 +468,21 @@ int V4L2M2MConverter::setInputCrop(const Stream *stream, Rectangle *rect)\n> }\n> \n> /**\n> - * \\copydoc libcamera::Converter::inputCropBounds\n> + * \\fn libcamera::V4L2M2MConverter::inputCropBounds()\n> + * \\copydoc libcamera::Converter::inputCropBounds()\n> + */\n> +\n> +/**\n> + * \\copydoc libcamera::Converter::inputCropBounds(const Stream *stream)\n> */\n> std::pair\n> V4L2M2MConverter::inputCropBounds(const Stream *stream)\n> {\n> \tauto iter = streams_.find(stream);\n> -\tif (iter == streams_.end())\n> +\tif (iter == streams_.end()) {\n> +\t\tLOG(Converter, Error) << \"Invalid output stream\";\n> \t\treturn {};\n> +\t}\n> \n> \treturn iter->second->inputCropBounds();\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 BEEBDC32AF\n\tfor ;\n\tTue, 17 Dec 2024 03:49:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7618667FBB;\n\tTue, 17 Dec 2024 04:49:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B2F067FB1\n\tfor ;\n\tTue, 17 Dec 2024 04:49:12 +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 A3DE23E;\n\tTue, 17 Dec 2024 04:48:34 +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=\"IdHaqHVK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734407315;\n\tbh=94TYvamrSizXtfDMlYLqjqFFe5SQIwMKYUnLlMZsGd4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IdHaqHVKbRDufNjcpsvo3SHUwv3h+s8219/xSzBWZYryBINST0JFU968KfG6At9dt\n\tbEItoTuUvz4NNhNb+pCHFc0yYzEa8NmVqAYhcyAbU6GldobVVTZxf1ckoQVSnFhSwh\n\tGB0ciEN3lzh32ggxgIVjCZSs8rWLpqfWyNkg6WSw=","Date":"Tue, 17 Dec 2024 12:49:06 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","Message-ID":"","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-9-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-9-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":32803,"web_url":"https://patchwork.libcamera.org/comment/32803/","msgid":"","date":"2024-12-17T07:53:16","subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","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 08:53:08PM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Thank you for the review.\n>\n> On Mon, Dec 16, 2024 at 07:08:02PM +0100, Jacopo Mondi wrote:\n> > Hi Stefan\n> >\n> > On Mon, Dec 16, 2024 at 04:40:48PM +0100, Stefan Klug wrote:\n> > > The inputCropBounds_ member of the V4L2M2MConverter::Stream class is\n> > > only initialized after a V4L2M2MConverter::configure() call, when the\n> > > streams are initialized.\n> > >\n> > > However, the converter has crop limits that do not depend on the\n> > > configured Streams, and which are currently not accessible from the\n> > > class interface.\n> >\n> > However you seem to store the absolute limts computed at class\n> > creation in inputCropBounds_ which gets updated at each configure()\n> > time.\n> >\n> > >\n> > > Add a new inputCropBounds() function to the V4L2M2MConverter class\n> > > that allows to retrieve the converter crop limits before any stream\n> > > is configured.\n> >\n> > But in this way, after a configuration, calling\n> >\n> > inputCropBounds()\n> > inputCropBounds(stream)\n> >\n> > return the same data.\n> >\n> > Or have I missed something ?\n>\n> Maybe, let's see. The V4L2M2MConverter and the V4L2M2MStream both have a\n> inputCropBounds_ member. The one on the converter is initialized at\n> converter construction time. The one on the stream is written when the\n> stream is configured. So the above two line should/could return\n> different things. Does that explain your concern?\n>\n> > When we discussed it I know I suggested to split the two functions\n> > apart, but my thinking was about being able to return the default crop\n> > rectangle despite the currently configured format. Sorry if I wasn't\n> > clear. Or do you actually prefer this version ?\n>\n> I think you were clear and I think it behaves the way you said :-)\n\nOh crabs, we have two inputCropBounds_ members indeed /0\\\n\nOk, then this makes it easy!\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> Cheers,\n> Stefan\n>\n> >\n> > >\n> > > This is particularly useful for pipelines to initialize controls and\n> > > properties and to implement validation before the Camera gets\n> > > configured.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > >\n> > > ---\n> > >\n> > > Changes in v4:\n> > > - Split off from libcamera: converter_v4l2_m2m: Improve crop bounds\n> > > support\n> > > - Added separate inputCropBounds() function to the dewarper class\n> > >\n> > > Fuxup inputCrop()\n> > > ---\n> > > include/libcamera/internal/converter.h | 1 +\n> > > .../internal/converter/converter_v4l2_m2m.h | 2 ++\n> > > src/libcamera/converter.cpp | 13 +++++++++++++\n> > > src/libcamera/converter/converter_v4l2_m2m.cpp | 18 ++++++++++++------\n> > > 4 files changed, 28 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\n> > > index ffbb6f345cd5..04187a2a96e8 100644\n> > > --- a/include/libcamera/internal/converter.h\n> > > +++ b/include/libcamera/internal/converter.h\n> > > @@ -66,6 +66,7 @@ public:\n> > > \t\t\t\t const std::map &outputs) = 0;\n> > >\n> > > \tvirtual int setInputCrop(const Stream *stream, Rectangle *rect) = 0;\n> > > +\tvirtual std::pair inputCropBounds() = 0;\n> > > \tvirtual std::pair inputCropBounds(const Stream *stream) = 0;\n> > >\n> > > \tSignal inputBufferReady;\n> > > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > index 9b8e43ff0b91..402a803959b9 100644\n> > > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n> > > @@ -60,6 +60,7 @@ public:\n> > > \t\t\t const std::map &outputs) override;\n> > >\n> > > \tint setInputCrop(const Stream *stream, Rectangle *rect) override;\n> > > +\tstd::pair inputCropBounds() override { return inputCropBounds_; }\n> > > \tstd::pair inputCropBounds(const Stream *stream) override;\n> > >\n> > > private:\n> > > @@ -106,6 +107,7 @@ private:\n> > >\n> > > \tstd::map> streams_;\n> > > \tstd::map queue_;\n> > > +\tstd::pair inputCropBounds_;\n> > > };\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\n> > > index 3a3f84344c5e..73c02fdcf4bb 100644\n> > > --- a/src/libcamera/converter.cpp\n> > > +++ b/src/libcamera/converter.cpp\n> > > @@ -185,6 +185,16 @@ Converter::~Converter()\n> > >\n> > > /**\n> > > * \\fn Converter::inputCropBounds()\n> > > + * \\brief Retrieve the crop bounds of the converter\n> > > + *\n> > > + * Retrieve the minimum and maximum crop bounds of the converter. This can be\n> > > + * used to query the crop bounds before configuring a stream.\n> > > + *\n> > > + * \\return A pair containing the minimum and maximum crop bound in that order\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Converter::inputCropBounds(const Stream *stream)\n> > > * \\brief Retrieve the crop bounds for \\a stream\n> > > * \\param[in] stream The output stream\n> > > *\n> > > @@ -195,6 +205,9 @@ Converter::~Converter()\n> > > * this function should be called after the \\a stream has been configured using\n> > > * configure().\n> > > *\n> > > + * When called with an unconfigured \\a stream, this function returns a pair of\n> > > + * null rectangles.\n> > > + *\n> > > * \\return A pair containing the minimum and maximum crop bound in that order\n> > > */\n> > >\n> > > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > index 8c341fe199f6..342aa32dab52 100644\n> > > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n> > > @@ -273,10 +273,9 @@ V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)\n> > > \t\treturn;\n> > > \t}\n> > >\n> > > -\tRectangle minCrop;\n> > > -\tRectangle maxCrop;\n> > > -\tret = getCropBounds(m2m_->output(), minCrop, maxCrop);\n> > > -\tif (!ret && minCrop != maxCrop) {\n> > > +\tret = getCropBounds(m2m_->output(), inputCropBounds_.first,\n> > > +\t\t\t inputCropBounds_.second);\n> > > +\tif (!ret && inputCropBounds_.first != inputCropBounds_.second) {\n> > > \t\tfeatures_ |= Feature::InputCrop;\n> > >\n> > > \t\tLOG(Converter, Info)\n> > > @@ -469,14 +468,21 @@ int V4L2M2MConverter::setInputCrop(const Stream *stream, Rectangle *rect)\n> > > }\n> > >\n> > > /**\n> > > - * \\copydoc libcamera::Converter::inputCropBounds\n> > > + * \\fn libcamera::V4L2M2MConverter::inputCropBounds()\n> > > + * \\copydoc libcamera::Converter::inputCropBounds()\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::Converter::inputCropBounds(const Stream *stream)\n> > > */\n> > > std::pair\n> > > V4L2M2MConverter::inputCropBounds(const Stream *stream)\n> > > {\n> > > \tauto iter = streams_.find(stream);\n> > > -\tif (iter == streams_.end())\n> > > +\tif (iter == streams_.end()) {\n> > > +\t\tLOG(Converter, Error) << \"Invalid output stream\";\n> > > \t\treturn {};\n> > > +\t}\n> > >\n> > > \treturn iter->second->inputCropBounds();\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 AA434C32F6\n\tfor ;\n\tTue, 17 Dec 2024 07:53:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFD9367FBB;\n\tTue, 17 Dec 2024 08:53:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FFE961899\n\tfor ;\n\tTue, 17 Dec 2024 08:53: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 006063E;\n\tTue, 17 Dec 2024 08:52:42 +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=\"aVbD3DeE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734421963;\n\tbh=0I95lPdBw5qXkVuhzlWzgUdewctGUakRdNtlLhDImPA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aVbD3DeE1BljOYCGDjMs3Ki5nkm7gd/dTT1uHWGyFCH/fc3Uxy25Hr7Y/mtylu6Y+\n\thR7XbjNsVkW+jBXYqiYGMaTTm9PSlwplZF8h0jBzzkMHMHAhpYJ6PxNQ5Il+z+Xc+p\n\tQ8/NHD9jTKsy/9dYXXtbZfWGfuik6dYjV2iujFNc=","Date":"Tue, 17 Dec 2024 08:53:16 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 08/20] libcamera: converter: Add function to query\n\tcrop bounds","Message-ID":"","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-9-stefan.klug@ideasonboard.com>\n\t\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\" "}}]