[{"id":32779,"web_url":"https://patchwork.libcamera.org/comment/32779/","msgid":"<ec4m4fsehf4i5ihsxd2vbxq7rlerpl4hjqpdtxyp5gwh5i262c@mdu5fsxhihkp>","date":"2024-12-16T18:14:05","subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","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:50PM +0100, Stefan Klug wrote:\n> Query the crop bounds on the dewarper instead of the stream in case the\n> camera was not yet configured when updateControls() gets called. This\n> provides sane defaults for the controls.\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>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 89946b782854..56192451eb3c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1220,8 +1220,11 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>  \tControlInfoMap::Map controls;\n>\n>  \tif (dewarper_) {\n> -\t\tstd::pair<Rectangle, Rectangle> cropLimits =\n> -\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> +\t\tstd::pair<Rectangle, Rectangle> cropLimits;\n> +\t\tif (dewarper_->isConfigured(&data->mainPathStream_))\n> +\t\t\tcropLimits = dewarper_->inputCropBounds(&data->mainPathStream_);\n> +\t\telse\n> +\t\t\tcropLimits = dewarper_->inputCropBounds();\n\nNow I wonder if the right way shouldn't have been\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 * ....\n\n- *\n- * When called with an unconfigured \\a stream, this function returns a pair of\n- * null rectangles.\n+ * When called with an unconfigured \\a stream, this function returns\n+ * the coverter default crop rectangle\n+ *\n\nSo that you could even skip this patch completely (and probably the\nisConfigured() one too).\n\nGenerally, I don't like API that have too many implicit behaviours and\nprefer the ones where the caller has to chose the right overload to\ncall, so that the logic is clear in the calling place and is harder to\nmis-uses, hence I might be biased. I'll let you decide really, I have already\npulled you in too many directions enough.\n\n>\n>  \t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n>  \t\t\t\t\t\t\t      cropLimits.second,\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 7F792C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Dec 2024 18:14:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 90B9A67F97;\n\tMon, 16 Dec 2024 19:14:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A0667F7F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 19:14:09 +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 C32C9160;\n\tMon, 16 Dec 2024 19:13:32 +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=\"Lp4EAth2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734372812;\n\tbh=Z5EWSwoeXcABX+ZzoJkRgYn34ClyFKdOApcgC7PnzDw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Lp4EAth2yJrvfU+lbTew+eRwcrM0nHiz3Q6NRN1UctmmGRIGOabap+k20BX0W49z2\n\tDFtkRck6Sk1qu16WgNTyQu+8/iWqPSXsII2q2gCiXako4kKd1Q9dsszd16yz/ifX8n\n\t+F5gFf48iXqi9MNgv2q3sIPlx9ykWh5/16tQpAl4=","Date":"Mon, 16 Dec 2024 19:14:05 +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 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","Message-ID":"<ec4m4fsehf4i5ihsxd2vbxq7rlerpl4hjqpdtxyp5gwh5i262c@mdu5fsxhihkp>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-11-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-11-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":32790,"web_url":"https://patchwork.libcamera.org/comment/32790/","msgid":"<eulszx6hzsvaj36md6rxyabo6jtqguykumkrtvxx3osfqnljgb@yvvr22g7ds3c>","date":"2024-12-16T20:47:16","subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","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:14:05PM +0100, Jacopo Mondi wrote:\n> Hi Stefan\n> \n> On Mon, Dec 16, 2024 at 04:40:50PM +0100, Stefan Klug wrote:\n> > Query the crop bounds on the dewarper instead of the stream in case the\n> > camera was not yet configured when updateControls() gets called. This\n> > provides sane defaults for the controls.\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> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++--\n> >  1 file changed, 5 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 89946b782854..56192451eb3c 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1220,8 +1220,11 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> >  \tControlInfoMap::Map controls;\n> >\n> >  \tif (dewarper_) {\n> > -\t\tstd::pair<Rectangle, Rectangle> cropLimits =\n> > -\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> > +\t\tstd::pair<Rectangle, Rectangle> cropLimits;\n> > +\t\tif (dewarper_->isConfigured(&data->mainPathStream_))\n> > +\t\t\tcropLimits = dewarper_->inputCropBounds(&data->mainPathStream_);\n> > +\t\telse\n> > +\t\t\tcropLimits = dewarper_->inputCropBounds();\n> \n> Now I wonder if the right way shouldn't have been\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>  * ....\n> \n> - *\n> - * When called with an unconfigured \\a stream, this function returns a pair of\n> - * null rectangles.\n> + * When called with an unconfigured \\a stream, this function returns\n> + * the coverter default crop rectangle\n> + *\n\nThat is exactly the version from v2 - righti? :-)\n\n> \n> So that you could even skip this patch completely (and probably the\n> isConfigured() one too).\n> \n> Generally, I don't like API that have too many implicit behaviours and\n> prefer the ones where the caller has to chose the right overload to\n> call, so that the logic is clear in the calling place and is harder to\n> mis-uses, hence I might be biased. I'll let you decide really, I have already\n> pulled you in too many directions enough.\n\nI liked the version from v2 better at first. Now that the explicit one\nis there I actually like it because a) You get an error if you pass a\nnon configured stream and b) it is clearer from the using code that\ndifferent things can happen there. In the end both version would do the\njob. If no one else objects with a good argument I'd leave it like it is\nnow.\n\nCheers,\nStefan\n\n> \n> >\n> >  \t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> >  \t\t\t\t\t\t\t      cropLimits.second,\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 BB73BC32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Dec 2024 20:47:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DD4E67FAF;\n\tMon, 16 Dec 2024 21:47: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 BF49862C8B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 21:47:19 +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 DF1AA675;\n\tMon, 16 Dec 2024 21:46: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=\"dWyQ1Duc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734382003;\n\tbh=1iEA+BCjYu1uP1k+/sNzCKEfmXiS2B9nskW0al4vOYw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dWyQ1DucyBGd6z84eua+0P9EGhI/+bfgzzTKyJKug+pV9DwTKwVX8YCrmLf9kF1YE\n\tnUB4gz7KB+k20n/PnI1uiKi0R4yx3YlZFquGV+OH/6nf5QvEWf6MKWKcFg8LiCI0Xj\n\tjo1PDiJ+j8OLX0y+tzXYV8SuYZYSfEjED6l6ypVM=","Date":"Mon, 16 Dec 2024 21:47:16 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","Message-ID":"<eulszx6hzsvaj36md6rxyabo6jtqguykumkrtvxx3osfqnljgb@yvvr22g7ds3c>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-11-stefan.klug@ideasonboard.com>\n\t<ec4m4fsehf4i5ihsxd2vbxq7rlerpl4hjqpdtxyp5gwh5i262c@mdu5fsxhihkp>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<ec4m4fsehf4i5ihsxd2vbxq7rlerpl4hjqpdtxyp5gwh5i262c@mdu5fsxhihkp>","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":32800,"web_url":"https://patchwork.libcamera.org/comment/32800/","msgid":"<Z2D06xQ4KOvZDIcz@pyrite.rasen.tech>","date":"2024-12-17T03:50:03","subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","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:50PM +0100, Stefan Klug wrote:\n> Query the crop bounds on the dewarper instead of the stream in case the\n> camera was not yet configured when updateControls() gets called. This\n> provides sane defaults for the controls.\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>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 89946b782854..56192451eb3c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1220,8 +1220,11 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>  \tControlInfoMap::Map controls;\n>  \n>  \tif (dewarper_) {\n> -\t\tstd::pair<Rectangle, Rectangle> cropLimits =\n> -\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> +\t\tstd::pair<Rectangle, Rectangle> cropLimits;\n> +\t\tif (dewarper_->isConfigured(&data->mainPathStream_))\n> +\t\t\tcropLimits = dewarper_->inputCropBounds(&data->mainPathStream_);\n> +\t\telse\n> +\t\t\tcropLimits = dewarper_->inputCropBounds();\n\nAh that's how it comes together, neat.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n>  \n>  \t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n>  \t\t\t\t\t\t\t      cropLimits.second,\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 B18E8C32AF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 03:50:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6253067FBB;\n\tTue, 17 Dec 2024 04:50:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E926867FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 04:50: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 103EF55;\n\tTue, 17 Dec 2024 04:49:32 +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=\"QYsuJ7Nw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734407374;\n\tbh=/iBmqlVav+itDV+W6AgR8XfCMh2ORWtIdBndRiSYCDE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QYsuJ7Nw79vICgCVhbk8TlRNAUwLPyFSIcJ8/ge67R9WbZK8pn4gJdid9b9trwN4L\n\tRR/UtsCbcnODKEtzorLiUmbWc4XAned2/qH3VBVWybWIErFli8sTXsJD62eXXuVlXG\n\t6wPl3l0voiOyJcwUdgX2daNCOJsw6eT5Jvx/c4r4=","Date":"Tue, 17 Dec 2024 12:50:03 +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 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","Message-ID":"<Z2D06xQ4KOvZDIcz@pyrite.rasen.tech>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-11-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-11-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":32805,"web_url":"https://patchwork.libcamera.org/comment/32805/","msgid":"<r73pkp4er36sxwvywgfg64ifzrrtr4sgkvtn5e3pxsqdnoai7w@nqmbgheeopot>","date":"2024-12-17T08:00:13","subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","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 09:47:16PM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Thank you for the review.\n>\n> On Mon, Dec 16, 2024 at 07:14:05PM +0100, Jacopo Mondi wrote:\n> > Hi Stefan\n> >\n> > On Mon, Dec 16, 2024 at 04:40:50PM +0100, Stefan Klug wrote:\n> > > Query the crop bounds on the dewarper instead of the stream in case the\n> > > camera was not yet configured when updateControls() gets called. This\n> > > provides sane defaults for the controls.\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> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++--\n> > >  1 file changed, 5 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 89946b782854..56192451eb3c 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -1220,8 +1220,11 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > >  \tControlInfoMap::Map controls;\n> > >\n> > >  \tif (dewarper_) {\n> > > -\t\tstd::pair<Rectangle, Rectangle> cropLimits =\n> > > -\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> > > +\t\tstd::pair<Rectangle, Rectangle> cropLimits;\n> > > +\t\tif (dewarper_->isConfigured(&data->mainPathStream_))\n> > > +\t\t\tcropLimits = dewarper_->inputCropBounds(&data->mainPathStream_);\n> > > +\t\telse\n> > > +\t\t\tcropLimits = dewarper_->inputCropBounds();\n> >\n> > Now I wonder if the right way shouldn't have been\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> >  * ....\n> >\n> > - *\n> > - * When called with an unconfigured \\a stream, this function returns a pair of\n> > - * null rectangles.\n> > + * When called with an unconfigured \\a stream, this function returns\n> > + * the coverter default crop rectangle\n> > + *\n>\n> That is exactly the version from v2 - righti? :-)\n>\n> >\n> > So that you could even skip this patch completely (and probably the\n> > isConfigured() one too).\n> >\n> > Generally, I don't like API that have too many implicit behaviours and\n> > prefer the ones where the caller has to chose the right overload to\n> > call, so that the logic is clear in the calling place and is harder to\n> > mis-uses, hence I might be biased. I'll let you decide really, I have already\n> > pulled you in too many directions enough.\n>\n> I liked the version from v2 better at first. Now that the explicit one\n> is there I actually like it because a) You get an error if you pass a\n> non configured stream and b) it is clearer from the using code that\n> different things can happen there. In the end both version would do the\n> job. If no one else objects with a good argument I'd leave it like it is\n> now.\n\nFine with me of course.\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n>\n> Cheers,\n> Stefan\n>\n> >\n> > >\n> > >  \t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> > >  \t\t\t\t\t\t\t      cropLimits.second,\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 AF2E4C32AF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 08:00:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AC2F67FC4;\n\tTue, 17 Dec 2024 09:00:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D2FB67EEF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 09:00:17 +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 D70623E;\n\tTue, 17 Dec 2024 08:59:39 +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=\"e/143C1X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734422380;\n\tbh=BuRXt3sFzrqCiuZmqvivxzuGtbvEVgUANYDwSLPPTrc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=e/143C1XYu+03k3x9lxF+g1Rkil9lAtPVH10nJEAg6fpw9LT1DyfJKCcbJPm8Zx34\n\tP26SThfMJWoeVgxqOQTab8vknQLCLbfpdFRVzEAt7Xuvg+vuF4FBI7Ro2pX8LOixJj\n\tlY2i5U6sJtb0vCAdNwR6n2Wu9CsD16eeuo/QsZ3g=","Date":"Tue, 17 Dec 2024 09:00:13 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 10/20] pipeline: rkisp1: Query dewarper crop bounds if\n\tno stream configured","Message-ID":"<r73pkp4er36sxwvywgfg64ifzrrtr4sgkvtn5e3pxsqdnoai7w@nqmbgheeopot>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-11-stefan.klug@ideasonboard.com>\n\t<ec4m4fsehf4i5ihsxd2vbxq7rlerpl4hjqpdtxyp5gwh5i262c@mdu5fsxhihkp>\n\t<eulszx6hzsvaj36md6rxyabo6jtqguykumkrtvxx3osfqnljgb@yvvr22g7ds3c>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<eulszx6hzsvaj36md6rxyabo6jtqguykumkrtvxx3osfqnljgb@yvvr22g7ds3c>","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>"}}]