[{"id":21454,"web_url":"https://patchwork.libcamera.org/comment/21454/","msgid":"<163827592245.3059017.1786876917025038752@Monstersaurus>","date":"2021-11-30T12:38:42","subject":"Re: [libcamera-devel] [PATCH v6 2/4] libcamera: add model() for\n\tretriving model name in V4L2Subdevice","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen (2021-11-30 10:51:55)\n> CameraSensor retrives model name from media entity. Move the heuristics\n> method into V4L2Subdevice, so CameraLens can reuse the function.\n> \n\n(The following line should be added while applying or for a next\nversion if needed).\n\n\"While here, fix a small typo for 'device'.\"\n\n> Signed-off-by: Han-Lin Chen \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 4 ++\n> src/libcamera/camera_sensor.cpp | 32 +---------------\n> src/libcamera/v4l2_subdevice.cpp | 42 +++++++++++++++++++++\n> 3 files changed, 48 insertions(+), 30 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 97b89fb9..763c4dec 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -61,6 +61,8 @@ public:\n> int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> Whence whence = ActiveFormat);\n> \n> + std::string model();\n> +\n> static std::unique_ptr\n> fromEntityName(const MediaDevice *media, const std::string &entity);\n> \n> @@ -75,6 +77,8 @@ private:\n> unsigned int code);\n> \n> const MediaEntity *entity_;\n> +\n> + std::string model_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 9fdb8c09..6151b32e 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -13,7 +13,6 @@\n> #include \n> #include \n> #include \n> -#include \n> #include \n> \n> #include \n> @@ -347,34 +346,7 @@ void CameraSensor::initTestPatternModes(\n> \n> int CameraSensor::initProperties()\n> {\n> - /*\n> - * Extract the camera sensor model name from the media entity name.\n> - *\n> - * There is no standardized naming scheme for sensor entities in the\n> - * Linux kernel at the moment.\n> - *\n> - * - The most common rule, used by I2C sensors, associates the model\n> - * name with the I2C bus number and address (e.g. 'imx219 0-0010').\n> - *\n> - * - When the sensor exposes multiple subdevs, the model name is\n> - * usually followed by a function name, as in the smiapp driver (e.g.\n> - * 'jt8ew9 pixel_array 0-0010').\n> - *\n> - * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.\n> - *\n> - * Other schemes probably exist. As a best effort heuristic, use the\n> - * part of the entity name before the first space if the name contains\n> - * an I2C address, and use the full entity name otherwise.\n> - */\n> - std::string entityName = entity_->name();\n> - std::regex i2cRegex{ \" [0-9]+-[0-9a-f]{4}\" };\n> - std::smatch match;\n> -\n> - if (std::regex_search(entityName, match, i2cRegex))\n> - model_ = entityName.substr(0, entityName.find(' '));\n> - else\n> - model_ = entityName;\n> -\n> + model_ = subdev_->model();\n> properties_.set(properties::Model, utils::toAscii(model_));\n> \n> /* Generate a unique ID for the sensor. */\n> @@ -832,7 +804,7 @@ int CameraSensor::generateId()\n> /*\n> * Virtual sensors not described in firmware\n> *\n> - * Verify it's a platform device and construct ID from the deive path\n> + * Verify it's a platform device and construct ID from the device path\n> * and model of sensor.\n> */\n> if (devPath.find(\"/sys/devices/platform/\", 0) == 0) {\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 023e2328..17ec9fdf 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -442,6 +443,47 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> return 0;\n> }\n> \n> +/**\n> + * \\brief Retrieve the model name\n> + * \\return The model name of the device\n> + */\n> +std::string V4L2Subdevice::model()\n> +{\n> + if (!model_.empty())\n> + return model_;\n> +\n\nWe don't normally use lazy initialisation, but I think this is OK in\nthis case.\n\nMaybe we might have a subdevice which doesn't ever read the model, so\nthere wouldn't be any point parsing the information...\n\nAnd it can't be accessed uninitialised, so it seems fine to me.\n\n\nReviewed-by: Kieran Bingham \n\n\n> + /*\n> + * Extract model name from the media entity name.\n> + *\n> + * There is no standardized naming scheme for sensor entities in the\n> + * Linux kernel at the moment.\n> + *\n> + * - The most common rule, used by I2C sensors, associates the model\n> + * name with the I2C bus number and address (e.g. 'imx219 0-0010').\n> + *\n> + * - When the sensor exposes multiple subdevs, the model name is\n> + * usually followed by a function name, as in the smiapp driver (e.g.\n> + * 'jt8ew9 pixel_array 0-0010').\n> + *\n> + * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.\n> + *\n> + * Other schemes probably exist. As a best effort heuristic, use the\n> + * part of the entity name before the first space if the name contains\n> + * an I2C address, and use the full entity name otherwise.\n> + */\n> + std::string entityName = entity_->name();\n> + std::regex i2cRegex{ \" [0-9]+-[0-9a-f]{4}\" };\n> + std::smatch match;\n> +\n> + std::string model;\n> + if (std::regex_search(entityName, match, i2cRegex))\n> + model_ = entityName.substr(0, entityName.find(' '));\n> + else\n> + model_ = entityName;\n> +\n> + return model_;\n> +}\n> +\n> /**\n> * \\brief Create a new video subdevice instance from \\a entity in media device\n> * \\a media\n> -- \n> 2.34.0.rc2.393.gf8c9666880-goog\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 CFBDEBF415\n\tfor ;\n\tTue, 30 Nov 2021 12:38:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C1636071A;\n\tTue, 30 Nov 2021 13:38:46 +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 07E14605C4\n\tfor ;\n\tTue, 30 Nov 2021 13:38:45 +0100 (CET)","from pendragon.ideasonboard.com\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 AC7588F0;\n\tTue, 30 Nov 2021 13:38:44 +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=\"HZZuOzQF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638275924;\n\tbh=fUNYN6TAv/t+GjBCFYaLl5L4huaWzT/8GEfn/9IVlFs=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=HZZuOzQFtSkpVjaZdPsMchB1hZ7C8EhU2KzZ4AH6X+q99w7bZWSpgS3yjNj4hNvJj\n\t3RS7zW2ICi3npMNWt0mq37gdunCiDe69LVxlDAsDXMoA0/hFPfj6bCl9Mugh5/ZovM\n\tfQ9rX9xYRiqy5LqGdokNN4/Hw0pelj/WWwAXJ+rY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211130105157.203856-3-hanlinchen@chromium.org>","References":"<20211130105157.203856-1-hanlinchen@chromium.org>\n\t<20211130105157.203856-3-hanlinchen@chromium.org>","From":"Kieran Bingham ","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 30 Nov 2021 12:38:42 +0000","Message-ID":"<163827592245.3059017.1786876917025038752@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v6 2/4] libcamera: add model() for\n\tretriving model name in V4L2Subdevice","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\" "}}]