[libcamera-devel] [PATCH 1/5] libcamera: Add pointer to MediaDevice to MediaObject
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Jan 4 17:24:57 CET 2019
Hi Jacopo,
Thanks for your patch.
On 2019-01-03 18:38:55 +0100, Jacopo Mondi wrote:
> Add a MediaDevice member field to the MediaObject class hierarcy.
> Each media object now has a reference to the media device it belongs to,
> and which it has been created by.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/include/media_object.h | 10 ++++++----
> src/libcamera/media_device.cpp | 8 ++++----
> src/libcamera/media_object.cpp | 30 +++++++++++++++++++---------
> 3 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h
> index 950a332..0f0bb29 100644
> --- a/src/libcamera/include/media_object.h
> +++ b/src/libcamera/include/media_object.h
> @@ -26,10 +26,12 @@ public:
> protected:
> friend class MediaDevice;
>
> - MediaObject(unsigned int id) : id_(id) { }
> + MediaObject(MediaDevice *media, unsigned int id) :
> + id_(id), media_(media) { }
Nit: I would swap the initialization settings if id_(), media_() to
media_(), id_() in order to match the arguments to the constructor.
> virtual ~MediaObject() { }
>
> unsigned int id_;
> + MediaDevice *media_;
> };
>
> class MediaLink : public MediaObject
> @@ -42,7 +44,7 @@ public:
> private:
> friend class MediaDevice;
>
> - MediaLink(const struct media_v2_link *link,
> + MediaLink(MediaDevice *media, const struct media_v2_link *link,
> MediaPad *source, MediaPad *sink);
> MediaLink(const MediaLink &) = delete;
> ~MediaLink() { }
> @@ -65,7 +67,7 @@ public:
> private:
> friend class MediaDevice;
>
> - MediaPad(const struct media_v2_pad *pad, MediaEntity *entity);
> + MediaPad(MediaDevice *media, const struct media_v2_pad *pad, MediaEntity *entity);
> MediaPad(const MediaPad &) = delete;
> ~MediaPad();
>
> @@ -93,7 +95,7 @@ public:
> private:
> friend class MediaDevice;
>
> - MediaEntity(const struct media_v2_entity *entity,
> + MediaEntity(MediaDevice *media, const struct media_v2_entity *entity,
> unsigned int major = 0, unsigned int minor = 0);
> MediaEntity(const MediaEntity &) = delete;
> ~MediaEntity();
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index cf4ff90..2f9490c 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -430,11 +430,11 @@ bool MediaDevice::populateEntities(const struct media_v2_topology &topology)
>
> MediaEntity *entity;
> if (iface)
> - entity = new MediaEntity(&mediaEntities[i],
> + entity = new MediaEntity(this, &mediaEntities[i],
> iface->devnode.major,
> iface->devnode.minor);
> else
> - entity = new MediaEntity(&mediaEntities[i]);
> + entity = new MediaEntity(this, &mediaEntities[i]);
>
> if (!addObject(entity)) {
> delete entity;
> @@ -464,7 +464,7 @@ bool MediaDevice::populatePads(const struct media_v2_topology &topology)
> return false;
> }
>
> - MediaPad *pad = new MediaPad(&mediaPads[i], mediaEntity);
> + MediaPad *pad = new MediaPad(this, &mediaPads[i], mediaEntity);
> if (!addObject(pad)) {
> delete pad;
> return false;
> @@ -509,7 +509,7 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)
> return false;
> }
>
> - MediaLink *link = new MediaLink(&mediaLinks[i], source, sink);
> + MediaLink *link = new MediaLink(this, &mediaLinks[i], source, sink);
> if (!addObject(link)) {
> delete link;
> return false;
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index 581e1c0..f1535e6 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -44,14 +44,16 @@ namespace libcamera {
> *
> * MediaObject is an abstract base class for all media objects in the media
> * graph. Every media graph object is identified by an id unique in the media
> - * device context, and this base class provides that.
> + * device context, and this base class provides both of them.
Both? It provides the unique id and what? Maybe I'm slow today :-) I
assume the other thing is a reference to the MediaDevice? If so how
about.
MediaObject is an abstract base class for all media objects in the
media graph. Each object is identified by a reference to the media
device it belongs to and a unique id withing that media devce.
This base class provide helpers to media objects to keep track of
these identifiers.
> *
> * \sa MediaEntity, MediaPad, MediaLink
> */
>
> /**
> * \fn MediaObject::MediaObject()
> - * \brief Construct a MediaObject with \a id
> + * \brief Construct a MediaObject with \a id, globally unique in the MediaDevice
> + * \a media
> + * \param media The media device this object belongs to
> * \param id The media object id
> *
> * The caller shall ensure unicity of the object id in the media device context.
> @@ -69,6 +71,11 @@ namespace libcamera {
> * \brief The media object id
> */
>
> +/**
> + * \var MediaObject::media_
> + * \brief The media device that constructed this object
How about
The media device the media object belongs to
Same comment bellow for documentation of the media argument. With this
addressed.
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> + */
> +
> /**
> * \class MediaLink
> * \brief The MediaLink represents a link between two pads in the media graph.
> @@ -82,13 +89,14 @@ namespace libcamera {
>
> /**
> * \brief Construct a MediaLink
> + * \param media The media device this entity belongs to
> * \param link The media link kernel data
> * \param source The source pad at the origin of the link
> * \param sink The sink pad at the destination of the link
> */
> -MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,
> - MediaPad *sink)
> - : MediaObject(link->id), source_(source),
> +MediaLink::MediaLink(MediaDevice *media, const struct media_v2_link *link,
> + MediaPad *source, MediaPad *sink)
> + : MediaObject(media, link->id), source_(source),
> sink_(sink), flags_(link->flags)
> {
> }
> @@ -135,11 +143,13 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,
>
> /**
> * \brief Construct a MediaPad
> + * \param media The media device this entity belongs to
> * \param pad The media pad kernel data
> * \param entity The entity the pad belongs to
> */
> -MediaPad::MediaPad(const struct media_v2_pad *pad, MediaEntity *entity)
> - : MediaObject(pad->id), index_(pad->index), entity_(entity),
> +MediaPad::MediaPad(MediaDevice *media, const struct media_v2_pad *pad,
> + MediaEntity *entity)
> + : MediaObject(media, pad->id), index_(pad->index), entity_(entity),
> flags_(pad->flags)
> {
> }
> @@ -283,13 +293,15 @@ int MediaEntity::setDeviceNode(const std::string &devnode)
>
> /**
> * \brief Construct a MediaEntity
> + * \param media The media device this entity belongs to
> * \param entity The media entity kernel data
> * \param major The major number of the entity associated interface
> * \param minor The minor number of the entity associated interface
> */
> -MediaEntity::MediaEntity(const struct media_v2_entity *entity,
> +MediaEntity::MediaEntity(MediaDevice *media,
> + const struct media_v2_entity *entity,
> unsigned int major, unsigned int minor)
> - : MediaObject(entity->id), name_(entity->name),
> + : MediaObject(media, entity->id), name_(entity->name),
> major_(major), minor_(minor)
> {
> }
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list