Hi Laurent,
Laurent Pinchart wrote:
> Hi Sakari,
>
> Thanks for the patch.
Thanks for the review!
> On Tuesday 20 December 2011 21:27:53 Sakari Ailus wrote:
>> From: Sakari Ailus <sakari.ailus@xxxxxx>
>>
>> Create a new control type called V4L2_CTRL_TYPE_INTEGER_MENU. Integer menu
>> controls are just like menu controls but the menu items are 64-bit integers
>> rather than strings.
>
> [snip]
>
>> diff --git a/drivers/media/video/v4l2-ctrls.c
>> b/drivers/media/video/v4l2-ctrls.c index 0f415da..083bb79 100644
>> --- a/drivers/media/video/v4l2-ctrls.c
>> +++ b/drivers/media/video/v4l2-ctrls.c
>
>> @@ -1775,16 +1797,22 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl,
>> struct v4l2_querymenu *qm)
>>
>> qm->reserved = 0;
>> /* Sanity checks */
>
> You should return -EINVAL here if the control is not of a menu type. It was
> done implictly before by the ctrl->qmenu == NULL check, but that's now
> conditioned to the control type being V4L2_CTRL_TYPE_MENU.
Good point. Fixed.
>> - if (ctrl->qmenu == NULL ||
>> + if ((ctrl->type == V4L2_CTRL_TYPE_MENU && ctrl->qmenu == NULL) ||
>> + (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU
>> + && ctrl->qmenu_int == NULL) ||
>> i < ctrl->minimum || i > ctrl->maximum)
>> return -EINVAL;
>> /* Use mask to see if this menu item should be skipped */
>> if (ctrl->menu_skip_mask & (1 << i))
>> return -EINVAL;
>> /* Empty menu items should also be skipped */
>> - if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
>> - return -EINVAL;
>> - strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
>> + if (ctrl->type == V4L2_CTRL_TYPE_MENU) {
>> + if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
>> + return -EINVAL;
>> + strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
>> + } else if (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU) {
>
> And you can then replace the else if by an else.
As well as this one.
>> + qm->value = ctrl->qmenu_int[i];
>> + }
>> return 0;
>> }
>> EXPORT_SYMBOL(v4l2_querymenu);
>
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
