<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your work.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 20 Oct 2021 at 12:08, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">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 from the StreamConfiguration is now handled<br>
appropriately in the V4L2Subdevice.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
include/libcamera/internal/v4l2_subdevice.h | 2 ++<br>
src/libcamera/camera_sensor.cpp | 1 +<br>
src/libcamera/v4l2_subdevice.cpp | 37 ++++++++++++++++++++-<br>
3 files changed, 39 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h<br>
index 97b89fb9..f3ab8454 100644<br>
--- a/include/libcamera/internal/v4l2_subdevice.h<br>
+++ b/include/libcamera/internal/v4l2_subdevice.h<br>
@@ -14,6 +14,7 @@<br>
#include <libcamera/base/class.h><br>
#include <libcamera/base/log.h><br>
<br>
+#include <libcamera/color_space.h><br>
#include <libcamera/geometry.h><br>
<br>
#include "libcamera/internal/formats.h"<br>
@@ -27,6 +28,7 @@ class MediaDevice;<br>
struct V4L2SubdeviceFormat {<br>
uint32_t mbus_code;<br>
Size size;<br>
+ ColorSpace colorSpace;<br>
<br>
const std::string toString() const;<br>
uint8_t bitsPerPixel() const;<br>
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp<br>
index 9fdb8c09..6fcd1c1d 100644<br>
--- a/src/libcamera/camera_sensor.cpp<br>
+++ b/src/libcamera/camera_sensor.cpp<br>
@@ -613,6 +613,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu<br>
V4L2SubdeviceFormat format{<br>
.mbus_code = bestCode,<br>
.size = *bestSize,<br>
+ .colorSpace = ColorSpace::Raw,<br>
};<br>
<br>
return format;<br>
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp<br>
index 023e2328..8538e66c 100644<br>
--- a/src/libcamera/v4l2_subdevice.cpp<br>
+++ b/src/libcamera/v4l2_subdevice.cpp<br>
@@ -168,6 +168,18 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {<br>
* \brief The image size in pixels<br>
*/<br>
<br>
+/**<br>
+ * \var V4L2SubdeviceFormat::colorSpace<br>
+ * \brief The color space of the pixels<br>
+ *<br>
+ * When setting or trying a format, passing in "Undefined" fields in the<br>
+ * ColorSpace is not recommended because the driver will then make an<br>
+ * arbitrary choice of its own. Choices made by the driver will be<br>
+ * passed back in the normal way, though note that "Undefined" values can<br>
+ * be returned if the device has chosen something that the ColorSpace<br>
+ * cannot represent.<br>
+ */<br>
+<br>
/**<br>
* \brief Assemble and return a string describing the format<br>
* \return A string describing the V4L2SubdeviceFormat<br>
@@ -400,6 +412,12 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
format->size.height = subdevFmt.format.height;<br>
format->mbus_code = subdevFmt.format.code;<br>
<br>
+ format->colorSpace = v4l2ToColorSpace(subdevFmt.format);<br>
+ if (!format->colorSpace.isFullyDefined())<br>
+ LOG(V4L2, Warning)<br>
+ << "Retrieved undefined color space: "<br>
+ << format->colorSpace.toString();<br>
+<br>
return 0;<br>
}<br>
<br>
@@ -418,6 +436,11 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
Whence whence)<br>
{<br>
+ if (!format->colorSpace.isFullyDefined())<br>
+ LOG(V4L2, Error)<br>
+ << "Trying to set undefined color space: "<br>
+ << format->colorSpace.toString();<br>
+<br></blockquote><div><br></div><div>Like the previous patch, should we return with an error code here? Other than that:</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></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">
struct v4l2_subdev_format subdevFmt = {};<br>
subdevFmt.which = whence == ActiveFormat ? V4L2_SUBDEV_FORMAT_ACTIVE<br>
: V4L2_SUBDEV_FORMAT_TRY;<br>
@@ -427,7 +450,13 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
subdevFmt.format.code = format->mbus_code;<br>
subdevFmt.format.field = V4L2_FIELD_NONE;<br>
<br>
- int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);<br>
+ int ret = colorSpaceToV4l2(format->colorSpace, subdevFmt.format);<br>
+ if (ret < 0)<br>
+ LOG(V4L2, Warning)<br>
+ << "Setting color space unrecognised by V4L2: "<br>
+ << format->colorSpace.toString();<br>
+<br>
+ ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);<br>
if (ret) {<br>
LOG(V4L2, Error)<br>
<< "Unable to set format on pad " << pad<br>
@@ -439,6 +468,12 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
format->size.height = subdevFmt.format.height;<br>
format->mbus_code = subdevFmt.format.code;<br>
<br>
+ format->colorSpace = v4l2ToColorSpace(subdevFmt.format);<br>
+ if (!format->colorSpace.isFullyDefined())<br>
+ LOG(V4L2, Warning)<br>
+ << "Undefined color space has been set: "<br>
+ << format->colorSpace.toString();<br>
+<br>
return 0;<br>
}<br>
<br>
-- <br>
2.20.1<br>
<br>
</blockquote></div></div>