Re: [PATCH] media: v4l2-ctrl: Add p_def to v4l2_ctrl_config

From: Hans Verkuil
Date: Wed Oct 16 2019 - 08:32:12 EST


On 10/16/19 2:20 PM, Ricardo Ribalda Delgado wrote:
> Hi Hans
>
> Not that awkward, the user has to use the brand new
> v4l2_ctrl_ptr_create() ;). But if you prefer void * I can make the
> change.

Well, a struct v4l2_ctrl_config is typically a static const, so you can't use
v4l2_ctrl_ptr_create().

Hmm, perhaps it is as easy as:

static const struct v4l2_area def_area = {
...
};

static const struct v4l2_ctrl_config ctrl = {
...

.p_def.p_area = &def_area,
...
};

Can you do a quick compile check that I am not overlooking anything?

If this works, then I'll take this patch.

Regards,

Hans

>
> Regards
>
> On Wed, Oct 16, 2019 at 2:17 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote:
>>
>> On 10/14/19 4:14 PM, Ricardo Ribalda Delgado wrote:
>>> This allows setting the default value on compound controls created via
>>> v4l2_ctrl_new_custom.
>>>
>>> Signed-off-by: Ricardo Ribalda Delgado <ribalda@xxxxxxxxxx>
>>> ---
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 2 +-
>>> include/media/v4l2-ctrls.h | 2 ++
>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index bf50d37ef6c1..12cf38f73f7b 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -2583,7 +2583,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
>>> type, min, max,
>>> is_menu ? cfg->menu_skip_mask : step, def,
>>> cfg->dims, cfg->elem_size,
>>> - flags, qmenu, qmenu_int, ptr_null, priv);
>>> + flags, qmenu, qmenu_int, cfg->p_def, priv);
>>> if (ctrl)
>>> ctrl->is_private = cfg->is_private;
>>> return ctrl;
>>> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>>> index 26205ba3a0a0..2fca5b823961 100644
>>> --- a/include/media/v4l2-ctrls.h
>>> +++ b/include/media/v4l2-ctrls.h
>>> @@ -375,6 +375,7 @@ struct v4l2_ctrl_handler {
>>> * @max: The control's maximum value.
>>> * @step: The control's step value for non-menu controls.
>>> * @def: The control's default value.
>>> + * @p_def: The control's default value for compound controls.
>>> * @dims: The size of each dimension.
>>> * @elem_size: The size in bytes of the control.
>>> * @flags: The control's flags.
>>> @@ -403,6 +404,7 @@ struct v4l2_ctrl_config {
>>> s64 max;
>>> u64 step;
>>> s64 def;
>>> + union v4l2_ctrl_ptr p_def;
>>> u32 dims[V4L2_CTRL_MAX_DIMS];
>>> u32 elem_size;
>>> u32 flags;
>>>
>>
>> I'm not sure about this. It might be a bit awkward to initialize p_def given that it is a union.
>>
>> Perhaps a simple void pointer would be easier?
>>
>> Regards,
>>
>> Hans