[libcamera-devel,v4,01/31] libcamera: formats: Add toString() methods
diff mbox series

Message ID 20190320163055.22056-2-jacopo@jmondi.org
State Accepted
Headers show
Series
  • libcamera: ipu3: Add ImgU support + multiple streams
Related show

Commit Message

Jacopo Mondi March 20, 2019, 4:30 p.m. UTC 
Add toString() helpers to pretty print out a V4L2Device or V4L2Subdevice
format.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@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(+)

Comments

Laurent Pinchart March 21, 2019, 8:42 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:25PM +0100, Jacopo Mondi wrote:
> Add toString() helpers to pretty print out a V4L2Device or V4L2Subdevice
> format.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@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 889c63b3fb63..97dc05288806 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

Please either put a space on each side of the -, or no space at all.

> +	   << std::setw(8) << fourcc;
> +
> +	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..11d22da25728 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

Same here.

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	   << std::setw(4) << mbus_code;
> +
> +	return ss.str();
> +}
> +
>  /**
>   * \class V4L2Subdevice
>   * \brief A V4L2 subdevice as exposed by the Linux kernel
Jacopo Mondi March 21, 2019, 10:34 a.m. UTC | #2 
Hi Laurent,

On Thu, Mar 21, 2019 at 10:42:48AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Mar 20, 2019 at 05:30:25PM +0100, Jacopo Mondi wrote:
> > Add toString() helpers to pretty print out a V4L2Device or V4L2Subdevice
> > format.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo@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 889c63b3fb63..97dc05288806 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
>
> Please either put a space on each side of the -, or no space at all.

I didn't get this, I thought we just wanted to standardize on a single
format between devices and subdevices.

I'll remove spaces from here and in all ther places in the series

Thanks
  j
>
> > +	   << std::setw(8) << fourcc;
> > +
> > +	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..11d22da25728 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
>
> Same here.
>
> With this fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > +	   << std::setw(4) << mbus_code;
> > +
> > +	return ss.str();
> > +}
> > +
> >  /**
> >   * \class V4L2Subdevice
> >   * \brief A V4L2 subdevice as exposed by the Linux kernel
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

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 889c63b3fb63..97dc05288806 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;
+
+	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..11d22da25728 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(4) << mbus_code;
+
+	return ss.str();
+}
+
 /**
  * \class V4L2Subdevice
  * \brief A V4L2 subdevice as exposed by the Linux kernel