| Message ID | 20190326083902.26121-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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 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@ideasonboard.com> > 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
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@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@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
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); + 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
Add a static method to V4L2Device class to create a V4L2Device instance from a media entity name. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/v4l2_device.h | 4 ++++ src/libcamera/v4l2_device.cpp | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+)