[{"id":15434,"web_url":"https://patchwork.libcamera.org/comment/15434/","msgid":"","date":"2021-03-03T09:47:56","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2021-03-02 21:49:48 +0200, Laurent Pinchart wrote:\n> When a sensor can upscale the image, the native sensor resolution isn't\n> equal to the largest size reported by the sensor. Use the active area\n> size instead, and default it to the largest enumerated size if the crop\n> rectangle targets are not supported.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> Changes since v1:\n> \n> - Don't set default pixelArraySize_ to override it later\n> \n> Note how this causes the pixelArraySize_ = sizes_.back() assignment to\n> be duplicated in CameraSensor::validateSensorDriver() and\n> CameraSensor::initVimcDefaultProperties(). Jacopo, do you prefer v1 or\n> v2 ?\n> \n> ---\n> include/libcamera/internal/camera_sensor.h | 3 +--\n> src/libcamera/camera_sensor.cpp | 23 +++++++++++-----------\n> 2 files changed, 13 insertions(+), 13 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index f22ffbfe9f97..71d012f795fe 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -53,7 +53,7 @@ public:\n> \tconst MediaEntity *entity() const { return entity_; }\n> \tconst std::vector &mbusCodes() const { return mbusCodes_; }\n> \tconst std::vector &sizes() const { return sizes_; }\n> -\tconst Size &resolution() const { return resolution_; }\n> +\tSize resolution() const { return activeArea_.size(); }\n> \n> \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> \t\t\t\t const Size &size) const;\n> @@ -87,7 +87,6 @@ private:\n> \tstd::string id_;\n> \n> \tV4L2Subdevice::Formats formats_;\n> -\tSize resolution_;\n> \tstd::vector mbusCodes_;\n> \tstd::vector sizes_;\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 8a1b9bd277df..8db6e8974a8d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -233,12 +233,6 @@ int CameraSensor::init()\n> \tauto last = std::unique(sizes_.begin(), sizes_.end());\n> \tsizes_.erase(last, sizes_.end());\n> \n> -\t/*\n> -\t * The sizes_ vector is sorted in ascending order, the resolution is\n> -\t * thus the last element of the vector.\n> -\t */\n> -\tresolution_ = sizes_.back();\n> -\n> \t/*\n> \t * VIMC is a bit special, as it does not yet support all the mandatory\n> \t * requirements regular sensors have to respect.\n> @@ -324,14 +318,20 @@ int CameraSensor::validateSensorDriver()\n> \tRectangle rect;\n> \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> \tif (ret) {\n> -\t\trect = Rectangle(resolution());\n> +\t\t/*\n> +\t\t * Default the pixel array size to the largest size supported\n> +\t\t * by the sensor. The sizes_ vector is sorted in ascending\n> +\t\t * order, the largest size is thus the last element.\n> +\t\t */\n> +\t\tpixelArraySize_ = sizes_.back();\n> +\n> \t\tLOG(CameraSensor, Warning)\n> \t\t\t<< \"The PixelArraySize property has been defaulted to \"\n> -\t\t\t<< rect.toString();\n> +\t\t\t<< pixelArraySize_.toString();\n> \t\terr = -EINVAL;\n> +\t} else {\n> +\t\tpixelArraySize_ = rect.size();\n> \t}\n> -\tpixelArraySize_.width = rect.width;\n> -\tpixelArraySize_.height = rect.height;\n> \n> \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);\n> \tif (ret) {\n> @@ -397,7 +397,8 @@ int CameraSensor::validateSensorDriver()\n> */\n> void CameraSensor::initVimcDefaultProperties()\n> {\n> -\tpixelArraySize_ = resolution();\n> +\t/* Use the largest supported size. */\n> +\tpixelArraySize_ = sizes_.back();\n> \tactiveArea_ = Rectangle(pixelArraySize_);\n> }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\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 675B9BD808\n\tfor ;\n\tWed, 3 Mar 2021 09:48:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4B4B68A98;\n\tWed, 3 Mar 2021 10:48:00 +0100 (CET)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 36C3C68A69\n\tfor ;\n\tWed, 3 Mar 2021 10:47:59 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id f1so36121649lfu.3\n\tfor ;\n\tWed, 03 Mar 2021 01:47:59 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt2sm2537076lfp.292.2021.03.03.01.47.57\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 03 Mar 2021 01:47:57 -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=\"psJEO9r1\"; 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=HQP2qJMX+Gc1UirdkyNItJ1rdbZdp7ciKJN3UcHL7Oc=;\n\tb=psJEO9r1ee7OKZL3uGTK3cp7pglgCuPN0PlHiE5sI0O14YrSWkUqC8tKwL4HGQvrex\n\t+u6U4iz/qvgM8CuvV6NeCQRU41py2oGhMu9aTupG8obkPyHRYdgY2KoxpVIHopsvNPl8\n\tKz1i8W3lygNBuXSm1FdhqV/u4hUlrqOy8EGSaDar7RBPTVKWQKpcl6I5UaC/W9McK5IE\n\tkqumBEI56HsUoXcSKvS06knISg4Qwyn+CuHmGVV5Pp+teBqQhROGkGrGs4tNWCVl6iB7\n\tpbmBqzEpBZKHVtjWGBfcxAypIbiUiMrr7zQ5TErnKai8GRcUeXkhbTvwlYCUDokoLypH\n\t9OHw==","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=HQP2qJMX+Gc1UirdkyNItJ1rdbZdp7ciKJN3UcHL7Oc=;\n\tb=Jt80enk5nt5xrZyQOQM/0i0s6xVKH79tIU54likkm/YbbtmWdyKseGX5FWgdn9HXr7\n\tqt6YZdmasMLlEFsPZe8vxw50Xl+Npq45cMnSBJKPRg5uxuis9UlTF1gseyetsUxLqyaY\n\tAdBzVokU0+e7xT96yR5HWEJKbYMuwKTzkLN4molCElsSv7W4z/vuOMdNL3MKmNqo79D4\n\tBWRCoNQ8i9IhHxmcUf6DZTlRt34Azx8+c99I1L5WVWLV/xZe2btvs7laUVC9GcUfzwHC\n\tJvMfwKRvO3/9BRfbgcD/8e6NWtHR7bVN/fL0tAl2h4p7yx726phv7IG+HEy/boCOdZF4\n\tumcg==","X-Gm-Message-State":"AOAM531ljura343VlM3mykDMAGTneppgnwqxUiNRMgCIHmvCxsFcD/2+\n\t4+qe3q9yRftI84U8PFkeFdeBYr0C/OLZ7g==","X-Google-Smtp-Source":"ABdhPJwnYeVWFz6S9mk3GRdDvrUOYO3K9eLGR/xRgMjAO6ho5jX22WagkCGhPPB6CC+x/tOuCsSFGA==","X-Received":"by 2002:a19:48:: with SMTP id 69mr14359821lfa.94.1614764878510; \n\tWed, 03 Mar 2021 01:47:58 -0800 (PST)","Date":"Wed, 3 Mar 2021 10:47:56 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210302194948.20396-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210302194948.20396-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","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":15442,"web_url":"https://patchwork.libcamera.org/comment/15442/","msgid":"<20210303130954.b45txdqteynuxzwn@uno.localdomain>","date":"2021-03-03T13:09:54","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Mar 02, 2021 at 09:49:48PM +0200, Laurent Pinchart wrote:\n> When a sensor can upscale the image, the native sensor resolution isn't\n> equal to the largest size reported by the sensor. Use the active area\n> size instead, and default it to the largest enumerated size if the crop\n> rectangle targets are not supported.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n>\n> - Don't set default pixelArraySize_ to override it later\n>\n> Note how this causes the pixelArraySize_ = sizes_.back() assignment to\n> be duplicated in CameraSensor::validateSensorDriver() and\n> CameraSensor::initVimcDefaultProperties(). Jacopo, do you prefer v1 or\n> v2 ?\n\nI think I slightly prefer this one as the two functions are in\nmutually exclusive call paths and it's easier to follow compared to an\ninitialization followed by an over-write.\n\nAnyway, we're talking details here.\n\n>\n> ---\n> include/libcamera/internal/camera_sensor.h | 3 +--\n> src/libcamera/camera_sensor.cpp | 23 +++++++++++-----------\n> 2 files changed, 13 insertions(+), 13 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index f22ffbfe9f97..71d012f795fe 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -53,7 +53,7 @@ public:\n> \tconst MediaEntity *entity() const { return entity_; }\n> \tconst std::vector &mbusCodes() const { return mbusCodes_; }\n> \tconst std::vector &sizes() const { return sizes_; }\n> -\tconst Size &resolution() const { return resolution_; }\n> +\tSize resolution() const { return activeArea_.size(); }\n>\n> \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> \t\t\t\t const Size &size) const;\n> @@ -87,7 +87,6 @@ private:\n> \tstd::string id_;\n>\n> \tV4L2Subdevice::Formats formats_;\n> -\tSize resolution_;\n> \tstd::vector mbusCodes_;\n> \tstd::vector sizes_;\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 8a1b9bd277df..8db6e8974a8d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -233,12 +233,6 @@ int CameraSensor::init()\n> \tauto last = std::unique(sizes_.begin(), sizes_.end());\n> \tsizes_.erase(last, sizes_.end());\n>\n> -\t/*\n> -\t * The sizes_ vector is sorted in ascending order, the resolution is\n> -\t * thus the last element of the vector.\n> -\t */\n> -\tresolution_ = sizes_.back();\n> -\n> \t/*\n> \t * VIMC is a bit special, as it does not yet support all the mandatory\n> \t * requirements regular sensors have to respect.\n> @@ -324,14 +318,20 @@ int CameraSensor::validateSensorDriver()\n> \tRectangle rect;\n> \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> \tif (ret) {\n> -\t\trect = Rectangle(resolution());\n> +\t\t/*\n> +\t\t * Default the pixel array size to the largest size supported\n> +\t\t * by the sensor. The sizes_ vector is sorted in ascending\n> +\t\t * order, the largest size is thus the last element.\n> +\t\t */\n> +\t\tpixelArraySize_ = sizes_.back();\n> +\n> \t\tLOG(CameraSensor, Warning)\n> \t\t\t<< \"The PixelArraySize property has been defaulted to \"\n> -\t\t\t<< rect.toString();\n> +\t\t\t<< pixelArraySize_.toString();\n> \t\terr = -EINVAL;\n> +\t} else {\n> +\t\tpixelArraySize_ = rect.size();\n\nCan't you\n if (ret) {\n rect = Rectangle(sizes_.back())\n ....\n }\n\n pixelArraySize = rect.size();\n\nAnyway, that's minor too.\n\nThe patch looks good.\nReviewed-by: Jacopo Mondi \n\nI recall you had a patch that inspected the list of the V4L2Subdevice\ncontrol info map instead of gong to the device to check if a control\nwas supported (iirc that help avoiding an error message being printed\nout by subdev_->getControls() in CameraSensor::sensorInfo()). Wasn't\nit paired with this one ?\n\n> \t}\n> -\tpixelArraySize_.width = rect.width;\n> -\tpixelArraySize_.height = rect.height;\n>\n> \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);\n> \tif (ret) {\n> @@ -397,7 +397,8 @@ int CameraSensor::validateSensorDriver()\n> */\n> void CameraSensor::initVimcDefaultProperties()\n> {\n> -\tpixelArraySize_ = resolution();\n> +\t/* Use the largest supported size. */\n> +\tpixelArraySize_ = sizes_.back();\n> \tactiveArea_ = Rectangle(pixelArraySize_);\n> }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 62CCFBD80C\n\tfor ;\n\tWed, 3 Mar 2021 13:09:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D833268A98;\n\tWed, 3 Mar 2021 14:09:27 +0100 (CET)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C738168A7E\n\tfor ;\n\tWed, 3 Mar 2021 14:09:26 +0100 (CET)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 47F304000C;\n\tWed, 3 Mar 2021 13:09:26 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Wed, 3 Mar 2021 14:09:54 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210303130954.b45txdqteynuxzwn@uno.localdomain>","References":"<20210302194948.20396-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210302194948.20396-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15445,"web_url":"https://patchwork.libcamera.org/comment/15445/","msgid":"","date":"2021-03-03T14:33:47","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Mar 03, 2021 at 02:09:54PM +0100, Jacopo Mondi wrote:\n> On Tue, Mar 02, 2021 at 09:49:48PM +0200, Laurent Pinchart wrote:\n> > When a sensor can upscale the image, the native sensor resolution isn't\n> > equal to the largest size reported by the sensor. Use the active area\n> > size instead, and default it to the largest enumerated size if the crop\n> > rectangle targets are not supported.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> >\n> > - Don't set default pixelArraySize_ to override it later\n> >\n> > Note how this causes the pixelArraySize_ = sizes_.back() assignment to\n> > be duplicated in CameraSensor::validateSensorDriver() and\n> > CameraSensor::initVimcDefaultProperties(). Jacopo, do you prefer v1 or\n> > v2 ?\n> \n> I think I slightly prefer this one as the two functions are in\n> mutually exclusive call paths and it's easier to follow compared to an\n> initialization followed by an over-write.\n\nWorks for me.\n\n> Anyway, we're talking details here.\n> \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 3 +--\n> > src/libcamera/camera_sensor.cpp | 23 +++++++++++-----------\n> > 2 files changed, 13 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index f22ffbfe9f97..71d012f795fe 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -53,7 +53,7 @@ public:\n> > \tconst MediaEntity *entity() const { return entity_; }\n> > \tconst std::vector &mbusCodes() const { return mbusCodes_; }\n> > \tconst std::vector &sizes() const { return sizes_; }\n> > -\tconst Size &resolution() const { return resolution_; }\n> > +\tSize resolution() const { return activeArea_.size(); }\n> >\n> > \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> > \t\t\t\t const Size &size) const;\n> > @@ -87,7 +87,6 @@ private:\n> > \tstd::string id_;\n> >\n> > \tV4L2Subdevice::Formats formats_;\n> > -\tSize resolution_;\n> > \tstd::vector mbusCodes_;\n> > \tstd::vector sizes_;\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 8a1b9bd277df..8db6e8974a8d 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -233,12 +233,6 @@ int CameraSensor::init()\n> > \tauto last = std::unique(sizes_.begin(), sizes_.end());\n> > \tsizes_.erase(last, sizes_.end());\n> >\n> > -\t/*\n> > -\t * The sizes_ vector is sorted in ascending order, the resolution is\n> > -\t * thus the last element of the vector.\n> > -\t */\n> > -\tresolution_ = sizes_.back();\n> > -\n> > \t/*\n> > \t * VIMC is a bit special, as it does not yet support all the mandatory\n> > \t * requirements regular sensors have to respect.\n> > @@ -324,14 +318,20 @@ int CameraSensor::validateSensorDriver()\n> > \tRectangle rect;\n> > \tint ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);\n> > \tif (ret) {\n> > -\t\trect = Rectangle(resolution());\n> > +\t\t/*\n> > +\t\t * Default the pixel array size to the largest size supported\n> > +\t\t * by the sensor. The sizes_ vector is sorted in ascending\n> > +\t\t * order, the largest size is thus the last element.\n> > +\t\t */\n> > +\t\tpixelArraySize_ = sizes_.back();\n> > +\n> > \t\tLOG(CameraSensor, Warning)\n> > \t\t\t<< \"The PixelArraySize property has been defaulted to \"\n> > -\t\t\t<< rect.toString();\n> > +\t\t\t<< pixelArraySize_.toString();\n> > \t\terr = -EINVAL;\n> > +\t} else {\n> > +\t\tpixelArraySize_ = rect.size();\n> \n> Can't you\n> if (ret) {\n> rect = Rectangle(sizes_.back())\n> ....\n> }\n> \n> pixelArraySize = rect.size();\n> \n> Anyway, that's minor too.\n\nThat would convert from size to rectangle and then to size again :-)\nIt's minor indeed.\n\n> The patch looks good.\n> Reviewed-by: Jacopo Mondi \n> \n> I recall you had a patch that inspected the list of the V4L2Subdevice\n> control info map instead of gong to the device to check if a control\n> was supported (iirc that help avoiding an error message being printed\n> out by subdev_->getControls() in CameraSensor::sensorInfo()). Wasn't\n> it paired with this one ?\n\nThat was part of commit 79266225d2af (\"libcamera: camera_sensor: Check\ncontrol availability from idmap\"), merged in master.\n\n> > \t}\n> > -\tpixelArraySize_.width = rect.width;\n> > -\tpixelArraySize_.height = rect.height;\n> >\n> > \tret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);\n> > \tif (ret) {\n> > @@ -397,7 +397,8 @@ int CameraSensor::validateSensorDriver()\n> > */\n> > void CameraSensor::initVimcDefaultProperties()\n> > {\n> > -\tpixelArraySize_ = resolution();\n> > +\t/* Use the largest supported size. */\n> > +\tpixelArraySize_ = sizes_.back();\n> > \tactiveArea_ = Rectangle(pixelArraySize_);\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 93122BD1F1\n\tfor ;\n\tWed, 3 Mar 2021 14:34:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16D2768A98;\n\tWed, 3 Mar 2021 15:34: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 9E61B68A7E\n\tfor ;\n\tWed, 3 Mar 2021 15:34:18 +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 08F6F8DE;\n\tWed, 3 Mar 2021 15:34:17 +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=\"Ulvw1Mmq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614782058;\n\tbh=dXf+4kS8SWtJaNdcdG2lBMX2OXSgakimCHW5oN99tlc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ulvw1MmqAuOfXeDC2cGZqgvN+PVnnCJkhQsgTFS+iEqAxhvkDDTVuZSY4fEeomJsp\n\t6Du2CdSPdMb90dCENyukyz/vipLmXbvGwdEghYa159mLJUUQERpmPE5vdk6jtgXgVn\n\t8A2/Nu1lkRZMERfk34DYKulmENo10ftzKEt7vR6I=","Date":"Wed, 3 Mar 2021 16:33:47 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210302194948.20396-1-laurent.pinchart@ideasonboard.com>\n\t<20210303130954.b45txdqteynuxzwn@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210303130954.b45txdqteynuxzwn@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: camera_sensor: Use\n\tactive area size as resolution","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]