[libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic stream usages

[Jacopo Mondi]

Hi Niklas,
   thank you for the patch

On Fri, Apr 05, 2019 at 04:02:53AM +0200, Niklas Söderlund wrote:
> In preparation of reworking how a default configuration is retrieved
> from a camera add stream usages. The usages will be used by applications
> to describe how they intend to use a camera and replace the Stream IDs
> when retrieving default configuration from the camera using
> streamConfiguration().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/stream.h | 40 +++++++++++++++++
>  src/libcamera/stream.cpp   | 92 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 132 insertions(+)
>
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 970c479627fab064..1dea696f2e5ca95d 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -8,6 +8,7 @@
>  #define __LIBCAMERA_STREAM_H__
>
>  #include <libcamera/buffer.h>
> +#include <libcamera/geometry.h>
>
>  namespace libcamera {
>
> @@ -21,9 +22,48 @@ struct StreamConfiguration {
>  	unsigned int bufferCount;
>  };
>
> +class StreamUsage
> +{
> +public:
> +	enum Role {
> +		StillCapture,
> +		VideoRecording,
> +		Viewfinder,
> +	};
> +
> +	Role role() const { return role_; }
> +	Size size() const { return size_; }

Maybe not that relevant as Size is just two integers, but
        Size &size() const
or maybe even
        const Size &size() const

Also
        const Role role() const
> +
> +protected:
> +	StreamUsage(Role role);

explicit

> +	StreamUsage(Role role, int width, int height);
> +
> +private:
> +	Role role_;
> +	Size size_;
> +};
> +
>  class Stream final
>  {
>  public:
> +	class StillCapture : public StreamUsage
> +	{
> +	public:
> +		StillCapture();
> +	};
> +
> +	class VideoRecording : public StreamUsage
> +	{
> +	public:
> +		VideoRecording();
> +	};
> +
> +	class Viewfinder : public StreamUsage
> +	{
> +	public:
> +		explicit Viewfinder(int width, int height);

No need for explicit, it takes two arguments.

> +	};
> +
>  	Stream();
>  	BufferPool &bufferPool() { return bufferPool_; }
>  	const StreamConfiguration &configuration() const { return configuration_; }
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index c4943c91b2e6ce13..816dff2d12acd923 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -60,6 +60,65 @@ namespace libcamera {
>   * \brief Requested number of buffers to allocate for the stream
>   */
>
> +/**
> + * \class StreamUsage
> + * \brief Stream usage information
> + *
> + * The StreamUsage class describes how an application intends to use a stream.
> + * Usages are specified by applications and passed to cameras, that then select
> + * the most appropriate streams and their default configurations.
> + */
> +
> +/**
> + * \enum StreamUsage::Role
> + * \brief Identify the role a stream is intended to play
> + * \var StreamUsage::StillCapture
> + * The stream is intended to capture high-resolution, high-quality still images
> + * with low frame rate. The captured frames may be exposed with flash.
> + * \var StreamUsage::VideoRecording
> + * The stream is intended to capture video for the purpose of recording or
> + * streaming. The video stream may produce a high frame rate and may be
> + * enhanced with video stabilization.
> + * \var StreamUsage::Viewfinder
> + * The stream is intended to capture video for the purpose of display on the
> + * local screen. The StreamUsage includes the desired resolution. Trade-offs
> + * between quality and usage of system resources are acceptable.
> + */
> +
> +/**
> + * \fn StreamUsage::role()
> + * \brief Retrieve the stream role
> + *
> + * \return The stream role
> + */
> +
> +/**
> + * \fn StreamUsage::size()
> + * \brief Retrieve desired size
> + *
> + * \return The desired size
> + */
> +
> +/**
> + * \brief Create a stream usage

should you somehow mention "a stream usage with sizes = 0" ?

> + * \param[in] role Stream role
> + */
> +StreamUsage::StreamUsage(Role role)
> +	: role_(role), size_(Size(0, 0))

Size default contructor initialize both fields to 0, so you can drop
the assignement.

> +{
> +}
> +
> +/**
> + * \brief Create a stream usage with size hint
> + * \param[in] role Stream role
> + * \param[in] width Desired width
> + * \param[in] height Desired height

"The desired" ?

> + */
> +StreamUsage::StreamUsage(Role role, int width, int height)
> +	: role_(role), size_(Size(width, height))
> +{
> +}
> +
>  /**
>   * \class Stream
>   * \brief Video stream for a camera
> @@ -78,6 +137,39 @@ namespace libcamera {
>   * optimal stream for the task.
>   */
>
> +/**
> + * \class Stream::StillCapture
> + * \brief Describe a still capture usage
> + */
> +Stream::StillCapture::StillCapture()
> +	: StreamUsage(Role::StillCapture)
> +{
> +}
> +
> +/**
> + * \class Stream::VideoRecording
> + * \brief Describe a video recording usage
> + */
> +Stream::VideoRecording::VideoRecording()
> +	: StreamUsage(Role::VideoRecording)
> +{
> +}
> +
> +/**
> + * \class Stream::Viewfinder
> + * \brief Describe a viewfinder usage
> + */
> +
> +/**
> + * \brief Create a viewfinder usage with dimension hints

Nit: you have used "size" everywhere in place of dimension if I'm not
wrong. I like dimension too though.

Thanks
   j

> + * \param[in] width Desired viewfinder width
> + * \param[in] height Desired viewfinder height
> + */
> +Stream::Viewfinder::Viewfinder(int width, int height)
> +	: StreamUsage(Role::Viewfinder, width, height)
> +{
> +}
> +
>  /**
>   * \brief Construct a stream with default parameters
>   */
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190405/091fbda2/attachment.sig>