[libcamera-devel] [PATCH 5/5] libcamera: Replace toString with operator<<() for format classes

[Laurent Pinchart]

Hi Kieran,

On Wed, May 04, 2022 at 10:08:16AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-04-29 22:23:48)
> > Now that format classes implement the stream formatting operator<<(),
> > use it instead of the toString() function.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/android/camera_capabilities.cpp           |  6 ++---
> >  src/android/camera_device.cpp                 |  4 ++--
> >  src/android/jpeg/encoder_libjpeg.cpp          |  2 +-
> >  src/android/jpeg/thumbnailer.cpp              |  2 +-
> >  src/android/mm/generic_camera_buffer.cpp      |  3 +--
> >  src/android/yuv/post_processor_yuv.cpp        |  4 ++--
> >  src/cam/camera_session.cpp                    |  2 +-
> >  src/cam/kms_sink.cpp                          |  2 +-
> >  src/libcamera/pipeline/ipu3/cio2.cpp          |  2 +-
> >  src/libcamera/pipeline/ipu3/imgu.cpp          |  6 ++---
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 20 ++++++++--------
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 10 ++++----
> >  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  6 ++---
> >  src/libcamera/pipeline/simple/converter.cpp   |  4 ++--
> >  src/libcamera/pipeline/simple/simple.cpp      |  9 ++++---
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  5 ++--
> >  src/libcamera/pipeline/vimc/vimc.cpp          |  2 +-
> >  src/libcamera/v4l2_subdevice.cpp              |  2 +-
> >  src/qcam/viewfinder_qt.cpp                    |  3 +--
> >  src/v4l2/v4l2_camera_proxy.cpp                |  2 +-
> >  test/bayer-format.cpp                         | 24 +++++++++----------
> >  test/camera-sensor.cpp                        |  2 +-
> >  22 files changed, 59 insertions(+), 63 deletions(-)

[snip]

> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index b3d0ddad83db..ca6b43cea5f1 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -206,7 +206,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> >         const auto it = formatInfoMap.find(mbus_code);
> >         if (it == formatInfoMap.end()) {
> >                 LOG(V4L2, Error) << "No information available for format '"
> > -                                << toString() << "'";
> > +                                << *this << "'";
> 
> This is probably the one time I'd prefer to see toString() rather than
> *this, but I don't care too much.

'*this* always feels a bit weird, mostly because it's not often
encountered I suppose. I went for it to try and remove as many usages of
toString() as possible, to see what would remain.

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >                 return 0;
> >         }
> >  
> > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp
> > index a05c75ed9e12..6844f998bbe0 100644
> > --- a/src/qcam/viewfinder_qt.cpp
> > +++ b/src/qcam/viewfinder_qt.cpp
> > @@ -67,8 +67,7 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,
> >  
> >                 image_ = QImage(size, QImage::Format_RGB32);
> >  
> > -               qInfo() << "Using software format conversion from"
> > -                       << format.toString().c_str();
> > +               qInfo() << "Using software format conversion from" << format;
> 
> I like the simplification here that we don't then need to call .c_str()
> on this. Was that actually required for qInfo() before?

Yes, qInfo() returns a QDebug object which has no operator<<(const
std::string &).

> >         } else {
> >                 qInfo() << "Zero-copy enabled";
> >         }

[snip]

-- 
Regards,

Laurent Pinchart