[{"id":4702,"web_url":"https://patchwork.libcamera.org/comment/4702/","msgid":"<20200501152415.GB2520266@oden.dyn.berto.se>","date":"2020-05-01T15:24:15","subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2020-05-01 17:16:51 +0200, Jacopo Mondi wrote:\n> Since the Linux kernel commit:\n> 69e39d40587b (\"media: vimc: Implement get/set selection in sink\")\n> the crop rectangle on the VIMC scaler sink pad needs to be\n> reset to match the requested format to avoid hitting a pipeline format\n> mis-configurations when the applied format is larger than the one\n> set in a previous capture session.\n> \n> As the V4L2 specification is not clear on what the correct behaviour\n> is, if the crop rectangle should be reset automatically or not on a\n> set_fmt operation, momentary fix the pipeline handler to please the\n> driver and manually reset the crop rectangle.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> \n> Tested on v5.6, please test on older kernel releases.\n> \n> ---\n> src/libcamera/pipeline/vimc/vimc.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index ccfd7f86d158..339d1cf653fb 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -224,6 +224,16 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tRectangle crop = {\n> +\t\t.x = 0,\n> +\t\t.y = 0,\n> +\t\t.width = subformat.size.width,\n> +\t\t.height = subformat.size.height,\n> +\t};\n> +\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> +\tif (ret && ret != -ENOTTY)\n> +\t\treturn ret;\n> +\n> \tsubformat.size = cfg.size;\n> \tret = data->scaler_->setFormat(1, &subformat);\n> \tif (ret)\n> --\n> 2.26.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C6BB8603F3\n\tfor ;\n\tFri, 1 May 2020 17:24:17 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id l11so4116789lfc.5\n\tfor ;\n\tFri, 01 May 2020 08:24:17 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tn14sm2397694lfe.31.2020.05.01.08.24.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 May 2020 08:24:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"0xPAD4vR\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=Jm7elecgnJwFLwphQua7zddJuWgsb1SYB/hSdWDqoIU=;\n\tb=0xPAD4vRVor32zd5i3m6EDD0Y8AHK0W/hbVYbGvRPaawl9DoLX/ANzU9O5idp0Htdu\n\t8n+ETNglw04VgRn1VFp8WFnHOXTb4D9QzpRwlEoNiKr7xDbCQXQmmDWEVr+JpMijg1Jz\n\tusxs5E1A19FE/A6fChY9quLlscF69vlaxEfr35FNKLegHBJ93+Y1vs39D7UVBz3HF3yP\n\tZvKmQU/NRrpoIYuG8nU5xAYq94j9A1S8v4XeczWlO1BcovwIuPLQESXwg5E+sHDF/0xD\n\tOzMBBhTHoFpqlA4ChA1ahShSjcznfnfS/2HXeIhXNa3CKOe1101MS4KwSUCxiDRqm1k/\n\tztlg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=Jm7elecgnJwFLwphQua7zddJuWgsb1SYB/hSdWDqoIU=;\n\tb=HgslJOOlrhlRPH/UtRKeH4SbG0NXm5Nw3muAso/cEh9Iahq1yz7Tbx8XCIYrxf8ZSI\n\tO2Ip8wAoqdmcn0yiqcyDuOgFUxvEAG5tVdeYbaXM1meVQ9naksud/ihiOVn5F6YiJowI\n\t27hpwsrGq15UUZwiOtayjXAMy93pw26R3iZS9Lm5LpRANIzpCQpXyvS9Y5ouYP8FZ7OB\n\tL21R5RFLyW2/fNKvX3nXbuwVzXjorAxiG3gs4esjdhe3VMHdyM8RhMrMiBrADGzytQMX\n\tQRrV80V6HhHAiPZDOuBzNBtlwEykpUIKnXD2Dr6KBCoGRSdrsZl7sJ9qt6j58+0OoVYY\n\tA2iw==","X-Gm-Message-State":"AGi0PualrVS9iMeZLhTDSNBREfQO0bFetueMS09MQInX1rsCDIS/YUtu\n\t3NxuXiHmv58rg9O9Q9gvq3T5tA==","X-Google-Smtp-Source":"APiQypKSqH2Em6CBi1iF2pRCzTGXU4hfn0ooyOdN4mfgS9HYLbX/YxKdSHtWW6SdRh/ZbNZCsslQBQ==","X-Received":"by 2002:a19:f719:: with SMTP id z25mr2994688lfe.63.1588346657059;\n\tFri, 01 May 2020 08:24:17 -0700 (PDT)","Date":"Fri, 1 May 2020 17:24:15 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501152415.GB2520266@oden.dyn.berto.se>","References":"<20200501151651.864358-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200501151651.864358-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","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","X-List-Received-Date":"Fri, 01 May 2020 15:24:18 -0000"}},{"id":4705,"web_url":"https://patchwork.libcamera.org/comment/4705/","msgid":"<20200501161909.GJ5951@pendragon.ideasonboard.com>","date":"2020-05-01T16:19:09","subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, May 01, 2020 at 05:16:51PM +0200, Jacopo Mondi wrote:\n> Since the Linux kernel commit:\n> 69e39d40587b (\"media: vimc: Implement get/set selection in sink\")\n> the crop rectangle on the VIMC scaler sink pad needs to be\n> reset to match the requested format to avoid hitting a pipeline format\n> mis-configurations when the applied format is larger than the one\n\ns/mis-configurations/misconfiguration/\n\n> set in a previous capture session.\n> \n> As the V4L2 specification is not clear on what the correct behaviour\n> is, if the crop rectangle should be reset automatically or not on a\n> set_fmt operation, momentary fix the pipeline handler to please the\n> driver and manually reset the crop rectangle.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> \n> Tested on v5.6, please test on older kernel releases.\n\nI'll test it now.\n\n> \n> ---\n> src/libcamera/pipeline/vimc/vimc.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index ccfd7f86d158..339d1cf653fb 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -224,6 +224,16 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tRectangle crop = {\n> +\t\t.x = 0,\n> +\t\t.y = 0,\n> +\t\t.width = subformat.size.width,\n> +\t\t.height = subformat.size.height,\n> +\t};\n> +\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> +\tif (ret && ret != -ENOTTY)\n> +\t\treturn ret;\n> +\n> \tsubformat.size = cfg.size;\n> \tret = data->scaler_->setFormat(1, &subformat);\n> \tif (ret)","headers":{"Return-Path":"","Received":["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 BEE1A603F3\n\tfor ;\n\tFri, 1 May 2020 18:19:12 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 463EA72C;\n\tFri, 1 May 2020 18:19:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rcktA6jL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588349952;\n\tbh=USdW2Z4KqT8zImhNbcGsRUxHbthnammDDXrF+LP/JRU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rcktA6jL0zkQZTKRYm5UWiNP2v7tVyZC0UBtxYXL8psyxvnr4qrslBRly5V3dMN8P\n\t1qY19resbMlvF3RsUY17lPZj+bZJy8COVNyZkINEhFKDhvDWC/y/gf5tyv3DVXBqgl\n\tdQUxyUulUjR+l74PIGxY6ZLilNThGzYqfoH/Qvr0=","Date":"Fri, 1 May 2020 19:19:09 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501161909.GJ5951@pendragon.ideasonboard.com>","References":"<20200501151651.864358-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200501151651.864358-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","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","X-List-Received-Date":"Fri, 01 May 2020 16:19:13 -0000"}},{"id":4706,"web_url":"https://patchwork.libcamera.org/comment/4706/","msgid":"<20200501164356.GK5951@pendragon.ideasonboard.com>","date":"2020-05-01T16:43:56","subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, May 01, 2020 at 07:19:10PM +0300, Laurent Pinchart wrote:\n> On Fri, May 01, 2020 at 05:16:51PM +0200, Jacopo Mondi wrote:\n> > Since the Linux kernel commit:\n> > 69e39d40587b (\"media: vimc: Implement get/set selection in sink\")\n> > the crop rectangle on the VIMC scaler sink pad needs to be\n> > reset to match the requested format to avoid hitting a pipeline format\n> > mis-configurations when the applied format is larger than the one\n> \n> s/mis-configurations/misconfiguration/\n> \n> > set in a previous capture session.\n> > \n> > As the V4L2 specification is not clear on what the correct behaviour\n> > is, if the crop rectangle should be reset automatically or not on a\n> > set_fmt operation, momentary fix the pipeline handler to please the\n> > driver and manually reset the crop rectangle.\n> > \n> > Signed-off-by: Jacopo Mondi \n> \n> Reviewed-by: Laurent Pinchart \n> \n> > ---\n> > \n> > Tested on v5.6, please test on older kernel releases.\n> \n> I'll test it now.\n\nTested-by: Laurent Pinchart # on v5.4.28\n\nI however get\n\n[5:10:58.023539356] [23523] ERROR V4L2 v4l2_subdevice.cpp:301 'Scaler': Unable to set rectangle 0 on pad 0: Inappropriate ioctl for device\n\nwhich I understand is expected as the driver doesn't support that ioctl\nyet.\n\nIf you think it's worth it, you may want to squash\n\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex 0b8a0a0c6890..0742b37c377a 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -12,6 +12,7 @@\n #include \n\n #include \n+#include \n\n #include \n #include \n@@ -226,15 +227,17 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n \tif (ret)\n \t\treturn ret;\n\n-\tRectangle crop = {\n-\t\t.x = 0,\n-\t\t.y = 0,\n-\t\t.width = subformat.size.width,\n-\t\t.height = subformat.size.height,\n-\t};\n-\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n-\tif (ret && ret != -ENOTTY)\n-\t\treturn ret;\n+\tif (data->media_->version() >= KERNEL_VERSION(5, 6, 0)) {\n+\t\tRectangle crop = {\n+\t\t\t.x = 0,\n+\t\t\t.y = 0,\n+\t\t\t.width = subformat.size.width,\n+\t\t\t.height = subformat.size.height,\n+\t\t};\n+\t\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\t}\n\n \tsubformat.size = cfg.size;\n \tret = data->scaler_->setFormat(1, &subformat);\n\n\nwith this patch, and base is on top of \n\n[PATCH 1/2] libcamera: media_device: Expose media device API version number\n[PATCH 2/2] libcamera: pipeline: vimc: Store media device pointer in camera data\n\n> > ---\n> > src/libcamera/pipeline/vimc/vimc.cpp | 10 ++++++++++\n> > 1 file changed, 10 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index ccfd7f86d158..339d1cf653fb 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -224,6 +224,16 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > +\tRectangle crop = {\n> > +\t\t.x = 0,\n> > +\t\t.y = 0,\n> > +\t\t.width = subformat.size.width,\n> > +\t\t.height = subformat.size.height,\n> > +\t};\n> > +\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> > +\tif (ret && ret != -ENOTTY)\n> > +\t\treturn ret;\n> > +\n> > \tsubformat.size = cfg.size;\n> > \tret = data->scaler_->setFormat(1, &subformat);\n> > \tif (ret)","headers":{"Return-Path":"","Received":["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 C2006603F3\n\tfor ;\n\tFri, 1 May 2020 18:44:01 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C84F172C;\n\tFri, 1 May 2020 18:44:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pfqM+EM9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588351441;\n\tbh=gwJgNgb4gpJToppaMpb6IUjBlrcR5CG8WB6nxuo2Bg8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pfqM+EM9r86xsrzzdbbXFw5fA/dSRxmsxH+dd5G893kuZG0sJ6Ufj0NcpzcEVJega\n\tePs8QMLVtUw66Gnyk5RVzbAEhUxCgZdBas2pk3KgVemZrzjd2vki/912Ix4XP4mO0u\n\t1pymnaKUq5SAYBG6GcT1wgyEKqbL0hq16n34iF1o=","Date":"Fri, 1 May 2020 19:43:56 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501164356.GK5951@pendragon.ideasonboard.com>","References":"<20200501151651.864358-1-jacopo@jmondi.org>\n\t<20200501161909.GJ5951@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200501161909.GJ5951@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","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","X-List-Received-Date":"Fri, 01 May 2020 16:44:02 -0000"}},{"id":4712,"web_url":"https://patchwork.libcamera.org/comment/4712/","msgid":"<20200501194302.rm357tlflgcqpnzl@uno.localdomain>","date":"2020-05-01T19:43:02","subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, May 01, 2020 at 07:43:56PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Fri, May 01, 2020 at 07:19:10PM +0300, Laurent Pinchart wrote:\n> > On Fri, May 01, 2020 at 05:16:51PM +0200, Jacopo Mondi wrote:\n> > > Since the Linux kernel commit:\n> > > 69e39d40587b (\"media: vimc: Implement get/set selection in sink\")\n> > > the crop rectangle on the VIMC scaler sink pad needs to be\n> > > reset to match the requested format to avoid hitting a pipeline format\n> > > mis-configurations when the applied format is larger than the one\n> >\n> > s/mis-configurations/misconfiguration/\n> >\n> > > set in a previous capture session.\n> > >\n> > > As the V4L2 specification is not clear on what the correct behaviour\n> > > is, if the crop rectangle should be reset automatically or not on a\n> > > set_fmt operation, momentary fix the pipeline handler to please the\n> > > driver and manually reset the crop rectangle.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > ---\n> > >\n> > > Tested on v5.6, please test on older kernel releases.\n> >\n> > I'll test it now.\n>\n> Tested-by: Laurent Pinchart # on v5.4.28\n>\n> I however get\n>\n> [5:10:58.023539356] [23523] ERROR V4L2 v4l2_subdevice.cpp:301 'Scaler': Unable to set rectangle 0 on pad 0: Inappropriate ioctl for device\n>\n> which I understand is expected as the driver doesn't support that ioctl\n> yet.\n>\n> If you think it's worth it, you may want to squash\n\nI'm debated... this is a pipeline specific fix, while we'll have the\nsame issue once we'll try to get crop rectangles to collect properties\nfrom sensor.. In that case, as long as selection API support is not\nmandatory, we will receive the same error message, while a less\nworrying one might be more appropriate...\n\nI was thinking of addressing this in the video device and subdevice\nclasses, and reduce the message level severity to INFO if -ENOTTY is\nreturned.\n\nThis would silence this error in VIMC as well, without requiring a\npipeline and version specific workaround..\n\nWhat do you think ?\n\n>\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 0b8a0a0c6890..0742b37c377a 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -12,6 +12,7 @@\n> #include \n>\n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -226,15 +227,17 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\tRectangle crop = {\n> -\t\t.x = 0,\n> -\t\t.y = 0,\n> -\t\t.width = subformat.size.width,\n> -\t\t.height = subformat.size.height,\n> -\t};\n> -\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> -\tif (ret && ret != -ENOTTY)\n> -\t\treturn ret;\n> +\tif (data->media_->version() >= KERNEL_VERSION(5, 6, 0)) {\n> +\t\tRectangle crop = {\n> +\t\t\t.x = 0,\n> +\t\t\t.y = 0,\n> +\t\t\t.width = subformat.size.width,\n> +\t\t\t.height = subformat.size.height,\n> +\t\t};\n> +\t\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n>\n> \tsubformat.size = cfg.size;\n> \tret = data->scaler_->setFormat(1, &subformat);\n>\n>\n> with this patch, and base is on top of\n>\n> [PATCH 1/2] libcamera: media_device: Expose media device API version number\n> [PATCH 2/2] libcamera: pipeline: vimc: Store media device pointer in camera data\n>\n> > > ---\n> > > src/libcamera/pipeline/vimc/vimc.cpp | 10 ++++++++++\n> > > 1 file changed, 10 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index ccfd7f86d158..339d1cf653fb 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -224,6 +224,16 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > >\n> > > +\tRectangle crop = {\n> > > +\t\t.x = 0,\n> > > +\t\t.y = 0,\n> > > +\t\t.width = subformat.size.width,\n> > > +\t\t.height = subformat.size.height,\n> > > +\t};\n> > > +\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> > > +\tif (ret && ret != -ENOTTY)\n> > > +\t\treturn ret;\n> > > +\n> > > \tsubformat.size = cfg.size;\n> > > \tret = data->scaler_->setFormat(1, &subformat);\n> > > \tif (ret)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB090603F8\n\tfor ;\n\tFri, 1 May 2020 21:39:52 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 48F82100002;\n\tFri, 1 May 2020 19:39:51 +0000 (UTC)"],"Date":"Fri, 1 May 2020 21:43:02 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501194302.rm357tlflgcqpnzl@uno.localdomain>","References":"<20200501151651.864358-1-jacopo@jmondi.org>\n\t<20200501161909.GJ5951@pendragon.ideasonboard.com>\n\t<20200501164356.GK5951@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200501164356.GK5951@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","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","X-List-Received-Date":"Fri, 01 May 2020 19:39:53 -0000"}},{"id":4713,"web_url":"https://patchwork.libcamera.org/comment/4713/","msgid":"<20200501205733.GA32621@pendragon.ideasonboard.com>","date":"2020-05-01T20:57:33","subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, May 01, 2020 at 09:43:02PM +0200, Jacopo Mondi wrote:\n> On Fri, May 01, 2020 at 07:43:56PM +0300, Laurent Pinchart wrote:\n> > On Fri, May 01, 2020 at 07:19:10PM +0300, Laurent Pinchart wrote:\n> > > On Fri, May 01, 2020 at 05:16:51PM +0200, Jacopo Mondi wrote:\n> > > > Since the Linux kernel commit:\n> > > > 69e39d40587b (\"media: vimc: Implement get/set selection in sink\")\n> > > > the crop rectangle on the VIMC scaler sink pad needs to be\n> > > > reset to match the requested format to avoid hitting a pipeline format\n> > > > mis-configurations when the applied format is larger than the one\n> > >\n> > > s/mis-configurations/misconfiguration/\n> > >\n> > > > set in a previous capture session.\n> > > >\n> > > > As the V4L2 specification is not clear on what the correct behaviour\n> > > > is, if the crop rectangle should be reset automatically or not on a\n> > > > set_fmt operation, momentary fix the pipeline handler to please the\n> > > > driver and manually reset the crop rectangle.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > > ---\n> > > >\n> > > > Tested on v5.6, please test on older kernel releases.\n> > >\n> > > I'll test it now.\n> >\n> > Tested-by: Laurent Pinchart # on v5.4.28\n> >\n> > I however get\n> >\n> > [5:10:58.023539356] [23523] ERROR V4L2 v4l2_subdevice.cpp:301 'Scaler': Unable to set rectangle 0 on pad 0: Inappropriate ioctl for device\n> >\n> > which I understand is expected as the driver doesn't support that ioctl\n> > yet.\n> >\n> > If you think it's worth it, you may want to squash\n> \n> I'm debated... this is a pipeline specific fix, while we'll have the\n> same issue once we'll try to get crop rectangles to collect properties\n> from sensor.. In that case, as long as selection API support is not\n> mandatory, we will receive the same error message, while a less\n> worrying one might be more appropriate...\n> \n> I was thinking of addressing this in the video device and subdevice\n> classes, and reduce the message level severity to INFO if -ENOTTY is\n> returned.\n> \n> This would silence this error in VIMC as well, without requiring a\n> pipeline and version specific workaround..\n> \n> What do you think ?\n\nIt's a good point. I think we'll need version checks in the future, but\nI'm fine delaying the two base patches until they're needed, and\naddressed this in the V4L2Subdevice class. We would then need to review\nthe users though, as what I really want to avoid is an error going\nunnoticed without any message whatsoever.\n\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 0b8a0a0c6890..0742b37c377a 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> >\n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > @@ -226,15 +227,17 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > -\tRectangle crop = {\n> > -\t\t.x = 0,\n> > -\t\t.y = 0,\n> > -\t\t.width = subformat.size.width,\n> > -\t\t.height = subformat.size.height,\n> > -\t};\n> > -\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> > -\tif (ret && ret != -ENOTTY)\n> > -\t\treturn ret;\n> > +\tif (data->media_->version() >= KERNEL_VERSION(5, 6, 0)) {\n> > +\t\tRectangle crop = {\n> > +\t\t\t.x = 0,\n> > +\t\t\t.y = 0,\n> > +\t\t\t.width = subformat.size.width,\n> > +\t\t\t.height = subformat.size.height,\n> > +\t\t};\n> > +\t\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> >\n> > \tsubformat.size = cfg.size;\n> > \tret = data->scaler_->setFormat(1, &subformat);\n> >\n> >\n> > with this patch, and base is on top of\n> >\n> > [PATCH 1/2] libcamera: media_device: Expose media device API version number\n> > [PATCH 2/2] libcamera: pipeline: vimc: Store media device pointer in camera data\n> >\n> > > > ---\n> > > > src/libcamera/pipeline/vimc/vimc.cpp | 10 ++++++++++\n> > > > 1 file changed, 10 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > > index ccfd7f86d158..339d1cf653fb 100644\n> > > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > > @@ -224,6 +224,16 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > > > \tif (ret)\n> > > > \t\treturn ret;\n> > > >\n> > > > +\tRectangle crop = {\n> > > > +\t\t.x = 0,\n> > > > +\t\t.y = 0,\n> > > > +\t\t.width = subformat.size.width,\n> > > > +\t\t.height = subformat.size.height,\n> > > > +\t};\n> > > > +\tret = data->scaler_->setSelection(0, V4L2_SEL_TGT_CROP, &crop);\n> > > > +\tif (ret && ret != -ENOTTY)\n> > > > +\t\treturn ret;\n> > > > +\n> > > > \tsubformat.size = cfg.size;\n> > > > \tret = data->scaler_->setFormat(1, &subformat);\n> > > > \tif (ret)","headers":{"Return-Path":"","Received":["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 AADC8603F8\n\tfor ;\n\tFri, 1 May 2020 22:57:38 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA16872C;\n\tFri, 1 May 2020 22:57:37 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TX4pWlaw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588366658;\n\tbh=z1+890jKyiDep0Do8gVeAis4qoUcyGaVQ4rJr0FSkNM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TX4pWlawfmNLig3uMGau6/bo1Kimp3a/9FMJuQoeVaYSTCpI2YLgIzexy2IVOLJia\n\t8gXtX8DXeuRQkZx7hJXv8S28KefS7JUYWQYfrP7xvUv7/RopDg4IHHw1pCAfFmis4i\n\t1khrUs82w8L+NtjGjP/gOndFl8QfDyJTFIxBc4NI=","Date":"Fri, 1 May 2020 23:57:33 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501205733.GA32621@pendragon.ideasonboard.com>","References":"<20200501151651.864358-1-jacopo@jmondi.org>\n\t<20200501161909.GJ5951@pendragon.ideasonboard.com>\n\t<20200501164356.GK5951@pendragon.ideasonboard.com>\n\t<20200501194302.rm357tlflgcqpnzl@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200501194302.rm357tlflgcqpnzl@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: vimc: Adjust crop rectangle","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","X-List-Received-Date":"Fri, 01 May 2020 20:57:39 -0000"}}]