Re: [RFC 03/17] vivi: Add an integer menu test control

2012-01-06 Thread Sakari Ailus 
Hi Laurent,

Laurent Pinchart wrote:
 Hi Sakari,
 
 Thanks for the patch.

Thanks for the review!

 On Tuesday 20 December 2011 21:27:55 Sakari Ailus wrote:
 From: Sakari Ailus sakari.ai...@iki.fi

 Add an integer menu test control for the vivi driver.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 ---
  drivers/media/video/vivi.c |   21 +
  1 files changed, 21 insertions(+), 0 deletions(-)

 diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
 index 7d754fb..763ec23 100644
 --- a/drivers/media/video/vivi.c
 +++ b/drivers/media/video/vivi.c
 @@ -177,6 +177,7 @@ struct vivi_dev {
  struct v4l2_ctrl   *menu;
  struct v4l2_ctrl   *string;
  struct v4l2_ctrl   *bitmask;
 +struct v4l2_ctrl   *int_menu;

  spinlock_t slock;
  struct mutex   mutex;
 @@ -503,6 +504,10 @@ static void vivi_fillbuff(struct vivi_dev *dev, struct
 vivi_buffer *buf) dev-boolean-cur.val,
  dev-menu-qmenu[dev-menu-cur.val],
  dev-string-cur.string);
 +snprintf(str, sizeof(str),  integer_menu %s, value %lld ,
 +dev-int_menu-qmenu[dev-int_menu-cur.val],
 
 Shouldn't you print the content of qmenu_int as a 64-bit integer instead ?

Oh, yes; I should. Also the value would be wrong, as well as the menu
item array --- should be the int one.

 +dev-int64-cur.val64);
 
 Shouldn't this be dev-int_menu-cur.val ?
 
 +gen_text(dev, vbuf, line++ * 16, 16, str);
  mutex_unlock(dev-ctrl_handler.lock);
  gen_text(dev, vbuf, line++ * 16, 16, str);
  if (dev-button_pressed) {
 @@ -1183,6 +1188,22 @@ static const struct v4l2_ctrl_config
 vivi_ctrl_bitmask = { .step = 0,
  };

 +static const s64 * const vivi_ctrl_int_menu_values[] = {
 +1, 1, 2, 3, 5, 8, 13, 21, 42,
 +};
 +
 +static const struct v4l2_ctrl_config vivi_ctrl_string = {
 +.ops = vivi_ctrl_ops,
 +.id = VIDI_CID_CUSTOM_BASE + 7
 +.name = Integer menu,
 +.type = V4L2_CTRL_TYPE_INTEGER_MENU,
 +.min = 1,
 +.max = 8,
 
 There are 9 values in your vivi_ctrl_int_menu_values array. Is 8 on purpose 
 here ?

I put it there to limit the maximum to 8 instead of 9, but 9 would be
equally good. I'll change it.

 +.def = 4,
 +.menu_skip_mask = 0x02,
 +.qmenu_int = vivi_ctrl_int_menu_values,
 +};
 +
  static const struct v4l2_file_operations vivi_fops = {
  .owner  = THIS_MODULE,
  .open   = v4l2_fh_open,
 


-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 03/17] vivi: Add an integer menu test control

2012-01-06 Thread Sakari Ailus 
Sakari Ailus wrote:
...
 I put it there to limit the maximum to 8 instead of 9, but 9 would be
 equally good. I'll change it.

Or not. 8 is still the index of the last value. min is one  to start the
menu from the second item. Would you like that to be changed to zero?

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 03/17] vivi: Add an integer menu test control

2012-01-05 Thread Laurent Pinchart 
Hi Sakari,

Thanks for the patch.

On Tuesday 20 December 2011 21:27:55 Sakari Ailus wrote:
 From: Sakari Ailus sakari.ai...@iki.fi
 
 Add an integer menu test control for the vivi driver.
 
 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 ---
  drivers/media/video/vivi.c |   21 +
  1 files changed, 21 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
 index 7d754fb..763ec23 100644
 --- a/drivers/media/video/vivi.c
 +++ b/drivers/media/video/vivi.c
 @@ -177,6 +177,7 @@ struct vivi_dev {
   struct v4l2_ctrl   *menu;
   struct v4l2_ctrl   *string;
   struct v4l2_ctrl   *bitmask;
 + struct v4l2_ctrl   *int_menu;
 
   spinlock_t slock;
   struct mutex   mutex;
 @@ -503,6 +504,10 @@ static void vivi_fillbuff(struct vivi_dev *dev, struct
 vivi_buffer *buf) dev-boolean-cur.val,
   dev-menu-qmenu[dev-menu-cur.val],
   dev-string-cur.string);
 + snprintf(str, sizeof(str),  integer_menu %s, value %lld ,
 + dev-int_menu-qmenu[dev-int_menu-cur.val],

Shouldn't you print the content of qmenu_int as a 64-bit integer instead ?

 + dev-int64-cur.val64);

Shouldn't this be dev-int_menu-cur.val ?

 + gen_text(dev, vbuf, line++ * 16, 16, str);
   mutex_unlock(dev-ctrl_handler.lock);
   gen_text(dev, vbuf, line++ * 16, 16, str);
   if (dev-button_pressed) {
 @@ -1183,6 +1188,22 @@ static const struct v4l2_ctrl_config
 vivi_ctrl_bitmask = { .step = 0,
  };
 
 +static const s64 * const vivi_ctrl_int_menu_values[] = {
 + 1, 1, 2, 3, 5, 8, 13, 21, 42,
 +};
 +
 +static const struct v4l2_ctrl_config vivi_ctrl_string = {
 + .ops = vivi_ctrl_ops,
 + .id = VIDI_CID_CUSTOM_BASE + 7
 + .name = Integer menu,
 + .type = V4L2_CTRL_TYPE_INTEGER_MENU,
 + .min = 1,
 + .max = 8,

There are 9 values in your vivi_ctrl_int_menu_values array. Is 8 on purpose 
here ?

 + .def = 4,
 + .menu_skip_mask = 0x02,
 + .qmenu_int = vivi_ctrl_int_menu_values,
 +};
 +
  static const struct v4l2_file_operations vivi_fops = {
   .owner  = THIS_MODULE,
   .open   = v4l2_fh_open,

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 03/17] vivi: Add an integer menu test control

2011-12-20 Thread Sakari Ailus 
From: Sakari Ailus sakari.ai...@iki.fi

Add an integer menu test control for the vivi driver.

Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
 drivers/media/video/vivi.c |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
index 7d754fb..763ec23 100644
--- a/drivers/media/video/vivi.c
+++ b/drivers/media/video/vivi.c
@@ -177,6 +177,7 @@ struct vivi_dev {
struct v4l2_ctrl   *menu;
struct v4l2_ctrl   *string;
struct v4l2_ctrl   *bitmask;
+   struct v4l2_ctrl   *int_menu;
 
spinlock_t slock;
struct mutex   mutex;
@@ -503,6 +504,10 @@ static void vivi_fillbuff(struct vivi_dev *dev, struct 
vivi_buffer *buf)
dev-boolean-cur.val,
dev-menu-qmenu[dev-menu-cur.val],
dev-string-cur.string);
+   snprintf(str, sizeof(str),  integer_menu %s, value %lld ,
+   dev-int_menu-qmenu[dev-int_menu-cur.val],
+   dev-int64-cur.val64);
+   gen_text(dev, vbuf, line++ * 16, 16, str);
mutex_unlock(dev-ctrl_handler.lock);
gen_text(dev, vbuf, line++ * 16, 16, str);
if (dev-button_pressed) {
@@ -1183,6 +1188,22 @@ static const struct v4l2_ctrl_config vivi_ctrl_bitmask = 
{
.step = 0,
 };
 
+static const s64 * const vivi_ctrl_int_menu_values[] = {
+   1, 1, 2, 3, 5, 8, 13, 21, 42,
+};
+
+static const struct v4l2_ctrl_config vivi_ctrl_string = {
+   .ops = vivi_ctrl_ops,
+   .id = VIDI_CID_CUSTOM_BASE + 7
+   .name = Integer menu,
+   .type = V4L2_CTRL_TYPE_INTEGER_MENU,
+   .min = 1,
+   .max = 8,
+   .def = 4,
+   .menu_skip_mask = 0x02,
+   .qmenu_int = vivi_ctrl_int_menu_values,
+};
+
 static const struct v4l2_file_operations vivi_fops = {
.owner  = THIS_MODULE,
.open   = v4l2_fh_open,
-- 
1.7.2.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html