[{"id":14453,"web_url":"https://patchwork.libcamera.org/comment/14453/","msgid":"","date":"2021-01-06T10:50:41","subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue 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 2021-01-05 13:31:22 +0100, Jacopo Mondi wrote:\n> As support for the V4L2_SEL_TGT_CROP selection target, used to read the\n> sensor analogue crop rectangle is schedule to become mandatory but is\n> still optional, cache a default value equal to the sensor's active area\n> size at sensor validation time to allow the creation of the\n> CameraSensorInfo in the case the driver does not support it.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 1 +\n> src/libcamera/camera_sensor.cpp | 27 ++++++++++++++--------\n> 2 files changed, 19 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 86902b85ada8..de6a0202d19a 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -86,6 +86,7 @@ private:\n> \n> \tSize pixelArraySize_;\n> \tRectangle activeArea_;\n> +\tRectangle analogueCrop_;\n> \n> \tControlList properties_;\n> };\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 92c90862215d..eda41980aa22 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -289,8 +289,10 @@ int CameraSensor::validateSensorDriver()\n> \n> \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &rect);\n> \tif (ret) {\n> +\t\tanalogueCrop_ = activeArea_;\n> \t\tLOG(CameraSensor, Warning)\n> -\t\t\t<< \"Failed to retrieve the sensor crop rectangle\";\n> +\t\t\t<< \"The analogue crop rectangle has been defaulted to: \"\n> +\t\t\t<< analogueCrop_.toString();\n> \t\terr = -EINVAL;\n> \t}\n> \n> @@ -643,13 +645,20 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \t */\n> \tinfo->activeAreaSize = { activeArea_.width, activeArea_.height };\n> \n> -\t/* It's mandatory for the subdevice to report its crop rectangle. */\n> -\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> -\tif (ret) {\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"Failed to construct camera sensor info: \"\n> -\t\t\t<< \"the camera sensor does not report the crop rectangle\";\n> -\t\treturn ret;\n> +\t/*\n> +\t * \\todo Support for retreiving the crop rectangle is scheduled to\n> +\t * become mandatory. For the time being use the default value if it has\n> +\t * been initialized at sensor driver validation time.\n> +\t */\n> +\tif (!analogueCrop_.isNull()) {\n> +\t\tinfo->analogCrop = analogueCrop_;\n> +\t} else {\n> +\t\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> +\t\tif (ret) {\n> +\t\t\tLOG(CameraSensor, Error)\n> +\t\t\t\t<< \"Failed to retrieve the sensor crop rectangle.\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n\nAs I understand 3/10 activeArea_ is static right? Would it make more \nsens to use it here instead of adding and caching it in analogueCrop_? \nThis would align the code here to always call getSelection() and \nhuddling the defaulting in the if (ret) code block? I think this would \nmake the code cleaner as the default as you point out is a temporary \nwork around.\n\n> \t}\n> \n> \t/*\n> @@ -664,7 +673,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \n> \t/* The bit depth and image size depend on the currently applied format. */\n> \tV4L2SubdeviceFormat format{};\n> -\tret = subdev_->getFormat(pad_, &format);\n> +\tint ret = subdev_->getFormat(pad_, &format);\n> \tif (ret)\n> \t\treturn ret;\n> \tinfo->bitsPerPixel = format.bitsPerPixel();\n> -- \n> 2.29.2\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 50D95C0F1A\n\tfor ;\n\tWed, 6 Jan 2021 10:50:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7F9A62D3E;\n\tWed, 6 Jan 2021 11:50:44 +0100 (CET)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94D336010B\n\tfor ;\n\tWed, 6 Jan 2021 11:50:43 +0100 (CET)","by mail-lf1-x136.google.com with SMTP id s26so5521913lfc.8\n\tfor ;\n\tWed, 06 Jan 2021 02:50:43 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt14sm305397ljg.48.2021.01.06.02.50.41\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 06 Jan 2021 02:50:42 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"d+ippsDs\"; dkim-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=lgtBtobtlPhqKeWpoL07dzYC8eNFaGKTbZjoDcIUop0=;\n\tb=d+ippsDsAl49iYn8EOncC7CfNOja4Nc5xWEKFjz7k5W640fp5tXMuYom2MDNmUG6nm\n\tvqDZlAVMfUSMEHPLXsCKOUkrLB09d1Xx+ICj0ygvbIlhZ+jmiFVHLiSUBsXi4hOIOc3d\n\tfGgc99V//P30FJAOWkvidDfMtOFwYpa5/yHIE414PER4Am0oT4a9jkeBmXybUvx5A0UI\n\tkF8v3HU4eFlZC1XeC/e9bdT85VPbArzRsUfAcxxCT4DeVPoglO0GYZYBgXO+yQ1+1Ssq\n\tOdDGceMnwkhMce7alVyQLRBYeyuy8QaITXG/h5qtU6xYUCQTAjE3WCqmhnig0papTxBp\n\t2yZg==","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=lgtBtobtlPhqKeWpoL07dzYC8eNFaGKTbZjoDcIUop0=;\n\tb=Y0eh9MEehcEf2xH12oOJL/gJDTF6rx1c7cRO5hEsts4EM0h7pbEigx8kJdJ/EOKhO3\n\tWzpgxBeS19RwNgUkxn7PeP0/b6n2AeIhm8jNd6nvqxMFoJQwPGo3kcuK1krv/Yorjwqa\n\ts7wE94b+OJxsQiID3En7P7XvgfONMFrkzFzHm0eJgUfxo6X/wwAJOvnpwIZfgO/JdGcA\n\td6ywgAjpvaN6ObaXhEEELNDj/z7MalOhK6B02NVEf4lnaY2abBa/5DnL/PfOQbBBoIkL\n\tSiMzDO7wP37W/yO7k4wTyEkQwuLm6RTbBq6BY+MPqh17RqyQ2k+dEUAeLBO9+k2dzLOd\n\ttz2w==","X-Gm-Message-State":"AOAM532nNfU7Q5PbguiU4qco2W2qMOEqRwXp6tnt9WURFMwg10w8IrNN\n\tL3ZbQ213uRdUwMMyyHcx8VIH3w==","X-Google-Smtp-Source":"ABdhPJxeBTxYl2uA0xFcnJhoEAD042bBVCiKbB/npM20y2f1i4jE7qaO+N9Yqt5k7iIfCihmYsrs1Q==","X-Received":"by 2002:a2e:9f14:: with SMTP id\n\tu20mr1930918ljk.244.1609930242741; \n\tWed, 06 Jan 2021 02:50:42 -0800 (PST)","Date":"Wed, 6 Jan 2021 11:50:41 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105123128.617543-1-jacopo@jmondi.org>\n\t<20210105123128.617543-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210105123128.617543-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue 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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14458,"web_url":"https://patchwork.libcamera.org/comment/14458/","msgid":"","date":"2021-01-07T02:44:59","subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue 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 Wed, Jan 06, 2021 at 11:50:41AM +0100, Niklas Söderlund wrote:\n> On 2021-01-05 13:31:22 +0100, Jacopo Mondi wrote:\n> > As support for the V4L2_SEL_TGT_CROP selection target, used to read the\n> > sensor analogue crop rectangle is schedule to become mandatory but is\n> > still optional, cache a default value equal to the sensor's active area\n> > size at sensor validation time to allow the creation of the\n> > CameraSensorInfo in the case the driver does not support it.\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 1 +\n> > src/libcamera/camera_sensor.cpp | 27 ++++++++++++++--------\n> > 2 files changed, 19 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 86902b85ada8..de6a0202d19a 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -86,6 +86,7 @@ private:\n> > \n> > \tSize pixelArraySize_;\n> > \tRectangle activeArea_;\n> > +\tRectangle analogueCrop_;\n> > \n> > \tControlList properties_;\n> > };\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 92c90862215d..eda41980aa22 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -289,8 +289,10 @@ int CameraSensor::validateSensorDriver()\n> > \n> > \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &rect);\n> > \tif (ret) {\n> > +\t\tanalogueCrop_ = activeArea_;\n> > \t\tLOG(CameraSensor, Warning)\n> > -\t\t\t<< \"Failed to retrieve the sensor crop rectangle\";\n> > +\t\t\t<< \"The analogue crop rectangle has been defaulted to: \"\n> > +\t\t\t<< analogueCrop_.toString();\n> > \t\terr = -EINVAL;\n> > \t}\n> > \n> > @@ -643,13 +645,20 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > \t */\n> > \tinfo->activeAreaSize = { activeArea_.width, activeArea_.height };\n> > \n> > -\t/* It's mandatory for the subdevice to report its crop rectangle. */\n> > -\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> > -\tif (ret) {\n> > -\t\tLOG(CameraSensor, Error)\n> > -\t\t\t<< \"Failed to construct camera sensor info: \"\n> > -\t\t\t<< \"the camera sensor does not report the crop rectangle\";\n> > -\t\treturn ret;\n> > +\t/*\n> > +\t * \\todo Support for retreiving the crop rectangle is scheduled to\n> > +\t * become mandatory. For the time being use the default value if it has\n> > +\t * been initialized at sensor driver validation time.\n> > +\t */\n> > +\tif (!analogueCrop_.isNull()) {\n> > +\t\tinfo->analogCrop = analogueCrop_;\n> > +\t} else {\n> > +\t\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> > +\t\tif (ret) {\n> > +\t\t\tLOG(CameraSensor, Error)\n> > +\t\t\t\t<< \"Failed to retrieve the sensor crop rectangle.\";\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> \n> As I understand 3/10 activeArea_ is static right? Would it make more \n> sens to use it here instead of adding and caching it in analogueCrop_? \n> This would align the code here to always call getSelection() and \n> huddling the defaulting in the if (ret) code block? I think this would \n> make the code cleaner as the default as you point out is a temporary \n> work around.\n\nI like this a bit better too. With if possible, or without this change,\n\nReviewed-by: Laurent Pinchart \n\n> > \t}\n> > \n> > \t/*\n> > @@ -664,7 +673,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > \n> > \t/* The bit depth and image size depend on the currently applied format. */\n> > \tV4L2SubdeviceFormat format{};\n> > -\tret = subdev_->getFormat(pad_, &format);\n> > +\tint ret = subdev_->getFormat(pad_, &format);\n> > \tif (ret)\n> > \t\treturn ret;\n> > \tinfo->bitsPerPixel = format.bitsPerPixel();","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 39DE8C0F1C\n\tfor ;\n\tThu, 7 Jan 2021 02:45:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7B87630BA;\n\tThu, 7 Jan 2021 03:45: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 5F1DA6010E\n\tfor ;\n\tThu, 7 Jan 2021 03:45:12 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0AB32E0;\n\tThu, 7 Jan 2021 03:45:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"M1Blqiym\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609987512;\n\tbh=JGqNx7FZRIHlOjIiTo8YkRUncjVDIrQOz/NsFrSPTmc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M1Blqiym+UV4q9dCv9NWd+nTGbH5/bG+54CaC70G9KvKOuiplEbiKmzWg+uGl9lrb\n\tVjumvyAfE+IBwU/jjmp9hcXSyroybuBBWcSNGTfD28FwgmQNzg9APi4+T4R4hgALs3\n\t8lqefF2h0Eo+ovSY8/zDGmhCdD01AfYwGiTXh1vk=","Date":"Thu, 7 Jan 2021 04:44:59 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105123128.617543-1-jacopo@jmondi.org>\n\t<20210105123128.617543-5-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue 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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14465,"web_url":"https://patchwork.libcamera.org/comment/14465/","msgid":"<20210107080327.s46yrlwl2i4ieuyb@uno.localdomain>","date":"2021-01-07T08:03:27","subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue crop rectangle","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent, Niklas,\n\nOn Thu, Jan 07, 2021 at 04:44:59AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Jan 06, 2021 at 11:50:41AM +0100, Niklas Söderlund wrote:\n> > On 2021-01-05 13:31:22 +0100, Jacopo Mondi wrote:\n> > > As support for the V4L2_SEL_TGT_CROP selection target, used to read the\n> > > sensor analogue crop rectangle is schedule to become mandatory but is\n> > > still optional, cache a default value equal to the sensor's active area\n> > > size at sensor validation time to allow the creation of the\n> > > CameraSensorInfo in the case the driver does not support it.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/internal/camera_sensor.h | 1 +\n> > > src/libcamera/camera_sensor.cpp | 27 ++++++++++++++--------\n> > > 2 files changed, 19 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > > index 86902b85ada8..de6a0202d19a 100644\n> > > --- a/include/libcamera/internal/camera_sensor.h\n> > > +++ b/include/libcamera/internal/camera_sensor.h\n> > > @@ -86,6 +86,7 @@ private:\n> > >\n> > > \tSize pixelArraySize_;\n> > > \tRectangle activeArea_;\n> > > +\tRectangle analogueCrop_;\n> > >\n> > > \tControlList properties_;\n> > > };\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 92c90862215d..eda41980aa22 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -289,8 +289,10 @@ int CameraSensor::validateSensorDriver()\n> > >\n> > > \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &rect);\n> > > \tif (ret) {\n> > > +\t\tanalogueCrop_ = activeArea_;\n> > > \t\tLOG(CameraSensor, Warning)\n> > > -\t\t\t<< \"Failed to retrieve the sensor crop rectangle\";\n> > > +\t\t\t<< \"The analogue crop rectangle has been defaulted to: \"\n> > > +\t\t\t<< analogueCrop_.toString();\n> > > \t\terr = -EINVAL;\n> > > \t}\n> > >\n> > > @@ -643,13 +645,20 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > > \t */\n> > > \tinfo->activeAreaSize = { activeArea_.width, activeArea_.height };\n> > >\n> > > -\t/* It's mandatory for the subdevice to report its crop rectangle. */\n> > > -\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> > > -\tif (ret) {\n> > > -\t\tLOG(CameraSensor, Error)\n> > > -\t\t\t<< \"Failed to construct camera sensor info: \"\n> > > -\t\t\t<< \"the camera sensor does not report the crop rectangle\";\n> > > -\t\treturn ret;\n> > > +\t/*\n> > > +\t * \\todo Support for retreiving the crop rectangle is scheduled to\n> > > +\t * become mandatory. For the time being use the default value if it has\n> > > +\t * been initialized at sensor driver validation time.\n> > > +\t */\n> > > +\tif (!analogueCrop_.isNull()) {\n> > > +\t\tinfo->analogCrop = analogueCrop_;\n> > > +\t} else {\n> > > +\t\tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop);\n> > > +\t\tif (ret) {\n> > > +\t\t\tLOG(CameraSensor, Error)\n> > > +\t\t\t\t<< \"Failed to retrieve the sensor crop rectangle.\";\n> > > +\t\t\treturn -EINVAL;\n> > > +\t\t}\n> >\n> > As I understand 3/10 activeArea_ is static right? Would it make more\n> > sens to use it here instead of adding and caching it in analogueCrop_?\n> > This would align the code here to always call getSelection() and\n> > huddling the defaulting in the if (ret) code block? I think this would\n> > make the code cleaner as the default as you point out is a temporary\n> > work around.\n>\n> I like this a bit better too. With if possible, or without this change,\n>\n\nYeah, makes sense, I'll fixe\n\n> Reviewed-by: Laurent Pinchart \n>\n> > > \t}\n> > >\n> > > \t/*\n> > > @@ -664,7 +673,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > >\n> > > \t/* The bit depth and image size depend on the currently applied format. */\n> > > \tV4L2SubdeviceFormat format{};\n> > > -\tret = subdev_->getFormat(pad_, &format);\n> > > +\tint ret = subdev_->getFormat(pad_, &format);\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > > \tinfo->bitsPerPixel = format.bitsPerPixel();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 C1B4BC0F1A\n\tfor ;\n\tThu, 7 Jan 2021 08:03:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42A57631D0;\n\tThu, 7 Jan 2021 09:03:14 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3671260318\n\tfor ;\n\tThu, 7 Jan 2021 09:03:13 +0100 (CET)","from uno.localdomain (mob-31-157-123-127.net.vodafone.it\n\t[31.157.123.127]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 2E757100005;\n\tThu, 7 Jan 2021 08:03:11 +0000 (UTC)"],"Date":"Thu, 7 Jan 2021 09:03:27 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210107080327.s46yrlwl2i4ieuyb@uno.localdomain>","References":"<20210105123128.617543-1-jacopo@jmondi.org>\n\t<20210105123128.617543-5-jacopo@jmondi.org>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 04/10] libcamera: camera_sensor:\n\tDefault analogue 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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]