[{"id":14383,"web_url":"https://patchwork.libcamera.org/comment/14383/","msgid":"<20201229031930.GC67590@pyrite.rasen.tech>","date":"2020-12-29T03:19:30","subject":"Re: [libcamera-devel] [PATCH v2 2/5] libcamera: camera_sensor:\n\tInitialize controls","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Dec 28, 2020 at 05:55:57PM +0100, Jacopo Mondi wrote:\n> Initialize the ControlInfoMap of sensor available controls by\n> reporting the sensor exposure time range.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Paul Elder \n\n> ---\n> include/libcamera/internal/camera_sensor.h | 2 +\n> src/libcamera/camera_sensor.cpp | 45 +++++++++++++++++++++-\n> 2 files changed, 46 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index aee10aa6e3c7..0357b2a630f7 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -71,6 +71,7 @@ private:\n> \tint generateId();\n> \tint validateSensorDriver();\n> \tint initProperties();\n> +\tint initControls();\n> \n> \tconst MediaEntity *entity_;\n> \tstd::unique_ptr subdev_;\n> @@ -85,6 +86,7 @@ private:\n> \tstd::vector sizes_;\n> \n> \tControlList properties_;\n> +\tControlInfoMap controls_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 71d7c862e69a..18dc6d723e27 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \"libcamera/internal/bayer_format.h\"\n> @@ -215,7 +216,7 @@ int CameraSensor::init()\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\treturn 0;\n> +\treturn initControls();\n> }\n> \n> int CameraSensor::validateSensorDriver()\n> @@ -229,6 +230,7 @@ int CameraSensor::validateSensorDriver()\n> \tconst std::vector mandatoryControls{\n> \t\tV4L2_CID_PIXEL_RATE,\n> \t\tV4L2_CID_HBLANK,\n> +\t\tV4L2_CID_EXPOSURE,\n> \t};\n> \n> \tControlList ctrls = subdev_->getControls(mandatoryControls);\n> @@ -428,6 +430,47 @@ int CameraSensor::initProperties()\n> \treturn 0;\n> }\n> \n> +int CameraSensor::initControls()\n> +{\n> +\tconst ControlInfoMap &v4l2Controls = subdev_->controls();\n> +\n> +\t/* Calculate exposure time limits expressed in micro-seconds. */\n> +\n> +\t/* Calculate the line length in pixels. */\n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_HBLANK });\n> +\tif (ctrls.empty())\n> +\t\treturn -EINVAL;\n> +\tint32_t hblank = ctrls.get(V4L2_CID_HBLANK).get();\n> +\n> +\tV4L2SubdeviceFormat format{};\n> +\tint ret = subdev_->getFormat(pad_, &format);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tint32_t ppl = format.size.width + hblank;\n> +\n> +\tconst ControlInfo &v4l2ExposureInfo = v4l2Controls.find(V4L2_CID_EXPOSURE)->second;\n> +\tint32_t minExposurePixels = v4l2ExposureInfo.min().get() * ppl;\n> +\tint32_t maxExposurePixels = v4l2ExposureInfo.max().get() * ppl;\n> +\tint32_t defExposurePixels = v4l2ExposureInfo.def().get() * ppl;\n> +\n> +\t/* Get the pixel rate (in useconds) and calculate the exposure timings. */\n> +\tconst ControlInfo &pixelRateInfo = v4l2Controls.find(V4L2_CID_PIXEL_RATE)->second;\n> +\tfloat minPixelRate = pixelRateInfo.min().get() / 1e6f;\n> +\tfloat maxPixelRate = pixelRateInfo.max().get() / 1e6f;\n> +\tfloat defPixelRate = pixelRateInfo.def().get() / 1e6f;\n> +\n> +\tint32_t minExposure = static_cast(minExposurePixels / maxPixelRate);\n> +\tint32_t maxExposure = static_cast(maxExposurePixels / minPixelRate);\n> +\tint32_t defExposure = static_cast(defExposurePixels / defPixelRate);\n> +\n> +\tControlInfoMap::Map map;\n> +\tmap[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> +\t\t\t\t\t\t defExposure);\n> +\tcontrols_ = std::move(map);\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\fn CameraSensor::model()\n> * \\brief Retrieve the sensor model name\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 F18CEC0F1C\n\tfor ;\n\tTue, 29 Dec 2020 03:19:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66B6E615B2;\n\tTue, 29 Dec 2020 04:19:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05F596031A\n\tfor ;\n\tTue, 29 Dec 2020 04:19:38 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5EB0298;\n\tTue, 29 Dec 2020 04:19:36 +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=\"WZwyJnuX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609211977;\n\tbh=UayDaqVlyaz8yVZRAO3cVblyLp/pQxAY3KG0VrEmb5k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WZwyJnuXbSUWDFF47LBnW5S7eDTLfmkCyWKG7mFxyD/Kev3m0Y/NSAa/6LMoI7R/r\n\tc7y3ZCCXSkwkZYl043bfx+o4SJ0S2qzHkBj50WAs+b9WU2SYyCauXu29HWApBtoK2q\n\tVUcPAvwW0YDF3fb+ivlygt5wQPaS0O4/pQ0zZ97s=","Date":"Tue, 29 Dec 2020 12:19:30 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20201229031930.GC67590@pyrite.rasen.tech>","References":"<20201228165600.53987-1-jacopo@jmondi.org>\n\t<20201228165600.53987-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201228165600.53987-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] libcamera: camera_sensor:\n\tInitialize controls","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":14414,"web_url":"https://patchwork.libcamera.org/comment/14414/","msgid":"","date":"2020-12-30T12:17:03","subject":"Re: [libcamera-devel] [PATCH v2 2/5] libcamera: camera_sensor:\n\tInitialize controls","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 Mon, Dec 28, 2020 at 05:55:57PM +0100, Jacopo Mondi wrote:\n> Initialize the ControlInfoMap of sensor available controls by\n> reporting the sensor exposure time range.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 2 +\n> src/libcamera/camera_sensor.cpp | 45 +++++++++++++++++++++-\n> 2 files changed, 46 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index aee10aa6e3c7..0357b2a630f7 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -71,6 +71,7 @@ private:\n> \tint generateId();\n> \tint validateSensorDriver();\n> \tint initProperties();\n> +\tint initControls();\n> \n> \tconst MediaEntity *entity_;\n> \tstd::unique_ptr subdev_;\n> @@ -85,6 +86,7 @@ private:\n> \tstd::vector sizes_;\n> \n> \tControlList properties_;\n> +\tControlInfoMap controls_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 71d7c862e69a..18dc6d723e27 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \"libcamera/internal/bayer_format.h\"\n> @@ -215,7 +216,7 @@ int CameraSensor::init()\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\treturn 0;\n> +\treturn initControls();\n> }\n> \n> int CameraSensor::validateSensorDriver()\n> @@ -229,6 +230,7 @@ int CameraSensor::validateSensorDriver()\n> \tconst std::vector mandatoryControls{\n> \t\tV4L2_CID_PIXEL_RATE,\n> \t\tV4L2_CID_HBLANK,\n> +\t\tV4L2_CID_EXPOSURE,\n> \t};\n> \n> \tControlList ctrls = subdev_->getControls(mandatoryControls);\n> @@ -428,6 +430,47 @@ int CameraSensor::initProperties()\n> \treturn 0;\n> }\n> \n> +int CameraSensor::initControls()\n> +{\n> +\tconst ControlInfoMap &v4l2Controls = subdev_->controls();\n> +\n> +\t/* Calculate exposure time limits expressed in micro-seconds. */\n> +\n> +\t/* Calculate the line length in pixels. */\n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_HBLANK });\n> +\tif (ctrls.empty())\n> +\t\treturn -EINVAL;\n> +\tint32_t hblank = ctrls.get(V4L2_CID_HBLANK).get();\n> +\n> +\tV4L2SubdeviceFormat format{};\n> +\tint ret = subdev_->getFormat(pad_, &format);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tint32_t ppl = format.size.width + hblank;\n\nI'd write pixelPerLine or lineLength to make this more explicit.\n\n> +\n> +\tconst ControlInfo &v4l2ExposureInfo = v4l2Controls.find(V4L2_CID_EXPOSURE)->second;\n> +\tint32_t minExposurePixels = v4l2ExposureInfo.min().get() * ppl;\n> +\tint32_t maxExposurePixels = v4l2ExposureInfo.max().get() * ppl;\n> +\tint32_t defExposurePixels = v4l2ExposureInfo.def().get() * ppl;\n> +\n> +\t/* Get the pixel rate (in useconds) and calculate the exposure timings. */\n> +\tconst ControlInfo &pixelRateInfo = v4l2Controls.find(V4L2_CID_PIXEL_RATE)->second;\n> +\tfloat minPixelRate = pixelRateInfo.min().get() / 1e6f;\n> +\tfloat maxPixelRate = pixelRateInfo.max().get() / 1e6f;\n> +\tfloat defPixelRate = pixelRateInfo.def().get() / 1e6f;\n\nThe pixel rate is fixed for a given sensor configuration. Sensors could\nexpose different pixel rates for different configurations, but it\ndoesn't necessarily mean that the exposure control minimum and maximum\nvalues will be compatible with any pixel rate. Furthermore, horizontal\nblanking may vary, and the horizontal active size definitely varies\nbetween different configurations. All this means that exposing an\nexposure time from the camera sensor class, with meaningful min/max\nvalues, will be more complicated than this.\n\nWe can start with this implementation, with a big \\todo comment\nexplaining that the API is a bit of a hack and that the problem requires\nmore design work. I'd however use the current pixel rate value, not the\nminimum, default and maximum, as there's a bigger chance in that case\nthat the result of the calculation would be meaningless.\n\n> +\n> +\tint32_t minExposure = static_cast(minExposurePixels / maxPixelRate);\n> +\tint32_t maxExposure = static_cast(maxExposurePixels / minPixelRate);\n> +\tint32_t defExposure = static_cast(defExposurePixels / defPixelRate);\n> +\n> +\tControlInfoMap::Map map;\n> +\tmap[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> +\t\t\t\t\t\t defExposure);\n> +\tcontrols_ = std::move(map);\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\fn CameraSensor::model()\n> * \\brief Retrieve the sensor model name","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 99E05C0F1A\n\tfor ;\n\tWed, 30 Dec 2020 12:17:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 198B1615B2;\n\tWed, 30 Dec 2020 13:17:17 +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 0EAF260320\n\tfor ;\n\tWed, 30 Dec 2020 13:17:15 +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 717032A3;\n\tWed, 30 Dec 2020 13:17:14 +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=\"gAxJ8tD8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609330634;\n\tbh=ygVePof1CEW7ycU8B1mSwnP+7UmT8VXR9z0Va3Z/ABE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gAxJ8tD8Q8F72/a3U3rs4X9qDouH5lq9w9pgDmtvytU+VREUEzuvS74jTC0lkhOYc\n\tozoqGXOM9e++74GfHQPRLTWN2rcFQgfYiBWuRBq0VRjQWfSx4wRj9cMJxeDjGFCiqY\n\tpmnsseGN/PSI1wA5XgTGYOrhI5iMy9lJpM0tKGkQ=","Date":"Wed, 30 Dec 2020 14:17:03 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20201228165600.53987-1-jacopo@jmondi.org>\n\t<20201228165600.53987-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201228165600.53987-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] libcamera: camera_sensor:\n\tInitialize controls","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\" "}}]