[libcamera-devel] [PATCH v3 2/7] libcamera: v4l2_device: Adjust colorspace based on pixel format
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 30 00:02:26 CEST 2022
Hi Umang,
Thank you for the patch.
On Mon, Aug 29, 2022 at 10:07:37PM +0530, Umang Jain via libcamera-devel wrote:
> V4L2 has no "none" YCbCr encoding, and thus reports an encoding for all
> formats, including RGB and raw formats. This causes the libcamera
> ColorSpace to report incorrect encodings for non-YUV formats. Fix it by
> overriding the encoding reported by the kernel to YCbCrEncoding::None
> for non-YUV pixel formats and media bus formats.
>
> Similarly, override the quantization range of non-YUV formats to full
> range, as limited range isn't used for RGB and raw formats.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> include/libcamera/internal/v4l2_device.h | 5 +++-
> src/libcamera/v4l2_device.cpp | 29 ++++++++++++++++++---
> src/libcamera/v4l2_subdevice.cpp | 32 ++++++++++++++++++++++--
> src/libcamera/v4l2_videodevice.cpp | 12 ++++++---
> 4 files changed, 67 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index a52a5f2c..75304be1 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -22,6 +22,8 @@
> #include <libcamera/color_space.h>
> #include <libcamera/controls.h>
>
> +#include "libcamera/internal/formats.h"
> +
> namespace libcamera {
>
> class EventNotifier;
> @@ -59,7 +61,8 @@ protected:
> int fd() const { return fd_.get(); }
>
> template<typename T>
> - static std::optional<ColorSpace> toColorSpace(const T &v4l2Format);
> + static std::optional<ColorSpace> toColorSpace(const T &v4l2Format,
> + PixelFormatInfo::ColourEncoding colourEncoding);
>
> template<typename T>
> static int fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v4l2Format);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index b22a981f..301620f8 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -24,6 +24,7 @@
> #include <libcamera/base/log.h>
> #include <libcamera/base/utils.h>
>
> +#include "libcamera/internal/formats.h"
> #include "libcamera/internal/sysfs.h"
>
> /**
> @@ -805,6 +806,7 @@ static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> /**
> * \brief Convert the color space fields in a V4L2 format to a ColorSpace
> * \param[in] v4l2Format A V4L2 format containing color space information
> + * \param[in] colourEncoding Type of colour encoding
> *
> * The colorspace, ycbcr_enc, xfer_func and quantization fields within a
> * V4L2 format structure are converted to a corresponding ColorSpace.
> @@ -816,7 +818,8 @@ static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> * \retval std::nullopt One or more V4L2 color space fields were not recognised
> */
> template<typename T>
> -std::optional<ColorSpace> V4L2Device::toColorSpace(const T &v4l2Format)
> +std::optional<ColorSpace> V4L2Device::toColorSpace(const T &v4l2Format,
> + PixelFormatInfo::ColourEncoding colourEncoding)
> {
> auto itColor = v4l2ToColorSpace.find(v4l2Format.colorspace);
> if (itColor == v4l2ToColorSpace.end())
> @@ -839,6 +842,14 @@ std::optional<ColorSpace> V4L2Device::toColorSpace(const T &v4l2Format)
> return std::nullopt;
>
> colorSpace.ycbcrEncoding = itYcbcrEncoding->second;
> +
> + /*
> + * V4L2 has no "none" encoding, override the value returned by
> + * the kernel for non-YUV formats as YCbCr encoding isn't
> + * applicable in that case.
> + */
> + if (colourEncoding != PixelFormatInfo::ColourEncodingYUV)
> + colorSpace.ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
> }
>
> if (v4l2Format.quantization != V4L2_QUANTIZATION_DEFAULT) {
> @@ -847,14 +858,24 @@ std::optional<ColorSpace> V4L2Device::toColorSpace(const T &v4l2Format)
> return std::nullopt;
>
> colorSpace.range = itRange->second;
> +
> + /*
> + * "Limited" quantization range is only meant for YUV formats.
> + * Override the range to "Full" for all other formats.
> + */
> + if (colourEncoding != PixelFormatInfo::ColourEncodingYUV)
> + colorSpace.range = ColorSpace::Range::Full;
> }
>
> return colorSpace;
> }
>
> -template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_pix_format &);
> -template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_pix_format_mplane &);
> -template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_mbus_framefmt &);
> +template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_pix_format &,
> + PixelFormatInfo::ColourEncoding);
> +template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_pix_format_mplane &,
> + PixelFormatInfo::ColourEncoding);
> +template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_mbus_framefmt &,
> + PixelFormatInfo::ColourEncoding);
>
> /**
> * \brief Fill in the color space fields of a V4L2 format from a ColorSpace
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 07a0bb07..0389f1b3 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -502,8 +502,22 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>
> format->size.width = subdevFmt.format.width;
> format->size.height = subdevFmt.format.height;
> +
> + PixelFormatInfo::ColourEncoding colourEncoding;
> + auto iter = formatInfoMap.find(subdevFmt.format.code);
> + if (iter != formatInfoMap.end()) {
> + colourEncoding = iter->second.colourEncoding;
> + } else {
> + LOG(V4L2, Warning)
> + << "Unknown subdev format "
> + << utils::hex(subdevFmt.format.code, 4)
> + << ", defaulting to RGB encoding";
> +
> + colourEncoding = PixelFormatInfo::ColourEncodingRGB;
> + }
> +
> format->mbus_code = subdevFmt.format.code;
I'd move this line above just after size.height, to keep all the color
space code grouped together.
> - format->colorSpace = toColorSpace(subdevFmt.format);
> + format->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);
>
> return 0;
> }
> @@ -548,8 +562,22 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>
> format->size.width = subdevFmt.format.width;
> format->size.height = subdevFmt.format.height;
> +
> + PixelFormatInfo::ColourEncoding colourEncoding;
> + auto iter = formatInfoMap.find(subdevFmt.format.code);
> + if (iter != formatInfoMap.end()) {
> + colourEncoding = iter->second.colourEncoding;
> + } else {
> + LOG(V4L2, Warning)
> + << "Unknown subdev format "
> + << utils::hex(subdevFmt.format.code, 4)
> + << ", defaulting to RGB encoding";
> +
> + colourEncoding = PixelFormatInfo::ColourEncodingRGB;
> + }
> +
> format->mbus_code = subdevFmt.format.code;
Same here.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> - format->colorSpace = toColorSpace(subdevFmt.format);
> + format->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);
>
> return 0;
> }
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 5a2d0e5b..0e3f5436 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -931,7 +931,8 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
> format->size.height = pix->height;
> format->fourcc = V4L2PixelFormat(pix->pixelformat);
> format->planesCount = pix->num_planes;
> - format->colorSpace = toColorSpace(*pix);
> + format->colorSpace =
> + toColorSpace(*pix, PixelFormatInfo::info(format->fourcc).colourEncoding);
>
> for (unsigned int i = 0; i < format->planesCount; ++i) {
> format->planes[i].bpl = pix->plane_fmt[i].bytesperline;
> @@ -987,7 +988,8 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)
> format->planes[i].bpl = pix->plane_fmt[i].bytesperline;
> format->planes[i].size = pix->plane_fmt[i].sizeimage;
> }
> - format->colorSpace = toColorSpace(*pix);
> + format->colorSpace =
> + toColorSpace(*pix, PixelFormatInfo::info(format->fourcc).colourEncoding);
>
> return 0;
> }
> @@ -1011,7 +1013,8 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
> format->planesCount = 1;
> format->planes[0].bpl = pix->bytesperline;
> format->planes[0].size = pix->sizeimage;
> - format->colorSpace = toColorSpace(*pix);
> + format->colorSpace =
> + toColorSpace(*pix, PixelFormatInfo::info(format->fourcc).colourEncoding);
>
> return 0;
> }
> @@ -1053,7 +1056,8 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
> format->planesCount = 1;
> format->planes[0].bpl = pix->bytesperline;
> format->planes[0].size = pix->sizeimage;
> - format->colorSpace = toColorSpace(*pix);
> + format->colorSpace =
> + toColorSpace(*pix, PixelFormatInfo::info(format->fourcc).colourEncoding);
>
> return 0;
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list