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

[Kieran Bingham]

Hi David,

Quoting David Plowman (2021-11-04 13:58:00)
> This is so that applications can choose appropriate color spaces which
> will then be passed down to the V4L2 devices.
> 
> There are two fields: the "requestedColorSpace" which is the one an
> application wants, and the "actualColorSpace" which is what the
> underlying hardware can deliver, and which is filled in by the
> validate() method.

Is it necessary to store both? We don't do that with the other fields...
the StreamConfiguration structure is used in different states to handle
that I think.

When a CameraConfiguration is 'generated' Potential StreamConfigurations
are 'offered' to the application.

Applications can either fill in those StreamConfigurations, (or start
from an empty one if it will set all fields anyway...) and 'validate'
the overall CameraConfiguration.

The StreamConfiguration at that point contains the 'requested' values
... and when returned back from validate() - it has the values that can
be used by hardware...


> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/stream.h |  4 ++++
>  src/libcamera/stream.cpp   | 25 +++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 0c55e716..fe491ff5 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,9 @@ struct StreamConfiguration {
>  
>         unsigned int bufferCount;
>  
> +       ColorSpace requestedColorSpace;
> +       ColorSpace actualColorSpace;

Given the above, I think this should be just 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..1ddbbb8c 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -329,6 +329,31 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>   * \brief Requested number of buffers to allocate for the stream
>   */
>  
> +/**
> + * \var StreamConfiguration::requestedColorSpace
> + * \brief The ColorSpace this stream should have
> + *
> + * The generateConfiguration method will generate reasonable default
> + * values (ColorSpace::Jpeg for stills, ColorSpace::Rec709 for video and
> + * ColorSpace::Raw for raw streams) but applications are free to overwrite
> + * this value.
> + */
> +
> +/**
> + * \var StreamConfiguration::actualColorSpace
> + * \brief The ColorSpace the will be used for this stream
> + *
> + * This field is filled in by CameraConfiguration::validate().
> + * Normally this should match the requestedColorSpace, but it may differ
> + * if the hardware does not support it.
> + *
> + * In general cameras may have different constraints here, for example,
> + * they may require all output streams to share the same color space.
> + * Sometimes the fields within this color space may report "Undefined"
> + * values if the hardware drivers are going to use a color space that
> + * is not recognised by the ColorSpace class.
> + */
> +
>  /**
>   * \fn StreamConfiguration::stream()
>   * \brief Retrieve the stream associated with the configuration
> -- 
> 2.20.1
>