[libcamera-devel] [PATCH] libcamera: v4l2_device: Log control id instead of errorIdx

[Jacopo Mondi]

Hi Umang

On Sun, Sep 25, 2022 at 05:53:24PM +0530, Umang Jain via libcamera-devel wrote:
> v4l2_ext_controls.errorIdx (in the case of single failing control for
> VIDIOC_*_EXT_CTRLS calls) represents the index of that control.
> Since it is a single control, we can print the control id rather than
> its index. This improves logging as the id can be easily co-related
> with the controls while reading the log.

If we do that we lose the ability to report errors during the
valiation step:

https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/vidioc-g-ext-ctrls.html?highlight=ext_ctrl#c.V4L.VIDIOC_S_EXT_CTRLS
If the error is associated with a particular control, then error_idx
is set to the index of that control. If the error is not related to a
specific control, or the validation step failed (see below), then
error_idx is set to count. The value is undefined if the ioctl
returned 0 (success).

There's more context in the documentation page about validation.

Also, if a single control has failed, error_idx will report the index
of such control already, so I'm missing what we gain here...

Thanks
   j

>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/libcamera/v4l2_device.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 83901763..c60f7c91 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -244,7 +244,8 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids)
>  		}
>
>  		/* A specific control failed. */
> -		LOG(V4L2, Error) << "Unable to read control " << errorIdx
> +		const unsigned int id = v4l2Ctrls[errorIdx].id;
> +		LOG(V4L2, Error) << "Unable to read control " << utils::hex(id)
>  				 << ": " << strerror(-ret);
>
>  		v4l2Ctrls.resize(errorIdx);
> @@ -354,7 +355,8 @@ int V4L2Device::setControls(ControlList *ctrls)
>  		}
>
>  		/* A specific control failed. */
> -		LOG(V4L2, Error) << "Unable to set control " << errorIdx
> +		const unsigned int id = v4l2Ctrls[errorIdx].id;
> +		LOG(V4L2, Error) << "Unable to set control " << utils::hex(id)
>  				 << ": " << strerror(-ret);
>
>  		v4l2Ctrls.resize(errorIdx);
> --
> 2.37.3
>