<div dir="ltr"><div dir="ltr">Hi David.<div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 8 Jun 2021 at 15:44, David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The ColorSpace class is added to the StreamConfiguration, and is now<br>
passed to V4L2 devices where it is handled appropriately. Note how<br>
this means that the colour space is configured per-stream (though<br>
platforms may restrict this).<br>
---<br>
include/libcamera/internal/v4l2_videodevice.h | 2 +<br>
include/libcamera/stream.h | 3 +<br>
src/libcamera/v4l2_videodevice.cpp | 112 ++++++++++++++++++<br>
3 files changed, 117 insertions(+)<br>
<br>
diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h<br>
index 7938343b..5122d422 100644<br>
--- a/include/libcamera/internal/v4l2_videodevice.h<br>
+++ b/include/libcamera/internal/v4l2_videodevice.h<br>
@@ -18,6 +18,7 @@<br>
<br>
#include <libcamera/buffer.h><br>
#include <libcamera/class.h><br>
+#include <libcamera/color_space.h><br>
#include <libcamera/geometry.h><br>
#include <libcamera/pixel_format.h><br>
#include <libcamera/signal.h><br>
@@ -162,6 +163,7 @@ public:<br>
<br>
V4L2PixelFormat fourcc;<br>
Size size;<br>
+ ColorSpace colorSpace;<br>
<br>
std::array<Plane, 3> planes;<br>
unsigned int planesCount = 0;<br>
diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h<br>
index bb47c390..f79c6987 100644<br>
--- a/include/libcamera/stream.h<br>
+++ b/include/libcamera/stream.h<br>
@@ -13,6 +13,7 @@<br>
#include <vector><br>
<br>
#include <libcamera/buffer.h><br>
+#include <libcamera/color_space.h><br>
#include <libcamera/geometry.h><br>
#include <libcamera/pixel_format.h><br>
<br>
@@ -47,6 +48,8 @@ struct StreamConfiguration {<br>
<br>
unsigned int bufferCount;<br>
<br>
+ ColorSpace colorSpace;<br>
+<br>
Stream *stream() const { return stream_; }<br>
void setStream(Stream *stream) { stream_ = stream; }<br>
const StreamFormats &formats() const { return formats_; }<br>
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp<br>
index 12c09dc7..d606f81d 100644<br>
--- a/src/libcamera/v4l2_videodevice.cpp<br>
+++ b/src/libcamera/v4l2_videodevice.cpp<br>
@@ -730,6 +730,114 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)<br>
return getFormatSingleplane(format);<br>
}<br>
<br>
+static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {<br>
+ { ColorSpace::RAW, V4L2_COLORSPACE_RAW },<br>
+ { ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },<br>
+ { ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },<br>
+ { ColorSpace::REC709, V4L2_COLORSPACE_REC709 },<br>
+ { ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },<br>
+};<br>
+<br>
+static const std::map<ColorSpace::Encoding, v4l2_ycbcr_encoding> encodingToV4l2 = {<br>
+ { ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },<br>
+ { ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },<br>
+ { ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },<br>
+};<br>
+<br>
+static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {<br>
+ { ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },<br>
+ { ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },<br>
+ { ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },<br>
+};<br>
+<br>
+static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {<br>
+ { ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },<br>
+ { ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },<br>
+};<br>
+<br>
+template<typename T><br>
+static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)<br></blockquote><div><br></div><div>Could you expand on why this (and below) needs to be templated?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+ if (!colorSpace.isFullyDefined())<br>
+ LOG(V4L2, Warning) << "Setting non-fully defined colour space"<br>
+ << colorSpace.toString();<br>
+<br>
+ v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;<br>
+ v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;<br>
+ v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;<br>
+ v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;<br>
+<br>
+ auto it_color = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),<br>
+ [&colorSpace](const std::pair<ColorSpace, v4l2_colorspace> &item) { return colorSpace == item.first; });<br></blockquote><div><br></div><div>Could you use a map for colorSpaceToV4l2 like you do for the others?</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ if (it_color != colorSpaceToV4l2.end())<br>
+ v4l2PixFormat.colorspace = it_color->second;<br>
+<br>
+ auto it_encoding = encodingToV4l2.find(colorSpace.encoding);<br>
+ if (it_encoding != encodingToV4l2.end())<br>
+ v4l2PixFormat.ycbcr_enc = it_encoding->second;<br>
+<br>
+ auto it_transfer = transferFunctionToV4l2.find(colorSpace.transferFunction);<br>
+ if (it_transfer != transferFunctionToV4l2.end())<br>
+ v4l2PixFormat.xfer_func = it_transfer->second;<br>
+<br>
+ auto it_range = rangeToV4l2.find(colorSpace.range);<br>
+ if (it_range != rangeToV4l2.end())<br>
+ v4l2PixFormat.quantization = it_range->second;<br>
+}<br>
+<br>
+static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {<br>
+ { V4L2_COLORSPACE_RAW, ColorSpace::RAW },<br>
+ { V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },<br>
+ { V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },<br>
+ { V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },<br>
+ { V4L2_COLORSPACE_REC709, ColorSpace::REC709 },<br>
+ { V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },<br>
+};<br>
+<br>
+static const std::map<uint32_t, ColorSpace::Encoding> v4l2ToEncoding = {<br>
+ { V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },<br>
+ { V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },<br>
+ { V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },<br>
+};<br>
+<br>
+static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunction = {<br>
+ { V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },<br>
+ { V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },<br>
+ { V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },<br>
+};<br>
+<br>
+static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {<br>
+ { V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },<br>
+ { V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },<br>
+};<br>
+<br>
+template<typename T><br>
+ColorSpace getColorSpace(const T &v4l2PixFormat)<br>
+{<br>
+ ColorSpace colorSpace;<br>
+<br>
+ auto it_color = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);<br>
+ if (it_color != v4l2ToColorSpace.end())<br>
+ colorSpace = it_color->second;<br>
+<br>
+ auto it_encoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);<br>
+ if (it_encoding != v4l2ToEncoding.end())<br>
+ colorSpace.encoding = it_encoding->second;<br>
+<br>
+ auto it_transfer = v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);<br>
+ if (it_transfer != v4l2ToTransferFunction.end())<br>
+ colorSpace.transferFunction = it_transfer->second;<br>
+<br>
+ auto it_range = v4l2ToRange.find(v4l2PixFormat.quantization);<br>
+ if (it_range != v4l2ToRange.end())<br>
+ colorSpace.range = it_range->second;<br>
+<br>
+ if (!colorSpace.isFullyDefined())<br>
+ LOG(V4L2, Warning) << "Returning non-fully defined colour space"<br>
+ << colorSpace.toString();<br>
+ return colorSpace;<br>
+}<br>
+<br>
/**<br>
* \brief Try an image format on the V4L2 video device<br>
* \param[inout] format The image format to test applicability to the video device<br>
@@ -839,6 +947,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)<br>
format->size.width = pix->width;<br>
format->size.height = pix->height;<br>
format->fourcc = V4L2PixelFormat(pix->pixelformat);<br>
+ format->colorSpace = getColorSpace(*pix);<br>
format->planesCount = pix->num_planes;<br>
<br>
for (unsigned int i = 0; i < format->planesCount; ++i) {<br>
@@ -861,6 +970,7 @@ int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)<br>
pix->pixelformat = format->fourcc;<br>
pix->num_planes = format->planesCount;<br>
pix->field = V4L2_FIELD_NONE;<br>
+ setColorSpace(format->colorSpace, *pix);<br>
<br>
ASSERT(pix->num_planes <= std::size(pix->plane_fmt));<br>
<br>
@@ -909,6 +1019,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)<br>
format->size.width = pix->width;<br>
format->size.height = pix->height;<br>
format->fourcc = V4L2PixelFormat(pix->pixelformat);<br>
+ format->colorSpace = getColorSpace(*pix);<br>
format->planesCount = 1;<br>
format->planes[0].bpl = pix->bytesperline;<br>
format->planes[0].size = pix->sizeimage;<br>
@@ -928,6 +1039,7 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)<br>
pix->pixelformat = format->fourcc;<br>
pix->bytesperline = format->planes[0].bpl;<br>
pix->field = V4L2_FIELD_NONE;<br>
+ setColorSpace(format->colorSpace, *pix);<br>
ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);<br>
if (ret) {<br>
LOG(V4L2, Error)<br>
-- <br>
2.20.1<br>
<br>
</blockquote></div></div>