[libcamera-devel] [PATCH v3 1/8] libcamera: v4l2_subdevice: Store media entity
Jacopo Mondi
jacopo at jmondi.org
Wed Feb 27 09:09:59 CET 2019
Hi Niklas,
On Tue, Feb 26, 2019 at 08:31:37PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2019-02-26 17:26:34 +0100, Jacopo Mondi wrote:
> > Store the media entity backing the V4L2Subdevice and add a deviceName()
> > method to retrieve the human readable name of the subdevice, which is
> > created using the name of the associated media entity.
> >
> > Use the deviceName() in error messages where appropriate, as the entity
> > name is sometimes more informative than the device node path.
>
> I would keep the device node in the error messages instead of the device
> name. As a user diagnosing the problem I would find it easier to
> associate the errno to a device node rather then a name. Also going from
> the device node to the device name is easier then the reveres using v4l2
> tools. Possibly you could print both.
I would agree if we were talking about video device nodes, but here
we're talking about subdevices.
It's very easy to go from entities from the devnode path with
media-ctl, and subdevices are inspected with usually with media-ctl
that presents them by their entities name.
Having the crude v4l-subdev path, is much less informative imho.
>
> With this fixed,
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/include/v4l2_subdevice.h | 8 +++++---
> > src/libcamera/v4l2_subdevice.cpp | 17 ++++++++++++-----
> > 2 files changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 82fa6685ab52..eac699a06109 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -9,9 +9,10 @@
> >
> > #include <string>
> >
> > +#include "media_object.h"
> > +
> > namespace libcamera {
> >
> > -class MediaEntity;
> > struct Rectangle;
> >
> > struct V4L2SubdeviceFormat {
> > @@ -31,7 +32,8 @@ public:
> > bool isOpen() const;
> > void close();
> >
> > - std::string deviceNode() const { return deviceNode_; }
> > + std::string deviceNode() const { return entity_->deviceNode(); }
> > + std::string deviceName() const { return entity_->name(); }
> >
> > int setCrop(unsigned int pad, Rectangle *rect);
> > int setCompose(unsigned int pad, Rectangle *rect);
> > @@ -43,7 +45,7 @@ private:
> > int setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect);
> >
> > - std::string deviceNode_;
> > + const MediaEntity *entity_;
> > int fd_;
> > };
> >
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index b436f73cc75f..56ecf3851cb0 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -88,7 +88,7 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)
> > * path
> > */
> > V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
> > - : deviceNode_(entity->deviceNode()), fd_(-1)
> > + : entity_(entity), fd_(-1)
> > {
> > }
> >
> > @@ -106,11 +106,11 @@ int V4L2Subdevice::open()
> > return -EBUSY;
> > }
> >
> > - ret = ::open(deviceNode_.c_str(), O_RDWR);
> > + ret = ::open(deviceNode().c_str(), O_RDWR);
> > if (ret < 0) {
> > ret = -errno;
> > LOG(V4L2Subdev, Error)
> > - << "Failed to open V4L2 subdevice '" << deviceNode_
> > + << "Failed to open V4L2 subdevice '" << deviceNode()
> > << "': " << strerror(-ret);
> > return ret;
> > }
> > @@ -147,6 +147,13 @@ void V4L2Subdevice::close()
> > * \return The subdevice's device node system path
> > */
> >
> > +/**
> > + * \fn V4L2Subdevice::deviceName()
> > + * \brief Retrieve the name of the media entity associated with the subdevice
> > + *
> > + * \return The name of the media entity the subdevice is associated to
> > + */
> > +
> > /**
> > * \brief Set a crop rectangle on one of the V4L2 subdevice pads
> > * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> > @@ -189,7 +196,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)
> > ret = -errno;
> > LOG(V4L2Subdev, Error)
> > << "Unable to get format on pad " << pad
> > - << " of " << deviceNode_ << ": " << strerror(-ret);
> > + << " of " << deviceName() << ": " << strerror(-ret);
> > return ret;
> > }
> >
> > @@ -255,7 +262,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > ret = -errno;
> > LOG(V4L2Subdev, Error)
> > << "Unable to set rectangle " << target << " on pad "
> > - << pad << " of " << deviceNode_ << ": "
> > + << pad << " of " << deviceName() << ": "
> > << strerror(-ret);
> > return ret;
> > }
> > --
> > 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/20190227/bceee453/attachment.sig>
More information about the libcamera-devel
mailing list