[libcamera-devel] [PATCH 2/3] libcamera: Support passing ColorSpaces to V4L2 drivers
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Thu Sep 23 08:26:56 CEST 2021
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 at 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
>
More information about the libcamera-devel
mailing list