Re: [PATCH v3 4/8] media: v4l: ctrls-api: Allow array update in __v4l2_ctrl_modify_range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/11/2022 14:34, Volodymyr Kharuk wrote:
> For V4L2_CID_TOF_TINT, which is dynamic array, it is required to use
> __v4l2_ctrl_modify_range.  So the idea is to use type_ops instead of u64
> from union. It will allow to work with any type.
> 
> Signed-off-by: Volodymyr Kharuk <vkh@xxxxxxxxxxx>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls-api.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> index d0a3aa3806fb..09735644a2f1 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> @@ -942,8 +942,6 @@ int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
>  	case V4L2_CTRL_TYPE_U8:
>  	case V4L2_CTRL_TYPE_U16:
>  	case V4L2_CTRL_TYPE_U32:
> -		if (ctrl->is_array)
> -			return -EINVAL;
>  		ret = check_range(ctrl->type, min, max, step, def);
>  		if (ret)
>  			return ret;
> @@ -960,17 +958,10 @@ int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
>  		ctrl->default_value = def;
>  	}
>  	cur_to_new(ctrl);
> -	if (validate_new(ctrl, ctrl->p_new)) {
> -		if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
> -			*ctrl->p_new.p_s64 = def;
> -		else
> -			*ctrl->p_new.p_s32 = def;
> -	}
> +	if (validate_new(ctrl, ctrl->p_new))
> +		ctrl->type_ops->init(ctrl, 0, ctrl->p_new);

Hmm, this sets *all* elements of the array to the default_value, not
just the elements whose value is out of range.

Is that what you want? Should this perhaps depend on the type of
control? I.e. in some cases it might make sense to do this, in other
cases it makes more sense to only adjust the elements that are out
of range.

How does that work for this TINT control?

Regards,

	Hans

>  
> -	if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
> -		value_changed = *ctrl->p_new.p_s64 != *ctrl->p_cur.p_s64;
> -	else
> -		value_changed = *ctrl->p_new.p_s32 != *ctrl->p_cur.p_s32;
> +	value_changed = !ctrl->type_ops->equal(ctrl, ctrl->p_cur, ctrl->p_new);
>  	if (value_changed)
>  		ret = set_ctrl(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE);
>  	else if (range_changed)




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux