{"id":13346,"url":"https://patchwork.libcamera.org/api/1.1/patches/13346/?format=json","web_url":"https://patchwork.libcamera.org/patch/13346/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210813143241.3528968-1-kieran.bingham@ideasonboard.com>","date":"2021-08-13T14:32:41","name":"[libcamera-devel] controls: Report required control names","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"ef89f7cc1f9ce278b7c8a8aab48cc33acbd5f18b","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/1.1/people/4/?format=json","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"delegate":{"id":11,"url":"https://patchwork.libcamera.org/api/1.1/users/11/?format=json","username":"kbingham","first_name":"Kieran","last_name":"Bingham","email":"kieran.bingham@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/13346/mbox/","series":[{"id":2355,"url":"https://patchwork.libcamera.org/api/1.1/series/2355/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2355","date":"2021-08-13T14:32:41","name":"[libcamera-devel] controls: Report required control names","version":1,"mbox":"https://patchwork.libcamera.org/series/2355/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13346/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13346/checks/","tags":{},"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 6DF33C3240\n\tfor ;\n\tFri, 13 Aug 2021 14:32:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0E976888F;\n\tFri, 13 Aug 2021 16:32:46 +0200 (CEST)","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 E2F75687FA\n\tfor ;\n\tFri, 13 Aug 2021 16:32:44 +0200 (CEST)","from Monstersaurus.local\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6F7D64A1;\n\tFri, 13 Aug 2021 16:32:44 +0200 (CEST)"],"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=\"NNjtX2vC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628865164;\n\tbh=6Dq0VavvQGxq7TIQtKK9uUSZKFiI0nzRENIUnF2IWow=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=NNjtX2vCciyFozawm+8vaUVcGlBkr8z7Um1yTJGW8jBIW6NM5t0phvOftsoHn0sfO\n\tqFQdJWQ//wS1YlkZHGmukE1t/eiuEpBaPqs1aOklZ+xP0Z+eAuVSykmWGQZyyAOeON\n\ty69LTKVgYMQ1wJImq+Tr4dfiQvfw5RY6qhlUM0Nk=","From":"Kieran Bingham ","To":"libcamera devel ","Date":"Fri, 13 Aug 2021 15:32:41 +0100","Message-Id":"<20210813143241.3528968-1-kieran.bingham@ideasonboard.com>","X-Mailer":"git-send-email 2.30.2","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH] controls: Report required control names","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The V4L2 controls which are optional, recommended, and mandatory report\nerrors if they are not found by CameraSensor::validateSensorDriver().\n\nThese errors report hex values for the controls, which can easily lead\nto confusion and incorrect assumption about which control needs to be\ninvestigated.\n\nThis can be seen occuring already in issues such as [0] and is generally\nan unfriendly result to report to users in a warning or error message.\n\nAdapt the tables of controls to store both the id and name of the\ncontrols to report a user friendly message that can ease diagnosis of\nany CameraSensor validation issues.\n\n [0] https://github.com/raspberrypi/linux/issues/4500\n\nSigned-off-by: Kieran Bingham \n\n---\nI'm aware of course of the checkstyle changes suggested on V4L2_CID, but\nI thought it was better to keep those macros 'scoped' here, and indented\naccordingly.\n---\n src/libcamera/camera_sensor.cpp | 45 ++++++++++++++++++++-------------\n 1 file changed, 27 insertions(+), 18 deletions(-)","diff":"diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 7a3864150c89..1409da1b071c 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -168,19 +168,26 @@ int CameraSensor::validateSensorDriver()\n {\n \tint err = 0;\n \n+\tstruct v4l2_cid {\n+\t\tconst uint32_t id;\n+\t\tconst char *name;\n+\t};\n+\n+\t#define V4L2_CID(x) { x, #x }\n+\n \t/*\n \t * Optional controls are used to register optional sensor properties. If\n \t * not present, some values will be defaulted.\n \t */\n-\tstatic constexpr uint32_t optionalControls[] = {\n-\t\tV4L2_CID_CAMERA_SENSOR_ROTATION,\n+\tstatic constexpr v4l2_cid optionalControls[] = {\n+\t\tV4L2_CID(V4L2_CID_CAMERA_SENSOR_ROTATION),\n \t};\n \n \tconst ControlIdMap &controls = subdev_->controls().idmap();\n-\tfor (uint32_t ctrl : optionalControls) {\n-\t\tif (!controls.count(ctrl))\n+\tfor (auto &ctrl : optionalControls) {\n+\t\tif (!controls.count(ctrl.id))\n \t\t\tLOG(CameraSensor, Debug)\n-\t\t\t\t<< \"Optional V4L2 control \" << utils::hex(ctrl)\n+\t\t\t\t<< \"Optional V4L2 control \" << ctrl.name\n \t\t\t\t<< \" not supported\";\n \t}\n \n@@ -188,14 +195,14 @@ int CameraSensor::validateSensorDriver()\n \t * Recommended controls are similar to optional controls, but will\n \t * become mandatory in the near future. Be loud if they're missing.\n \t */\n-\tstatic constexpr uint32_t recommendedControls[] = {\n-\t\tV4L2_CID_CAMERA_ORIENTATION,\n+\tstatic constexpr v4l2_cid recommendedControls[] = {\n+\t\tV4L2_CID(V4L2_CID_CAMERA_ORIENTATION),\n \t};\n \n-\tfor (uint32_t ctrl : recommendedControls) {\n-\t\tif (!controls.count(ctrl)) {\n+\tfor (auto &ctrl : recommendedControls) {\n+\t\tif (!controls.count(ctrl.id)) {\n \t\t\tLOG(CameraSensor, Warning)\n-\t\t\t\t<< \"Recommended V4L2 control \" << utils::hex(ctrl)\n+\t\t\t\t<< \"Recommended V4L2 control \" << ctrl.name\n \t\t\t\t<< \" not supported\";\n \t\t\terr = -EINVAL;\n \t\t}\n@@ -259,18 +266,20 @@ int CameraSensor::validateSensorDriver()\n \t * For raw sensors, make sure the sensor driver supports the controls\n \t * required by the CameraSensor class.\n \t */\n-\tstatic constexpr uint32_t mandatoryControls[] = {\n-\t\tV4L2_CID_EXPOSURE,\n-\t\tV4L2_CID_HBLANK,\n-\t\tV4L2_CID_PIXEL_RATE,\n-\t\tV4L2_CID_VBLANK,\n+\tstatic constexpr v4l2_cid mandatoryControls[] = {\n+\t\tV4L2_CID(V4L2_CID_EXPOSURE),\n+\t\tV4L2_CID(V4L2_CID_HBLANK),\n+\t\tV4L2_CID(V4L2_CID_PIXEL_RATE),\n+\t\tV4L2_CID(V4L2_CID_VBLANK),\n \t};\n \n+\t#undef V4L2_CID\n+\n \terr = 0;\n-\tfor (uint32_t ctrl : mandatoryControls) {\n-\t\tif (!controls.count(ctrl)) {\n+\tfor (auto &ctrl : mandatoryControls) {\n+\t\tif (!controls.count(ctrl.id)) {\n \t\t\tLOG(CameraSensor, Error)\n-\t\t\t\t<< \"Mandatory V4L2 control \" << utils::hex(ctrl)\n+\t\t\t\t<< \"Mandatory V4L2 control \" << ctrl.name\n \t\t\t\t<< \" not available\";\n \t\t\terr = -EINVAL;\n \t\t}\n","prefixes":["libcamera-devel"]}