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

[Jacopo Mondi]

Hi Niklas,

On Fri, Jan 04, 2019 at 05:33:16PM +0100, Niklas Söderlund wrote:
> 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.
>

Ah, right, no need to search for the source and sink pads here.
I'll fix.

> > +}
> > +
> > +/**
> > + * \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.
>

Ack for this and for the comments style in general.

Thanks
  j

> > +	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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190107/96444bec/attachment.sig>