[libcamera-devel] [PATCH 2/5] libcamera: media_device: Add function to get a MediaLink

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-01-03 18:38:56 +0100, Jacopo Mondi wrote:
> Add three overloaded functions 'link()' to retrieve a link between two
> pads. Each overloaded implementation exposes a different method to
> identify the source and sink entities.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/media_device.h |   6 ++
>  src/libcamera/media_device.cpp       | 119 +++++++++++++++++++++++++++
>  2 files changed, 125 insertions(+)
> 
> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
> index 9f45fc7..3228ad5 100644
> --- a/src/libcamera/include/media_device.h
> +++ b/src/libcamera/include/media_device.h
> @@ -40,6 +40,12 @@ public:
>  	const std::vector<MediaEntity *> &entities() const { return entities_; }
>  	MediaEntity *getEntityByName(const std::string &name) const;
>  
> +	MediaLink *link(const std::string sourceName, unsigned int sourceIdx,
> +			const std::string sinkName, unsigned int sinkIdx) const;
> +	MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
> +			const MediaEntity *sink, unsigned int sinkIdx) const;
> +	MediaLink *link(const MediaPad *source, const MediaPad *sink) const;
> +
>  private:
>  	std::string driver_;
>  	std::string devnode_;
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 2f9490c..6892300 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -306,6 +306,125 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const
>  	return nullptr;
>  }
>  
> +/**
> + * \brief Return the MediaLink that connects two entities identified by name
> + * \param sourceName The source entity name
> + * \param sourceIdx The index of the source pad
> + * \param sinkName The sink entity name
> + * \param sinkIdx The index of the sink pad
> + *
> + * Find link that connects the pads at index \a sourceIdx of the source
> + * entity with name \a sourceName, to the pad at index \a sinkIdx of the
> + * sink entity with name \a sinkName, if any.
> + *
> + * \sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const
> + * \sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const
> + *
> + * \return The link that connects the two entities, nullptr otherwise
> + */
> +MediaLink *MediaDevice::link(const std::string sourceName, unsigned int sourceIdx,
> +			     const std::string sinkName, unsigned int sinkIdx) const
> +{
> +	const MediaEntity *source = getEntityByName(sourceName);
> +	if (!source) {
> +		LOG(Error) << "Failed to find entity with name: "
> +			   << sourceName << "\n";
> +		return nullptr;
> +	}
> +
> +	const MediaPad *sourcePad = source->getPadByIndex(sourceIdx);
> +	if (!sourcePad) {
> +		LOG(Error) << "Entity \"" << sourceName << "\" "
> +			   << "has no pad at index " << sourceIdx << "\n";
> +		return nullptr;
> +	}
> +
> +	const MediaEntity *sink = getEntityByName(sinkName);
> +	if (!sink) {
> +		LOG(Error) << "Failed to find entity with name: "
> +			   << sinkName << "\n";
> +		return nullptr;
> +	}
> +
> +	const MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);
> +	if (!sinkPad) {
> +		LOG(Error) << "Entity \"" << sinkName << "\" "
> +			   << "has no pad at index " << sinkIdx << "\n";
> +		return nullptr;
> +	}
> +
> +	return link(sourcePad, sinkPad);

How about just resolving the source and sink MediaEntity's here and then

    return link(source, sourceIdx, sink, sinkIdx);

This would reduce the code duplication.

> +}
> +
> +/**
> + * \brief Return the MediaLink that connects two entities
> + * \param source The source entity
> + * \param sourceIdx The index of the source pad
> + * \param sink The sink entity
> + * \param sinkIdx The index of the sink pad
> + *
> + * Find link that connects the pads at index \a sourceIdx of the source
> + * entity \a source, to the pad at index \a sinkIdx of the sink entity \a
> + * sink, if any.
> + *
> + * \sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const
> + * \sa MediaDevice::link(const MediaPad *source, const MediaPad *sink) const
> + *
> + * \return The link that connects the two entities, nullptr otherwise
> + */
> +MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx,
> +			     const MediaEntity *sink, unsigned int sinkIdx) const
> +{
> +
> +	const MediaPad *sourcePad = source->getPadByIndex(sourceIdx);
> +	if (!sourcePad) {
> +		LOG(Error) << "Entity \"" << source->name() << "\" "
> +			   << "has no pad at index " << sourceIdx << "\n";
> +		return nullptr;
> +	}
> +
> +	const MediaPad *sinkPad = sink->getPadByIndex(sinkIdx);
> +	if (!sinkPad) {
> +		LOG(Error) << "Entity \"" << sink->name() << "\" "
> +			   << "has no pad at index " << sinkIdx << "\n";
> +		return nullptr;
> +	}
> +
> +	return link(sourcePad, sinkPad);
> +}
> +
> +/**
> + * \brief Return the MediaLink that connects two pads
> + * \param source The source pad
> + * \param sink The sink pad
> + *
> + * \sa MediaDevice::link(const std::string sourceName, unsigned int sourceIdx, const std::string sinkName, unsigned int sinkIdx) const
> + * \sa MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx) const
> + *
> + * \return The link that connects the two pads, nullptr otherwise
> + */
> +MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad *sink) const
> +{
> +	if (!source || !sink)
> +		return nullptr;
> +
> +	/*
> +	 * Now that we have made sure all instances are valid, compare
> +	 * their ids to find a matching link.
> +	 */

I like comments in the code it self. I don't like (but I still write 
them my self) comments that are written in this tens, how about.

    Compare pad ids to find the link which connects the source and sink 
    pads.

> +	for (MediaLink *link : source->links()) {
> +		if (link->sink()->id() != sink->id())
> +			continue;
> +
> +		if (link->sink()->entity()->id() != sink->entity()->id())
> +			continue;
> +
> +		return link;
> +	}
> +
> +	return nullptr;
> +}
> +
>  /**
>   * \var MediaDevice::objects_
>   * \brief Global map of media objects (entities, pads, links) keyed by their
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund