[libcamera-devel] [PATCH v2 01/14] libcamera: formats: Add toString() methods

[Jacopo Mondi]

HI Laurent,

On Wed, Mar 13, 2019 at 06:59:17PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Mar 12, 2019 at 01:12:29PM +0100, Jacopo Mondi wrote:
> > Add toString() helpers to pretty print out a V4L2Device or V4L2Subdevice
> > format.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/include/v4l2_device.h    |  2 ++
> >  src/libcamera/include/v4l2_subdevice.h |  2 ++
> >  src/libcamera/v4l2_device.cpp          | 18 ++++++++++++++++++
> >  src/libcamera/v4l2_subdevice.cpp       | 18 ++++++++++++++++++
> >  4 files changed, 40 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index 5c379fac66dc..258deee8d461 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -100,6 +100,8 @@ public:
> >  		uint32_t bpl;
> >  	} planes[3];
> >  	unsigned int planesCount;
> > +
> > +	const std::string toString() const;
> >  };
> >
> >  class V4L2Device : protected Loggable
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 1cc0fab73103..700e66bcddac 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -21,6 +21,8 @@ struct V4L2SubdeviceFormat {
> >  	uint32_t mbus_code;
> >  	uint32_t width;
> >  	uint32_t height;
> > +
> > +	const std::string toString() const;
> >  };
> >
> >  class V4L2Subdevice : protected Loggable
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index a88a5f5ff036..0bdcfc1d3c8f 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -6,6 +6,8 @@
> >   */
> >
> >  #include <fcntl.h>
> > +#include <iomanip>
> > +#include <sstream>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> >  #include <sys/mman.h>
> > @@ -223,6 +225,22 @@ LOG_DEFINE_CATEGORY(V4L2)
> >   * \brief The number of valid data planes
> >   */
> >
> > +/**
> > + * \brief Assemble and return a string describing the format
> > + *
> > + * \return A string describing the V4L2DeviceFormat
> > + */
> > +const std::string V4L2DeviceFormat::toString() const
> > +{
> > +	std::stringstream ss;
> > +
> > +	ss.fill(0);
> > +	ss << width << "x" << height << "- 0x" << std::hex
> > +	   << std::setw(8) << fourcc << " planes: " << planesCount;
>
> As we standardize on a textual representation of a format, I think we
> should consider how it should look like. Would it make sense to avoid
> spaces, in order to use the same representation as command line
> arguments to the cam tool without the need for quotes ? Do we need to
> print the number of planes ?
>

How would you re-use the argument? The cam tool options syntax is
-f width=1920,height=1080

Would you print out the format with 'width' and 'heigh' ?

Thanks
  j

> > +
> > +	return ss.str();
> > +}
> > +
> >  /**
> >   * \class V4L2Device
> >   * \brief V4L2Device object and API
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 5f58904bf9e6..7c56ba3ba510 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -6,6 +6,8 @@
> >   */
> >
> >  #include <fcntl.h>
> > +#include <iomanip>
> > +#include <sstream>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> >  #include <unistd.h>
> > @@ -69,6 +71,22 @@ LOG_DEFINE_CATEGORY(V4L2Subdev)
> >   * \brief The image height in pixels
> >   */
> >
> > +/**
> > + * \brief Assemble and return a string describing the format
> > + *
> > + * \return A string describing the V4L2SubdeviceFormat
> > + */
> > +const std::string V4L2SubdeviceFormat::toString() const
> > +{
> > +	std::stringstream ss;
> > +
> > +	ss.fill(0);
> > +	ss << width << "x" << height << " - 0x" << std::hex << std::setw(8)
> > +	   << mbus_code;
>
> Same question here regarding spaces. And I think you can limit the
> mbus_code to 4 digits.
>
> > +
> > +	return ss.str();
> > +}
> > +
> >  /**
> >   * \class V4L2Subdevice
> >   * \brief A V4L2 subdevice as exposed by the Linux kernel
>
> --
> 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/20190314/cac1b055/attachment.sig>