[libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace fields to StreamConfiguration

[Jacopo Mondi]

Hi David,

On Thu, Nov 18, 2021 at 03:19:28PM +0000, David Plowman wrote:
> This is so that applications can choose appropriate color spaces which
> will then be passed down to the V4L2 devices.
>
> The ColorSpace field is actually optional. If it is not set you will
> get the driver's default color space.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/stream.h |  3 +++
>  src/libcamera/stream.cpp   | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 0c55e716..bea88eb4 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -12,6 +12,7 @@
>  #include <string>
>  #include <vector>
>
> +#include <libcamera/color_space.h>
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
> @@ -47,6 +48,8 @@ struct StreamConfiguration {
>
>  	unsigned int bufferCount;
>
> +	std::optional<ColorSpace> colorSpace;
> +
>  	Stream *stream() const { return stream_; }
>  	void setStream(Stream *stream) { stream_ = stream; }
>  	const StreamFormats &formats() const { return formats_; }
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index b421e17e..300b3af7 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -329,6 +329,20 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>   * \brief Requested number of buffers to allocate for the stream
>   */
>
> +/**
> + * \var StreamConfiguration::colorSpace
> + * \brief The ColorSpace for this stream
> + *
> + * A suitable color space may be set here or chosen by an application.

What is the difference between "set here" OR "chosen by an application" ?
Aren't application in charge to select a ColorSpace in the
configuration ?


> + * Alternatively the color space may be left unset, in which case it will
> + * be up to the driver to choose a color space.

I wil model this from the application point of view.

* This field allow to select a ColorSpace to use for the Stream.
*
* The field is optional and application can chose to leave it unset.
* The library will adjust it to what the Camera can provide and the
* CameraConfiguration this StreamConfiguration is part of will be
* adjusted.

> + *
> + * If the system cannot deliver the requested color space, the validate()
> + * method will overwrite the value here with what it can deliver. Note that
> + * platforms may enforce extra contraints here, such as requiring all output
> + * streams to share the same color space.

What about something along the lines of:

* If a specific ColorSpace is requested here but the Camera cannot
* produce it, the StreamConfiguration will be Adjusted to what can be
* delivered, as there might be platform specific contraints to respect
* such as all output streams sharing the same color space.

Feel free to adjust, as a native speaker you'll surely do better.

Thanks
   j

> + */
> +
>  /**
>   * \fn StreamConfiguration::stream()
>   * \brief Retrieve the stream associated with the configuration
> --
> 2.20.1
>