Hi Hans,

On 01/21/2013 09:25 AM, Hans Verkuil wrote:
Hi Sylwester!

On Sat January 19 2013 22:27:21 Sylwester Nawrocki wrote:
This patch adds a helper function that allows to modify range,
i.e. minimum, maximum, step and default value of a v4l2 control,
after the control has been created and initialized. This is helpful
in situations when range of a control depends on user configurable
parameters, e.g. camera sensor absolute exposure time depending on
an output image resolution and frame rate.

v4l2_ctrl_modify_range() function allows to modify range of an
INTEGER, BOOL, MENU, INTEGER_MENU and BITMASK type controls.

Based on a patch from Hans Verkuil http://patchwork.linuxtv.org/patch/8654.

Signed-off-by: Sylwester Nawrocki<sylvester.nawro...@gmail.com>

This is a very nice patch. I found only one small mistake:

Thank you for the review. In fact, I didn't change a lot in the original
patch ;) But I have to admit I had to spent some time to understand what's
going on in this code and how to resolve all merge conflicts. :-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index f6ee201..d68fb57 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2721,10 +2751,44 @@ int v4l2_ctrl_s_ctrl_int64(struct v4l2_ctrl *ctrl, s64 
val)
        /* It's a driver bug if this happens. */
        WARN_ON(ctrl->type != V4L2_CTRL_TYPE_INTEGER64);
        c.value64 = val;
-       return set_ctrl(NULL, ctrl,&c);
+       return set_ctrl_lock(NULL, ctrl,&c);
  }
  EXPORT_SYMBOL(v4l2_ctrl_s_ctrl_int64);

+int v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
+                       s32 min, s32 max, u32 step, s32 def)
+{
+       int ret = check_range(ctrl->type, min, max, step, def);
+       struct v4l2_ext_control c;
+
+       switch (ctrl->type) {
+       case V4L2_CTRL_TYPE_INTEGER:
+       case V4L2_CTRL_TYPE_BOOLEAN:
+       case V4L2_CTRL_TYPE_MENU:
+       case V4L2_CTRL_TYPE_BITMASK:

TYPE_INTEGER_MENU is missing here!

Thanks, fixed.

+               if (ret)
+                       return ret;
+               break;
+       default:
+               return -EINVAL;
+       }
+       v4l2_ctrl_lock(ctrl);
+       ctrl->minimum = min;
+       ctrl->maximum = max;
+       ctrl->step = step;
+       ctrl->default_value = def;
+       c.value = ctrl->cur.val;
+       if (validate_new(ctrl,&c))
+               c.value = def;
+       if (c.value != ctrl->cur.val)
+               ret = set_ctrl(NULL, ctrl,&c, V4L2_EVENT_CTRL_CH_RANGE);
+       else
+               send_event(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE);
+       v4l2_ctrl_unlock(ctrl);
+       return ret;
+}
+EXPORT_SYMBOL(v4l2_ctrl_modify_range);
+
  static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned 
elems)
  {
        struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id);

After correcting that missing case you can add my ack:

Acked-by: Hans Verkuil<hans.verk...@cisco.com>

Thank you. I'll post the next version shortly.

--

Regards,
Sylwester
--
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

Reply via email to