[{"id":31534,"web_url":"https://patchwork.libcamera.org/comment/31534/","msgid":"<172790621516.2371408.15923515984877278215@ping.linuxembedded.co.uk>","date":"2024-10-02T21:56:55","subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-10-02 22:26:31)\n> Provide a MediaPipeline class to help identifing and managing pipelines across\n> a MediaDevice graph.\n> \n> Signed-off-by: Kieran Bingham \n> \n> ---\n> v2:\n> \n> - use srcPads to clearly identify which pads are managed\n> - Only report enabling links when a change is made\n> \n> - fix header includes\n> \n> - Fix includes\n> - Remove period at end of briefs\n> \n> - Document function parameters\n> - expand documentation throughout\n> - Fix debug log capitalisation\n> \n> - reduce scope of single use 'ret'\n> \n> Signed-off-by: Kieran Bingham \n\n... snippity ...\n\n> +\n> +/**\n> + * \\brief Configure the entities of this MediaPipeline\n> + * \\param[in] sensor The configured CameraSensor to propogate\n\nHow did I miss that.\n\nWill add\n\n * \\param[inout] format The format to propogate through the pipeline\n\n> + *\n> + * Propagate formats through each of the entities of the Pipeline, validating\n> + * that each one was not adjusted by the driver from the desired format.\n> + *\n\nMaybe I should add \n\n * The format is updated with the final accepted format of the last\n * entity of the pipeline.\n\n> + * \\return 0 on success or a negative errno otherwise\n> + */\n> +int MediaPipeline::configure(CameraSensor *sensor, V4L2SubdeviceFormat *format)\n> +{\n> + int ret;\n> +\n> + for (const Entity &e : entities_) {\n> + /* The sensor is configured through the CameraSensor */\n> + if (!e.sourceLink)\n> + break;\n> +\n> + MediaLink *link = e.sourceLink;\n> + MediaPad *source = link->source();\n> + MediaPad *sink = link->sink();\n> +\n> + /* 'format' already contains the sensor configuration */\n> + if (source->entity() != sensor->entity()) {\n> + /* todo: Add MediaDevice cache to reduce FD pressure */\n> + V4L2Subdevice subdev(source->entity());\n> + ret = subdev.open();\n> + if (ret)\n> + return ret;\n> +\n> + ret = subdev.getFormat(source->index(), format);\n> + if (ret < 0)\n> + return ret;\n> + }\n> +\n> + V4L2SubdeviceFormat sourceFormat = *format;\n> + /* todo: Add MediaDevice cache to reduce FD pressure */\n> + V4L2Subdevice subdev(sink->entity());\n> + ret = subdev.open();\n> + if (ret)\n> + return ret;\n> +\n> + ret = subdev.setFormat(sink->index(), format);\n> + if (ret < 0)\n> + return ret;\n> +\n> + if (format->code != sourceFormat.code ||\n> + format->size != sourceFormat.size) {\n> + LOG(MediaPipeline, Debug)\n> + << \"Source '\" << *source\n> + << \" produces \" << sourceFormat\n> + << \", sink '\" << *sink\n> + << \" requires \" << format;\n> + return -EINVAL;\n> + }\n> +\n> + LOG(MediaPipeline, Debug)\n> + << \"Link \" << *link << \" configured with format \"\n> + << format;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index aa9ab0291854..2c0f8603b231 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_internal_sources = files([\n> 'mapped_framebuffer.cpp',\n> 'media_device.cpp',\n> 'media_object.cpp',\n> + 'media_pipeline.cpp',\n> 'pipeline_handler.cpp',\n> 'process.cpp',\n> 'pub_key.cpp',\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 83C97C3257\n\tfor ;\n\tWed, 2 Oct 2024 21:57:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1378063528;\n\tWed, 2 Oct 2024 23:56:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2CC686351F\n\tfor ;\n\tWed, 2 Oct 2024 23:56:58 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 60991581\n\tfor ;\n\tWed, 2 Oct 2024 23:55:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SqjLedVk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727906125;\n\tbh=NIoUsxN2HxRNzcgp+o/Ohu1oSTumhE/OSLpWUXZmd9Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=SqjLedVkqIRocgbDwFRLhdE49tXeg1TsAWyuxu1GS4s0uuWo4rlp/84y72leXz18f\n\tiEoNxy4H8XjXHabX+fZWE2cS5RFPQTEOMQqKWfwfxf+HyZ9GIBgE0MVNXsNACZF6XD\n\t2HiYc7iJlLWclhOAJDq3l+oekiqEs6B7kAsxUlEI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241002212632.2463458-4-kieran.bingham@ideasonboard.com>","References":"<20241002212632.2463458-1-kieran.bingham@ideasonboard.com>\n\t<20241002212632.2463458-4-kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","From":"Kieran Bingham ","Cc":"","To":"libcamera devel ","Date":"Wed, 02 Oct 2024 22:56:55 +0100","Message-ID":"<172790621516.2371408.15923515984877278215@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}},{"id":31543,"web_url":"https://patchwork.libcamera.org/comment/31543/","msgid":"<5e1c6160-ef28-4fb1-99a4-f2b35e336125@ideasonboard.com>","date":"2024-10-03T06:24:11","subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran\n\nThank you for the patch.\n\nOn 03/10/24 2:56 am, Kieran Bingham wrote:\n> Provide a MediaPipeline class to help identifing and managing pipelines across\n\ns/identifing/identifying/\n> a MediaDevice graph.\n>\n> Signed-off-by: Kieran Bingham \n>\n> ---\n> v2:\n>\n> - use srcPads to clearly identify which pads are managed\n> - Only report enabling links when a change is made\n>\n> - fix header includes\n>\n> - Fix includes\n> - Remove period at end of briefs\n>\n> - Document function parameters\n> - expand documentation throughout\n> - Fix debug log capitalisation\n>\n> - reduce scope of single use 'ret'\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/media_pipeline.h | 59 ++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/media_pipeline.cpp | 307 ++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 368 insertions(+)\n> create mode 100644 include/libcamera/internal/media_pipeline.h\n> create mode 100644 src/libcamera/media_pipeline.cpp\n>\n> diff --git a/include/libcamera/internal/media_pipeline.h b/include/libcamera/internal/media_pipeline.h\n> new file mode 100644\n> index 000000000000..ca4a86a88a1e\n> --- /dev/null\n> +++ b/include/libcamera/internal/media_pipeline.h\n> @@ -0,0 +1,59 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas on Board Oy\n> + *\n> + * Media pipeline handler\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class CameraSensor;\n> +class MediaEntity;\n> +class MediaLink;\n> +class MediaPad;\n> +struct V4L2SubdeviceFormat;\n> +\n> +class MediaPipeline\n> +{\n> +public:\n> +\tint init(MediaEntity *source, std::string sink);\n> +\tint initLinks();\n> +\tint configure(CameraSensor *sensor, V4L2SubdeviceFormat *);\n\nIs skipping of function param name intentional ? I missed this in v1 review.\n\nRest looks good,\n\nReviewed-by: Umang Jain \n> +\n> +private:\n> +\tstruct Entity {\n> +\t\t/* The media entity, always valid. */\n> +\t\tMediaEntity *entity;\n> +\t\t/*\n> +\t\t * Whether or not the entity is a subdev that supports the\n> +\t\t * routing API.\n> +\t\t */\n> +\t\tbool supportsRouting;\n> +\t\t/*\n> +\t\t * The local sink pad connected to the upstream entity, null for\n> +\t\t * the camera sensor at the beginning of the pipeline.\n> +\t\t */\n> +\t\tconst MediaPad *sink;\n> +\t\t/*\n> +\t\t * The local source pad connected to the downstream entity, null\n> +\t\t * for the video node at the end of the pipeline.\n> +\t\t */\n> +\t\tconst MediaPad *source;\n> +\t\t/*\n> +\t\t * The link on the source pad, to the downstream entity, null\n> +\t\t * for the video node at the end of the pipeline.\n> +\t\t */\n> +\t\tMediaLink *sourceLink;\n> +\t};\n> +\n> +\tstd::vector routedSourcePads(MediaPad *sink);\n> +\tstd::list entities_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 1c5eef9cab80..60a35d3e0357 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -30,6 +30,7 @@ libcamera_internal_headers = files([\n> 'mapped_framebuffer.h',\n> 'media_device.h',\n> 'media_object.h',\n> + 'media_pipeline.h',\n> 'pipeline_handler.h',\n> 'process.h',\n> 'pub_key.h',\n> diff --git a/src/libcamera/media_pipeline.cpp b/src/libcamera/media_pipeline.cpp\n> new file mode 100644\n> index 000000000000..dba8084ddc97\n> --- /dev/null\n> +++ b/src/libcamera/media_pipeline.cpp\n> @@ -0,0 +1,307 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas on Board Oy\n> + *\n> + * Media pipeline support\n> + */\n> +\n> +#include \"libcamera/internal/media_pipeline.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/media_object.h\"\n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +\n> +/**\n> + * \\file media_pipeline.h\n> + * \\brief Provide a representation of a pipeline of devices using the Media\n> + * Controller\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(MediaPipeline)\n> +\n> +/**\n> + * \\class MediaPipeline\n> + * \\brief The MediaPipeline represents a set of entities that together form a\n> + * data path for stream data\n> + *\n> + * A MediaPipeline instance is constructed from a sink and a source between\n> + * two entities in a media graph.\n> + */\n> +\n> +/**\n> + * \\brief Retrieve all source pads connected to a sink pad through active routes\n> + * \\param[in] sink The sink pad to examine\n> + *\n> + * Examine the entity using the V4L2 Subdevice Routing API to collect all the\n> + * source pads which are connected with an active route to the sink pad.\n> + *\n> + * \\return A vector of source MediaPads\n> + */\n> +std::vector MediaPipeline::routedSourcePads(MediaPad *sink)\n> +{\n> +\tMediaEntity *entity = sink->entity();\n> +\tstd::unique_ptr subdev =\n> +\t\tstd::make_unique(entity);\n> +\n> +\tint ret = subdev->open();\n> +\tif (ret < 0)\n> +\t\treturn {};\n> +\n> +\tV4L2Subdevice::Routing routing = {};\n> +\tret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);\n> +\tif (ret < 0)\n> +\t\treturn {};\n> +\n> +\tstd::vector pads;\n> +\n> +\tfor (const V4L2Subdevice::Route &route : routing) {\n> +\t\tif (sink->index() != route.sink.pad ||\n> +\t\t !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))\n> +\t\t\tcontinue;\n> +\n> +\t\tconst MediaPad *pad = entity->getPadByIndex(route.source.pad);\n> +\t\tif (!pad) {\n> +\t\t\tLOG(MediaPipeline, Warning)\n> +\t\t\t\t<< \"Entity \" << entity->name()\n> +\t\t\t\t<< \" has invalid route source pad \"\n> +\t\t\t\t<< route.source.pad;\n> +\t\t}\n> +\n> +\t\tpads.push_back(pad);\n> +\t}\n> +\n> +\treturn pads;\n> +}\n> +\n> +/**\n> + * \\brief Find the path from source to sink\n> + * \\param[in] source The source entity to start from\n> + * \\param[in] sink The sink entity name to search for\n> + *\n> + * Starting from a source entity, determine the shortest path to the target\n> + * described by sink.\n> + *\n> + * If sink can not be found, or a route from source to sink can not be achieved,\n> + * an error of -ENOLINK will be returned.\n> + *\n> + * When successful, the MediaPipeline will internally store the representation\n> + * of entities and links to describe the path between the two entities.\n> + *\n> + * It is expected that the Source entity is a sensor represented by the\n> + * CameraSensor class.\n> + *\n> + * \\return 0 on success, a negative errno otherwise\n> + */\n> +int MediaPipeline::init(MediaEntity *source, std::string sink)\n> +{\n> +\t/*\n> +\t * Find the shortest path between from the source and the\n> +\t * target entity.\n> +\t */\n> +\tstd::unordered_set visited;\n> +\tstd::queue> queue;\n> +\n> +\t/* Remember at each entity where we came from. */\n> +\tstd::unordered_map parents;\n> +\tMediaEntity *entity = nullptr;\n> +\tMediaEntity *target = nullptr;\n> +\tMediaPad *sinkPad;\n> +\n> +\tqueue.push({ source, nullptr });\n> +\n> +\twhile (!queue.empty()) {\n> +\t\tstd::tie(entity, sinkPad) = queue.front();\n> +\t\tqueue.pop();\n> +\n> +\t\t/* Found the target device. */\n> +\t\tif (entity->name() == sink) {\n> +\t\t\tLOG(MediaPipeline, Debug)\n> +\t\t\t\t<< \"Found pipeline target \" << entity->name();\n> +\t\t\ttarget = entity;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tvisited.insert(entity);\n> +\n> +\t\t/*\n> +\t\t * Add direct downstream entities to the search queue. If the\n> +\t\t * current entity supports the subdev internal routing API,\n> +\t\t * restrict the search to downstream entities reachable through\n> +\t\t * active routes.\n> +\t\t */\n> +\n> +\t\tstd::vector srcPads;\n> +\t\tbool supportsRouting = false;\n> +\n> +\t\tif (sinkPad) {\n> +\t\t\tsrcPads = routedSourcePads(sinkPad);\n> +\t\t\tif (!srcPads.empty())\n> +\t\t\t\tsupportsRouting = true;\n> +\t\t}\n> +\n> +\t\tif (srcPads.empty()) {\n> +\t\t\tfor (const MediaPad *pad : entity->pads()) {\n> +\t\t\t\tif (!(pad->flags() & MEDIA_PAD_FL_SOURCE))\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\tsrcPads.push_back(pad);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tfor (const MediaPad *srcPad : srcPads) {\n> +\t\t\tfor (MediaLink *link : srcPad->links()) {\n> +\t\t\t\tMediaEntity *next = link->sink()->entity();\n> +\t\t\t\tif (visited.find(next) == visited.end()) {\n> +\t\t\t\t\tqueue.push({ next, link->sink() });\n> +\n> +\t\t\t\t\tEntity e{ entity, supportsRouting, sinkPad,\n> +\t\t\t\t\t\t srcPad, link };\n> +\t\t\t\t\tparents.insert({ next, e });\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\tif (!target) {\n> +\t\tLOG(MediaPipeline, Error) << \"Failed to connect \" << source->name();\n> +\t\treturn -ENOLINK;\n> +\t}\n> +\n> +\t/*\n> +\t * With the parents, we can follow back our way from the capture device\n> +\t * to the sensor. Store all the entities in the pipeline, from the\n> +\t * camera sensor to the video node, in entities_.\n> +\t */\n> +\tentities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n> +\n> +\tfor (auto it = parents.find(entity); it != parents.end();\n> +\t it = parents.find(entity)) {\n> +\t\tconst Entity &e = it->second;\n> +\t\tentities_.push_front(e);\n> +\t\tentity = e.entity;\n> +\t}\n> +\n> +\tLOG(MediaPipeline, Info)\n> +\t\t<< \"Found pipeline: \"\n> +\t\t<< utils::join(entities_, \" -> \",\n> +\t\t\t [](const Entity &e) {\n> +\t\t\t\t std::string s = \"[\";\n> +\t\t\t\t if (e.sink)\n> +\t\t\t\t\t s += std::to_string(e.sink->index()) + \"|\";\n> +\t\t\t\t s += e.entity->name();\n> +\t\t\t\t if (e.source)\n> +\t\t\t\t\t s += \"|\" + std::to_string(e.source->index());\n> +\t\t\t\t s += \"]\";\n> +\t\t\t\t return s;\n> +\t\t\t });\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Initialise and enable all links through the MediaPipeline\n> + * \\return 0 on success, or a negative errno otherwise\n> + */\n> +int MediaPipeline::initLinks()\n> +{\n> +\tMediaLink *sinkLink = nullptr;\n> +\tfor (Entity &e : entities_) {\n> +\t\t/* Sensor entities have no connected sink. */\n> +\t\tif (!sinkLink) {\n> +\t\t\tsinkLink = e.sourceLink;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (!(sinkLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> +\t\t\tLOG(MediaPipeline, Debug) << \"Enabling : \" << *sinkLink;\n> +\n> +\t\t\tint ret = sinkLink->setEnabled(true);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tsinkLink = e.sourceLink;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the entities of this MediaPipeline\n> + * \\param[in] sensor The configured CameraSensor to propogate\n> + *\n> + * Propagate formats through each of the entities of the Pipeline, validating\n> + * that each one was not adjusted by the driver from the desired format.\n> + *\n> + * \\return 0 on success or a negative errno otherwise\n> + */\n> +int MediaPipeline::configure(CameraSensor *sensor, V4L2SubdeviceFormat *format)\n> +{\n> +\tint ret;\n> +\n> +\tfor (const Entity &e : entities_) {\n> +\t\t/* The sensor is configured through the CameraSensor */\n> +\t\tif (!e.sourceLink)\n> +\t\t\tbreak;\n> +\n> +\t\tMediaLink *link = e.sourceLink;\n> +\t\tMediaPad *source = link->source();\n> +\t\tMediaPad *sink = link->sink();\n> +\n> +\t\t/* 'format' already contains the sensor configuration */\n> +\t\tif (source->entity() != sensor->entity()) {\n> +\t\t\t/* todo: Add MediaDevice cache to reduce FD pressure */\n> +\t\t\tV4L2Subdevice subdev(source->entity());\n> +\t\t\tret = subdev.open();\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tret = subdev.getFormat(source->index(), format);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tV4L2SubdeviceFormat sourceFormat = *format;\n> +\t\t/* todo: Add MediaDevice cache to reduce FD pressure */\n> +\t\tV4L2Subdevice subdev(sink->entity());\n> +\t\tret = subdev.open();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\tret = subdev.setFormat(sink->index(), format);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\tif (format->code != sourceFormat.code ||\n> +\t\t format->size != sourceFormat.size) {\n> +\t\t\tLOG(MediaPipeline, Debug)\n> +\t\t\t\t<< \"Source '\" << *source\n> +\t\t\t\t<< \" produces \" << sourceFormat\n> +\t\t\t\t<< \", sink '\" << *sink\n> +\t\t\t\t<< \" requires \" << format;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tLOG(MediaPipeline, Debug)\n> +\t\t\t<< \"Link \" << *link << \" configured with format \"\n> +\t\t\t<< format;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index aa9ab0291854..2c0f8603b231 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_internal_sources = files([\n> 'mapped_framebuffer.cpp',\n> 'media_device.cpp',\n> 'media_object.cpp',\n> + 'media_pipeline.cpp',\n> 'pipeline_handler.cpp',\n> 'process.cpp',\n> 'pub_key.cpp',","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 01766C3257\n\tfor ;\n\tThu, 3 Oct 2024 06:24:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D630663525;\n\tThu, 3 Oct 2024 08:24:17 +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 0BF0862C8F\n\tfor ;\n\tThu, 3 Oct 2024 08:24:16 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 581754C7;\n\tThu, 3 Oct 2024 08:22:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kEb9xzGu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727936562;\n\tbh=VwWnfiuiLsdsPuVZk3Ms83jCGWHK1vPWo8oZxjddhic=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=kEb9xzGuRIqJxSF8eBVou6P0JJ8e95+8i26fufODWbGeo9SJrgeV1GNEH/wAqIkzw\n\tRQfGtuMArpdY1/YAEBXzX9rQYNZDWJg1gKVVJ1HCsrB5jJKHwqwQl3/Q7ALEPAG2WG\n\tE1Gl5S9gW7PN9fkoSUwfy7Vq8k5FnACcOqVbBpb4=","Message-ID":"<5e1c6160-ef28-4fb1-99a4-f2b35e336125@ideasonboard.com>","Date":"Thu, 3 Oct 2024 11:54:11 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","To":"Kieran Bingham ,\n\tlibcamera devel ","References":"<20241002212632.2463458-1-kieran.bingham@ideasonboard.com>\n\t<20241002212632.2463458-4-kieran.bingham@ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20241002212632.2463458-4-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":31629,"web_url":"https://patchwork.libcamera.org/comment/31629/","msgid":"<172842650489.206884.171070782539889564@ping.linuxembedded.co.uk>","date":"2024-10-08T22:28:24","subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-10-03 07:24:11)\n> Hi Kieran\n> \n> Thank you for the patch.\n> \n> On 03/10/24 2:56 am, Kieran Bingham wrote:\n> > Provide a MediaPipeline class to help identifing and managing pipelines across\n> \n> s/identifing/identifying/\n> > a MediaDevice graph.\n> >\n> > Signed-off-by: Kieran Bingham \n> >\n> > ---\n> > v2:\n> >\n> > - use srcPads to clearly identify which pads are managed\n> > - Only report enabling links when a change is made\n> >\n> > - fix header includes\n> >\n> > - Fix includes\n> > - Remove period at end of briefs\n> >\n> > - Document function parameters\n> > - expand documentation throughout\n> > - Fix debug log capitalisation\n> >\n> > - reduce scope of single use 'ret'\n> >\n> > Signed-off-by: Kieran Bingham \n> > ---\n> > include/libcamera/internal/media_pipeline.h | 59 ++++\n> > include/libcamera/internal/meson.build | 1 +\n> > src/libcamera/media_pipeline.cpp | 307 ++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > 4 files changed, 368 insertions(+)\n> > create mode 100644 include/libcamera/internal/media_pipeline.h\n> > create mode 100644 src/libcamera/media_pipeline.cpp\n> >\n> > diff --git a/include/libcamera/internal/media_pipeline.h b/include/libcamera/internal/media_pipeline.h\n> > new file mode 100644\n> > index 000000000000..ca4a86a88a1e\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/media_pipeline.h\n> > @@ -0,0 +1,59 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas on Board Oy\n> > + *\n> > + * Media pipeline handler\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class CameraSensor;\n> > +class MediaEntity;\n> > +class MediaLink;\n> > +class MediaPad;\n> > +struct V4L2SubdeviceFormat;\n> > +\n> > +class MediaPipeline\n> > +{\n> > +public:\n> > + int init(MediaEntity *source, std::string sink);\n> > + int initLinks();\n> > + int configure(CameraSensor *sensor, V4L2SubdeviceFormat *);\n> \n> Is skipping of function param name intentional ? I missed this in v1 review.\n\nNo, I'll add it as 'format' to match the function implementation.\n\n\n> \n> Rest looks good,\n> \n> Reviewed-by: Umang Jain \n\nThanks\n\n--\nKieran\n\n\n> > +\n> > +private:\n> > + struct Entity {\n> > + /* The media entity, always valid. */\n> > + MediaEntity *entity;\n> > + /*\n> > + * Whether or not the entity is a subdev that supports the\n> > + * routing API.\n> > + */\n> > + bool supportsRouting;\n> > + /*\n> > + * The local sink pad connected to the upstream entity, null for\n> > + * the camera sensor at the beginning of the pipeline.\n> > + */\n> > + const MediaPad *sink;\n> > + /*\n> > + * The local source pad connected to the downstream entity, null\n> > + * for the video node at the end of the pipeline.\n> > + */\n> > + const MediaPad *source;\n> > + /*\n> > + * The link on the source pad, to the downstream entity, null\n> > + * for the video node at the end of the pipeline.\n> > + */\n> > + MediaLink *sourceLink;\n> > + };\n> > +\n> > + std::vector routedSourcePads(MediaPad *sink);\n> > + std::list entities_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 1c5eef9cab80..60a35d3e0357 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -30,6 +30,7 @@ libcamera_internal_headers = files([\n> > 'mapped_framebuffer.h',\n> > 'media_device.h',\n> > 'media_object.h',\n> > + 'media_pipeline.h',\n> > 'pipeline_handler.h',\n> > 'process.h',\n> > 'pub_key.h',\n> > diff --git a/src/libcamera/media_pipeline.cpp b/src/libcamera/media_pipeline.cpp\n> > new file mode 100644\n> > index 000000000000..dba8084ddc97\n> > --- /dev/null\n> > +++ b/src/libcamera/media_pipeline.cpp\n> > @@ -0,0 +1,307 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas on Board Oy\n> > + *\n> > + * Media pipeline support\n> > + */\n> > +\n> > +#include \"libcamera/internal/media_pipeline.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/camera_sensor.h\"\n> > +#include \"libcamera/internal/media_device.h\"\n> > +#include \"libcamera/internal/media_object.h\"\n> > +#include \"libcamera/internal/v4l2_subdevice.h\"\n> > +\n> > +/**\n> > + * \\file media_pipeline.h\n> > + * \\brief Provide a representation of a pipeline of devices using the Media\n> > + * Controller\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(MediaPipeline)\n> > +\n> > +/**\n> > + * \\class MediaPipeline\n> > + * \\brief The MediaPipeline represents a set of entities that together form a\n> > + * data path for stream data\n> > + *\n> > + * A MediaPipeline instance is constructed from a sink and a source between\n> > + * two entities in a media graph.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Retrieve all source pads connected to a sink pad through active routes\n> > + * \\param[in] sink The sink pad to examine\n> > + *\n> > + * Examine the entity using the V4L2 Subdevice Routing API to collect all the\n> > + * source pads which are connected with an active route to the sink pad.\n> > + *\n> > + * \\return A vector of source MediaPads\n> > + */\n> > +std::vector MediaPipeline::routedSourcePads(MediaPad *sink)\n> > +{\n> > + MediaEntity *entity = sink->entity();\n> > + std::unique_ptr subdev =\n> > + std::make_unique(entity);\n> > +\n> > + int ret = subdev->open();\n> > + if (ret < 0)\n> > + return {};\n> > +\n> > + V4L2Subdevice::Routing routing = {};\n> > + ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);\n> > + if (ret < 0)\n> > + return {};\n> > +\n> > + std::vector pads;\n> > +\n> > + for (const V4L2Subdevice::Route &route : routing) {\n> > + if (sink->index() != route.sink.pad ||\n> > + !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))\n> > + continue;\n> > +\n> > + const MediaPad *pad = entity->getPadByIndex(route.source.pad);\n> > + if (!pad) {\n> > + LOG(MediaPipeline, Warning)\n> > + << \"Entity \" << entity->name()\n> > + << \" has invalid route source pad \"\n> > + << route.source.pad;\n> > + }\n> > +\n> > + pads.push_back(pad);\n> > + }\n> > +\n> > + return pads;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Find the path from source to sink\n> > + * \\param[in] source The source entity to start from\n> > + * \\param[in] sink The sink entity name to search for\n> > + *\n> > + * Starting from a source entity, determine the shortest path to the target\n> > + * described by sink.\n> > + *\n> > + * If sink can not be found, or a route from source to sink can not be achieved,\n> > + * an error of -ENOLINK will be returned.\n> > + *\n> > + * When successful, the MediaPipeline will internally store the representation\n> > + * of entities and links to describe the path between the two entities.\n> > + *\n> > + * It is expected that the Source entity is a sensor represented by the\n> > + * CameraSensor class.\n> > + *\n> > + * \\return 0 on success, a negative errno otherwise\n> > + */\n> > +int MediaPipeline::init(MediaEntity *source, std::string sink)\n> > +{\n> > + /*\n> > + * Find the shortest path between from the source and the\n> > + * target entity.\n> > + */\n> > + std::unordered_set visited;\n> > + std::queue> queue;\n> > +\n> > + /* Remember at each entity where we came from. */\n> > + std::unordered_map parents;\n> > + MediaEntity *entity = nullptr;\n> > + MediaEntity *target = nullptr;\n> > + MediaPad *sinkPad;\n> > +\n> > + queue.push({ source, nullptr });\n> > +\n> > + while (!queue.empty()) {\n> > + std::tie(entity, sinkPad) = queue.front();\n> > + queue.pop();\n> > +\n> > + /* Found the target device. */\n> > + if (entity->name() == sink) {\n> > + LOG(MediaPipeline, Debug)\n> > + << \"Found pipeline target \" << entity->name();\n> > + target = entity;\n> > + break;\n> > + }\n> > +\n> > + visited.insert(entity);\n> > +\n> > + /*\n> > + * Add direct downstream entities to the search queue. If the\n> > + * current entity supports the subdev internal routing API,\n> > + * restrict the search to downstream entities reachable through\n> > + * active routes.\n> > + */\n> > +\n> > + std::vector srcPads;\n> > + bool supportsRouting = false;\n> > +\n> > + if (sinkPad) {\n> > + srcPads = routedSourcePads(sinkPad);\n> > + if (!srcPads.empty())\n> > + supportsRouting = true;\n> > + }\n> > +\n> > + if (srcPads.empty()) {\n> > + for (const MediaPad *pad : entity->pads()) {\n> > + if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))\n> > + continue;\n> > + srcPads.push_back(pad);\n> > + }\n> > + }\n> > +\n> > + for (const MediaPad *srcPad : srcPads) {\n> > + for (MediaLink *link : srcPad->links()) {\n> > + MediaEntity *next = link->sink()->entity();\n> > + if (visited.find(next) == visited.end()) {\n> > + queue.push({ next, link->sink() });\n> > +\n> > + Entity e{ entity, supportsRouting, sinkPad,\n> > + srcPad, link };\n> > + parents.insert({ next, e });\n> > + }\n> > + }\n> > + }\n> > + }\n> > +\n> > + if (!target) {\n> > + LOG(MediaPipeline, Error) << \"Failed to connect \" << source->name();\n> > + return -ENOLINK;\n> > + }\n> > +\n> > + /*\n> > + * With the parents, we can follow back our way from the capture device\n> > + * to the sensor. Store all the entities in the pipeline, from the\n> > + * camera sensor to the video node, in entities_.\n> > + */\n> > + entities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n> > +\n> > + for (auto it = parents.find(entity); it != parents.end();\n> > + it = parents.find(entity)) {\n> > + const Entity &e = it->second;\n> > + entities_.push_front(e);\n> > + entity = e.entity;\n> > + }\n> > +\n> > + LOG(MediaPipeline, Info)\n> > + << \"Found pipeline: \"\n> > + << utils::join(entities_, \" -> \",\n> > + [](const Entity &e) {\n> > + std::string s = \"[\";\n> > + if (e.sink)\n> > + s += std::to_string(e.sink->index()) + \"|\";\n> > + s += e.entity->name();\n> > + if (e.source)\n> > + s += \"|\" + std::to_string(e.source->index());\n> > + s += \"]\";\n> > + return s;\n> > + });\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Initialise and enable all links through the MediaPipeline\n> > + * \\return 0 on success, or a negative errno otherwise\n> > + */\n> > +int MediaPipeline::initLinks()\n> > +{\n> > + MediaLink *sinkLink = nullptr;\n> > + for (Entity &e : entities_) {\n> > + /* Sensor entities have no connected sink. */\n> > + if (!sinkLink) {\n> > + sinkLink = e.sourceLink;\n> > + continue;\n> > + }\n> > +\n> > + if (!(sinkLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> > + LOG(MediaPipeline, Debug) << \"Enabling : \" << *sinkLink;\n> > +\n> > + int ret = sinkLink->setEnabled(true);\n> > + if (ret < 0)\n> > + return ret;\n> > + }\n> > +\n> > + sinkLink = e.sourceLink;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Configure the entities of this MediaPipeline\n> > + * \\param[in] sensor The configured CameraSensor to propogate\n> > + *\n> > + * Propagate formats through each of the entities of the Pipeline, validating\n> > + * that each one was not adjusted by the driver from the desired format.\n> > + *\n> > + * \\return 0 on success or a negative errno otherwise\n> > + */\n> > +int MediaPipeline::configure(CameraSensor *sensor, V4L2SubdeviceFormat *format)\n> > +{\n> > + int ret;\n> > +\n> > + for (const Entity &e : entities_) {\n> > + /* The sensor is configured through the CameraSensor */\n> > + if (!e.sourceLink)\n> > + break;\n> > +\n> > + MediaLink *link = e.sourceLink;\n> > + MediaPad *source = link->source();\n> > + MediaPad *sink = link->sink();\n> > +\n> > + /* 'format' already contains the sensor configuration */\n> > + if (source->entity() != sensor->entity()) {\n> > + /* todo: Add MediaDevice cache to reduce FD pressure */\n> > + V4L2Subdevice subdev(source->entity());\n> > + ret = subdev.open();\n> > + if (ret)\n> > + return ret;\n> > +\n> > + ret = subdev.getFormat(source->index(), format);\n> > + if (ret < 0)\n> > + return ret;\n> > + }\n> > +\n> > + V4L2SubdeviceFormat sourceFormat = *format;\n> > + /* todo: Add MediaDevice cache to reduce FD pressure */\n> > + V4L2Subdevice subdev(sink->entity());\n> > + ret = subdev.open();\n> > + if (ret)\n> > + return ret;\n> > +\n> > + ret = subdev.setFormat(sink->index(), format);\n> > + if (ret < 0)\n> > + return ret;\n> > +\n> > + if (format->code != sourceFormat.code ||\n> > + format->size != sourceFormat.size) {\n> > + LOG(MediaPipeline, Debug)\n> > + << \"Source '\" << *source\n> > + << \" produces \" << sourceFormat\n> > + << \", sink '\" << *sink\n> > + << \" requires \" << format;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + LOG(MediaPipeline, Debug)\n> > + << \"Link \" << *link << \" configured with format \"\n> > + << format;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index aa9ab0291854..2c0f8603b231 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -41,6 +41,7 @@ libcamera_internal_sources = files([\n> > 'mapped_framebuffer.cpp',\n> > 'media_device.cpp',\n> > 'media_object.cpp',\n> > + 'media_pipeline.cpp',\n> > 'pipeline_handler.cpp',\n> > 'process.cpp',\n> > 'pub_key.cpp',\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 6D529BE080\n\tfor ;\n\tTue, 8 Oct 2024 22:28:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C69306536B;\n\tWed, 9 Oct 2024 00:28:48 +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 391466351F\n\tfor ;\n\tWed, 9 Oct 2024 00:28:28 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B4E614D4;\n\tWed, 9 Oct 2024 00:26:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MiqhZs9f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728426410;\n\tbh=qB8MUUougRYCQWVqY2SG8vkP2hdc05pylhXC9RWgy+U=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=MiqhZs9faR3lpFheozZmfU/WgjVozYRnNd1b70839kl5Twh8VwxN20/TkMH7EGLG4\n\tR+aFiQqc2UKO0P5D8VgvxqhA7ZV4NQzv8ORPc/xYGhApcoppEYQKFJvyUt9PbGR6jq\n\th+DPTN0s533OyMdf/00JkcxEfHA/d5Wd5a+z5lm4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<5e1c6160-ef28-4fb1-99a4-f2b35e336125@ideasonboard.com>","References":"<20241002212632.2463458-1-kieran.bingham@ideasonboard.com>\n\t<20241002212632.2463458-4-kieran.bingham@ideasonboard.com>\n\t<5e1c6160-ef28-4fb1-99a4-f2b35e336125@ideasonboard.com>","Subject":"Re: [PATCH v2 3/4] libcamera: internal: Add MediaPipeline helper","From":"Kieran Bingham ","To":"Umang Jain ,\n\tlibcamera devel ","Date":"Tue, 08 Oct 2024 23:28:24 +0100","Message-ID":"<172842650489.206884.171070782539889564@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]