| 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: <201209110955.26208.hverkuil@xs4all.nl>
Date: Tue, 11 Sep 2012 09:55:26 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Prabhakar Lad <prabhakar.lad@...com>
Cc: LMML <linux-media@...r.kernel.org>,
dlos <davinci-linux-open-source@...ux.davincidsp.com>,
linux-kernel@...r.kernel.org,
Manjunath Hadli <manjunath.hadli@...com>,
linux-doc@...r.kernel.org, Hans Verkuil <hans.verkuil@...co.com>,
Sakari Ailus <sakari.ailus@....fi>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Mauro Carvalho Chehab <mchehab@...radead.org>,
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 v2] media: v4l2-ctrl: add a helper function to modify the menu
Hi Pradhakar,
Looks good, but I have a number of style/grammar issues which I've corrected
below.
On Tue 11 September 2012 09:39:02 Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@...com>
>
> Add a helper function to modify the menu, max and default value
> to set.
>
> 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 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 | 30 +++++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls.c | 17 +++++++++++++++
> include/media/v4l2-ctrls.h | 11 +++++++++
> 3 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
> index 43da22b..160368a 100644
> --- a/Documentation/video4linux/v4l2-controls.txt
> +++ b/Documentation/video4linux/v4l2-controls.txt
> @@ -367,6 +367,36 @@ it to 0 means that all menu items are supported.
> You set this mask either through the v4l2_ctrl_config struct for a custom
> control, or by calling v4l2_ctrl_new_std_menu().
>
> +Changing the menu:
> +There are situations when menu items may be device specific, in such cases the
> +framework provides the helper function to change the menu.
> +
> +void v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, const char * const *qmenu,
> + s32 max, u32 menu_skip_mask, s32 def);
> +
> +A good example is the test pattern generation, the capture/display/sensors have
> +the capability to generate test patterns. This test patterns are hardware
> +specific, In such case the menu will vary from device to device.
> +
> +This helper, function is used to modify the menu, max, mask and the default
> +value to set.
> +
> +Example for usage:
> + static const char * const test_pattern[] = {
> + "Disabled",
> + "Vertical Bars",
> + "Vertical Bars",
> + "Solid Black",
> + "Solid White",
> + NULL
> + };
> + struct v4l2_ctrl *test_pattern_ctrl =
> + v4l2_ctrl_new_std_menu(&foo->ctrl_handler, &foo_ctrl_ops,
> + V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0,
> + V4L2_TEST_PATTERN_DISABLED);
> +
> + v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 5, 0x3, 1);
> +
There are a number of style/grammar issues with the text above. I've
corrected them in the revised version below. That replaces your version
completely. So just to make it clear: I've dropped the first line ('Changing
the menu:') because I don't think that line is necessary.
New text:
There are situations where menu items may be device specific. In such cases the
framework provides a helper function to change the menu:
void v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, const char * const *qmenu,
s32 max, u32 menu_skip_mask, s32 def);
A good example 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.
This helper function is used to modify the menu, max, mask and the default
value of the control.
Example:
static const char * const test_pattern[] = {
"Disabled",
"Vertical Bars",
"Solid Black",
"Solid White",
NULL
};
struct v4l2_ctrl *test_pattern_ctrl =
v4l2_ctrl_new_std_menu(&foo->ctrl_handler, &foo_ctrl_ops,
V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0,
V4L2_TEST_PATTERN_DISABLED);
v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 3, 0,
V4L2_TEST_PATTERN_DISABLED);
>
> Custom Controls
> ===============
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d731422..d89b460 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2666,3 +2666,20 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait)
> return 0;
> }
> EXPORT_SYMBOL(v4l2_ctrl_poll);
> +
> +/* Helper function for modifying the menu */
> +void v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, const char * const *qmenu,
> + s32 max, u32 menu_skip_mask, s32 def)
> +{
> + if (WARN_ON(ctrl->type != V4L2_CTRL_TYPE_MENU || qmenu == NULL))
> + return;
> +
> + if (WARN_ON(def < 0 || def > max))
> + return;
> +
> + ctrl->qmenu = qmenu;
> + ctrl->maximum = max;
> + ctrl->menu_skip_mask = menu_skip_mask;
> + ctrl->cur.val = ctrl->val = ctrl->default_value = def;
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_modify_menu);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 776605f..0c91b4e 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -488,6 +488,17 @@ static inline void v4l2_ctrl_unlock(struct v4l2_ctrl *ctrl)
> mutex_unlock(ctrl->handler->lock);
> }
>
> +/**
> + * v4l2_ctrl_modify_menu() - This function is used to modify the menu.
> + * @ctrl: The control of which menu should be changed.
"The control whose menu should be modified."
> + * @qmenu: The new menu to which control will point to.
"The new menu."
> + * @max: Maximum value of the control.
> + * @menu_skip_mask: The control's skip mask for menu controls.
> + * @def: The default value for control to be set.
> + */
> +void v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, const char * const *qmenu,
> + s32 max, u32 menu_skip_mask, s32 def);
> +
> /** v4l2_ctrl_g_ctrl() - Helper function to get the control's value from within a driver.
> * @ctrl: The control.
> *
>
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