[{"id":28831,"web_url":"https://patchwork.libcamera.org/comment/28831/","msgid":"","date":"2024-03-04T17:32:28","subject":"Re: [PATCH/RFC 15/32] libcamera: camera_sensor: Drop\n\tupdateControlInfo() function","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Fri, Mar 01, 2024 at 11:21:04PM +0200, Laurent Pinchart wrote:\n> The CameraSensor::updateControlInfo() function is a wrapper around the\n> same function of the V4L2Subdevice class. It was meant to be called by\n> pipeline handlers that modify the sensor configuration directly,\n> bypassing the CameraSensor::setFormat() function. This never happened,\n> and the function is called once only, internally to the CameraSensor\n> class. No external users are foreseen, drop the function and call\n> V4L2Subdevice::updateControlInfo() directly.\n>\n> Signed-off-by: Laurent Pinchart \n\nThis could indeed be fast-tracked\n\nReviewed-by: Jacopo Mondi \n\n> ---\n> include/libcamera/internal/camera_sensor.h | 2 --\n> src/libcamera/sensor/camera_sensor.cpp | 20 ++------------------\n> 2 files changed, 2 insertions(+), 20 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 60a8b106d175..b2f077b9cc75 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -75,8 +75,6 @@ public:\n> \tconst ControlList &properties() const { return properties_; }\n> \tint sensorInfo(IPACameraSensorInfo *info) const;\n>\n> -\tvoid updateControlInfo();\n> -\n> \tCameraLens *focusLens() { return focusLens_.get(); }\n>\n> \tTransform computeTransform(Orientation *orientation) const;\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index af5d97f35de1..545f89d036df 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -814,7 +814,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\tupdateControlInfo();\n> +\tsubdev_->updateControlInfo();\n> \treturn 0;\n> }\n>\n> @@ -924,9 +924,7 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config,\n> *\n> * Control information is updated automatically to reflect the current sensor\n> * configuration when the setFormat() function is called, without invalidating\n> - * any iterator on the ControlInfoMap. A manual update can also be forced by\n> - * calling the updateControlInfo() function for pipeline handlers that change\n> - * the sensor configuration wihtout using setFormat().\n> + * any iterator on the ControlInfoMap.\n> *\n> * \\return A map of the V4L2 controls supported by the sensor\n> */\n> @@ -1013,10 +1011,6 @@ int CameraSensor::setControls(ControlList *ctrls)\n> * Sensor information is only available for raw sensors. When called for a YUV\n> * sensor, this function returns -EINVAL.\n> *\n> - * Pipeline handlers that do not change the sensor format using the setFormat()\n> - * function may need to call updateControlInfo() beforehand, to ensure all the\n> - * control ranges are up to date.\n> - *\n> * \\return 0 on success, a negative error code otherwise\n> */\n> int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> @@ -1094,16 +1088,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> \treturn 0;\n> }\n>\n> -/**\n> - * \\fn void CameraSensor::updateControlInfo()\n> - * \\brief Update the sensor's ControlInfoMap in case they have changed\n> - * \\sa V4L2Device::updateControlInfo()\n> - */\n> -void CameraSensor::updateControlInfo()\n> -{\n> -\tsubdev_->updateControlInfo();\n> -}\n> -\n> /**\n> * \\fn CameraSensor::focusLens()\n> * \\brief Retrieve the focus lens controller\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 A8459C326B\n\tfor ;\n\tMon, 4 Mar 2024 17:32:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED1166286C;\n\tMon, 4 Mar 2024 18:32:32 +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 38FBD627FC\n\tfor ;\n\tMon, 4 Mar 2024 18:32:31 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9E821BD1;\n\tMon, 4 Mar 2024 18:32:14 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"adQSApSS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709573534;\n\tbh=03CV/pd3T3LCPpERcD/vTeiAS9EPtLG9H3zDsQ635Qs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=adQSApSSE89prQiXQBznlHDUwFRgrT8dgQGpkGFDeNi5qsJCh9SKEzZHIo/U7TyIq\n\tIU+/usV4zk9G0QS7jUtd0jNU6z/aLKGVEOjJrWZj1+NfQa2KCwqpd5TMD4FksGeuZl\n\tS+uRRWXdH9WS0ReeqoiynlppqObUS42j3UoK4HGo=","Date":"Mon, 4 Mar 2024 18:32:28 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Subject":"Re: [PATCH/RFC 15/32] libcamera: camera_sensor: Drop\n\tupdateControlInfo() function","Message-ID":"","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-16-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240301212121.9072-16-laurent.pinchart@ideasonboard.com>","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, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28885,"web_url":"https://patchwork.libcamera.org/comment/28885/","msgid":"<20240306161224.y45rlcvq5xnhwal5@jasper>","date":"2024-03-06T16:12:24","subject":"Re: [PATCH/RFC 15/32] libcamera: camera_sensor: Drop\n\tupdateControlInfo() function","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, Mar 01, 2024 at 11:21:04PM +0200, Laurent Pinchart wrote:\n> The CameraSensor::updateControlInfo() function is a wrapper around the\n> same function of the V4L2Subdevice class. It was meant to be called by\n> pipeline handlers that modify the sensor configuration directly,\n> bypassing the CameraSensor::setFormat() function. This never happened,\n> and the function is called once only, internally to the CameraSensor\n> class. No external users are foreseen, drop the function and call\n> V4L2Subdevice::updateControlInfo() directly.\n> \n> Signed-off-by: Laurent Pinchart \n\nNice. I overcautiously called it in my SimplePipeline changes after setFormat()\nThis can now be removed.\n\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\n> ---\n> include/libcamera/internal/camera_sensor.h | 2 --\n> src/libcamera/sensor/camera_sensor.cpp | 20 ++------------------\n> 2 files changed, 2 insertions(+), 20 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 60a8b106d175..b2f077b9cc75 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -75,8 +75,6 @@ public:\n> \tconst ControlList &properties() const { return properties_; }\n> \tint sensorInfo(IPACameraSensorInfo *info) const;\n> \n> -\tvoid updateControlInfo();\n> -\n> \tCameraLens *focusLens() { return focusLens_.get(); }\n> \n> \tTransform computeTransform(Orientation *orientation) const;\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index af5d97f35de1..545f89d036df 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -814,7 +814,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tupdateControlInfo();\n> +\tsubdev_->updateControlInfo();\n> \treturn 0;\n> }\n> \n> @@ -924,9 +924,7 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config,\n> *\n> * Control information is updated automatically to reflect the current sensor\n> * configuration when the setFormat() function is called, without invalidating\n> - * any iterator on the ControlInfoMap. A manual update can also be forced by\n> - * calling the updateControlInfo() function for pipeline handlers that change\n> - * the sensor configuration wihtout using setFormat().\n> + * any iterator on the ControlInfoMap.\n> *\n> * \\return A map of the V4L2 controls supported by the sensor\n> */\n> @@ -1013,10 +1011,6 @@ int CameraSensor::setControls(ControlList *ctrls)\n> * Sensor information is only available for raw sensors. When called for a YUV\n> * sensor, this function returns -EINVAL.\n> *\n> - * Pipeline handlers that do not change the sensor format using the setFormat()\n> - * function may need to call updateControlInfo() beforehand, to ensure all the\n> - * control ranges are up to date.\n> - *\n> * \\return 0 on success, a negative error code otherwise\n> */\n> int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> @@ -1094,16 +1088,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> \treturn 0;\n> }\n> \n> -/**\n> - * \\fn void CameraSensor::updateControlInfo()\n> - * \\brief Update the sensor's ControlInfoMap in case they have changed\n> - * \\sa V4L2Device::updateControlInfo()\n> - */\n> -void CameraSensor::updateControlInfo()\n> -{\n> -\tsubdev_->updateControlInfo();\n> -}\n> -\n> /**\n> * \\fn CameraSensor::focusLens()\n> * \\brief Retrieve the focus lens controller\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 136EDC0F2A\n\tfor ;\n\tWed, 6 Mar 2024 16:12:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84AA362867;\n\tWed, 6 Mar 2024 17:12:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 782B361C8F\n\tfor ;\n\tWed, 6 Mar 2024 17:12:27 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:dd65:a3ea:5f4d:989a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8053EBD1;\n\tWed, 6 Mar 2024 17:12:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ow5SgYdp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709741529;\n\tbh=nOYrr1Lc6c6LnQsuzDAJcgn4jqMT9UOiXwOncU3sylw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ow5SgYdp7PRw7fLtXzQdJVYnHehfA5Q4jDcjs/lspt0acJnYwVQalZyUtKUvgqg3V\n\tXbab++dVFnZN79Jk5EGO+IPbSUIZghEgj92iES6JmNuUsl/ij0HxeqTXlZnud9ZzRP\n\tWka4H/9emQH8Df5Q9Ipp4UnYh1KsAYeJjr139RHE=","Date":"Wed, 6 Mar 2024 17:12:24 +0100","From":"Stefan Klug ","To":"Laurent Pinchart ","Subject":"Re: [PATCH/RFC 15/32] libcamera: camera_sensor: Drop\n\tupdateControlInfo() function","Message-ID":"<20240306161224.y45rlcvq5xnhwal5@jasper>","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-16-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240301212121.9072-16-laurent.pinchart@ideasonboard.com>","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, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]