[{"id":27662,"web_url":"https://patchwork.libcamera.org/comment/27662/","msgid":"<169221955628.695434.4143946212912662525@ping.linuxembedded.co.uk>","date":"2023-08-16T20:59:16","subject":"Re: [libcamera-devel] [RFC PATCH 1/5] libcamera: pipeline:\n\tuvcvideo: Add UVC metadata node","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Gabby,\n\nQuoting Gabby George (2023-08-14 12:28:45)\n> Identify and open the UVC metadata's video node. This will give us\n> access to metadata associated with the video stream of the uvc camera.\n> The user will not have access to this video node and will not need to\n> manage its data in any way.\n> \n> Signed-off-by: Gabby George \n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 36 +++++++++++++++++++-\n> 1 file changed, 35 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 38f48a5d..4470d8a2 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -49,10 +49,13 @@ public:\n> const std::string &id() const { return id_; }\n> \n> std::unique_ptr video_;\n> + std::unique_ptr metadata_;\n> Stream stream_;\n> std::map> formats_;\n> \n> private:\n> + int initMetadata(MediaDevice *media);\n> +\n> bool generateId();\n> \n> std::string id_;\n> @@ -411,6 +414,37 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> return true;\n> }\n> \n> +int UVCCameraData::initMetadata(MediaDevice *media)\n> +{\n> + int ret;\n> +\n> + const std::vector &entities = media->entities();\n> + std::string dev_node_name = video_->deviceNode();\n> + auto metadata = std::find_if(entities.begin(), entities.end(),\n> + [&dev_node_name](MediaEntity *e) {\n> + return e->type() == MediaEntity::Type::V4L2VideoDevice && !(e->flags() & MEDIA_ENT_FL_DEFAULT);\n> + });\n> +\n> + if (metadata == entities.end()) {\n> + LOG(UVC, Error) << \"Could not find a metadata video device.\";\n> + return -ENODEV;\n> + }\n> +\n> + /* configure the metadata node */\n> + metadata_ = std::make_unique(*metadata);\n> + ret = metadata_->open();\n> + if (ret)\n> + return ret;\n> +\n> + if (!(metadata_->caps().isMeta())) {\n> + /* if the caps do not have the metadata attribute\n> + * (shouldn't happen) */\n\nKeep an eye on the coding styles, multiline comments should be formatted\ndifferently. Try not to be too succint with comments, we like to keep to\nfull sentences with valid grammar in the code base. Any comments should\nexplain reasoning and the situation.\n\nPerhaps:\n\n\t/*\n\t * UVC Devices are usually only anticipated to expose two\n\t * devices, so we assume the non-default device is the metadata\n\t * device node.\n\t */\n\nI do wonder if there are more complex devices out there that might break\nthis assumption, though I haven't personally come across any.\n\nLaurent - any feedback here? Can we continue with this assumption? or do\nwe have to do a full search here, opening every entity to check each\ncaps?\n\n> + metadata_ = NULL;\n> + return -EINVAL;\n> + }\n> + return 0;\n> +}\n> +\n> int UVCCameraData::init(MediaDevice *media)\n> {\n> int ret;\n> @@ -512,7 +546,7 @@ int UVCCameraData::init(MediaDevice *media)\n> \n> controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);\n> \n> - return 0;\n> + return initMetadata(media);\n\nHrm .... if this fails - we can no longer open the camera at all.\n\nMaybe we should just report an error and continue without the metadata\nnode just as we already do?\n\n--\nKieran\n\n\n> }\n> \n> bool UVCCameraData::generateId()\n> -- \n> 2.34.1\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 ADEDBBF415\n\tfor ;\n\tWed, 16 Aug 2023 20:59:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22F71628D3;\n\tWed, 16 Aug 2023 22:59:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89A2B628D2\n\tfor ;\n\tWed, 16 Aug 2023 22:59:19 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C316283;\n\tWed, 16 Aug 2023 22:58:06 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1692219563;\n\tbh=pQ1pcL57ZvGXiKbwj/kQEonIiBH5gTCv+xVJL4diX+M=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=BUJk81xmCIihi9TR70r2Gpx3638M5ImMi1xUB0rotuCKjh4lMMBtW0EhlRm1+fdbg\n\tG90XCy3yQTV6RgksQer3xBONRiovf5rr8ejB6UJc3jr/EMsi+YLs5FgO6V7i9u1eXF\n\tSnidU8zMdqgDn9Gn0woUUUcfBcJaFysz3fnOXuIztXgzXRd/RoQ6rBs9TFQkVbHxTW\n\tCFZfexFoSWcVmwGey5gVSaqF1xC1T96FfgvYfIEn6SkkW9KtgvIcIKMnRj7Keo7V3f\n\t4iLtZ5fCqtAwjCGWwrU/lfOHlFzN9V6lT44dfkGC+WpTnF674T1M2hTj6K4SI2zrOm\n\tzEzBDOFUdcL/Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1692219486;\n\tbh=pQ1pcL57ZvGXiKbwj/kQEonIiBH5gTCv+xVJL4diX+M=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=RPPy4zLj3F3kKkPcDlzVxl5mqIYWlREd/f0QEgiBVn6iQnrfdCc0Af7Xp4nE4nPZE\n\ttDnEfZBMZd0ABF0JxRzPCSi364EBhyty1awBAcqR0iG9q4sDN8dsbbG+pIZ9GMD62f\n\tmaGZmw1i8HIhNHw37zk8sLgUQ990g2VB8R0foTHI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RPPy4zLj\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230814112849.176943-2-gabbymg94@gmail.com>","References":"<20230814112849.176943-1-gabbymg94@gmail.com>\n\t<20230814112849.176943-2-gabbymg94@gmail.com>","To":"gabbymg94@gmail.com, libcamera-devel@lists.libcamera.org,\n\tvedantparanjape160201@gmail.com","Date":"Wed, 16 Aug 2023 21:59:16 +0100","Message-ID":"<169221955628.695434.4143946212912662525@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH 1/5] libcamera: pipeline:\n\tuvcvideo: Add UVC metadata node","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]