<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Laurent,<br>
</p>
<div class="moz-cite-prefix">On 8/1/22 01:47, Laurent Pinchart
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:YubjaPko1t7g4elW@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">Hi Umang,
Thank you for the patch.
On Fri, Jul 29, 2022 at 08:20:42PM +0530, Umang Jain via libcamera-devel wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">The colorspace fields as read-only from an application point of view,
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
s/as read-only/are read-only/
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">both on video devices and on subdevs, unless the
V4L2_PIX_FMT_FLAG_SET_CSC or V4L2_MBUS_FRAMEFMT_SET_CSC flags
(respectively) are set when calling the S_FMT ioctl.
Signed-off-by: Umang Jain <a class="moz-txt-link-rfc2396E" href="mailto:umang.jain@ideasonboard.com"><umang.jain@ideasonboard.com></a>
---
src/libcamera/v4l2_subdevice.cpp | 3 ++-
src/libcamera/v4l2_videodevice.cpp | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 98a3911a..815c676e 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -459,7 +459,8 @@ int <a class="moz-txt-link-freetext" href="V4L2Subdevice::setFormat(unsigned">V4L2Subdevice::setFormat(unsigned</a> int pad, V4L2SubdeviceFormat *format,
subdevFmt.format.height = format->size.height;
subdevFmt.format.code = format->mbus_code;
subdevFmt.format.field = V4L2_FIELD_NONE;
- fromColorSpace(format->colorSpace, subdevFmt.format);
+ if (fromColorSpace(format->colorSpace, subdevFmt.format) == 0)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
If format->colorSpace is nullopt, should be set the CSC flag ?</pre>
</blockquote>
<p><br>
</p>
<p>Good question, I remember I was wondering that as well...</p>
<p>The documentation mentions:</p>
<p>'''</p>
<p><code class="docutils literal notranslate"><span class="pre">V4L2_MBUS_FRAMEFMT_SET_CSC</span></code>
then the application can set this
field on the source pad to request a specific colorspace for the
media
bus data</p>
<p>'''</p>
<p>So it seems flag is used to request a "specific" colorspace. So
*_DEFAULT are specific, I don't think so.</p>
<p>So I think if format->colorSpace is nullopt, we should avoid
setting this flag, unless, we start mapping to *_DEFAULT to some
kind of specifics in the future (in libcamera).<br>
</p>
<blockquote type="cite"
cite="mid:YubjaPko1t7g4elW@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ subdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
The flag is meant for source pads only, can you avoid setting it on sink
pads ?</pre>
</blockquote>
<p><br>
</p>
<p>ack.<br>
</p>
<blockquote type="cite"
cite="mid:YubjaPko1t7g4elW@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
if (ret) {
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 63911339..a969f7fa 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -940,7 +940,8 @@ int <a class="moz-txt-link-freetext" href="V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat">V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat</a> *format, bool set)
pix->pixelformat = format->fourcc;
pix->num_planes = format->planesCount;
pix->field = V4L2_FIELD_NONE;
- fromColorSpace(format->colorSpace, *pix);
+ if (fromColorSpace(format->colorSpace, *pix) == 0)
+ pix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Similarly, the CSC flag is meant for capture devices only.</pre>
</blockquote>
<p><br>
</p>
<p>ack<br>
</p>
<blockquote type="cite"
cite="mid:YubjaPko1t7g4elW@pendragon.ideasonboard.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
ASSERT(pix->num_planes <= <a class="moz-txt-link-freetext" href="std::size(pix">std::size(pix</a>->plane_fmt));
@@ -1010,7 +1011,8 @@ int <a class="moz-txt-link-freetext" href="V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat">V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat</a> *format, bool set)
pix->pixelformat = format->fourcc;
pix->bytesperline = format->planes[0].bpl;
pix->field = V4L2_FIELD_NONE;
- fromColorSpace(format->colorSpace, *pix);
+ if (fromColorSpace(format->colorSpace, *pix) == 0)
+ pix->flags |= V4L2_PIX_FMT_FLAG_SET_CSC;
ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
if (ret) {
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>