[libcamera-devel,v4,2/7] libcamera: v4l2_device: Adjust colorspace based on pixel format
diff mbox series

Message ID 20220830074725.1059643-3-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Colospace adjustment and gstreamer mapping
Related show

Commit Message

Umang Jain Aug. 30, 2022, 7:47 a.m. UTC 
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@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/v4l2_device.h |  5 +++-
 src/libcamera/v4l2_device.cpp            | 29 +++++++++++++++++++----
 src/libcamera/v4l2_subdevice.cpp         | 30 ++++++++++++++++++++++--
 src/libcamera/v4l2_videodevice.cpp       | 12 ++++++----
 4 files changed, 65 insertions(+), 11 deletions(-)

Comments

Nicolas Dufresne via libcamera-devel Aug. 31, 2022, 10:09 p.m. UTC | #1 
On Tue, Aug 30, 2022 at 01:17:20PM +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@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  include/libcamera/internal/v4l2_device.h |  5 +++-
>  src/libcamera/v4l2_device.cpp            | 29 +++++++++++++++++++----
>  src/libcamera/v4l2_subdevice.cpp         | 30 ++++++++++++++++++++++--
>  src/libcamera/v4l2_videodevice.cpp       | 12 ++++++----
>  4 files changed, 65 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..95bfde34 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -503,7 +503,20 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  	format->size.width = subdevFmt.format.width;
>  	format->size.height = subdevFmt.format.height;
>  	format->mbus_code = subdevFmt.format.code;
> -	format->colorSpace = toColorSpace(subdevFmt.format);
> +
> +	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->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);
>  
>  	return 0;
>  }
> @@ -549,7 +562,20 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  	format->size.width = subdevFmt.format.width;
>  	format->size.height = subdevFmt.format.height;
>  	format->mbus_code = subdevFmt.format.code;
> -	format->colorSpace = toColorSpace(subdevFmt.format);
> +
> +	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->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;
>  }
> -- 
> 2.37.2
>

Patch
diff mbox series 

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..95bfde34 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -503,7 +503,20 @@  int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
 	format->size.width = subdevFmt.format.width;
 	format->size.height = subdevFmt.format.height;
 	format->mbus_code = subdevFmt.format.code;
-	format->colorSpace = toColorSpace(subdevFmt.format);
+
+	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->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);
 
 	return 0;
 }
@@ -549,7 +562,20 @@  int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
 	format->size.width = subdevFmt.format.width;
 	format->size.height = subdevFmt.format.height;
 	format->mbus_code = subdevFmt.format.code;
-	format->colorSpace = toColorSpace(subdevFmt.format);
+
+	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->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;
 }