[libcamera-devel,2/3] libcamera: Support passing ColorSpaces to V4L2 drivers
diff mbox series

Message ID 20210805142154.20324-3-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Colour spaces
Related show

Commit Message

David Plowman Aug. 5, 2021, 2:21 p.m. UTC
The ColorSpace class is added to the StreamConfiguration, and is now
passed to V4L2 devices where it is handled appropriately. Note how
this means that the colour space is configured per-stream (though
platforms may restrict this).

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/internal/v4l2_videodevice.h |   2 +
 include/libcamera/stream.h                    |   3 +
 src/libcamera/v4l2_videodevice.cpp            | 117 ++++++++++++++++++
 3 files changed, 122 insertions(+)

Comments

Paul Elder Sept. 23, 2021, 6:26 a.m. UTC | #1
Hi David,

On Thu, Aug 05, 2021 at 03:21:53PM +0100, David Plowman wrote:
> The ColorSpace class is added to the StreamConfiguration, and is now
> passed to V4L2 devices where it is handled appropriately. Note how
> this means that the colour space is configured per-stream (though
> platforms may restrict this).
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_videodevice.h |   2 +
>  include/libcamera/stream.h                    |   3 +
>  src/libcamera/v4l2_videodevice.cpp            | 117 ++++++++++++++++++
>  3 files changed, 122 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index e767ec84..c05899cf 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -20,6 +20,7 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/signal.h>
>  
> +#include <libcamera/color_space.h>
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
> @@ -163,6 +164,7 @@ public:
>  
>  	V4L2PixelFormat fourcc;
>  	Size size;
> +	ColorSpace colorSpace;
>  
>  	std::array<Plane, 3> planes;
>  	unsigned int planesCount = 0;
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 0c55e716..131f7733 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,8 @@ struct StreamConfiguration {
>  
>  	unsigned int bufferCount;
>  
> +	ColorSpace colorSpace;
> +
>  	Stream *stream() const { return stream_; }
>  	void setStream(Stream *stream) { stream_ = stream; }
>  	const StreamFormats &formats() const { return formats_; }
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index da2af6a1..7f30412c 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -366,6 +366,11 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
>   * \brief The image size in pixels
>   */
>  
> +/**
> + * \var V4L2DeviceFormat::colorSpace
> + * \brief The color space of the pixels
> + */
> +
>  /**
>   * \var V4L2DeviceFormat::fourcc
>   * \brief The fourcc code describing the pixel encoding scheme
> @@ -731,6 +736,114 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
>  		return getFormatSingleplane(format);
>  }
>  
> +static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {

What's wrong with a map? iirc ColorSpace has operator==() so
shouldn't find() below *just work*?

> +	{ ColorSpace::RAW, V4L2_COLORSPACE_RAW },
> +	{ ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },
> +	{ ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },
> +	{ ColorSpace::REC709, V4L2_COLORSPACE_REC709 },
> +	{ ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::Encoding, v4l2_ycbcr_encoding> encodingToV4l2 = {
> +	{ ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },
> +	{ ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },
> +	{ ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {
> +	{ ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },
> +	{ ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },
> +	{ ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },
> +};
> +
> +static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> +	{ ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },
> +	{ ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },
> +};

Also I would move the reverse-maps that you have below here, so that
it's easier to see them together if changes need to be made on them.
(Does C++ have a double-sided map...?)

> +
> +template<typename T>
> +static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)
> +{
> +	if (!colorSpace.isFullyDefined())
> +		LOG(V4L2, Warning) << "Setting non-fully defined colour space"
> +				   << colorSpace.toString();
> +
> +	v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;
> +	v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +	v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;
> +
> +	auto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),
> +				     [&colorSpace](const std::pair<ColorSpace, v4l2_colorspace> &item) { return colorSpace == item.first; });
> +	if (itColor != colorSpaceToV4l2.end())
> +		v4l2PixFormat.colorspace = itColor->second;
> +
> +	auto itEncoding = encodingToV4l2.find(colorSpace.encoding);
> +	if (itEncoding != encodingToV4l2.end())
> +		v4l2PixFormat.ycbcr_enc = itEncoding->second;
> +
> +	auto itTransfer = transferFunctionToV4l2.find(colorSpace.transferFunction);
> +	if (itTransfer != transferFunctionToV4l2.end())
> +		v4l2PixFormat.xfer_func = itTransfer->second;
> +
> +	auto itRange = rangeToV4l2.find(colorSpace.range);
> +	if (itRange != rangeToV4l2.end())
> +		v4l2PixFormat.quantization = itRange->second;
> +}
> +
> +static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> +	{ V4L2_COLORSPACE_RAW, ColorSpace::RAW },
> +	{ V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },
> +	{ V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },
> +	{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },
> +	{ V4L2_COLORSPACE_REC709, ColorSpace::REC709 },
> +	{ V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::Encoding> v4l2ToEncoding = {
> +	{ V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },
> +	{ V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },
> +	{ V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunction = {
> +	{ V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },
> +	{ V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },
> +	{ V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
> +	{ V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },
> +	{ V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },
> +};
> +
> +template<typename T>
> +static ColorSpace getColorSpace(const T &v4l2PixFormat)
> +{
> +	ColorSpace colorSpace;
> +
> +	auto itColor = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);
> +	if (itColor != v4l2ToColorSpace.end())
> +		colorSpace = itColor->second;
> +
> +	auto itEncoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);
> +	if (itEncoding != v4l2ToEncoding.end())
> +		colorSpace.encoding = itEncoding->second;
> +
> +	auto itTransfer = v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);
> +	if (itTransfer != v4l2ToTransferFunction.end())
> +		colorSpace.transferFunction = itTransfer->second;
> +
> +	auto itRange = v4l2ToRange.find(v4l2PixFormat.quantization);
> +	if (itRange != v4l2ToRange.end())
> +		colorSpace.range = itRange->second;
> +
> +	if (!colorSpace.isFullyDefined())
> +		LOG(V4L2, Warning) << "Returning non-fully defined colour space"
> +				   << colorSpace.toString();
> +	return colorSpace;
> +}
> +
>  /**
>   * \brief Try an image format on the V4L2 video device
>   * \param[inout] format The image format to test applicability to the video device
> @@ -840,6 +953,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
>  	format->size.width = pix->width;
>  	format->size.height = pix->height;
>  	format->fourcc = V4L2PixelFormat(pix->pixelformat);
> +	format->colorSpace = getColorSpace(*pix);
>  	format->planesCount = pix->num_planes;
>  
>  	for (unsigned int i = 0; i < format->planesCount; ++i) {
> @@ -862,6 +976,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)
>  	pix->pixelformat = format->fourcc;
>  	pix->num_planes = format->planesCount;
>  	pix->field = V4L2_FIELD_NONE;
> +	setColorSpace(format->colorSpace, *pix);

I suppose if no color space is set in the StreamConfiguration then it'll
just configure default.

Otherwise, looks good to me.


Paul

>  
>  	ASSERT(pix->num_planes <= std::size(pix->plane_fmt));
>  
> @@ -910,6 +1025,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
>  	format->size.width = pix->width;
>  	format->size.height = pix->height;
>  	format->fourcc = V4L2PixelFormat(pix->pixelformat);
> +	format->colorSpace = getColorSpace(*pix);
>  	format->planesCount = 1;
>  	format->planes[0].bpl = pix->bytesperline;
>  	format->planes[0].size = pix->sizeimage;
> @@ -929,6 +1045,7 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
>  	pix->pixelformat = format->fourcc;
>  	pix->bytesperline = format->planes[0].bpl;
>  	pix->field = V4L2_FIELD_NONE;
> +	setColorSpace(format->colorSpace, *pix);
>  	ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
>  	if (ret) {
>  		LOG(V4L2, Error)
> -- 
> 2.20.1
>
Nicolas Dufresne Sept. 23, 2021, 1:03 p.m. UTC | #2
Le jeudi 05 août 2021 à 15:21 +0100, David Plowman a écrit :
> The ColorSpace class is added to the StreamConfiguration, and is now
> passed to V4L2 devices where it is handled appropriately. Note how
> this means that the colour space is configured per-stream (though
> platforms may restrict this).

V4L2 being fail safe, it will update the chosen colorspace to a supported one.
How do you report back this change, I don't see any code related to that ?

p.s. to complete the GStreamer integration, we need to advertise the final
colorspace rather then the initially preferred one.

> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_videodevice.h |   2 +
>  include/libcamera/stream.h                    |   3 +
>  src/libcamera/v4l2_videodevice.cpp            | 117 ++++++++++++++++++
>  3 files changed, 122 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index e767ec84..c05899cf 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -20,6 +20,7 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/signal.h>
>  
> +#include <libcamera/color_space.h>
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
> @@ -163,6 +164,7 @@ public:
>  
>  	V4L2PixelFormat fourcc;
>  	Size size;
> +	ColorSpace colorSpace;
>  
>  	std::array<Plane, 3> planes;
>  	unsigned int planesCount = 0;
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 0c55e716..131f7733 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,8 @@ struct StreamConfiguration {
>  
>  	unsigned int bufferCount;
>  
> +	ColorSpace colorSpace;
> +
>  	Stream *stream() const { return stream_; }
>  	void setStream(Stream *stream) { stream_ = stream; }
>  	const StreamFormats &formats() const { return formats_; }
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index da2af6a1..7f30412c 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -366,6 +366,11 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
>   * \brief The image size in pixels
>   */
>  
> +/**
> + * \var V4L2DeviceFormat::colorSpace
> + * \brief The color space of the pixels
> + */
> +
>  /**
>   * \var V4L2DeviceFormat::fourcc
>   * \brief The fourcc code describing the pixel encoding scheme
> @@ -731,6 +736,114 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
>  		return getFormatSingleplane(format);
>  }
>  
> +static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {
> +	{ ColorSpace::RAW, V4L2_COLORSPACE_RAW },
> +	{ ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },
> +	{ ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },
> +	{ ColorSpace::REC709, V4L2_COLORSPACE_REC709 },
> +	{ ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::Encoding, v4l2_ycbcr_encoding> encodingToV4l2 = {
> +	{ ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },
> +	{ ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },
> +	{ ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {
> +	{ ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },
> +	{ ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },
> +	{ ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },
> +};
> +
> +static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> +	{ ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },
> +	{ ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },
> +};
> +
> +template<typename T>
> +static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)
> +{
> +	if (!colorSpace.isFullyDefined())
> +		LOG(V4L2, Warning) << "Setting non-fully defined colour space"
> +				   << colorSpace.toString();
> +
> +	v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;
> +	v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +	v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;
> +
> +	auto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),
> +				     [&colorSpace](const std::pair<ColorSpace, v4l2_colorspace> &item) { return colorSpace == item.first; });
> +	if (itColor != colorSpaceToV4l2.end())
> +		v4l2PixFormat.colorspace = itColor->second;
> +
> +	auto itEncoding = encodingToV4l2.find(colorSpace.encoding);
> +	if (itEncoding != encodingToV4l2.end())
> +		v4l2PixFormat.ycbcr_enc = itEncoding->second;
> +
> +	auto itTransfer = transferFunctionToV4l2.find(colorSpace.transferFunction);
> +	if (itTransfer != transferFunctionToV4l2.end())
> +		v4l2PixFormat.xfer_func = itTransfer->second;
> +
> +	auto itRange = rangeToV4l2.find(colorSpace.range);
> +	if (itRange != rangeToV4l2.end())
> +		v4l2PixFormat.quantization = itRange->second;
> +}
> +
> +static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> +	{ V4L2_COLORSPACE_RAW, ColorSpace::RAW },
> +	{ V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },
> +	{ V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },
> +	{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },
> +	{ V4L2_COLORSPACE_REC709, ColorSpace::REC709 },
> +	{ V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::Encoding> v4l2ToEncoding = {
> +	{ V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },
> +	{ V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },
> +	{ V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunction = {
> +	{ V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },
> +	{ V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },
> +	{ V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },
> +};
> +
> +static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
> +	{ V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },
> +	{ V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },
> +};
> +
> +template<typename T>
> +static ColorSpace getColorSpace(const T &v4l2PixFormat)
> +{
> +	ColorSpace colorSpace;
> +
> +	auto itColor = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);
> +	if (itColor != v4l2ToColorSpace.end())
> +		colorSpace = itColor->second;
> +
> +	auto itEncoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);
> +	if (itEncoding != v4l2ToEncoding.end())
> +		colorSpace.encoding = itEncoding->second;
> +
> +	auto itTransfer = v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);
> +	if (itTransfer != v4l2ToTransferFunction.end())
> +		colorSpace.transferFunction = itTransfer->second;
> +
> +	auto itRange = v4l2ToRange.find(v4l2PixFormat.quantization);
> +	if (itRange != v4l2ToRange.end())
> +		colorSpace.range = itRange->second;
> +
> +	if (!colorSpace.isFullyDefined())
> +		LOG(V4L2, Warning) << "Returning non-fully defined colour space"
> +				   << colorSpace.toString();
> +	return colorSpace;
> +}
> +
>  /**
>   * \brief Try an image format on the V4L2 video device
>   * \param[inout] format The image format to test applicability to the video device
> @@ -840,6 +953,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
>  	format->size.width = pix->width;
>  	format->size.height = pix->height;
>  	format->fourcc = V4L2PixelFormat(pix->pixelformat);
> +	format->colorSpace = getColorSpace(*pix);
>  	format->planesCount = pix->num_planes;
>  
>  	for (unsigned int i = 0; i < format->planesCount; ++i) {
> @@ -862,6 +976,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)
>  	pix->pixelformat = format->fourcc;
>  	pix->num_planes = format->planesCount;
>  	pix->field = V4L2_FIELD_NONE;
> +	setColorSpace(format->colorSpace, *pix);
>  
>  	ASSERT(pix->num_planes <= std::size(pix->plane_fmt));
>  
> @@ -910,6 +1025,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
>  	format->size.width = pix->width;
>  	format->size.height = pix->height;
>  	format->fourcc = V4L2PixelFormat(pix->pixelformat);
> +	format->colorSpace = getColorSpace(*pix);
>  	format->planesCount = 1;
>  	format->planes[0].bpl = pix->bytesperline;
>  	format->planes[0].size = pix->sizeimage;
> @@ -929,6 +1045,7 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
>  	pix->pixelformat = format->fourcc;
>  	pix->bytesperline = format->planes[0].bpl;
>  	pix->field = V4L2_FIELD_NONE;
> +	setColorSpace(format->colorSpace, *pix);
>  	ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
>  	if (ret) {
>  		LOG(V4L2, Error)
David Plowman Sept. 24, 2021, 11:55 a.m. UTC | #3
Hi

Thanks for the feedback, that wasn't something I realised! Thanks also
to everyone else for their comments higher up this thread - I'll roll
all that into a new version over the next few days and re-post.

Best regards
David

On Thu, 23 Sept 2021 at 14:03, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le jeudi 05 août 2021 à 15:21 +0100, David Plowman a écrit :
> > The ColorSpace class is added to the StreamConfiguration, and is now
> > passed to V4L2 devices where it is handled appropriately. Note how
> > this means that the colour space is configured per-stream (though
> > platforms may restrict this).
>
> V4L2 being fail safe, it will update the chosen colorspace to a supported one.
> How do you report back this change, I don't see any code related to that ?
>
> p.s. to complete the GStreamer integration, we need to advertise the final
> colorspace rather then the initially preferred one.
>
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  include/libcamera/internal/v4l2_videodevice.h |   2 +
> >  include/libcamera/stream.h                    |   3 +
> >  src/libcamera/v4l2_videodevice.cpp            | 117 ++++++++++++++++++
> >  3 files changed, 122 insertions(+)
> >
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index e767ec84..c05899cf 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -20,6 +20,7 @@
> >  #include <libcamera/base/log.h>
> >  #include <libcamera/base/signal.h>
> >
> > +#include <libcamera/color_space.h>
> >  #include <libcamera/framebuffer.h>
> >  #include <libcamera/geometry.h>
> >  #include <libcamera/pixel_format.h>
> > @@ -163,6 +164,7 @@ public:
> >
> >       V4L2PixelFormat fourcc;
> >       Size size;
> > +     ColorSpace colorSpace;
> >
> >       std::array<Plane, 3> planes;
> >       unsigned int planesCount = 0;
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index 0c55e716..131f7733 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,8 @@ struct StreamConfiguration {
> >
> >       unsigned int bufferCount;
> >
> > +     ColorSpace colorSpace;
> > +
> >       Stream *stream() const { return stream_; }
> >       void setStream(Stream *stream) { stream_ = stream; }
> >       const StreamFormats &formats() const { return formats_; }
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index da2af6a1..7f30412c 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -366,6 +366,11 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
> >   * \brief The image size in pixels
> >   */
> >
> > +/**
> > + * \var V4L2DeviceFormat::colorSpace
> > + * \brief The color space of the pixels
> > + */
> > +
> >  /**
> >   * \var V4L2DeviceFormat::fourcc
> >   * \brief The fourcc code describing the pixel encoding scheme
> > @@ -731,6 +736,114 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
> >               return getFormatSingleplane(format);
> >  }
> >
> > +static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {
> > +     { ColorSpace::RAW, V4L2_COLORSPACE_RAW },
> > +     { ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },
> > +     { ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },
> > +     { ColorSpace::REC709, V4L2_COLORSPACE_REC709 },
> > +     { ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },
> > +};
> > +
> > +static const std::map<ColorSpace::Encoding, v4l2_ycbcr_encoding> encodingToV4l2 = {
> > +     { ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },
> > +     { ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },
> > +     { ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },
> > +};
> > +
> > +static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {
> > +     { ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },
> > +     { ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },
> > +     { ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },
> > +};
> > +
> > +static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> > +     { ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },
> > +     { ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },
> > +};
> > +
> > +template<typename T>
> > +static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)
> > +{
> > +     if (!colorSpace.isFullyDefined())
> > +             LOG(V4L2, Warning) << "Setting non-fully defined colour space"
> > +                                << colorSpace.toString();
> > +
> > +     v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;
> > +     v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > +     v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > +     v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;
> > +
> > +     auto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),
> > +                                  [&colorSpace](const std::pair<ColorSpace, v4l2_colorspace> &item) { return colorSpace == item.first; });
> > +     if (itColor != colorSpaceToV4l2.end())
> > +             v4l2PixFormat.colorspace = itColor->second;
> > +
> > +     auto itEncoding = encodingToV4l2.find(colorSpace.encoding);
> > +     if (itEncoding != encodingToV4l2.end())
> > +             v4l2PixFormat.ycbcr_enc = itEncoding->second;
> > +
> > +     auto itTransfer = transferFunctionToV4l2.find(colorSpace.transferFunction);
> > +     if (itTransfer != transferFunctionToV4l2.end())
> > +             v4l2PixFormat.xfer_func = itTransfer->second;
> > +
> > +     auto itRange = rangeToV4l2.find(colorSpace.range);
> > +     if (itRange != rangeToV4l2.end())
> > +             v4l2PixFormat.quantization = itRange->second;
> > +}
> > +
> > +static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> > +     { V4L2_COLORSPACE_RAW, ColorSpace::RAW },
> > +     { V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },
> > +     { V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },
> > +     { V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },
> > +     { V4L2_COLORSPACE_REC709, ColorSpace::REC709 },
> > +     { V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },
> > +};
> > +
> > +static const std::map<uint32_t, ColorSpace::Encoding> v4l2ToEncoding = {
> > +     { V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },
> > +     { V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },
> > +     { V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },
> > +};
> > +
> > +static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunction = {
> > +     { V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },
> > +     { V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },
> > +     { V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },
> > +};
> > +
> > +static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
> > +     { V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },
> > +     { V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },
> > +};
> > +
> > +template<typename T>
> > +static ColorSpace getColorSpace(const T &v4l2PixFormat)
> > +{
> > +     ColorSpace colorSpace;
> > +
> > +     auto itColor = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);
> > +     if (itColor != v4l2ToColorSpace.end())
> > +             colorSpace = itColor->second;
> > +
> > +     auto itEncoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);
> > +     if (itEncoding != v4l2ToEncoding.end())
> > +             colorSpace.encoding = itEncoding->second;
> > +
> > +     auto itTransfer = v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);
> > +     if (itTransfer != v4l2ToTransferFunction.end())
> > +             colorSpace.transferFunction = itTransfer->second;
> > +
> > +     auto itRange = v4l2ToRange.find(v4l2PixFormat.quantization);
> > +     if (itRange != v4l2ToRange.end())
> > +             colorSpace.range = itRange->second;
> > +
> > +     if (!colorSpace.isFullyDefined())
> > +             LOG(V4L2, Warning) << "Returning non-fully defined colour space"
> > +                                << colorSpace.toString();
> > +     return colorSpace;
> > +}
> > +
> >  /**
> >   * \brief Try an image format on the V4L2 video device
> >   * \param[inout] format The image format to test applicability to the video device
> > @@ -840,6 +953,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
> >       format->size.width = pix->width;
> >       format->size.height = pix->height;
> >       format->fourcc = V4L2PixelFormat(pix->pixelformat);
> > +     format->colorSpace = getColorSpace(*pix);
> >       format->planesCount = pix->num_planes;
> >
> >       for (unsigned int i = 0; i < format->planesCount; ++i) {
> > @@ -862,6 +976,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)
> >       pix->pixelformat = format->fourcc;
> >       pix->num_planes = format->planesCount;
> >       pix->field = V4L2_FIELD_NONE;
> > +     setColorSpace(format->colorSpace, *pix);
> >
> >       ASSERT(pix->num_planes <= std::size(pix->plane_fmt));
> >
> > @@ -910,6 +1025,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
> >       format->size.width = pix->width;
> >       format->size.height = pix->height;
> >       format->fourcc = V4L2PixelFormat(pix->pixelformat);
> > +     format->colorSpace = getColorSpace(*pix);
> >       format->planesCount = 1;
> >       format->planes[0].bpl = pix->bytesperline;
> >       format->planes[0].size = pix->sizeimage;
> > @@ -929,6 +1045,7 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
> >       pix->pixelformat = format->fourcc;
> >       pix->bytesperline = format->planes[0].bpl;
> >       pix->field = V4L2_FIELD_NONE;
> > +     setColorSpace(format->colorSpace, *pix);
> >       ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
> >       if (ret) {
> >               LOG(V4L2, Error)
>
>

Patch
diff mbox series

diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
index e767ec84..c05899cf 100644
--- a/include/libcamera/internal/v4l2_videodevice.h
+++ b/include/libcamera/internal/v4l2_videodevice.h
@@ -20,6 +20,7 @@ 
 #include <libcamera/base/log.h>
 #include <libcamera/base/signal.h>
 
+#include <libcamera/color_space.h>
 #include <libcamera/framebuffer.h>
 #include <libcamera/geometry.h>
 #include <libcamera/pixel_format.h>
@@ -163,6 +164,7 @@  public:
 
 	V4L2PixelFormat fourcc;
 	Size size;
+	ColorSpace colorSpace;
 
 	std::array<Plane, 3> planes;
 	unsigned int planesCount = 0;
diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
index 0c55e716..131f7733 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,8 @@  struct StreamConfiguration {
 
 	unsigned int bufferCount;
 
+	ColorSpace colorSpace;
+
 	Stream *stream() const { return stream_; }
 	void setStream(Stream *stream) { stream_ = stream; }
 	const StreamFormats &formats() const { return formats_; }
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index da2af6a1..7f30412c 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -366,6 +366,11 @@  bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
  * \brief The image size in pixels
  */
 
+/**
+ * \var V4L2DeviceFormat::colorSpace
+ * \brief The color space of the pixels
+ */
+
 /**
  * \var V4L2DeviceFormat::fourcc
  * \brief The fourcc code describing the pixel encoding scheme
@@ -731,6 +736,114 @@  int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
 		return getFormatSingleplane(format);
 }
 
+static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {
+	{ ColorSpace::RAW, V4L2_COLORSPACE_RAW },
+	{ ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },
+	{ ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },
+	{ ColorSpace::REC709, V4L2_COLORSPACE_REC709 },
+	{ ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },
+};
+
+static const std::map<ColorSpace::Encoding, v4l2_ycbcr_encoding> encodingToV4l2 = {
+	{ ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },
+	{ ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },
+	{ ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },
+};
+
+static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {
+	{ ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },
+	{ ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },
+	{ ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },
+};
+
+static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
+	{ ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },
+	{ ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },
+};
+
+template<typename T>
+static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)
+{
+	if (!colorSpace.isFullyDefined())
+		LOG(V4L2, Warning) << "Setting non-fully defined colour space"
+				   << colorSpace.toString();
+
+	v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;
+	v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;
+	v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;
+
+	auto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),
+				     [&colorSpace](const std::pair<ColorSpace, v4l2_colorspace> &item) { return colorSpace == item.first; });
+	if (itColor != colorSpaceToV4l2.end())
+		v4l2PixFormat.colorspace = itColor->second;
+
+	auto itEncoding = encodingToV4l2.find(colorSpace.encoding);
+	if (itEncoding != encodingToV4l2.end())
+		v4l2PixFormat.ycbcr_enc = itEncoding->second;
+
+	auto itTransfer = transferFunctionToV4l2.find(colorSpace.transferFunction);
+	if (itTransfer != transferFunctionToV4l2.end())
+		v4l2PixFormat.xfer_func = itTransfer->second;
+
+	auto itRange = rangeToV4l2.find(colorSpace.range);
+	if (itRange != rangeToV4l2.end())
+		v4l2PixFormat.quantization = itRange->second;
+}
+
+static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
+	{ V4L2_COLORSPACE_RAW, ColorSpace::RAW },
+	{ V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },
+	{ V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },
+	{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },
+	{ V4L2_COLORSPACE_REC709, ColorSpace::REC709 },
+	{ V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },
+};
+
+static const std::map<uint32_t, ColorSpace::Encoding> v4l2ToEncoding = {
+	{ V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },
+	{ V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },
+	{ V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },
+};
+
+static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunction = {
+	{ V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },
+	{ V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },
+	{ V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },
+};
+
+static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
+	{ V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },
+	{ V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },
+};
+
+template<typename T>
+static ColorSpace getColorSpace(const T &v4l2PixFormat)
+{
+	ColorSpace colorSpace;
+
+	auto itColor = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);
+	if (itColor != v4l2ToColorSpace.end())
+		colorSpace = itColor->second;
+
+	auto itEncoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);
+	if (itEncoding != v4l2ToEncoding.end())
+		colorSpace.encoding = itEncoding->second;
+
+	auto itTransfer = v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);
+	if (itTransfer != v4l2ToTransferFunction.end())
+		colorSpace.transferFunction = itTransfer->second;
+
+	auto itRange = v4l2ToRange.find(v4l2PixFormat.quantization);
+	if (itRange != v4l2ToRange.end())
+		colorSpace.range = itRange->second;
+
+	if (!colorSpace.isFullyDefined())
+		LOG(V4L2, Warning) << "Returning non-fully defined colour space"
+				   << colorSpace.toString();
+	return colorSpace;
+}
+
 /**
  * \brief Try an image format on the V4L2 video device
  * \param[inout] format The image format to test applicability to the video device
@@ -840,6 +953,7 @@  int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
 	format->size.width = pix->width;
 	format->size.height = pix->height;
 	format->fourcc = V4L2PixelFormat(pix->pixelformat);
+	format->colorSpace = getColorSpace(*pix);
 	format->planesCount = pix->num_planes;
 
 	for (unsigned int i = 0; i < format->planesCount; ++i) {
@@ -862,6 +976,7 @@  int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)
 	pix->pixelformat = format->fourcc;
 	pix->num_planes = format->planesCount;
 	pix->field = V4L2_FIELD_NONE;
+	setColorSpace(format->colorSpace, *pix);
 
 	ASSERT(pix->num_planes <= std::size(pix->plane_fmt));
 
@@ -910,6 +1025,7 @@  int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
 	format->size.width = pix->width;
 	format->size.height = pix->height;
 	format->fourcc = V4L2PixelFormat(pix->pixelformat);
+	format->colorSpace = getColorSpace(*pix);
 	format->planesCount = 1;
 	format->planes[0].bpl = pix->bytesperline;
 	format->planes[0].size = pix->sizeimage;
@@ -929,6 +1045,7 @@  int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
 	pix->pixelformat = format->fourcc;
 	pix->bytesperline = format->planes[0].bpl;
 	pix->field = V4L2_FIELD_NONE;
+	setColorSpace(format->colorSpace, *pix);
 	ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
 	if (ret) {
 		LOG(V4L2, Error)