2019-10-16 16:35:09

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH v2] media: v4l2-ctrl: Add p_def to v4l2_ctrl_config

This allows setting the default value on compound controls created via
v4l2_ctrl_new_custom.

Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 3 ++-
include/media/v4l2-ctrls.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index bf50d37ef6c1..939aa110daa0 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2583,7 +2583,8 @@ 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,
+ v4l2_ctrl_ptr_create((void *)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..d08d19a4ae34 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;
+ const void *p_def;
u32 dims[V4L2_CTRL_MAX_DIMS];
u32 elem_size;
u32 flags;
--
2.23.0


2019-10-21 12:11:03

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v2] media: v4l2-ctrl: Add p_def to v4l2_ctrl_config

On 10/16/19 4:55 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 <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 3 ++-
> include/media/v4l2-ctrls.h | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index bf50d37ef6c1..939aa110daa0 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2583,7 +2583,8 @@ 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,
> + v4l2_ctrl_ptr_create((void *)cfg->p_def), priv);

I don't like this cast. I've been experimenting a bit and this works fine:

#include <stdlib.h>
#include <stdio.h>

union v4l2_ctrl_ptr {
int *p_int;
const int *p_int_const;
};

struct v4l2_ctrl_config {
union v4l2_ctrl_ptr p;
};

static const int i = 5;

static const struct v4l2_ctrl_config c = {
.p.p_int_const = &i,
};

int main(int argc, char **argv)
{
printf("%d\n", *c.p.p_int_const);
return 0;
}

So I would like to see a p_const being added to the union, and I think we
need a v4l2_ctrl_const_ptr_create() helper.

In a separate patch it would be really nice to replace any uses of .p with
.p_const where possible to signal that the contents isn't modified.

Regards,

Hans

> 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..d08d19a4ae34 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;
> + const void *p_def;
> u32 dims[V4L2_CTRL_MAX_DIMS];
> u32 elem_size;
> u32 flags;
>