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@ti.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@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.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
Re: [PATCH v4] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
Hi Hans, Thanks for the review. On Wed, Sep 19, 2012 at 1:23 PM, Hans Verkuil hverk...@xs4all.nl wrote: 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@ti.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@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.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 Ok. +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. Ok. + 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 Ok. +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. Ok. +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
[PATCH v4] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
From: Lad, Prabhakar prabhakar@ti.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@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.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 +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); + 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 +v4l2_ctrl_new_std_menu but takes a extra parameter qmenu_user, which is +driver specific menu but yet a 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) { + handler_set_err(hdl, -EINVAL); +