[v2,2/5] libcamera: v4l2_{video, sub}device: {get, set, try}Format(): Log format
diff mbox series

Message ID 20260910160309.940584-2-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v2,1/5] libcamera: v4l2_videodevice: V4L2DeviceFormat: Expand formatting
Related show

Commit Message

Barnabás Pőcze Sept. 10, 2026, 4:03 p.m. UTC 
These functions have not been logging the v4l2 formats that they receive and
the ones that the device actually returns. This makes debugging much more
difficult. And multiple pipeline handlers work around this by adding logging
around `setFormat()` calls, but those can be missed, and they don't necessarily
include the path of the device node.

So add these log messages into the `{get,set,try}Format()` methods themselves.

Example:

  DEBUG V4L2 v4l2_videodevice.cpp:859 /dev/video0[13:cap]: trying format: 640x480-YUYV[]/Unset
  DEBUG V4L2 v4l2_videodevice.cpp:882 /dev/video0[13:cap]: returned format: 640x480-YUYV[614400/1280]/Rec709/Rec709/Rec601/Limited

  DEBUG V4L2 v4l2_subdevice.cpp:1424 'imx708': setting format on 0/0: 4608x2592-SRGGB10_1X10/Unset
  DEBUG V4L2 v4l2_subdevice.cpp:1439 'imx708': returned format on 0/0: 4608x2592-SRGGB10_1X10/RAW

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---

changes in v2:
  * same changes in `V4L2Subdevice`

---
 src/libcamera/v4l2_subdevice.cpp   |  6 ++++
 src/libcamera/v4l2_videodevice.cpp | 48 +++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 8 deletions(-)

--
2.55.0

Comments

Kieran Bingham Sept. 10, 2026, 4:25 p.m. UTC | #1 
Quoting Barnabás Pőcze (2026-09-10 17:03:06)
> These functions have not been logging the v4l2 formats that they receive and
> the ones that the device actually returns. This makes debugging much more
> difficult. And multiple pipeline handlers work around this by adding logging
> around `setFormat()` calls, but those can be missed, and they don't necessarily
> include the path of the device node.
> 
> So add these log messages into the `{get,set,try}Format()` methods themselves.
> 
> Example:
> 
>   DEBUG V4L2 v4l2_videodevice.cpp:859 /dev/video0[13:cap]: trying format: 640x480-YUYV[]/Unset
>   DEBUG V4L2 v4l2_videodevice.cpp:882 /dev/video0[13:cap]: returned format: 640x480-YUYV[614400/1280]/Rec709/Rec709/Rec601/Limited
> 
>   DEBUG V4L2 v4l2_subdevice.cpp:1424 'imx708': setting format on 0/0: 4608x2592-SRGGB10_1X10/Unset
>   DEBUG V4L2 v4l2_subdevice.cpp:1439 'imx708': returned format on 0/0: 4608x2592-SRGGB10_1X10/RAW
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> 
> changes in v2:
>   * same changes in `V4L2Subdevice`


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> ---
>  src/libcamera/v4l2_subdevice.cpp   |  6 ++++
>  src/libcamera/v4l2_videodevice.cpp | 48 +++++++++++++++++++++++++-----
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index a69de154e8..511cc12e2f 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -1374,6 +1374,8 @@ int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,
>         format->code = subdevFmt.format.code;
>         format->colorSpace = toColorSpace(subdevFmt.format);
> 
> +       LOG(V4L2, Debug) << "returned format on " << stream << ": " << *format;
> +
>         return 0;
>  }
> 
> @@ -1419,6 +1421,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
>                         subdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
>         }
> 
> +       LOG(V4L2, Debug) << "setting format on " << stream << ": " << *format;
> +
>         int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
>         if (ret) {
>                 LOG(V4L2, Error)
> @@ -1432,6 +1436,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
>         format->code = subdevFmt.format.code;
>         format->colorSpace = toColorSpace(subdevFmt.format);
> 
> +       LOG(V4L2, Debug) << "returned format on " << stream << ": " << *format;
> +
>         return 0;
>  }
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index bc539884e5..ea6270fdf2 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -814,19 +814,32 @@ std::string V4L2VideoDevice::logPrefix() const
>   */
>  int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
>  {
> +       int ret;
> +
>         switch (bufferType_) {
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>         case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> -               return getFormatSingleplane(format);
> +               ret = getFormatSingleplane(format);
> +               break;
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>         case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> -               return getFormatMultiplane(format);
> +               ret = getFormatMultiplane(format);
> +               break;
>         case V4L2_BUF_TYPE_META_CAPTURE:
>         case V4L2_BUF_TYPE_META_OUTPUT:
> -               return getFormatMeta(format);
> +               ret = getFormatMeta(format);
> +               break;
>         default:
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               break;
>         }
> +
> +       if (ret)
> +               return ret;
> +
> +       LOG(V4L2, Debug) << "returned format: " << *format;
> +
> +       return 0;
>  }
> 
>  /**
> @@ -841,19 +854,34 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
>   */
>  int V4L2VideoDevice::tryFormat(V4L2DeviceFormat *format)
>  {
> +       int ret;
> +
> +       LOG(V4L2, Debug) << "trying format: " << *format;
> +
>         switch (bufferType_) {
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>         case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> -               return trySetFormatSingleplane(format, false);
> +               ret = trySetFormatSingleplane(format, false);
> +               break;
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>         case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> -               return trySetFormatMultiplane(format, false);
> +               ret = trySetFormatMultiplane(format, false);
> +               break;
>         case V4L2_BUF_TYPE_META_CAPTURE:
>         case V4L2_BUF_TYPE_META_OUTPUT:
> -               return trySetFormatMeta(format, false);
> +               ret = trySetFormatMeta(format, false);
> +               break;
>         default:
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               break;
>         }
> +
> +       if (ret)
> +               return ret;
> +
> +       LOG(V4L2, Debug) << "returned format: " << *format;
> +
> +       return 0;
>  }
> 
>  /**
> @@ -869,6 +897,8 @@ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
>  {
>         int ret;
> 
> +       LOG(V4L2, Debug) << "setting format: " << *format;
> +
>         switch (bufferType_) {
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>         case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> @@ -894,6 +924,8 @@ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
>         format_ = *format;
>         formatInfo_ = &PixelFormatInfo::info(format_.fourcc);
> 
> +       LOG(V4L2, Debug) << "returned format: " << *format;
> +
>         return 0;
>  }
> 
> --
> 2.55.0

Patch
diff mbox series 

diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index a69de154e8..511cc12e2f 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -1374,6 +1374,8 @@  int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,
 	format->code = subdevFmt.format.code;
 	format->colorSpace = toColorSpace(subdevFmt.format);

+	LOG(V4L2, Debug) << "returned format on " << stream << ": " << *format;
+
 	return 0;
 }

@@ -1419,6 +1421,8 @@  int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
 			subdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
 	}

+	LOG(V4L2, Debug) << "setting format on " << stream << ": " << *format;
+
 	int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
 	if (ret) {
 		LOG(V4L2, Error)
@@ -1432,6 +1436,8 @@  int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
 	format->code = subdevFmt.format.code;
 	format->colorSpace = toColorSpace(subdevFmt.format);

+	LOG(V4L2, Debug) << "returned format on " << stream << ": " << *format;
+
 	return 0;
 }

diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index bc539884e5..ea6270fdf2 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -814,19 +814,32 @@  std::string V4L2VideoDevice::logPrefix() const
  */
 int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
 {
+	int ret;
+
 	switch (bufferType_) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
-		return getFormatSingleplane(format);
+		ret = getFormatSingleplane(format);
+		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		return getFormatMultiplane(format);
+		ret = getFormatMultiplane(format);
+		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
 	case V4L2_BUF_TYPE_META_OUTPUT:
-		return getFormatMeta(format);
+		ret = getFormatMeta(format);
+		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		break;
 	}
+
+	if (ret)
+		return ret;
+
+	LOG(V4L2, Debug) << "returned format: " << *format;
+
+	return 0;
 }

 /**
@@ -841,19 +854,34 @@  int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format)
  */
 int V4L2VideoDevice::tryFormat(V4L2DeviceFormat *format)
 {
+	int ret;
+
+	LOG(V4L2, Debug) << "trying format: " << *format;
+
 	switch (bufferType_) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
-		return trySetFormatSingleplane(format, false);
+		ret = trySetFormatSingleplane(format, false);
+		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		return trySetFormatMultiplane(format, false);
+		ret = trySetFormatMultiplane(format, false);
+		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
 	case V4L2_BUF_TYPE_META_OUTPUT:
-		return trySetFormatMeta(format, false);
+		ret = trySetFormatMeta(format, false);
+		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		break;
 	}
+
+	if (ret)
+		return ret;
+
+	LOG(V4L2, Debug) << "returned format: " << *format;
+
+	return 0;
 }

 /**
@@ -869,6 +897,8 @@  int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
 {
 	int ret;

+	LOG(V4L2, Debug) << "setting format: " << *format;
+
 	switch (bufferType_) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
@@ -894,6 +924,8 @@  int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
 	format_ = *format;
 	formatInfo_ = &PixelFormatInfo::info(format_.fourcc);

+	LOG(V4L2, Debug) << "returned format: " << *format;
+
 	return 0;
 }