[libcamera-devel] [RFC v1 1/2] libcamera: media_device: Add enumerateMediaWalks() helper
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Feb 1 01:19:37 CET 2022
Quoting Naushir Patuck (2022-01-13 10:25:28)
> Add a new helper function to the MediaDevice class which returns a vector of
> all possible media device topologies starting at the sensor entity and walking
> to an endpoint for the media device. Each of these topologies is called a
> MediaWalk. For each entity in a MediaWalk, we store a MediaEntity pointer
> together with the source and sink MediaLink pointers.
>
> As an example, consider the media graph below:
>
> +----------+
> | CSI2 |
> +-----^----+
> |
> +---+---+
> | Mux1 <-------+
> +--^----+ |
> | |
> +-----+---+ +---+---+
> | Sensor1 | | Mux2 |<--+
> +---------+ +-^-----+ |
> | |
> +-------+-+ +---+-----+
> | Sensor2 | | Sensor3 |
> +---------+ +---------+
>
> This would return three MediaDevice::MediaWalk structures looking like:
>
> 1)
> +----------+
> | CSI2 |
> +-----^----+
> |
> +-----+----+
> | Mux1 |
> +-----^----+
> |
> +-----+----+
> | Sensor1 |
> +----------+
>
> 2)
> +----------+
> | CSI2 |
> +-----^----+
> |
> +-----+----+
> | Mux1 |
> +-----^----+
> |
> +-----+----+
> | Mux2 |
> +-----^----+
> |
> +-----+----+
> | Sensor2 |
> +----------+
>
> 3)
> +----------+
> | CSI2 |
> +-----^----+
> |
> +-----+----+
> | Mux1 |
> +-----^----+
> |
> +-----+----+
> | Mux2 |
> +-----^----+
> |
> +-----+----+
> | Sensor3 |
> +----------+
>
I think this fits and would be easier / clearer to see.
1) 2) 3)
+----------+ +----------+ +----------+
| CSI2 | | CSI2 | | CSI2 |
+-----^----+ +-----^----+ +-----^----+
| | |
+-----+----+ +-----+----+ +-----+----+
| Mux1 | | Mux1 | | Mux1 |
+-----^----+ +-----^----+ +-----^----+
| | |
+-----+----+ +-----+----+ +-----+----+
| Sensor1 | | Mux2 | | Mux2 |
+----------+ +-----^----+ +-----^----+
| |
+-----+----+ +-----+----+
| Sensor2 | | Sensor3 |
+----------+ +----------+
*formatted with https://asciiflow.com/#/
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/internal/media_device.h | 12 ++
> src/libcamera/media_device.cpp | 135 ++++++++++++++++++++++
> 2 files changed, 147 insertions(+)
>
> diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h
> index 6e2a63f38229..63fcbe423eb9 100644
> --- a/include/libcamera/internal/media_device.h
> +++ b/include/libcamera/internal/media_device.h
> @@ -25,6 +25,13 @@ namespace libcamera {
> class MediaDevice : protected Loggable
> {
> public:
> + struct EntityParams {
> + MediaEntity *entity;
> + MediaLink *sinkLink;
> + MediaLink *sourceLink;
> + };
> + using MediaWalk = std::vector<EntityParams>;
> +
> MediaDevice(const std::string &deviceNode);
> ~MediaDevice();
>
> @@ -47,6 +54,8 @@ public:
> const std::vector<MediaEntity *> &entities() const { return entities_; }
> MediaEntity *getEntityByName(const std::string &name) const;
>
> + std::vector<MediaWalk> enumerateMediaWalks() const;
> +
> MediaLink *link(const std::string &sourceName, unsigned int sourceIdx,
> const std::string &sinkName, unsigned int sinkIdx);
> MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
> @@ -77,6 +86,9 @@ private:
> friend int MediaLink::setEnabled(bool enable);
> int setupLink(const MediaLink *link, unsigned int flags);
>
> + void walkGraph(std::vector<MediaWalk> *mediaWalks, MediaWalk *walk,
> + MediaEntity *entity, MediaLink *sinkLink) const;
> +
> std::string driver_;
> std::string deviceNode_;
> std::string model_;
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 941f86c25f66..60b397e205a2 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -340,6 +340,105 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const
> return nullptr;
> }
>
> +/**
> + * \fn MediaDevice::enumerateMediaWalks()
> + * \brief Retrieve the list of all possible MediaWalks available to this device
> + *
> + * Enumerate the media graph and return all possible permutations of unique
> + * sub-graphs where the first entity is a sensor device. These sub-graphs are
> + * stored as a \a MediaDevice::MediaWalk structure, where each element gives the
> + * device entity and associated sink pad link. The first entity in this structure
> + * is asensor device, with the sink pad link set to a nullptr.
/asensor/a sensor/
> + *
> + * As an example, consider the media graph below:
> + *
> + * +----------+
> + * | CSI2 |
> + * +-----^----+
> + * |
> + * +---+---+
> + * | Mux1 <-------+
I'd probably fix this with a | before the <- too
> + * +--^----+ |
> + * | |
> + * +-----+---+ +---+---+
> + * | Sensor1 | | Mux2 |<--+
> + * +---------+ +-^-----+ |
> + * | |
> + * +-------+-+ +---+-----+
> + * | Sensor2 | | Sensor3 |
> + * +---------+ +---------+
> + *
> + * This would return three \a MediaDevice::MediaWalk structures looking like:
> + *
> + * 1)
> + * +----------+
> + * | CSI2 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Mux1 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Sensor1 |
> + * +----------+
> + *
> + * 2)
> + * +----------+
> + * | CSI2 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Mux1 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Mux2 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Sensor2 |
> + * +----------+
> + *
> + * 3)
> + * +----------+
> + * | CSI2 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Mux1 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Mux2 |
> + * +-----^----+
> + * |
> + * +-----+----+
> + * | Sensor3 |
> + * +----------+
And as with the commit message, those would be nicer if they were in
a horizontal row I think.
That's easy to fix with asciiflow, so here's a copy/paste to fix either
now or while applying.
1) 2) 3)
+----------+ +----------+ +----------+
| CSI2 | | CSI2 | | CSI2 |
+-----^----+ +-----^----+ +-----^----+
| | |
+-----+----+ +-----+----+ +-----+----+
| Mux1 | | Mux1 | | Mux1 |
+-----^----+ +-----^----+ +-----^----+
| | |
+-----+----+ +-----+----+ +-----+----+
| Sensor1 | | Mux2 | | Mux2 |
+----------+ +-----^----+ +-----^----+
| |
+-----+----+ +-----+----+
| Sensor2 | | Sensor3 |
+----------+ +----------+
> + *
> + * \return A vector of MediaWalk structures available
> + */
> +std::vector<MediaDevice::MediaWalk> MediaDevice::enumerateMediaWalks() const
> +{
> + std::vector<MediaWalk> mediaWalks;
> +
> + for (MediaEntity *entity : entities_) {
> + /* Only perform enumeration starting with sensor entities */
> + if (entity->function() != MEDIA_ENT_F_CAM_SENSOR)
> + continue;
> + /*
> + * Start with an empty MediaWalk structure, and walk the graph
> + * with the sensor entity at the top, and a nullptr as the sink
> + * pad link.
> + */
> + mediaWalks.push_back({});
> + walkGraph(&mediaWalks, &mediaWalks.back(), entity, nullptr);
> + }
> +
> + return mediaWalks;
> +}
> +
> /**
> * \brief Retrieve the MediaLink connecting two pads, identified by entity
> * names and pad indexes
> @@ -800,4 +899,40 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
> return 0;
> }
>
> +void MediaDevice::walkGraph(std::vector<MediaWalk> *mediaWalks, MediaWalk *walk,
> + MediaEntity *entity, MediaLink *sinkLink) const
> +{
> + bool newWalk = false;
> +
> + walk->push_back({ entity, sinkLink, nullptr });
> +
> + for (const MediaPad *pad : entity->pads()) {
> + /* We only walk down the graph, so ignore any sink pads */
> + if (pad->flags() & MEDIA_PAD_FL_SINK)
> + continue;
> +
> + for (MediaLink *nextLink : pad->links()) {
> + MediaDevice::MediaWalk *nextWalk = walk;
> + /*
> + * If this is a new branch in the graph, create a new
> + * MediaWalk structure in our list, and populate it with
> + * a copy of the curret MediaWalk. Any subsequent recursion
/curret/current/
I think this looks useful, and I've already found myself having to write
similar code else where so I beleive this will be reusable, and perhaps
extended.
I fear that in the future things may get more complex and end up
requiring specific helpers for specific entities that we match while
walking a graph. For example we've been working on devices like GMSL or
FPDLink which handle serialising and deserialising mutliple devices
through a single connection, or some paths may have more complexity that
need sovling. Maybe that will be with more support from the pipeline
handlers.
But ... I think those will be dealt with as we hit it.
With the typos fixed, and optionally making those graphs in a row ...
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> + * into this graph branch will cause it to diverge.
> + */
> + if (newWalk) {
> + mediaWalks->push_back(*walk);
> + nextWalk = &mediaWalks->back();
> + }
> +
> + /* Record the source pad link for the current entity */
> + walk->back().sourceLink = nextLink;
> +
> + /* Recurse into the next entity for this source pad link */
> + MediaEntity *nextEntity = nextLink->sink()->entity();
> + walkGraph(mediaWalks, nextWalk, nextEntity, nextLink);
> + newWalk = true;
> + }
> + }
> +}
> +
> } /* namespace libcamera */
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list