[libcamera-devel,v4,1/6] libcamera: stream: add initial Stream class

433 diff mbox series

Add an initial Stream implementation. The idea is that once capability
support is added to the library each stream will describe its
capabilities using this class. An application will then select one or
more streams based on these capabilities and use them to configure the
camera and capture.

At this stage the Stream class is empty as capabilities are yet to be
added. The class is still useful as it will be used to communicate how
many streams a Camera object provides.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/libcamera.h |  1 +
 include/libcamera/meson.build |  1 +
 include/libcamera/stream.h    | 18 +++++++++++++
 src/libcamera/meson.build     |  1 +
 src/libcamera/stream.cpp      | 51 +++++++++++++++++++++++++++++++++++
 5 files changed, 72 insertions(+)
 create mode 100644 include/libcamera/stream.h
 create mode 100644 src/libcamera/stream.cpp

Hi Niklas,

On 29/01/2019 02:00, Niklas Söderlund wrote:
> Add an initial Stream implementation. The idea is that once capability
> support is added to the library each stream will describe its
> capabilities using this class. An application will then select one or
> more streams based on these capabilities and use them to configure the
> camera and capture.
> 
> At this stage the Stream class is empty as capabilities are yet to be
> added. The class is still useful as it will be used to communicate how
> many streams a Camera object provides.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Some queries about capitalisation in the Doxygen - but I'm you can
handle that however you see fit.

--
Kieran


> ---
>  include/libcamera/libcamera.h |  1 +
>  include/libcamera/meson.build |  1 +
>  include/libcamera/stream.h    | 18 +++++++++++++
>  src/libcamera/meson.build     |  1 +
>  src/libcamera/stream.cpp      | 51 +++++++++++++++++++++++++++++++++++
>  5 files changed, 72 insertions(+)
>  create mode 100644 include/libcamera/stream.h
>  create mode 100644 src/libcamera/stream.cpp
> 
> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h
> index c0511cf6d662b63f..272dfd5e4a67d5de 100644
> --- a/include/libcamera/libcamera.h
> +++ b/include/libcamera/libcamera.h
> @@ -12,6 +12,7 @@
>  #include <libcamera/event_dispatcher.h>
>  #include <libcamera/event_notifier.h>
>  #include <libcamera/signal.h>
> +#include <libcamera/stream.h>
>  #include <libcamera/timer.h>
>  
>  #endif /* __LIBCAMERA_LIBCAMERA_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index d7cb55ba4a49e1e8..54a680787e5c17aa 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_api = files([
>      'event_notifier.h',
>      'libcamera.h',
>      'signal.h',
> +    'stream.h',
>      'timer.h',
>  ])
>  
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> new file mode 100644
> index 0000000000000000..4f47d85ed6382b36
> --- /dev/null
> +++ b/include/libcamera/stream.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * stream.h - Video stream for a Camera
> + */
> +#ifndef __LIBCAMERA_STREAM_H__
> +#define __LIBCAMERA_STREAM_H__
> +
> +namespace libcamera {
> +
> +class Stream final
> +{
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_STREAM_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index f9f25c0ecf1564cc..9f6ff99eebe2f5bc 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -10,6 +10,7 @@ libcamera_sources = files([
>      'media_object.cpp',
>      'pipeline_handler.cpp',
>      'signal.cpp',
> +    'stream.cpp',
>      'timer.cpp',
>      'v4l2_device.cpp',
>  ])
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> new file mode 100644
> index 0000000000000000..01f4e5008af8ac46
> --- /dev/null
> +++ b/src/libcamera/stream.cpp
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * stream.cpp - Video stream for a Camera
> + */
> +
> +#include <libcamera/stream.h>
> +
> +/**
> + * \file stream.h
> + * \brief Video stream for a Camera
> + *
> + * A camera device can provide frames in different resolutions and formats

Should we capitalise 'camera' as it's the name of one of our classes?



> + * concurrently from a single image source. The Stream class represents
> + * one of the multiple concurrent streams.
> + *
> + * All streams exposed by a camera device share the same image source and are

Same here ? 'Camera'?

> + * thus not fully independent. Parameters related to the image source, such as
> + * the exposure time or flash control, are common to all streams. Other
> + * parameters, such as format or resolution, may be specified per-stream,
> + * depending on the capabilities of the camera device.
> + *
> + * Camera devices expose at least one stream, and may expose additional streams

Stream? on the same basis?

> + * based on the device capabilities. This can be used, for instance, to
> + * implement concurrent viewfinder and video capture, or concurrent viewfinder,
> + * video capture and still image capture.
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class Stream
> + * \brief Video stream for a camera
> + *
> + * The Stream class models of all static information which are associated
> + * with a single video stream. Stream are exposed by the Camera object they

s/video stream/video Stream/ ?

s/Stream are exposed by/Streams are exposed by/

> + * belong to.
> + *
> + * Some cameras are capable of supplying more then one stream from the same

Some Cameras, ... one Stream ?

> + * video source. In such cases an application can inspect all available streams
> + * and select the ones that best fit its use case.
> + *
> + * \todo Add capabilities to the stream API. Without this the Stream class only
> + * serves to reveal how many streams of unknown capabilities a camera supports.
> + * This in itself is productive as it allows applications to configure and
> + * capture from one or more streams even if they won't be able to select the
> + * optimal stream for the task.

I agree - even just for abstraction - it's better to get this in soon to
model the layouts.


> + */
> +
> +} /* namespace libcamera */
>

Hi Kieran,

Thanks for your feedback.

On 2019-01-29 09:45:22 +0000, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 29/01/2019 02:00, Niklas Söderlund wrote:
> > Add an initial Stream implementation. The idea is that once capability
> > support is added to the library each stream will describe its
> > capabilities using this class. An application will then select one or
> > more streams based on these capabilities and use them to configure the
> > camera and capture.
> > 
> > At this stage the Stream class is empty as capabilities are yet to be
> > added. The class is still useful as it will be used to communicate how
> > many streams a Camera object provides.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Some queries about capitalisation in the Doxygen - but I'm you can
> handle that however you see fit.

I have tried to no capitalise them as I think the text flows nicer. I 
have no strong feelings in this area and are open to change if that is 
the consensus. I have left them as is for now.

> 
> --
> Kieran
> 
> 
> > ---
> >  include/libcamera/libcamera.h |  1 +
> >  include/libcamera/meson.build |  1 +
> >  include/libcamera/stream.h    | 18 +++++++++++++
> >  src/libcamera/meson.build     |  1 +
> >  src/libcamera/stream.cpp      | 51 +++++++++++++++++++++++++++++++++++
> >  5 files changed, 72 insertions(+)
> >  create mode 100644 include/libcamera/stream.h
> >  create mode 100644 src/libcamera/stream.cpp
> > 
> > diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h
> > index c0511cf6d662b63f..272dfd5e4a67d5de 100644
> > --- a/include/libcamera/libcamera.h
> > +++ b/include/libcamera/libcamera.h
> > @@ -12,6 +12,7 @@
> >  #include <libcamera/event_dispatcher.h>
> >  #include <libcamera/event_notifier.h>
> >  #include <libcamera/signal.h>
> > +#include <libcamera/stream.h>
> >  #include <libcamera/timer.h>
> >  
> >  #endif /* __LIBCAMERA_LIBCAMERA_H__ */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index d7cb55ba4a49e1e8..54a680787e5c17aa 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -5,6 +5,7 @@ libcamera_api = files([
> >      'event_notifier.h',
> >      'libcamera.h',
> >      'signal.h',
> > +    'stream.h',
> >      'timer.h',
> >  ])
> >  
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > new file mode 100644
> > index 0000000000000000..4f47d85ed6382b36
> > --- /dev/null
> > +++ b/include/libcamera/stream.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * stream.h - Video stream for a Camera
> > + */
> > +#ifndef __LIBCAMERA_STREAM_H__
> > +#define __LIBCAMERA_STREAM_H__
> > +
> > +namespace libcamera {
> > +
> > +class Stream final
> > +{
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_STREAM_H__ */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index f9f25c0ecf1564cc..9f6ff99eebe2f5bc 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -10,6 +10,7 @@ libcamera_sources = files([
> >      'media_object.cpp',
> >      'pipeline_handler.cpp',
> >      'signal.cpp',
> > +    'stream.cpp',
> >      'timer.cpp',
> >      'v4l2_device.cpp',
> >  ])
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > new file mode 100644
> > index 0000000000000000..01f4e5008af8ac46
> > --- /dev/null
> > +++ b/src/libcamera/stream.cpp
> > @@ -0,0 +1,51 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * stream.cpp - Video stream for a Camera
> > + */
> > +
> > +#include <libcamera/stream.h>
> > +
> > +/**
> > + * \file stream.h
> > + * \brief Video stream for a Camera
> > + *
> > + * A camera device can provide frames in different resolutions and formats
> 
> Should we capitalise 'camera' as it's the name of one of our classes?
> 
> 
> 
> > + * concurrently from a single image source. The Stream class represents
> > + * one of the multiple concurrent streams.
> > + *
> > + * All streams exposed by a camera device share the same image source and are
> 
> Same here ? 'Camera'?
> 
> > + * thus not fully independent. Parameters related to the image source, such as
> > + * the exposure time or flash control, are common to all streams. Other
> > + * parameters, such as format or resolution, may be specified per-stream,
> > + * depending on the capabilities of the camera device.
> > + *
> > + * Camera devices expose at least one stream, and may expose additional streams
> 
> Stream? on the same basis?
> 
> > + * based on the device capabilities. This can be used, for instance, to
> > + * implement concurrent viewfinder and video capture, or concurrent viewfinder,
> > + * video capture and still image capture.
> > + */
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \class Stream
> > + * \brief Video stream for a camera
> > + *
> > + * The Stream class models of all static information which are associated
> > + * with a single video stream. Stream are exposed by the Camera object they
> 
> s/video stream/video Stream/ ?
> 
> s/Stream are exposed by/Streams are exposed by/

Thanks!

> 
> > + * belong to.
> > + *
> > + * Some cameras are capable of supplying more then one stream from the same
> 
> Some Cameras, ... one Stream ?

s/Some//.

> 
> > + * video source. In such cases an application can inspect all available streams
> > + * and select the ones that best fit its use case.
> > + *
> > + * \todo Add capabilities to the stream API. Without this the Stream class only
> > + * serves to reveal how many streams of unknown capabilities a camera supports.
> > + * This in itself is productive as it allows applications to configure and
> > + * capture from one or more streams even if they won't be able to select the
> > + * optimal stream for the task.
> 
> I agree - even just for abstraction - it's better to get this in soon to
> model the layouts.
> 
> 
> > + */
> > +
> > +} /* namespace libcamera */
> > 
> 
> -- 
> Regards
> --
> Kieran