[{"id":14652,"web_url":"https://patchwork.libcamera.org/comment/14652/","msgid":"<01161e11-4b57-c86c-7fdd-27e268b50186@ideasonboard.com>","date":"2021-01-21T14:17:18","subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 19/01/2021 15:37, Jacopo Mondi wrote:\n> Calculate the controls::Exposure limits at camera registration time\n> and register it in the list of camera supported controls.\n> \n> Cache the default exposure value to report it in the request metadata.\n> \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 66 ++++++++++++++++++++++++++--\n> 1 file changed, 62 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 73304ea73050..f928af4d92a2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;\n> static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> \n> -static const ControlInfoMap IPU3Controls = {\n> +static const ControlInfoMap::Map IPU3Controls = {\n> \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> };\n> \n> @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), exposureTime_(0)\n> \t{\n> \t}\n> \n> @@ -62,6 +62,8 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tuint32_t exposureTime_;\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -119,6 +121,7 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n> \n> +\tint initControls(IPU3CameraData *data);\n> \tint registerCameras();\n> \n> \tint allocateBuffers(Camera *camera);\n> @@ -730,6 +733,58 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> \treturn ret == 0;\n> }\n> \n> +/**\n> + * \\brief Initialize the camera controls\n> + * \\param[in] data The camera data\n> + *\n> + * Initialize the camera controls as the union of the static pipeline handler\n> + * controls (IPU3Controls) and controls created dynamically from the sensor\n> + * capabilities.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n\nProbably more a cosmetic and \"can wait\" thing: initControls is used for\nexposure only...\nIf there is more controls in the future, shouldn't we create a\ninitExposureControl and call it from initControls (which will ease its\nreading if more are added) ?\n\n> +{\n> +\tconst CameraSensor *sensor = data->cio2_.sensor();\n> +\tCameraSensorInfo sensorInfo{};\n> +\n> +\tint ret = sensor->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tControlInfoMap::Map controls = IPU3Controls;\n> +\n> +\t/*\n> +\t * Compute exposure time limits.\n> +\t *\n> +\t * \\todo The exposure limits depend on the sensor configuration.\n> +\t * Initialize the control using the line length and pixel rate of the\n> +\t * current configuration converted to microseconds. Use the\n> +\t * V4L2_CID_EXPOSURE control to get exposure min, max and default and\n> +\t * convert it from lines to microseconds.\n> +\t */\n> +\tdouble lineDuration = sensorInfo.lineLength\n> +\t\t\t / (sensorInfo.pixelRate / 1e6f);\n> +\tconst ControlInfoMap &sensorControls = sensor->controls();\n> +\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> +\tint32_t minExposure = v4l2Exposure.min().get() * lineDuration;\n> +\tint32_t maxExposure = v4l2Exposure.max().get() * lineDuration;\n> +\tint32_t defExposure = v4l2Exposure.def().get() * lineDuration;\n> +\n> +\t/*\n> +\t * \\todo Report the actual exposure time, use the default for the\n> +\t * moment.\n> +\t */\n> +\tdata->exposureTime_ = defExposure;\n> +\n> +\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> +\t\t\t\t\t\t\tdefExposure);\n\nBasically, we are transforming an undefined exposure value into an\nexposureTime value in a new ControlInfo... ?\nMeaning the camera_sensor could be able to use it to convert from time\nto exposure maybe ?\nI am wondering if it could not be used to generalize the CamHelper from\nraspberrypi to a more general cam_helper class used through the pipeline ?\nNot sure my question is crystal clear :-p.\n\n> +\tdata->controlInfo_ = std::move(controls);\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Initialise ImgU and CIO2 devices associated with cameras\n> *\n> @@ -775,8 +830,9 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n> \n> -\t\t/* Initialze the camera controls. */\n> -\t\tdata->controlInfo_ = IPU3Controls;\n> +\t\tret = initControls(data.get());\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> \n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> @@ -841,6 +897,8 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \n> \t/* Mark the request as complete. */\n> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> +\t/* \\todo Move the ExposureTime control to the IPA. */\n> +\trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> \tpipe_->completeRequest(request);\n> }\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 5F3C9C0F2A\n\tfor ;\n\tThu, 21 Jan 2021 14:17:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6BD6681F0;\n\tThu, 21 Jan 2021 15:17:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45B78681DB\n\tfor ;\n\tThu, 21 Jan 2021 15:17:19 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:4d81:9b04:8287:3bd1] (unknown\n\t[IPv6:2a01:e0a:169:7140:4d81:9b04:8287:3bd1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DEA2B50E;\n\tThu, 21 Jan 2021 15:17:18 +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=\"SiOq3a+q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611238638;\n\tbh=3R5eSAccpnMnddl1Maj6WpUQpeL1a7gyJSvIhZzSBdk=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=SiOq3a+q9Gx1/2XL/aBjmJSG8T/WM8vwY4MCgOS+AR2jOMdPpGnizKObsw/fAhEab\n\tq+5up8oB5Tf5E5FtZ5LhJwXsWDvB1t/v5a+PEMmaiU7DuWi0ZEyW940g5d8gLjklkl\n\tsoT/vv+NEeWYTLibidggqQNvK8acU/d6dGsmsxJk=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-4-jacopo@jmondi.org>","From":"Jean-Michel Hautbois ","Message-ID":"<01161e11-4b57-c86c-7fdd-27e268b50186@ideasonboard.com>","Date":"Thu, 21 Jan 2021 15:17:18 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.5.0","MIME-Version":"1.0","In-Reply-To":"<20210119143711.153517-4-jacopo@jmondi.org>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14658,"web_url":"https://patchwork.libcamera.org/comment/14658/","msgid":"<20210121155131.a3e574li53a5hgdw@uno.localdomain>","date":"2021-01-21T15:51:31","subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello Jean-Michel,\n\nOn Thu, Jan 21, 2021 at 03:17:18PM +0100, Jean-Michel Hautbois wrote:\n> Hi Jacopo,\n>\n> On 19/01/2021 15:37, Jacopo Mondi wrote:\n> > Calculate the controls::Exposure limits at camera registration time\n> > and register it in the list of camera supported controls.\n> >\n> > Cache the default exposure value to report it in the request metadata.\n> >\n> > Reviewed-by: Laurent Pinchart \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 66 ++++++++++++++++++++++++++--\n> > 1 file changed, 62 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 73304ea73050..f928af4d92a2 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;\n> > static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> >\n> > -static const ControlInfoMap IPU3Controls = {\n> > +static const ControlInfoMap::Map IPU3Controls = {\n> > \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> > };\n> >\n> > @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData\n> > {\n> > public:\n> > \tIPU3CameraData(PipelineHandler *pipe)\n> > -\t\t: CameraData(pipe)\n> > +\t\t: CameraData(pipe), exposureTime_(0)\n> > \t{\n> > \t}\n> >\n> > @@ -62,6 +62,8 @@ public:\n> > \tStream outStream_;\n> > \tStream vfStream_;\n> > \tStream rawStream_;\n> > +\n> > +\tuint32_t exposureTime_;\n> > };\n> >\n> > class IPU3CameraConfiguration : public CameraConfiguration\n> > @@ -119,6 +121,7 @@ private:\n> > \t\t\tPipelineHandler::cameraData(camera));\n> > \t}\n> >\n> > +\tint initControls(IPU3CameraData *data);\n> > \tint registerCameras();\n> >\n> > \tint allocateBuffers(Camera *camera);\n> > @@ -730,6 +733,58 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > \treturn ret == 0;\n> > }\n> >\n> > +/**\n> > + * \\brief Initialize the camera controls\n> > + * \\param[in] data The camera data\n> > + *\n> > + * Initialize the camera controls as the union of the static pipeline handler\n> > + * controls (IPU3Controls) and controls created dynamically from the sensor\n> > + * capabilities.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n>\n> Probably more a cosmetic and \"can wait\" thing: initControls is used for\n> exposure only...\n> If there is more controls in the future, shouldn't we create a\n> initExposureControl and call it from initControls (which will ease its\n> reading if more are added) ?\n>\n\nPossibly. This series adds already one more control (ScalerCrop) and\nmore will come.\n\nSo far this didn't bother me for being too long as a function, but we\ncan consider splitting already indeed.\n\n> > +{\n> > +\tconst CameraSensor *sensor = data->cio2_.sensor();\n> > +\tCameraSensorInfo sensorInfo{};\n> > +\n> > +\tint ret = sensor->sensorInfo(&sensorInfo);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tControlInfoMap::Map controls = IPU3Controls;\n> > +\n> > +\t/*\n> > +\t * Compute exposure time limits.\n> > +\t *\n> > +\t * \\todo The exposure limits depend on the sensor configuration.\n> > +\t * Initialize the control using the line length and pixel rate of the\n> > +\t * current configuration converted to microseconds. Use the\n> > +\t * V4L2_CID_EXPOSURE control to get exposure min, max and default and\n> > +\t * convert it from lines to microseconds.\n> > +\t */\n> > +\tdouble lineDuration = sensorInfo.lineLength\n> > +\t\t\t / (sensorInfo.pixelRate / 1e6f);\n> > +\tconst ControlInfoMap &sensorControls = sensor->controls();\n> > +\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> > +\tint32_t minExposure = v4l2Exposure.min().get() * lineDuration;\n> > +\tint32_t maxExposure = v4l2Exposure.max().get() * lineDuration;\n> > +\tint32_t defExposure = v4l2Exposure.def().get() * lineDuration;\n> > +\n> > +\t/*\n> > +\t * \\todo Report the actual exposure time, use the default for the\n> > +\t * moment.\n> > +\t */\n> > +\tdata->exposureTime_ = defExposure;\n> > +\n> > +\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> > +\t\t\t\t\t\t\tdefExposure);\n>\n> Basically, we are transforming an undefined exposure value into an\n> exposureTime value in a new ControlInfo... ?\n\nWhy 'undefined' ? The thing here is that all these controls are\ndependent on the sensor configuration, so establishing a good initial\nvalue can only be done by implementing a policy. Here I've chose to\nuse the current configuration, but there might be better options.\n\nOr are you referring to the todo note for data->exposureTime_ ?\n\n> Meaning the camera_sensor could be able to use it to convert from time\n> to exposure maybe ?\n\nIt could indeed. The thing is that we haven't yet established an\ninterface for the controls the CameraSensor should expose.\n\nRight now the class reports V4L2 controls, and operates on the same\nset of controls in set/getControls(). Also, sensor controls are transported\nto the IPA as V4L2 controls, not libcamera and this has to be taken into\naccount.\n\nAdding an interface to report libcamera Control[Info] created by\ninspecting V4L2 control limits might be an option, but IPU3 is the\nsole user of that interface so we might need a few more data point to\nactually see what interface is better suited.\n\nIn brief, yes, the initialization of the ControlInfoMap accessible by\napplications through libcamera::Camera::controls() could be\ncentralized or moved to helpers, but a few more things have to be\nstraighten up first to make the right (or the less wrong, if you\nprefer) call.\n\n> I am wondering if it could not be used to generalize the CamHelper from\n> raspberrypi to a more general cam_helper class used through the pipeline ?\n> Not sure my question is crystal clear :-p.\n\nThe CamHelper functionalities should be generalized, I agree. The part\nthat serves to report static sensor information (ie the controls'\ndelay) will probably be moved to a CameraSensorDatabase which has been\non the list already. Afaict CamHelper won't help with controls\ninitialization, but in general yes, we should aim to generalize those\ncomponents.\n\nThanks\n j\n\n>\n> > +\tdata->controlInfo_ = std::move(controls);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /**\n> > * \\brief Initialise ImgU and CIO2 devices associated with cameras\n> > *\n> > @@ -775,8 +830,9 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\t/* Initialize the camera properties. */\n> > \t\tdata->properties_ = cio2->sensor()->properties();\n> >\n> > -\t\t/* Initialze the camera controls. */\n> > -\t\tdata->controlInfo_ = IPU3Controls;\n> > +\t\tret = initControls(data.get());\n> > +\t\tif (ret)\n> > +\t\t\tcontinue;\n> >\n> > \t\t/**\n> > \t\t * \\todo Dynamically assign ImgU and output devices to each\n> > @@ -841,6 +897,8 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> >\n> > \t/* Mark the request as complete. */\n> > \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> > +\t/* \\todo Move the ExposureTime control to the IPA. */\n> > +\trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> > \tpipe_->completeRequest(request);\n> > }\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 F1EA8BD808\n\tfor ;\n\tThu, 21 Jan 2021 15:51:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CDA5D68201;\n\tThu, 21 Jan 2021 16:51:13 +0100 (CET)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5F3E681E9\n\tfor ;\n\tThu, 21 Jan 2021 16:51:12 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 6E65B1C000F;\n\tThu, 21 Jan 2021 15:51:12 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 21 Jan 2021 16:51:31 +0100","From":"Jacopo Mondi ","To":"Jean-Michel Hautbois ","Message-ID":"<20210121155131.a3e574li53a5hgdw@uno.localdomain>","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-4-jacopo@jmondi.org>\n\t<01161e11-4b57-c86c-7fdd-27e268b50186@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<01161e11-4b57-c86c-7fdd-27e268b50186@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","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":14753,"web_url":"https://patchwork.libcamera.org/comment/14753/","msgid":"","date":"2021-01-25T11:21:11","subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","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 Tue, Jan 19, 2021 at 03:37:03PM +0100, Jacopo Mondi wrote:\n> Calculate the controls::Exposure limits at camera registration time\n> and register it in the list of camera supported controls.\n> \n> Cache the default exposure value to report it in the request metadata.\n> \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 66 ++++++++++++++++++++++++++--\n> 1 file changed, 62 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 73304ea73050..f928af4d92a2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;\n> static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> \n> -static const ControlInfoMap IPU3Controls = {\n> +static const ControlInfoMap::Map IPU3Controls = {\n> \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> };\n> \n> @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), exposureTime_(0)\n> \t{\n> \t}\n> \n> @@ -62,6 +62,8 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tuint32_t exposureTime_;\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -119,6 +121,7 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n> \n> +\tint initControls(IPU3CameraData *data);\n> \tint registerCameras();\n> \n> \tint allocateBuffers(Camera *camera);\n> @@ -730,6 +733,58 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> \treturn ret == 0;\n> }\n> \n> +/**\n> + * \\brief Initialize the camera controls\n> + * \\param[in] data The camera data\n> + *\n> + * Initialize the camera controls as the union of the static pipeline handler\n> + * controls (IPU3Controls) and controls created dynamically from the sensor\n> + * capabilities.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n> +{\n> +\tconst CameraSensor *sensor = data->cio2_.sensor();\n> +\tCameraSensorInfo sensorInfo{};\n> +\n> +\tint ret = sensor->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tControlInfoMap::Map controls = IPU3Controls;\n> +\n> +\t/*\n> +\t * Compute exposure time limits.\n> +\t *\n> +\t * \\todo The exposure limits depend on the sensor configuration.\n> +\t * Initialize the control using the line length and pixel rate of the\n> +\t * current configuration converted to microseconds. Use the\n> +\t * V4L2_CID_EXPOSURE control to get exposure min, max and default and\n> +\t * convert it from lines to microseconds.\n> +\t */\n> +\tdouble lineDuration = sensorInfo.lineLength\n> +\t\t\t / (sensorInfo.pixelRate / 1e6f);\n\nAs you use a double, I'd write s/1e6f/1e6/.\n\n> +\tconst ControlInfoMap &sensorControls = sensor->controls();\n> +\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> +\tint32_t minExposure = v4l2Exposure.min().get() * lineDuration;\n> +\tint32_t maxExposure = v4l2Exposure.max().get() * lineDuration;\n> +\tint32_t defExposure = v4l2Exposure.def().get() * lineDuration;\n> +\n> +\t/*\n> +\t * \\todo Report the actual exposure time, use the default for the\n> +\t * moment.\n> +\t */\n> +\tdata->exposureTime_ = defExposure;\n> +\n> +\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> +\t\t\t\t\t\t\tdefExposure);\n> +\n> +\tdata->controlInfo_ = std::move(controls);\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Initialise ImgU and CIO2 devices associated with cameras\n> *\n> @@ -775,8 +830,9 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n> \n> -\t\t/* Initialze the camera controls. */\n> -\t\tdata->controlInfo_ = IPU3Controls;\n> +\t\tret = initControls(data.get());\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> \n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> @@ -841,6 +897,8 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \n> \t/* Mark the request as complete. */\n> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> +\t/* \\todo Move the ExposureTime control to the IPA. */\n> +\trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> \tpipe_->completeRequest(request);\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 B5479C0F2B\n\tfor ;\n\tMon, 25 Jan 2021 11:21:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3EA20682C0;\n\tMon, 25 Jan 2021 12:21:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74F5D6030B\n\tfor ;\n\tMon, 25 Jan 2021 12:21:31 +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 DADDD331;\n\tMon, 25 Jan 2021 12:21:30 +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=\"rF2qG5fK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611573691;\n\tbh=JZFYaLwS3n/MuIPVWNv6WVDUU74ejGXGYesWZFVZ/RY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rF2qG5fKSkhzlROT1p6ePFLhvUXho32ZqZlSmus5m9Oe4UhQXmogt2Ygf87XFvA6b\n\tF6yy6oG+kaZwhkvwxz0AVHkA7R3w+6WGEPAxh7qRy8dywnl5yep7KNs+26/beFqFEi\n\tFwlGEH6Z33H1OqSR/r07wscj+lmzpUBDNnn6h854=","Date":"Mon, 25 Jan 2021 13:21:11 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210119143711.153517-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 03/11] libcamera: ipu3: Register\n\tExposure control","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\" "}}]