[{"id":31893,"web_url":"https://patchwork.libcamera.org/comment/31893/","msgid":"","date":"2024-10-23T22:31:40","subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Kieran, thanks for the patch\n\nOn 09/10/2024 00:13, Kieran Bingham wrote:\n> Provide a MediaPipeline class to help identifing and managing pipelines across\n> a MediaDevice graph.\n>\n> Reviewed-by: Umang Jain \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> - fix header includes\n> - Fix includes\n> - Remove period at end of briefs\n> - Document function parameters\n> - expand documentation throughout\n> - Fix debug log capitalisation\n> - reduce scope of single use 'ret'\n>\n> v3:\n> - Add doxygen documentation for the format parameter of configure()\n> - Add 'format' identifier to configure parameter\n>\n> include/libcamera/internal/media_pipeline.h | 59 ++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/media_pipeline.cpp | 311 ++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 372 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..ee773b892719\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\nI think I would mildly prefer not to refer to something not derived from class PipelineHandler with \nthe phrase \"pipeline handler\" but it's not a big deal\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 *format);\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..ec78b78e2f75\n> --- /dev/null\n> +++ b/src/libcamera/media_pipeline.cpp\n> @@ -0,0 +1,311 @@\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\nCouldn't this result in a nullptr being pushed into the vector, and then dereferenced in ::init()? I \ndoubt it's actually possible as presumably the kernel wouldn't return such invalid data, but still.\n\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.\nIs it worth validating with the entity function? Up to you.\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\nThat's a bit untidy...maybe something liiiiike:\n\n\nstd::ostringstream s;\n\n\ns << \"[\";\n\ne.sink && s << e.sink->index() << \"|\";\n\ns << e.entity.name();\n\ne.source && s << \"|\" << e.source->index();\n\ns << \"]\"\n\n\nreturn s.str()\n\n\nOr maybe we just need a proper std::string.format() utility until c++ 20\n\n\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> + * \\param[inout] format The format to propogate through the pipeline\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> + * The format is updated with the final accepted format of the last entity of\n> + * the pipeline.\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 */\ns/todo:/\\todo/\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 7694DC3200\n\tfor ;\n\tWed, 23 Oct 2024 22:31:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77E8E65393;\n\tThu, 24 Oct 2024 00:31:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C4466053E\n\tfor ;\n\tThu, 24 Oct 2024 00:31:44 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 60010353\n\tfor ;\n\tThu, 24 Oct 2024 00:29:56 +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=\"UQ6KBTPr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729722596;\n\tbh=PYwKtvSp9Scf8QHnwoK0h2AM6+ZktQlm9js7lm44fLk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=UQ6KBTPrgJ2IcIwbqdO7293gO1+VLht49DnkpthNutAWdo74QUfl1TlyVh0HIcbpr\n\tZgZmFgR9qgfB4P2EFYrMDp3WrM6k9Y+1kwmT2plSONJGifr7MCwhoctaAkuqBfhfmG\n\t7pogUf254CT99AnF/v4Ebf7s4aG4+fl9xEql6peU=","Message-ID":"","Date":"Wed, 23 Oct 2024 23:31:40 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","To":"libcamera-devel@lists.libcamera.org","References":"<20241008231314.744556-1-kieran.bingham@ideasonboard.com>\n\t<20241008231314.744556-4-kieran.bingham@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20241008231314.744556-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":31902,"web_url":"https://patchwork.libcamera.org/comment/31902/","msgid":"","date":"2024-10-24T09:20:41","subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"So coming back to this after reading patch 4\n\nOn 23/10/2024 23:31, Dan Scally wrote:\n> Hi Kieran, thanks for the patch\n>\n> On 09/10/2024 00:13, Kieran Bingham wrote:\n>> Provide a MediaPipeline class to help identifing and managing pipelines across\n>> a MediaDevice graph.\n>>\n>> Reviewed-by: Umang Jain \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>> - fix header includes\n>> - Fix includes\n>> - Remove period at end of briefs\n>> - Document function parameters\n>> - expand documentation throughout\n>> - Fix debug log capitalisation\n>> - reduce scope of single use 'ret'\n>>\n>> v3:\n>> - Add doxygen documentation for the format parameter of configure()\n>> - Add 'format' identifier to configure parameter\n>>\n>>   include/libcamera/internal/media_pipeline.h |  59 ++++\n>>   include/libcamera/internal/meson.build      |   1 +\n>>   src/libcamera/media_pipeline.cpp            | 311 ++++++++++++++++++++\n>>   src/libcamera/meson.build                   |   1 +\n>>   4 files changed, 372 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 \n>> b/include/libcamera/internal/media_pipeline.h\n>> new file mode 100644\n>> index 000000000000..ee773b892719\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> I think I would mildly prefer not to refer to something not derived from class PipelineHandler \n> with the phrase \"pipeline handler\" but it's not a big deal\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 *format);\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>> +    };\nI think I'd make the comments a bit clearer that it might be a subdevice entity at the end (though \nsource and sourceLink will still be null)\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..ec78b78e2f75\n>> --- /dev/null\n>> +++ b/src/libcamera/media_pipeline.cpp\n>> @@ -0,0 +1,311 @@\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> Couldn't this result in a nullptr being pushed into the vector, and then dereferenced in ::init()? \n> I doubt it's actually possible as presumably the kernel wouldn't return such invalid data, but still.\n>\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> Is it worth validating with the entity function? Up to you.\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\n\nAnd same here perhaps - there might not be a video node at the end of the pipeline.\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> That's a bit untidy...maybe something liiiiike:\n>\n>\n> std::ostringstream s;\n>\n>\n> s << \"[\";\n>\n> e.sink && s << e.sink->index() << \"|\";\n>\n> s << e.entity.name();\n>\n> e.source && s << \"|\" << e.source->index();\n>\n> s << \"]\"\n>\n>\n> return s.str()\n>\n>\n> Or maybe we just need a proper std::string.format() utility until c++ 20\n>\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>> + * \\param[inout] format The format to propogate through the pipeline\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>> + * The format is updated with the final accepted format of the last entity of\n>> + * 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> s/todo:/\\todo/\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',","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 4BAB8BE08B\n\tfor ;\n\tThu, 24 Oct 2024 09:20:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69E9D65394;\n\tThu, 24 Oct 2024 11:20:45 +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 11B46603ED\n\tfor ;\n\tThu, 24 Oct 2024 11:20:44 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 204773D5\n\tfor ;\n\tThu, 24 Oct 2024 11:18:56 +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=\"KmtHS1qN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729761536;\n\tbh=Ni3pmtUfuJZaJSyc3EAYMj8pwYCRA8HDIOt1cChDNv8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=KmtHS1qNkmUDIpuFHE9XTZnnnLgjocb7+YMb3crlqnk6gFljol67VqtJlUQ3jXPvz\n\toc9k7eSJFL8QCFZH3Hh9RsV2wdkerHQo6Xba03G+aDCW52OcsVExWz8aFIQW7NzgpL\n\tWDQtWXkvQIQhGcZGSMql0sGx8Oc+qzBM4EXu05AA=","Message-ID":"","Date":"Thu, 24 Oct 2024 10:20:41 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","To":"libcamera-devel@lists.libcamera.org","References":"<20241008231314.744556-1-kieran.bingham@ideasonboard.com>\n\t<20241008231314.744556-4-kieran.bingham@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":31911,"web_url":"https://patchwork.libcamera.org/comment/31911/","msgid":"<6dtzrwbbo37zdiio3axwlwfdtul3bgnc5x2ykyvp4ogzgvccdx@qgfohc7cn7rf>","date":"2024-10-24T16:22:37","subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Wed, Oct 09, 2024 at 12:13:13AM +0100, Kieran Bingham wrote:\n> Provide a MediaPipeline class to help identifing and managing pipelines across\n> a MediaDevice graph.\n>\n> Reviewed-by: Umang Jain \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> - fix header includes\n> - Fix includes\n> - Remove period at end of briefs\n> - Document function parameters\n> - expand documentation throughout\n> - Fix debug log capitalisation\n> - reduce scope of single use 'ret'\n>\n> v3:\n> - Add doxygen documentation for the format parameter of configure()\n> - Add 'format' identifier to configure parameter\n>\n> include/libcamera/internal/media_pipeline.h | 59 ++++\n> include/libcamera/internal/meson.build | 1 +\n> src/libcamera/media_pipeline.cpp | 311 ++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 372 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..ee773b892719\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 *format);\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..ec78b78e2f75\n> --- /dev/null\n> +++ b/src/libcamera/media_pipeline.cpp\n> @@ -0,0 +1,311 @@\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\nI still don't see how this can't create contentions with the pipeline\nhandler\n\n int V4L2Device::open(unsigned int flags)\n {\n if (isOpen()) {\n LOG(V4L2, Error) << \"Device already open\";\n return -EBUSY;\n }\n\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\nby \\a sink\n\n> + *\n> + * If sink can not be found, or a route from source to sink can not be achieved,\n\nif \\a sink\n\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\nThis confuses me a bit as the class seems to be generically connect\nentities and doesn't require to start from a sensor. Or does it ?\n\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\ns/between from/between/\n\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> + * \\param[inout] format The format to propogate through the pipeline\n\ns/propogate/propagate/\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> + * The format is updated with the final accepted format of the last entity of\n> + * the pipeline.\n> + *\n> + * \\return 0 on success or a negative errno otherwise\n> + */\n> +int MediaPipeline::configure(CameraSensor *sensor, V4L2SubdeviceFormat *format)\n\nIs this the right API ? A caller should pass in the sensor (as\npointer, is it modifiable?) and its format. What if this function only\ntakes a sensor a returns the format applied at the end of the pipeline ?\n\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\nSorry, but !sourceLink doesn't indentify the video device (while the\ncomment seems to hint it identifies the sensor)\n\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\nI'm still a bit scared about contentions of the subdevices. Probably,\nif a pipeline handler uses MediaPipeline it is not supposed to handle\nsubdevs by itself ?\n\nAll minors though\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\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',\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 8BEABC3220\n\tfor ;\n\tThu, 24 Oct 2024 16:22:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 98ED465393;\n\tThu, 24 Oct 2024 18:22:45 +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 E910E618C1\n\tfor ;\n\tThu, 24 Oct 2024 18:22:43 +0200 (CEST)","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 8FE9D2E0;\n\tThu, 24 Oct 2024 18:20:55 +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=\"Yeq+2qWP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729786855;\n\tbh=HpWdvOY40rGg3NS+y8rQCmKnCVZLOoSNtZGeIw1DM7g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Yeq+2qWPJnKpeCtUjT5FFATugVkRROCRz9/sDysU2A51eG1zq2IfVtM8CSiy327hJ\n\tjxmF2X6A+DBpKX0pJ6Ymgj3JSY1h3iZzyFDFHjOpK4pD4gSoyAxMNgbDaRijbltIlK\n\tvy0NffvZasL4ikvJJuaNt6H+wMRemPDOocY4YpwM=","Date":"Thu, 24 Oct 2024 18:22:37 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"libcamera devel , \n\tUmang Jain ","Subject":"Re: [PATCH v3 3/4] libcamera: internal: Add MediaPipeline helper","Message-ID":"<6dtzrwbbo37zdiio3axwlwfdtul3bgnc5x2ykyvp4ogzgvccdx@qgfohc7cn7rf>","References":"<20241008231314.744556-1-kieran.bingham@ideasonboard.com>\n\t<20241008231314.744556-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241008231314.744556-4-kieran.bingham@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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]