[libcamera-devel,v4,2/6] libcamera: stream: add initial StreamConfiguration structure

435 diff mbox series

Add an initial StreamConfiguration implementation to hold configuration
data for a single stream of a Camera. In its current form not many
configuration parameters are supported but it's expected the number of
options will grow over time.

At this stage the pixel format is represented as an unsigned int to
allow for easy mapping to the V4L2 API. This might be subject to change
in the future as we finalize how libcamera shall represent pixel
formats.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/stream.h |  6 ++++++
 src/libcamera/stream.cpp   | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

Hi Niklas,

On 29/01/2019 02:00, Niklas Söderlund wrote:
> Add an initial StreamConfiguration implementation to hold configuration
> data for a single stream of a Camera. In its current form not many
> configuration parameters are supported but it's expected the number of
> options will grow over time.
> 
> At this stage the pixel format is represented as an unsigned int to
> allow for easy mapping to the V4L2 API. This might be subject to change
> in the future as we finalize how libcamera shall represent pixel
> formats.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/stream.h |  6 ++++++
>  src/libcamera/stream.cpp   | 26 ++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 4f47d85ed6382b36..4b24dd841dd64b64 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -13,6 +13,12 @@ class Stream final
>  {
>  };
>  
> +struct StreamConfiguration {
> +	unsigned int width;
> +	unsigned int height;
> +	unsigned int pixelFormat;
> +};
> +


Should we consider a single global 'Format Configuration' structure or
object as Jacopo has been looking at ? or would that be too generic? (or
even too specific if it has planar information etc)



>  } /* namespace libcamera */
>  
>  #endif /* __LIBCAMERA_STREAM_H__ */
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 01f4e5008af8ac46..ef55f947574e241c 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -48,4 +48,30 @@ namespace libcamera {
>   * optimal stream for the task.
>   */
>  
> +/**
> + * \struct StreamConfiguration
> + * \brief Configuration parameters for a stream
> + *
> + * The StreamConfiguration structure models all information which can be
> + * configured for a single video stream.
> + */
> +
> +/**
> + * \var StreamConfiguration::width
> + * \brief Stream width in pixels
> + */
> +
> +/**
> + * \var StreamConfiguration::height
> + * \brief Stream height in pixels
> + */
> +
> +/**
> + * \var StreamConfiguration::pixelFormat
> + * \brief Stream pixel format
> + *
> + * This is a little endian four character code representation of the pixel
> + * format described in V4L2 using the V4L2_PIX_FMT_* definitions.
> + */
> +
>  } /* namespace libcamera */
>

Hi Kieran,

Thanks for your feedback.

On 2019-01-29 09:47:43 +0000, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 29/01/2019 02:00, Niklas Söderlund wrote:
> > Add an initial StreamConfiguration implementation to hold configuration
> > data for a single stream of a Camera. In its current form not many
> > configuration parameters are supported but it's expected the number of
> > options will grow over time.
> > 
> > At this stage the pixel format is represented as an unsigned int to
> > allow for easy mapping to the V4L2 API. This might be subject to change
> > in the future as we finalize how libcamera shall represent pixel
> > formats.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  include/libcamera/stream.h |  6 ++++++
> >  src/libcamera/stream.cpp   | 26 ++++++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index 4f47d85ed6382b36..4b24dd841dd64b64 100644
> > --- a/include/libcamera/stream.h
> > +++ b/include/libcamera/stream.h
> > @@ -13,6 +13,12 @@ class Stream final
> >  {
> >  };
> >  
> > +struct StreamConfiguration {
> > +	unsigned int width;
> > +	unsigned int height;
> > +	unsigned int pixelFormat;
> > +};
> > +
> 
> 
> Should we consider a single global 'Format Configuration' structure or
> object as Jacopo has been looking at ? or would that be too generic? (or
> even too specific if it has planar information etc)

I'm not sure I follow you. The idea for StreamConfiguration is to hold 
all configuration parameters that can act on a stream before stream on.  
Once the stream is running I think we will need a different object to 
handle parameters that can change in runtime, such as controls. I think 
we need to tackle this once we start working with queuing requests 
instead of buffers :-)

> 
> 
> 
> >  } /* namespace libcamera */
> >  
> >  #endif /* __LIBCAMERA_STREAM_H__ */
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 01f4e5008af8ac46..ef55f947574e241c 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -48,4 +48,30 @@ namespace libcamera {
> >   * optimal stream for the task.
> >   */
> >  
> > +/**
> > + * \struct StreamConfiguration
> > + * \brief Configuration parameters for a stream
> > + *
> > + * The StreamConfiguration structure models all information which can be
> > + * configured for a single video stream.
> > + */
> > +
> > +/**
> > + * \var StreamConfiguration::width
> > + * \brief Stream width in pixels
> > + */
> > +
> > +/**
> > + * \var StreamConfiguration::height
> > + * \brief Stream height in pixels
> > + */
> > +
> > +/**
> > + * \var StreamConfiguration::pixelFormat
> > + * \brief Stream pixel format
> > + *
> > + * This is a little endian four character code representation of the pixel
> > + * format described in V4L2 using the V4L2_PIX_FMT_* definitions.
> > + */
> > +
> >  } /* namespace libcamera */
> > 
> 
> -- 
> Regards
> --
> Kieran