[libcamera-devel] [PATCH 2/3] libcamera: stream: Add and use toString() method to StreamConfiguration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Apr 19 12:24:08 CEST 2019
Hi Jacopo,
On Fri, Apr 19, 2019 at 10:50:39AM +0200, Jacopo Mondi wrote:
> On Thu, Apr 18, 2019 at 06:44:52PM +0300, Laurent Pinchart wrote:
> > Add a toString() method to the StreamConfiguration class, and replace
> > all manually coded implementations through the source code.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > include/libcamera/stream.h | 4 ++++
> > src/libcamera/camera.cpp | 4 +---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +----
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++----------
> > src/libcamera/stream.cpp | 19 +++++++++++++++++++
> > 5 files changed, 29 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index 8a47930f8614..3caaefc9f8e9 100644
> > --- a/include/libcamera/stream.h
> > +++ b/include/libcamera/stream.h
> > @@ -7,6 +7,8 @@
> > #ifndef __LIBCAMERA_STREAM_H__
> > #define __LIBCAMERA_STREAM_H__
> >
> > +#include <string>
> > +
> > #include <libcamera/buffer.h>
> > #include <libcamera/geometry.h>
> >
> > @@ -20,6 +22,8 @@ struct StreamConfiguration {
> > unsigned int pixelFormat;
> >
> > unsigned int bufferCount;
> > +
> > + std::string toString() const;
> > };
> >
> > class StreamUsage
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 69406b515700..a52769626446 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -596,9 +596,7 @@ int Camera::configureStreams(const CameraConfiguration &config)
> > return -EINVAL;
> >
> > const StreamConfiguration &cfg = config[stream];
> > - msg << " (" << index << ") " << cfg.width << "x"
> > - << cfg.height << "-0x" << std::hex << std::setfill('0')
> > - << std::setw(8) << cfg.pixelFormat;
> > + msg << " (" << index << ") " << cfg.toString();
> >
> > index++;
> > }
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 405d6548fd01..7443224d4f45 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -224,10 +224,7 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> > config->pixelFormat = V4L2_PIX_FMT_NV12;
> > config->bufferCount = IPU3_BUFFER_COUNT;
> >
> > - LOG(IPU3, Debug)
> > - << "Stream format set to " << config->width << "x"
> > - << config->height << "-0x" << std::hex << std::setfill('0')
> > - << std::setw(8) << config->pixelFormat;
> > + LOG(IPU3, Debug) << "Stream format set to " << config->toString();
> >
> > return configs;
> > }
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 8ed0ba84780a..d21c6266c6ba 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -124,10 +124,7 @@ CameraConfiguration PipelineHandlerRkISP1::streamConfiguration(Camera *camera,
> >
> > configs[&data->stream_] = config;
> >
> > - LOG(RkISP1, Debug)
> > - << "Stream format set to " << config.width << "x"
> > - << config.height << "-0x" << std::hex << std::setfill('0')
> > - << std::setw(8) << config.pixelFormat;
> > + LOG(RkISP1, Debug) << "Stream format set to " << config.toString();
> >
> > return configs;
> > }
> > @@ -223,7 +220,7 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera,
> > V4L2DeviceFormat outputFormat = {};
> > outputFormat.width = cfg.width;
> > outputFormat.height = cfg.height;
> > - outputFormat.fourcc = V4L2_PIX_FMT_NV12;
> > + outputFormat.fourcc = cfg.pixelFormat;
> > outputFormat.planesCount = 2;
> >
> > ret = video_->setFormat(&outputFormat);
> > @@ -232,12 +229,9 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera,
> >
> > if (outputFormat.width != cfg.width ||
> > outputFormat.height != cfg.height ||
> > - outputFormat.fourcc != V4L2_PIX_FMT_NV12) {
> > + outputFormat.fourcc != cfg.pixelFormat) {
>
> This and the above hunk seems to be unrelated to this patch to me...
I included them here to match the log message, but you're right, they're
worth a separate patch. I'll fix that.
> > LOG(RkISP1, Error)
> > - << "Unable to configure capture in " << cfg.width
> > - << "x" << cfg.height << "-0x"
> > - << std::hex << std::setfill('0') << std::setw(8)
> > - << V4L2_PIX_FMT_NV12;
> > + << "Unable to configure capture in " << cfg.toString();
> > return -EINVAL;
> > }
> >
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index d4ef506cea95..06db9797ff7e 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -5,6 +5,9 @@
> > * stream.cpp - Video stream for a Camera
> > */
> >
> > +#include <iomanip>
> > +#include <sstream>
> > +
> > #include <libcamera/stream.h>
> >
> > /**
> > @@ -55,6 +58,22 @@ namespace libcamera {
> > * format described in V4L2 using the V4L2_PIX_FMT_* definitions.
> > */
> >
> > +/**
> > + * \brief Assemble and return a string describing the configuration
> > + *
> > + * \return A string describing the StreamConfiguration
> > + */
> > +std::string StreamConfiguration::toString() const
> > +{
> > + std::stringstream ss;
> > +
> > + ss.fill(0);
> > + ss << width << "x" << height << "-0x" << std::hex
> > + << std::setw(8) << pixelFormat;
> > +
> > + return ss.str();
> > +}
> > +
>
> In the struct declaration the toString() method comes after the
> bufferCount member, if we want to follow the declaration order.
Oops, I'll fix that.
> Apart from that:
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > /**
> > * \var StreamConfiguration::bufferCount
> > * \brief Requested number of buffers to allocate for the stream
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list