[libcamera-devel] [PATCH v5 06/19] libcamera: v4l2_device: Create device from entity name

[Jacopo Mondi]

Hi Laurent,

On Tue, Apr 02, 2019 at 01:01:37AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Mar 26, 2019 at 09:38:49AM +0100, Jacopo Mondi wrote:
> > Add a static method to V4L2Device class to create a V4L2Device instance
> > from a media entity name.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/include/v4l2_device.h |  4 ++++
> >  src/libcamera/v4l2_device.cpp       | 22 ++++++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index 258deee8d461..482d7891dd49 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -22,6 +22,7 @@ namespace libcamera {
> >  class Buffer;
> >  class BufferPool;
> >  class EventNotifier;
> > +class MediaDevice;
> >  class MediaEntity;
> >
> >  struct V4L2Capability final : v4l2_capability {
> > @@ -107,6 +108,9 @@ public:
> >  class V4L2Device : protected Loggable
> >  {
> >  public:
> > +	static V4L2Device *fromEntityName(const MediaDevice *media,
> > +					  const std::string &name);
> > +
>
> I would move this down, as static methods are usually put after the
> non-static ones.
>

I haven't found this rule anywhere in the code style guides we refer
to. I can move them down if that's how we generally do things.

> I wonder if there would be an advantage in turning this into a
> constructor for the V4L2Device class, but in any case that could be done
> later, so, with the above fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks
  j

>
> >  	explicit V4L2Device(const std::string &deviceNode);
> >  	explicit V4L2Device(const MediaEntity *entity);
> >  	V4L2Device(const V4L2Device &) = delete;
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 93d81517fedb..68be5a064258 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -19,6 +19,7 @@
> >  #include <libcamera/event_notifier.h>
> >
> >  #include "log.h"
> > +#include "media_device.h"
> >  #include "media_object.h"
> >  #include "v4l2_device.h"
> >
> > @@ -264,6 +265,27 @@ const std::string V4L2DeviceFormat::toString() const
> >   * released.
> >   */
> >
> > +/**
> > + * \brief Create a new video device instance from entity with \a name in
> > + * media device \a media
> > + * \param[in] media The media device where the entity is registered
> > + * \param[in] name The media entity name
> > + *
> > + * Releasing memory of the newly created instance is responsibility of the
> > + * caller of this function.
> > + *
> > + * \return A newly created V4L2Device on success, nullptr otherwise
> > + */
> > +V4L2Device *V4L2Device::fromEntityName(const MediaDevice *media,
> > +				       const std::string &name)
> > +{
> > +	MediaEntity *entity = media->getEntityByName(name);
> > +	if (!entity)
> > +		return nullptr;
> > +
> > +	return new V4L2Device(entity);
> > +}
> > +
> >  /**
> >   * \brief Construct a V4L2Device
> >   * \param deviceNode The file-system path to the video device node
>
> --
> Regards,
>
> Laurent Pinchart
-------------- 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/20190402/3e22159d/attachment.sig>