[v3] libcamera: stream: Add operator<<(StreamConfiguration)

22236 diff mbox series

The StreamConfiguration class only implements toString() but doesn't
offer an overload of operator<<() which is more convenient to use.

Add an overload for operator<<(StreamConfiguration) and re-implement
StreamConfiguration::toString() on top of it.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 include/libcamera/stream.h |  2 ++
 src/libcamera/stream.cpp   | 18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

Hi Jacopo,

Thank you for the patch.

On Fri, Dec 06, 2024 at 05:55:58PM +0100, Jacopo Mondi wrote:
> The StreamConfiguration class only implements toString() but doesn't
> offer an overload of operator<<() which is more convenient to use.
> 
> Add an overload for operator<<(StreamConfiguration) and re-implement
> StreamConfiguration::toString() on top of it.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  include/libcamera/stream.h |  2 ++
>  src/libcamera/stream.cpp   | 18 +++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 071b71698acb..ea228aea7d56 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -61,6 +61,8 @@ private:
>  	StreamFormats formats_;
>  };
>  
> +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg);
> +
>  enum class StreamRole {
>  	Raw,
>  	StillCapture,
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 1f75dbbc5b64..179e75df445b 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -392,7 +392,23 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>   */
>  std::string StreamConfiguration::toString() const
>  {
> -	return size.toString() + "-" + pixelFormat.toString();
> +	std::stringstream ss;
> +	ss << *this;
> +
> +	return ss.str();
> +}
> +
> +/**
> + * \brief Insert a text representation of a StreamConfiguration into an output
> + * stream
> + * \param[in] out The output stream
> + * \param[in] cfg The StreamConfiguration
> + * \return The output stream \a out
> + */
> +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg)
> +{
> +	out << cfg.size.toString() + "-" + cfg.pixelFormat.toString();

If we want to make streams usage efficient here, this should be

	out << cfg.size << "-" << cfg.pixelFormat;

With that,

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

> +	return out;
>  }
>  
>  /**

Hi


2024. december 6., péntek 19:15 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:

> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Fri, Dec 06, 2024 at 05:55:58PM +0100, Jacopo Mondi wrote:
> > The StreamConfiguration class only implements toString() but doesn't
> > offer an overload of operator<<() which is more convenient to use.
> >
> > Add an overload for operator<<(StreamConfiguration) and re-implement
> > StreamConfiguration::toString() on top of it.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  include/libcamera/stream.h |  2 ++
> >  src/libcamera/stream.cpp   | 18 +++++++++++++++++-
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index 071b71698acb..ea228aea7d56 100644
> > --- a/include/libcamera/stream.h
> > +++ b/include/libcamera/stream.h
> > @@ -61,6 +61,8 @@ private:
> >  	StreamFormats formats_;
> >  };
> >
> > +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg);
> > +
> >  enum class StreamRole {
> >  	Raw,
> >  	StillCapture,
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 1f75dbbc5b64..179e75df445b 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -392,7 +392,23 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> >   */
> >  std::string StreamConfiguration::toString() const
> >  {
> > -	return size.toString() + "-" + pixelFormat.toString();
> > +	std::stringstream ss;
> > +	ss << *this;
> > +
> > +	return ss.str();
> > +}
> > +
> > +/**
> > + * \brief Insert a text representation of a StreamConfiguration into an output
> > + * stream
> > + * \param[in] out The output stream
> > + * \param[in] cfg The StreamConfiguration
> > + * \return The output stream \a out
> > + */
> > +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg)
> > +{
> > +	out << cfg.size.toString() + "-" + cfg.pixelFormat.toString();
> 
> If we want to make streams usage efficient here, this should be
> 
> 	out << cfg.size << "-" << cfg.pixelFormat;
> [...]

I believe the argument should be `const StreamConfiguration&` in addition.


Regards,
Barnabás Pőcze

On Fri, Dec 06, 2024 at 06:19:47PM +0000, Barnabás Pőcze wrote:
> 2024. december 6., péntek 19:15 keltezéssel, Laurent Pinchart írta:
> > On Fri, Dec 06, 2024 at 05:55:58PM +0100, Jacopo Mondi wrote:
> > > The StreamConfiguration class only implements toString() but doesn't
> > > offer an overload of operator<<() which is more convenient to use.
> > >
> > > Add an overload for operator<<(StreamConfiguration) and re-implement
> > > StreamConfiguration::toString() on top of it.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > > ---
> > >  include/libcamera/stream.h |  2 ++
> > >  src/libcamera/stream.cpp   | 18 +++++++++++++++++-
> > >  2 files changed, 19 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > > index 071b71698acb..ea228aea7d56 100644
> > > --- a/include/libcamera/stream.h
> > > +++ b/include/libcamera/stream.h
> > > @@ -61,6 +61,8 @@ private:
> > >  	StreamFormats formats_;
> > >  };
> > >
> > > +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg);
> > > +
> > >  enum class StreamRole {
> > >  	Raw,
> > >  	StillCapture,
> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > > index 1f75dbbc5b64..179e75df445b 100644
> > > --- a/src/libcamera/stream.cpp
> > > +++ b/src/libcamera/stream.cpp
> > > @@ -392,7 +392,23 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> > >   */
> > >  std::string StreamConfiguration::toString() const
> > >  {
> > > -	return size.toString() + "-" + pixelFormat.toString();
> > > +	std::stringstream ss;
> > > +	ss << *this;
> > > +
> > > +	return ss.str();
> > > +}
> > > +
> > > +/**
> > > + * \brief Insert a text representation of a StreamConfiguration into an output
> > > + * stream
> > > + * \param[in] out The output stream
> > > + * \param[in] cfg The StreamConfiguration
> > > + * \return The output stream \a out
> > > + */
> > > +std::ostream &operator<<(std::ostream &out, StreamConfiguration cfg)
> > > +{
> > > +	out << cfg.size.toString() + "-" + cfg.pixelFormat.toString();
> > 
> > If we want to make streams usage efficient here, this should be
> > 
> > 	out << cfg.size << "-" << cfg.pixelFormat;
> > [...]
> 
> I believe the argument should be `const StreamConfiguration&` in addition.

Yes, that too, I've missed it.