[libcamera-devel] [PATCH v2 3/6] libcamera: colorspace: Rectify the ColorSpace::Srgb preset

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Wed, Aug 24, 2022 at 09:54:22PM +0530, Umang Jain via libcamera-devel wrote:
> Rectify the ColorSpace::Srgb to denote that it does not use
> any Y'Cbcr encoding and uses full range.
> 
> The kernel on the other hand, recommends to use Rec601 as the encoding
> for V4L2_COLORSPACE_SRGB. It is not very explicit but it can be
> inferred that the kernel assumes V4L2_COLORSPACE_SRGB is a YUV-encoded
> one. However, when the data is in RGB, no encoding is required (and
> this is denoted by YcbcrEncoding::None in libcamera).
> 
> Hence, to be clear on the libcamera colorspace API, rectify the
> ColorSpace::Srgb preset to use YcbcrEncoding::None.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/libcamera/color_space.cpp | 14 +++++++-------
>  src/libcamera/v4l2_device.cpp |  7 +++++--
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp
> index f0d6109a..cb47acf9 100644
> --- a/src/libcamera/color_space.cpp
> +++ b/src/libcamera/color_space.cpp
> @@ -47,7 +47,10 @@ namespace libcamera {
>   *
>   * Note that there is no guarantee of a 1:1 mapping between color space names
>   * and definitions in libcamera and V4L2. A notable difference is that the sYCC
> - * libcamera color space is called JPEG in V4L2 due to historical reasons.
> + * libcamera color space is called JPEG in V4L2 due to historical reasons. On
> + * a similar note, the sRGB colorspace defined in the kernel assumes a Y'CbCr
> + * encoding to it which is not true. Hence the ColorSpace::sRGB is defined
> + * differently in libcamera (with no Y'CbCr encoding and full range).

 * and definitions in libcamera and V4L2. Two notable differences are
 *
 * - The sRGB libcamera color space is defined for RGB formats only with no
 *   Y'CbCr encoding and a full quantization range, while the V4L2 SRGB color
 *   space has a Y'CbCr encoding and a limited quantization range.
 * - The sYCC libcamera color space is called JPEG in V4L2 due to historical
 *   reasons.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>   *
>   * \todo Define the color space fully in the libcamera API to avoid referencing
>   * V4L2
> @@ -250,16 +253,13 @@ const ColorSpace ColorSpace::Raw = {
>  };
>  
>  /**
> - * \brief A constant representing the sRGB color space
> - *
> - * This is identical to the sYCC color space except that the Y'CbCr
> - * range is limited rather than full.
> + * \brief A constant representing the sRGB color space (RGB formats only)
>   */
>  const ColorSpace ColorSpace::Srgb = {
>  	Primaries::Rec709,
>  	TransferFunction::Srgb,
> -	YcbcrEncoding::Rec601,
> -	Range::Limited
> +	YcbcrEncoding::None,
> +	Range::Full
>  };
>  
>  /**
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 1fb08b9d..a9e34ee1 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -746,7 +746,11 @@ void V4L2Device::eventAvailable()
>  
>  static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
>  	{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },
> -	{ V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },
> +	{ V4L2_COLORSPACE_SRGB, {
> +		ColorSpace::Primaries::Rec709,
> +		ColorSpace::TransferFunction::Srgb,
> +		ColorSpace::YcbcrEncoding::Rec601,
> +		ColorSpace::Range::Limited } },
>  	{ V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },
>  	{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },
>  	{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },
> @@ -772,7 +776,6 @@ static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
>  
>  static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {
>  	{ ColorSpace::Raw, V4L2_COLORSPACE_RAW },
> -	{ ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },
>  	{ ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },
>  	{ ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
>  	{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },

-- 
Regards,

Laurent Pinchart