[libcamera-devel] [RFC PATCH 1/4] libcamera: colorspace: Rectify ColorSpace::Srgb preset
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 16 04:17:57 CEST 2022
Hi Umang,
Thank you for the patch.
On Wed, Aug 03, 2022 at 12:27:16AM +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 | 9 +++------
> src/libcamera/v4l2_device.cpp | 2 --
> 2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp
> index caf39760..73148228 100644
> --- a/src/libcamera/color_space.cpp
> +++ b/src/libcamera/color_space.cpp
> @@ -254,16 +254,13 @@ const ColorSpace ColorSpace::Jpeg = {
> };
>
> /**
> - * \brief A constant representing the sRGB color space
> - *
> - * This is identical to the JPEG color space except that the Y'CbCr
> - * range is limited rather than full.
> + * \brief A constant representing the sRGB color space (non-YUV format)
I would write "(RGB formats only)" to be even clearer.
> */
> 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 3fc8438f..a4446fbf 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -746,7 +746,6 @@ void V4L2Device::eventAvailable()
> static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> { V4L2_COLORSPACE_RAW, ColorSpace::Raw },
> { V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },
> - { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },
This will cause an issue, as V4L2Device::toColorSpace() will return
nullopt if the kernel reports V4L2_COLORSPACE_SRGB. You should instead
use the kernel definition of the SRGB color space here:
{ V4L2_COLORSPACE_SRGB, {
ColorSpace::Primaries::Rec709,
ColorSpace::TransferFunction::Srgb,
ColorSpace::YcbcrEncoding::Rec601,
ColorSpace::Range::Limited
} },
However, that won't be enough. V4L2Device::toColorSpace() also needs to
set colorSpace.ycbcrEncoding to YcbcrEncoding::None and colorSpace.range
to Range::Full if the format is an RGB format. I think it would be best
to do so in a patch before this one.
> { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },
> { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },
> { V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },
> @@ -772,7 +771,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::Jpeg, V4L2_COLORSPACE_JPEG },
> - { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },
This, on the other hand, should be fine.
> { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
> { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },
> { ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list