On 10/16/19 2:39 PM, Ricardo Ribalda Delgado wrote:
> Hi Hans:
>
> On Wed, Oct 16, 2019 at 2:32 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote:
>>
>> 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.
>
> Testing with gcc 9.2.1
>
> This works fine, no warning/error:
>
> static struct v4l2_area unit_size = {
> .width = UNIT_SIZE,
> .height = UNIT_SIZE,
> };
> static struct v4l2_ctrl_config area_ctrl = {
> .type = V4L2_CTRL_TYPE_AREA,
> .flags = V4L2_CTRL_FLAG_READ_ONLY,
> .p_def.p_area = &unit_size,
> };
>
> but if unit_size is set as CONST:
> static const struct v4l2_area
>
> Then:
> drivers/qtec/qtec_sony.c: In function ‘qtec_sony_probe’:
> drivers/qtec/qtec_sony.c:3151:19: warning: initialization discards
> ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> 3151 | .p_def.p_area = &unit_size,
> |
Hmm. So we need a const void *p_def instead.
Regards,
Hans
>
>>
>> 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
>>
