Message ID | 20190418154453.20142-2-laurent.pinchart@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Laurent, 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@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... > 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. Apart from that: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > /** > * \var StreamConfiguration::bufferCount > * \brief Requested number of buffers to allocate for the stream > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@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@jmondi.org> > > > /** > > * \var StreamConfiguration::bufferCount > > * \brief Requested number of buffers to allocate for the stream
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) { 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(); +} + /** * \var StreamConfiguration::bufferCount * \brief Requested number of buffers to allocate for the stream
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@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(-)