[libcamera-devel] [PATCH 0/5] libcamera: More operator<<() for stream output

[Jacopo Mondi]

Ah, right, different use cases, so having both is fine

For the series: add Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

On Tue, May 03, 2022 at 05:39:44PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, May 03, 2022 at 09:04:14AM +0200, Jacopo Mondi wrote:
> > Hi Laurent,
> >
> > On Sat, Apr 30, 2022 at 12:23:43AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > Hello,
> > >
> > > This patch series extends Han-Lin's operator<<() implementation for
> > > geometry classes to format classes. There isn't much else to mention in
> > > the cover letter, please see individual patches for details.
> >
> > We now have both .toString() and operator<< for printing. Is this
> > intentional ? Shouldn't we stabilize on only one of the two ?
>
> I think both make sense (as long as one of the two is implemented based
> on the other internally), as they serve different purposes. operator<<()
> certainly simplifies logging, but not all string representations are
> used in logs. toString() has few users so far, mostly in tests, for
> instance in
>
> 	if (PixelFormat().toString() != "<INVALID>") {
> 		cerr << "Failed to convert default PixelFormat to string"
> 		     << endl;
> 		return TestFail;
> 	}
>
> It would need to be written as
>
> 	std::stringstream ss;
> 	ss << PixelFormat();
> 	if (ss.str() != "<INVALID>") {
> 		...
> 	}
>
> which isn't very convenient. I can imagine more use cases in
> applications, for instance in qcam where available resolutions could be
> displayed in the UI.
>
> > > Laurent Pinchart (5):
> > >   libcamera: geometry: Add missing ostream header in geometry.h
> > >   libcamera: Add operator<<() for pixel format classes
> > >   libcamera: bayer_format: Add operator<<()
> > >   libcamera: Add operator<<() for V4L2 format classes
> > >   libcamera: Replace toString with operator<<() for format classes
> > >
> > >  include/libcamera/geometry.h                  |  1 +
> > >  include/libcamera/internal/bayer_format.h     |  3 +
> > >  include/libcamera/internal/v4l2_pixelformat.h |  3 +
> > >  include/libcamera/internal/v4l2_subdevice.h   |  3 +
> > >  include/libcamera/internal/v4l2_videodevice.h |  3 +
> > >  include/libcamera/pixel_format.h              |  3 +
> > >  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/bayer_format.cpp                | 57 ++++++++++++-------
> > >  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/pixel_format.cpp                | 12 ++++
> > >  src/libcamera/v4l2_pixelformat.cpp            | 13 +++++
> > >  src/libcamera/v4l2_subdevice.cpp              | 36 ++++++++----
> > >  src/libcamera/v4l2_videodevice.cpp            | 16 +++++-
> > >  src/qcam/viewfinder_qt.cpp                    |  3 +-
> > >  src/v4l2/v4l2_camera_proxy.cpp                |  2 +-
> > >  test/bayer-format.cpp                         | 24 ++++----
> > >  test/camera-sensor.cpp                        |  2 +-
> > >  32 files changed, 175 insertions(+), 95 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart