[libcamera-devel] [PATCH 08/11] licamera: v4l2_device: Support reading U8 array controls

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Mar 09, 2020 at 05:24:11PM +0100, Jacopo Mondi wrote:
> Add support to retrieve the value of array controls of type
> V4L2_CTRL_TYPE_U8.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/v4l2_device.cpp | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 950e6286b84d..464df6a1fe18 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -185,6 +185,22 @@ int V4L2Device::getControls(ControlList *ctrls)
>  			return -EINVAL;
>  		}
>  
> +		const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id];
> +		if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
> +			if (info.type != V4L2_CTRL_TYPE_U8) {
> +				/*
> +				 * \todo Fail loudly to notify support for
> +				 * other array control types is not implemented.
> +				 */
> +				LOG(V4L2, Error) << "Unsupported type";
> +				return -EINVAL;
> +			}
> +
> +			size_t sizeBytes = info.elems * info.elem_size;
> +			v4l2Ctrls[i].p_u8 = new uint8_t[sizeBytes];
> +			v4l2Ctrls[i].size = sizeBytes;

Having to allocate extra memory here isn't very nice :-S I think that we
should try to instead write directly in the ControlValue data,
especially given that the updateControls() function should lose handling
of the V4L2_CTRL_TYPE_U8 type as explained in the review of patch 07/11.
I have a patch to implement this in ControlValue, I'll send it.

> +		}
> +
>  		v4l2Ctrls[i].id = id;
>  		i++;
>  	}
> @@ -214,6 +230,16 @@ int V4L2Device::getControls(ControlList *ctrls)
>  
>  	updateControls(ctrls, v4l2Ctrls, count);
>  
> +	/* Release memory reserved for array controls. */
> +	for (unsigned int i = 0; i < count; i++) {
> +		unsigned int id = v4l2Ctrls[i].id;
> +		const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id];
> +
> +		/* \todo Adjust to support more array control types. */
> +		if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD)
> +			delete[] v4l2Ctrls[i].p_u8;
> +	}
> +
>  	return ret;
>  }
>  

-- 
Regards,

Laurent Pinchart