| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <201209190953.45560.hverkuil@xs4all.nl>
Date: Wed, 19 Sep 2012 09:53:45 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Prabhakar Lad <prabhakar.csengg@...il.com>
Cc: LMML <linux-media@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
Mauro Carvalho Chehab <mchehab@...radead.org>,
linux-doc@...r.kernel.org, "Lad, Prabhakar" <prabhakar.lad@...com>,
Manjunath Hadli <manjunath.hadli@...com>,
Hans Verkuil <hans.verkuil@...co.com>,
Sakari Ailus <sakari.ailus@....fi>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Hans de Goede <hdegoede@...hat.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Guennadi Liakhovetski <g.liakhovetski@....de>,
Rob Landley <rob@...dley.net>
Subject: Re: [PATCH v4] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
Hi Prabhakar,
I found some grammar issues, but also some (small) things that should be changed.
On Tue 18 September 2012 20:54:38 Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@...com>
>
> Add helper function v4l2_ctrl_new_std_menu_items(), which adds
> a standard menu control, with driver specific menu.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@...com>
> Signed-off-by: Manjunath Hadli <manjunath.hadli@...com>
> Cc: Hans Verkuil <hans.verkuil@...co.com>
> Cc: Sakari Ailus <sakari.ailus@....fi>
> Cc: Sylwester Nawrocki <s.nawrocki@...sung.com>
> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@...radead.org>
> Cc: Hans de Goede <hdegoede@...hat.com>
> Cc: Kyungmin Park <kyungmin.park@...sung.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@....de>
> Cc: Rob Landley <rob@...dley.net>
> ---
> Changes for v4:
> 1: Rather then adding a function to modify the menu, added a helper
> function, that creates a new standard control with user specific
> menu.
>
> Changes for v3:
> 1: Fixed style/grammer issues as pointed by Hans.
> Thanks Hans for providing the description.
>
> Changes for v2:
> 1: Fixed review comments from Hans, to have return type as
> void, add WARN_ON() for fail conditions, allow this fucntion
> to modify the menu of custom controls.
>
> Documentation/video4linux/v4l2-controls.txt | 25 ++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 28 +++++++++++++++++++++++++++
> include/media/v4l2-ctrls.h | 23 ++++++++++++++++++++++
> 3 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
> index 43da22b..ad8e172 100644
> --- a/Documentation/video4linux/v4l2-controls.txt
> +++ b/Documentation/video4linux/v4l2-controls.txt
> @@ -136,11 +136,25 @@ Or alternatively for integer menu controls, by calling v4l2_ctrl_new_int_menu:
> const struct v4l2_ctrl_ops *ops,
> u32 id, s32 max, s32 def, const s64 *qmenu_int);
>
> +Standard menu controls with driver specific menu are added by calling
with driver -> with a driver
> +v4l2_ctrl_new_std_menu_items:
> +
> + struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(
> + struct v4l2_ctrl_handler *hdl,
> + const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
> + s32 skip_mask, s32 def, const char * const *qmenu_user);
I would recommend that qmenu_user is just renamed to qmenu.
The _user suffix suggests that this is a userspace-provided menu, which
is not the case.
> +
> These functions are typically called right after the v4l2_ctrl_handler_init:
>
> static const s64 exp_bias_qmenu[] = {
> -2, -1, 0, 1, 2
> };
> + static const char * const test_pattern[] = {
> + "Disabled",
> + "Vertical Bars",
> + "Solid Black",
> + "Solid White",
> + };
>
> v4l2_ctrl_handler_init(&foo->ctrl_handler, nr_of_controls);
> v4l2_ctrl_new_std(&foo->ctrl_handler, &foo_ctrl_ops,
> @@ -156,6 +170,9 @@ These functions are typically called right after the v4l2_ctrl_handler_init:
> ARRAY_SIZE(exp_bias_qmenu) - 1,
> ARRAY_SIZE(exp_bias_qmenu) / 2 - 1,
> exp_bias_qmenu);
> + v4l2_ctrl_new_std_menu_items(&foo->ctrl_handler, &foo_ctrl_ops,
> + V4L2_CID_TEST_PATTERN, ARRAY_SIZE(test_pattern) - 1, 0,
> + 0, test_pattern);
> ...
> if (foo->ctrl_handler.error) {
> int err = foo->ctrl_handler.error;
> @@ -185,6 +202,14 @@ v4l2_ctrl_new_std_menu in that it doesn't have the mask argument and takes
> as the last argument an array of signed 64-bit integers that form an exact
> menu item list.
>
> +The v4l2_ctrl_new_std_menu_items funtion is very similar as
funtion -> function
similar as -> similar to
> +v4l2_ctrl_new_std_menu but takes a extra parameter qmenu_user, which is
a extra -> an extra
> +driver specific menu but yet a standard menu control.
the driver specific menu for an otherwise standard menu control.
> +A good example for this control is the test pattern control for
> +capture/display/sensors devices that have the capability to generate test
> +patterns. These test patterns are hardware specific, so the contents of the
> +menu will vary from device to device.
> +
> Note that if something fails, the function will return NULL or an error and
> set ctrl_handler->error to the error code. If ctrl_handler->error was already
> set, then it will just return and do nothing. This is also true for
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d731422..9ac1b75 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1649,6 +1649,34 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
> }
> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
>
> +/* Helper function for standard menu controls with user defined menu */
> +struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
> + const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
> + s32 mask, s32 def, const char * const *qmenu_user)
> +{
> + const char * const *qmenu = v4l2_ctrl_get_menu(id);
> + const char *name;
> + enum v4l2_ctrl_type type;
> + s32 min;
> + s32 step;
> + u32 flags;
> +
> + if (!qmenu) {
This test should be inverted: if (qmenu)
Add a comment that v4l2_ctrl_new_std_menu_items() may only be called for
standard controls *without* a standard menu.
> + handler_set_err(hdl, -EINVAL);
> + return NULL;
> + }
> +
> + v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> + if (type != V4L2_CTRL_TYPE_MENU) {
> + handler_set_err(hdl, -EINVAL);
> + return NULL;
> + }
> + return v4l2_ctrl_new(hdl, ops, id, name, type,
> + 0, max, mask, def, flags, qmenu_user, NULL, NULL);
> +
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
> +
> /* Helper function for standard integer menu controls */
> struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
> const struct v4l2_ctrl_ops *ops,
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 776605f..e0dd392 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -351,6 +351,29 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
> const struct v4l2_ctrl_ops *ops,
> u32 id, s32 max, s32 mask, s32 def);
>
> +/** v4l2_ctrl_new_std_menu_items() - Create a new standard V4L2 menu control
> + * with driver specific menu.
> + * @hdl: The control handler.
> + * @ops: The control ops.
> + * @id: The control ID.
> + * @max: The control's maximum value.
> + * @mask: The control's skip mask for menu controls. This makes it
> + * easy to skip menu items that are not valid. If bit X is set,
> + * then menu item X is skipped. Of course, this only works for
> + * menus with <= 32 menu items. There are no menus that come
> + * close to that number, so this is OK. Should we ever need more,
> + * then this will have to be extended to a u64 or a bit array.
> + * @def: The control's default value.
> + * @qmenu_user:The new menu.
> + *
> + * Same as v4l2_ctrl_new_std_menu().but @qmenu_user will be the menu to
.but -> , but
'will be the driver specific menu of this control.'
> + * which the control will be pointing to.
> + *
> + */
> +struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
> + const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
> + s32 mask, s32 def, const char * const *qmenu_user);
> +
> /** v4l2_ctrl_new_int_menu() - Create a new standard V4L2 integer menu control.
> * @hdl: The control handler.
> * @ops: The control ops.
>
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists