Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
but there are some controls (limited powerline), that start with a value
different than 0.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 092decfdaa62..3b20b23abd1e 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
switch (mapping->v4l2_type) {
case V4L2_CTRL_TYPE_MENU:
- v4l2_ctrl->minimum = 0;
+ v4l2_ctrl->minimum = mapping->menu_min;
v4l2_ctrl->maximum = mapping->menu_count - 1;
v4l2_ctrl->step = 1;
@@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
goto done;
}
- if (query_menu->index >= mapping->menu_count) {
+ if (query_menu->index < mapping->menu_min ||
+ query_menu->index >= mapping->menu_count) {
ret = -EINVAL;
goto done;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index fff5c5c99a3d..6ceb7f7b964d 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -254,6 +254,7 @@ struct uvc_control_mapping {
u32 data_type;
const struct uvc_menu_info *menu_info;
+ u32 menu_min;
u32 menu_count;
u32 master_id;
--
2.36.1.476.g0c4daa206d-goog
Hi Laurent
On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart
<[email protected]> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > but there are some controls (limited powerline), that start with a value
> > different than 0.
> >
> > Signed-off-by: Ricardo Ribalda <[email protected]>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 092decfdaa62..3b20b23abd1e 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> >
> > switch (mapping->v4l2_type) {
> > case V4L2_CTRL_TYPE_MENU:
> > - v4l2_ctrl->minimum = 0;
> > + v4l2_ctrl->minimum = mapping->menu_min;
> > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > v4l2_ctrl->step = 1;
> >
> > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > goto done;
> > }
> >
> > - if (query_menu->index >= mapping->menu_count) {
> > + if (query_menu->index < mapping->menu_min ||
> > + query_menu->index >= mapping->menu_count) {
> > ret = -EINVAL;
> > goto done;
> > }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index fff5c5c99a3d..6ceb7f7b964d 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > u32 data_type;
> >
> > const struct uvc_menu_info *menu_info;
> > + u32 menu_min;
> > u32 menu_count;
>
> That's a bit of a stop-gap measure, could we turn it into a bitmask
> instead ?
Unfortunately that is uAPI :(
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
We have to keep the control type and its values.
Regards!
>
> >
> > u32 master_id;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi Ricardo,
Thank you for the patch.
On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> but there are some controls (limited powerline), that start with a value
> different than 0.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 092decfdaa62..3b20b23abd1e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>
> switch (mapping->v4l2_type) {
> case V4L2_CTRL_TYPE_MENU:
> - v4l2_ctrl->minimum = 0;
> + v4l2_ctrl->minimum = mapping->menu_min;
> v4l2_ctrl->maximum = mapping->menu_count - 1;
> v4l2_ctrl->step = 1;
>
> @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> goto done;
> }
>
> - if (query_menu->index >= mapping->menu_count) {
> + if (query_menu->index < mapping->menu_min ||
> + query_menu->index >= mapping->menu_count) {
> ret = -EINVAL;
> goto done;
> }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index fff5c5c99a3d..6ceb7f7b964d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> u32 data_type;
>
> const struct uvc_menu_info *menu_info;
> + u32 menu_min;
> u32 menu_count;
That's a bit of a stop-gap measure, could we turn it into a bitmask
instead ?
>
> u32 master_id;
--
Regards,
Laurent Pinchart
Hi Laurent
On Fri, 17 Jun 2022 at 16:11, Laurent Pinchart
<[email protected]> wrote:
>
> Hi Ricardo,
>
> On Fri, Jun 17, 2022 at 03:55:52PM +0200, Ricardo Ribalda wrote:
> > On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart wrote:
> > > On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > > > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > > > but there are some controls (limited powerline), that start with a value
> > > > different than 0.
> > > >
> > > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > > ---
> > > > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > > > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > index 092decfdaa62..3b20b23abd1e 100644
> > > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> > > >
> > > > switch (mapping->v4l2_type) {
> > > > case V4L2_CTRL_TYPE_MENU:
> > > > - v4l2_ctrl->minimum = 0;
> > > > + v4l2_ctrl->minimum = mapping->menu_min;
> > > > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > > > v4l2_ctrl->step = 1;
> > > >
> > > > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > > > goto done;
> > > > }
> > > >
> > > > - if (query_menu->index >= mapping->menu_count) {
> > > > + if (query_menu->index < mapping->menu_min ||
> > > > + query_menu->index >= mapping->menu_count) {
> > > > ret = -EINVAL;
> > > > goto done;
> > > > }
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index fff5c5c99a3d..6ceb7f7b964d 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > > > u32 data_type;
> > > >
> > > > const struct uvc_menu_info *menu_info;
> > > > + u32 menu_min;
> > > > u32 menu_count;
> > >
> > > That's a bit of a stop-gap measure, could we turn it into a bitmask
> > > instead ?
> >
> > Unfortunately that is uAPI :(
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
> >
> > We have to keep the control type and its values.
>
> Sure, I didn't mean changing that, but replacing menu_min and menu_count
> in uvc_control_mapping with a menu_mask that stores a bitmask of all
> supported values. This will allow skipping the first value in the power
> line frequency control case, but will also support skipping other menu
> entries for other controls in the future.
Ahh gotcha. Will implement that in the next version.
Thanks!
>
> > > >
> > > > u32 master_id;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi Ricardo,
On Fri, Jun 17, 2022 at 03:55:52PM +0200, Ricardo Ribalda wrote:
> On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart wrote:
> > On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > > but there are some controls (limited powerline), that start with a value
> > > different than 0.
> > >
> > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > ---
> > > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index 092decfdaa62..3b20b23abd1e 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> > >
> > > switch (mapping->v4l2_type) {
> > > case V4L2_CTRL_TYPE_MENU:
> > > - v4l2_ctrl->minimum = 0;
> > > + v4l2_ctrl->minimum = mapping->menu_min;
> > > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > > v4l2_ctrl->step = 1;
> > >
> > > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > > goto done;
> > > }
> > >
> > > - if (query_menu->index >= mapping->menu_count) {
> > > + if (query_menu->index < mapping->menu_min ||
> > > + query_menu->index >= mapping->menu_count) {
> > > ret = -EINVAL;
> > > goto done;
> > > }
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index fff5c5c99a3d..6ceb7f7b964d 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > > u32 data_type;
> > >
> > > const struct uvc_menu_info *menu_info;
> > > + u32 menu_min;
> > > u32 menu_count;
> >
> > That's a bit of a stop-gap measure, could we turn it into a bitmask
> > instead ?
>
> Unfortunately that is uAPI :(
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
>
> We have to keep the control type and its values.
Sure, I didn't mean changing that, but replacing menu_min and menu_count
in uvc_control_mapping with a menu_mask that stores a bitmask of all
supported values. This will allow skipping the first value in the power
line frequency control case, but will also support skipping other menu
entries for other controls in the future.
> > >
> > > u32 master_id;
--
Regards,
Laurent Pinchart