[{"id":31972,"web_url":"https://patchwork.libcamera.org/comment/31972/","msgid":"<04ae3f0b-c2f8-4553-9b49-302cc638c0c7@ideasonboard.com>","date":"2024-10-31T05:37:00","subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Stefan\n\nOn 30/10/24 10:04 pm, Stefan Klug wrote:\n> The target crop rectangle is initialized with the crop of the default\n> sensor mode. This is incorrect when a different sensor mode gets\n> selected. Fix that by updating the crop rectangle when changing the\n> sensor mode.\n>\n> Signed-off-by: Stefan Klug \n> ---\n> drivers/media/i2c/imx283.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c\n> index 3174d5ffd2d7..c8863c9e0ccf 100644\n> --- a/drivers/media/i2c/imx283.c\n> +++ b/drivers/media/i2c/imx283.c\n> @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> \t\t\t\t struct v4l2_subdev_state *sd_state,\n> \t\t\t\t struct v4l2_subdev_format *fmt)\n> {\n> +\tstruct v4l2_rect *crop;\n> \tstruct v4l2_mbus_framefmt *format;\n> \tconst struct imx283_mode *mode;\n> \tstruct imx283 *imx283 = to_imx283(sd);\n> @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> \n> \t*format = fmt->format;\n> \n> +\tcrop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD);\n> +\t*crop = mode->crop;\n> +\n\nOne thing to note, is the crop for binning modes.\n\nDo you need to report\n\n     mode->crop.width / mode->hbin_ratio\n     mode->crop.height / mode->vbin_ratio\n\nfor those modes?\n\n> \treturn 0;\n> }\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 65FBCC3237\n\tfor ;\n\tThu, 31 Oct 2024 05:37:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DEC76539A;\n\tThu, 31 Oct 2024 06:37:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B10560396\n\tfor ;\n\tThu, 31 Oct 2024 06:37:05 +0100 (CET)","from [IPV6:2405:201:2015:f873:c173:4b:4a04:3a21] (unknown\n\t[IPv6:2405:201:2015:f873:c173:4b:4a04:3a21])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4574C842;\n\tThu, 31 Oct 2024 06:37:00 +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=\"OKpE+t+q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730353021;\n\tbh=8ewBrgbIPlY7S598KZ4WcC1h6n5PO4F3mmIRKZ6AtM4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=OKpE+t+qPMQOYoB57FJnYTYtjRaHdMyIEI4/ljy4cx0jWoNQ1VHVpXMdqkfJkz1Er\n\tYYeXe4r7f1x4AA3BZa+JW6jYE+/o3hJXIvzhiN74lxVEOli1gT99J3Xdt1dUwBm1FQ\n\tjHKHa2L5Zi5lMTqntp980qIGnqWWknJVdMLTasbM=","Message-ID":"<04ae3f0b-c2f8-4553-9b49-302cc638c0c7@ideasonboard.com>","Date":"Thu, 31 Oct 2024 11:07:00 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Cc":"Sakari Ailus ,\n\tKieran Bingham ,\n\tMauro Carvalho Chehab ","References":"<20241030163439.245035-1-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20241030163439.245035-1-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31976,"web_url":"https://patchwork.libcamera.org/comment/31976/","msgid":"","date":"2024-10-31T09:13:13","subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Oct 31, 2024 at 11:07:00AM +0530, Umang Jain wrote:\n> Hi Stefan\n> \n> On 30/10/24 10:04 pm, Stefan Klug wrote:\n> > The target crop rectangle is initialized with the crop of the default\n> > sensor mode. This is incorrect when a different sensor mode gets\n> > selected. Fix that by updating the crop rectangle when changing the\n> > sensor mode.\n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > drivers/media/i2c/imx283.c | 4 ++++\n> > 1 file changed, 4 insertions(+)\n> > \n> > diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c\n> > index 3174d5ffd2d7..c8863c9e0ccf 100644\n> > --- a/drivers/media/i2c/imx283.c\n> > +++ b/drivers/media/i2c/imx283.c\n> > @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> > \t\t\t\t struct v4l2_subdev_state *sd_state,\n> > \t\t\t\t struct v4l2_subdev_format *fmt)\n> > {\n> > +\tstruct v4l2_rect *crop;\n> > \tstruct v4l2_mbus_framefmt *format;\n> > \tconst struct imx283_mode *mode;\n> > \tstruct imx283 *imx283 = to_imx283(sd);\n> > @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> > \t*format = fmt->format;\n> > +\tcrop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD);\n> > +\t*crop = mode->crop;\n> > +\n> \n> One thing to note, is the crop for binning modes.\n> \n> Do you need to report\n> \n>     mode->crop.width / mode->hbin_ratio\n>     mode->crop.height / mode->vbin_ratio\n> \n> for those modes?\n\nGood point. I was naively assuming that it has the same semantics as we\nuse for ScalerCrop in libcamera where it is explicitly stated that the\ncoordinates are in sensor pixels without binning. That has the added\nadvantage that we can deduce the binning factor from TGT_CROP and the\nactual output size. However I couldn't find a precise specification for\nthat in the linux docs. \n\nMaybe Sakari or Laurent have a definiteve answer there?\n\nBest regards,\nStefan\n\n> \n> > \treturn 0;\n> > }\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 505C5C31E9\n\tfor ;\n\tThu, 31 Oct 2024 09:13:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 01488653A9;\n\tThu, 31 Oct 2024 10:13:20 +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 944B76539F\n\tfor ;\n\tThu, 31 Oct 2024 10:13:17 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:c433:a011:b9cf:d32c])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3AD8D842;\n\tThu, 31 Oct 2024 10:13:13 +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=\"rj7zytZF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730365993;\n\tbh=gqnaDtZtuISCM8zTQgzltHmUGJlE1EtGHbAL+f5QyvY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rj7zytZFTQ42WfgEkXHKy4HFyxdM9Hq1aTmwWll4DJ7Ef8BjlMeS7EBlHyuiSkTm4\n\t5/hc8hV39g36KoSRArCTygpEjYWmM+kcF4OOy4Y3w4936Bj38dNTn+XsnBUgI9/i6z\n\tkT/G4th/M+9kE1Ylm/v0fom1TwJU+b1YwUTUE5Ls=","Date":"Thu, 31 Oct 2024 10:13:13 +0100","From":"Stefan Klug ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org, \n\tlinux-kernel@vger.kernel.org,\n\tSakari Ailus , \n\tKieran Bingham ,\n\tMauro Carvalho Chehab , \n\tLaurent Pinchart ","Subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","Message-ID":"","References":"<20241030163439.245035-1-stefan.klug@ideasonboard.com>\n\t<04ae3f0b-c2f8-4553-9b49-302cc638c0c7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<04ae3f0b-c2f8-4553-9b49-302cc638c0c7@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":31978,"web_url":"https://patchwork.libcamera.org/comment/31978/","msgid":"<20241031093859.GB2473@pendragon.ideasonboard.com>","date":"2024-10-31T09:38:59","subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Oct 31, 2024 at 10:13:13AM +0100, Stefan Klug wrote:\n> On Thu, Oct 31, 2024 at 11:07:00AM +0530, Umang Jain wrote:\n> > On 30/10/24 10:04 pm, Stefan Klug wrote:\n> > > The target crop rectangle is initialized with the crop of the default\n> > > sensor mode. This is incorrect when a different sensor mode gets\n> > > selected. Fix that by updating the crop rectangle when changing the\n> > > sensor mode.\n> > > \n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > drivers/media/i2c/imx283.c | 4 ++++\n> > > 1 file changed, 4 insertions(+)\n> > > \n> > > diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c\n> > > index 3174d5ffd2d7..c8863c9e0ccf 100644\n> > > --- a/drivers/media/i2c/imx283.c\n> > > +++ b/drivers/media/i2c/imx283.c\n> > > @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> > > \t\t\t\t struct v4l2_subdev_state *sd_state,\n> > > \t\t\t\t struct v4l2_subdev_format *fmt)\n> > > {\n> > > +\tstruct v4l2_rect *crop;\n> > > \tstruct v4l2_mbus_framefmt *format;\n> > > \tconst struct imx283_mode *mode;\n> > > \tstruct imx283 *imx283 = to_imx283(sd);\n> > > @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,\n> > > \t*format = fmt->format;\n> > > +\tcrop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD);\n> > > +\t*crop = mode->crop;\n> > > +\n> > \n> > One thing to note, is the crop for binning modes.\n> > \n> > Do you need to report\n> > \n> >     mode->crop.width / mode->hbin_ratio\n> >     mode->crop.height / mode->vbin_ratio\n> > \n> > for those modes?\n> \n> Good point. I was naively assuming that it has the same semantics as we\n> use for ScalerCrop in libcamera where it is explicitly stated that the\n> coordinates are in sensor pixels without binning. That has the added\n> advantage that we can deduce the binning factor from TGT_CROP and the\n> actual output size. However I couldn't find a precise specification for\n> that in the linux docs. \n> \n> Maybe Sakari or Laurent have a definiteve answer there?\n\nThis is not standardized in V4L2, and different drivers implement\ndifferent semantics. There's an ongoing effort to fix this, see\nhttps://lore.kernel.org/r/20241011075535.588140-1-sakari.ailus@linux.intel.com.\nReviews are appreciated :-)\n\n> > > \treturn 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 A9C04C3237\n\tfor ;\n\tThu, 31 Oct 2024 09:39:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD70F6539F;\n\tThu, 31 Oct 2024 10:39: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 6E492618BF\n\tfor ;\n\tThu, 31 Oct 2024 10:39:07 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D7AE29EC;\n\tThu, 31 Oct 2024 10:39:02 +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=\"ESlbbYyo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730367543;\n\tbh=LRz1h79bJN4EYNQ1JU/SY7GS2AoN+ilnUVUKkNbp1+s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ESlbbYyoYhKGDZUFCQhHxFQx28YfAGaDtvJf0E5BG6Us8nDoBVE1sLhzB/C0wt8jH\n\tX8PVJ5860rPxXXg0u7kb5c8zxnMJzvogtvmhE6iGXC68+/MFjy1QJe3RzaIdJdYU3V\n\twqFm3OheKqq96zZZCjGxny3uErcw6VWWdsj321mE=","Date":"Thu, 31 Oct 2024 11:38:59 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org,\n\tSakari Ailus , \n\tKieran Bingham ,\n\tMauro Carvalho Chehab ","Subject":"Re: [PATCH] media: imx283: Report correct V4L2_SEL_TGT_CROP","Message-ID":"<20241031093859.GB2473@pendragon.ideasonboard.com>","References":"<20241030163439.245035-1-stefan.klug@ideasonboard.com>\n\t<04ae3f0b-c2f8-4553-9b49-302cc638c0c7@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","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\" "}}]