* [PATCH] media: uvcvideo: Override default flags
@ 2024-06-02 6:50 Daniel Schaefer
2024-06-03 11:49 ` Ricardo Ribalda
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Schaefer @ 2024-06-02 6:50 UTC (permalink / raw)
To: linux-media
Cc: Daniel Schaefer, Edgar Thier, Kieran Bingham,
Mauro Carvalho Chehab, Laurent Pinchart, Kieran Levin
When the UVC device has a control that is readonly it doesn't set the
SET_CUR flag. For example the privacy control has SET_CUR flag set in
the defaults in the `uvc_ctrls` variable. Even if the device does not
have it set, it's not cleared by uvc_ctrl_get_flags.
Originally written with assignment in commit 859086ae3636 ("media:
uvcvideo: Apply flags from device to actual properties"). But changed to
|= in commit 0dc68cabdb62 ("media: uvcvideo: Prevent setting unavailable
flags"). It would not clear the default flags.
With this patch applied the correct flags are reported to user space.
Tested with:
```
> v4l2-ctl --list-ctrls | grep privacy
privacy 0x009a0910 (bool) : default=0 value=0 flags=read-only
```
Cc: Edgar Thier <info@edgarthier.net>
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Kieran Levin <ktl@frame.work>
Signed-off-by: Daniel Schaefer <dhs@frame.work>
---
drivers/media/usb/uvc/uvc_ctrl.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 4b685f883e4d..f50542e26542 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2031,15 +2031,23 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev,
else
ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
dev->intfnum, info->selector, data, 1);
- if (!ret)
- info->flags |= (data[0] & UVC_CONTROL_CAP_GET ?
- UVC_CTRL_FLAG_GET_CUR : 0)
- | (data[0] & UVC_CONTROL_CAP_SET ?
- UVC_CTRL_FLAG_SET_CUR : 0)
- | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
- UVC_CTRL_FLAG_AUTO_UPDATE : 0)
- | (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS ?
- UVC_CTRL_FLAG_ASYNCHRONOUS : 0);
+ if (!ret) {
+ info->flags = (data[0] & UVC_CONTROL_CAP_GET)
+ ? (info->flags | UVC_CTRL_FLAG_GET_CUR)
+ : (info->flags & ~UVC_CTRL_FLAG_GET_CUR);
+
+ info->flags = (data[0] & UVC_CONTROL_CAP_SET)
+ ? (info->flags | UVC_CTRL_FLAG_SET_CUR)
+ : (info->flags & ~UVC_CTRL_FLAG_SET_CUR);
+
+ info->flags = (data[0] & UVC_CONTROL_CAP_AUTOUPDATE)
+ ? (info->flags | UVC_CTRL_FLAG_AUTO_UPDATE)
+ : (info->flags & ~UVC_CTRL_FLAG_AUTO_UPDATE);
+
+ info->flags = (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS)
+ ? (info->flags | UVC_CTRL_FLAG_ASYNCHRONOUS)
+ : (info->flags & ~UVC_CTRL_FLAG_ASYNCHRONOUS);
+ }
kfree(data);
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] media: uvcvideo: Override default flags 2024-06-02 6:50 [PATCH] media: uvcvideo: Override default flags Daniel Schaefer @ 2024-06-03 11:49 ` Ricardo Ribalda 2024-06-05 7:12 ` Daniel Schaefer 2024-06-16 23:20 ` Laurent Pinchart 0 siblings, 2 replies; 4+ messages in thread From: Ricardo Ribalda @ 2024-06-03 11:49 UTC (permalink / raw) To: Daniel Schaefer Cc: linux-media, Edgar Thier, Kieran Bingham, Mauro Carvalho Chehab, Laurent Pinchart, Kieran Levin Hi Daniel Thanks for the patch. Some minor nits. Feel free to ignore it if you prefer your style. On Sun, 2 Jun 2024 at 08:52, Daniel Schaefer <dhs@frame.work> wrote: > > When the UVC device has a control that is readonly it doesn't set the > SET_CUR flag. For example the privacy control has SET_CUR flag set in > the defaults in the `uvc_ctrls` variable. Even if the device does not > have it set, it's not cleared by uvc_ctrl_get_flags. > > Originally written with assignment in commit 859086ae3636 ("media: > uvcvideo: Apply flags from device to actual properties"). But changed to > |= in commit 0dc68cabdb62 ("media: uvcvideo: Prevent setting unavailable > flags"). It would not clear the default flags. > > With this patch applied the correct flags are reported to user space. > Tested with: > > ``` > > v4l2-ctl --list-ctrls | grep privacy > privacy 0x009a0910 (bool) : default=0 value=0 flags=read-only > ``` > > Cc: Edgar Thier <info@edgarthier.net> > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Kieran Levin <ktl@frame.work> > Signed-off-by: Daniel Schaefer <dhs@frame.work> Fixes: 0dc68cabdb62 ("media: uvcvideo: Prevent setting unavailable flags") Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 4b685f883e4d..f50542e26542 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2031,15 +2031,23 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, > else > ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, > dev->intfnum, info->selector, data, 1); > - if (!ret) > - info->flags |= (data[0] & UVC_CONTROL_CAP_GET ? > - UVC_CTRL_FLAG_GET_CUR : 0) > - | (data[0] & UVC_CONTROL_CAP_SET ? > - UVC_CTRL_FLAG_SET_CUR : 0) > - | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ? > - UVC_CTRL_FLAG_AUTO_UPDATE : 0) > - | (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS ? > - UVC_CTRL_FLAG_ASYNCHRONOUS : 0); > + if (!ret) { > + info->flags = (data[0] & UVC_CONTROL_CAP_GET) > + ? (info->flags | UVC_CTRL_FLAG_GET_CUR) > + : (info->flags & ~UVC_CTRL_FLAG_GET_CUR); > + > + info->flags = (data[0] & UVC_CONTROL_CAP_SET) > + ? (info->flags | UVC_CTRL_FLAG_SET_CUR) > + : (info->flags & ~UVC_CTRL_FLAG_SET_CUR); > + > + info->flags = (data[0] & UVC_CONTROL_CAP_AUTOUPDATE) > + ? (info->flags | UVC_CTRL_FLAG_AUTO_UPDATE) > + : (info->flags & ~UVC_CTRL_FLAG_AUTO_UPDATE); > + > + info->flags = (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS) > + ? (info->flags | UVC_CTRL_FLAG_ASYNCHRONOUS) > + : (info->flags & ~UVC_CTRL_FLAG_ASYNCHRONOUS); > + } nit: I would have done it as diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 4b685f883e4d..c453a67e1407 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -2031,7 +2031,12 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, else ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum, info->selector, data, 1); - if (!ret) + if (!ret) { + info->flags &= ~(UVC_CTRL_FLAG_GET_CUR | + UVC_CTRL_FLAG_SET_CUR | + UVC_CTRL_FLAG_AUTO_UPDATE | + UVC_CTRL_FLAG_ASYNCHRONOUS); + info->flags |= (data[0] & UVC_CONTROL_CAP_GET ? UVC_CTRL_FLAG_GET_CUR : 0) | (data[0] & UVC_CONTROL_CAP_SET ? @@ -2040,6 +2045,7 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, UVC_CTRL_FLAG_AUTO_UPDATE : 0) | (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS ? UVC_CTRL_FLAG_ASYNCHRONOUS : 0); + } > > kfree(data); > return ret; > -- > 2.43.0 > > -- Ricardo Ribalda ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] media: uvcvideo: Override default flags 2024-06-03 11:49 ` Ricardo Ribalda @ 2024-06-05 7:12 ` Daniel Schaefer 2024-06-16 23:20 ` Laurent Pinchart 1 sibling, 0 replies; 4+ messages in thread From: Daniel Schaefer @ 2024-06-05 7:12 UTC (permalink / raw) To: Ricardo Ribalda Cc: linux-media, Edgar Thier, Kieran Bingham, Mauro Carvalho Chehab, Laurent Pinchart, Kieran Levin On Mon, Jun 3, 2024 at 7:50 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > Hi Daniel > > Thanks for the patch. Some minor nits. > > Feel free to ignore it if you prefer your style. > Sure, that's also a nice way to do it. I'm okay with either one, whatever the media subsystem maintainers prefer. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: uvcvideo: Override default flags 2024-06-03 11:49 ` Ricardo Ribalda 2024-06-05 7:12 ` Daniel Schaefer @ 2024-06-16 23:20 ` Laurent Pinchart 1 sibling, 0 replies; 4+ messages in thread From: Laurent Pinchart @ 2024-06-16 23:20 UTC (permalink / raw) To: Ricardo Ribalda Cc: Daniel Schaefer, linux-media, Edgar Thier, Kieran Bingham, Mauro Carvalho Chehab, Kieran Levin On Mon, Jun 03, 2024 at 01:49:51PM +0200, Ricardo Ribalda wrote: > Hi Daniel > > Thanks for the patch. Some minor nits. > > Feel free to ignore it if you prefer your style. > > On Sun, 2 Jun 2024 at 08:52, Daniel Schaefer <dhs@frame.work> wrote: > > > > When the UVC device has a control that is readonly it doesn't set the > > SET_CUR flag. For example the privacy control has SET_CUR flag set in > > the defaults in the `uvc_ctrls` variable. Even if the device does not > > have it set, it's not cleared by uvc_ctrl_get_flags. > > > > Originally written with assignment in commit 859086ae3636 ("media: > > uvcvideo: Apply flags from device to actual properties"). But changed to > > |= in commit 0dc68cabdb62 ("media: uvcvideo: Prevent setting unavailable > > flags"). It would not clear the default flags. > > > > With this patch applied the correct flags are reported to user space. > > Tested with: > > > > ``` > > > v4l2-ctl --list-ctrls | grep privacy > > privacy 0x009a0910 (bool) : default=0 value=0 flags=read-only > > ``` > > > > Cc: Edgar Thier <info@edgarthier.net> > > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Kieran Levin <ktl@frame.work> > > Signed-off-by: Daniel Schaefer <dhs@frame.work> > Fixes: 0dc68cabdb62 ("media: uvcvideo: Prevent setting unavailable flags") > > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > drivers/media/usb/uvc/uvc_ctrl.c | 26 +++++++++++++++++--------- > > 1 file changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > > index 4b685f883e4d..f50542e26542 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -2031,15 +2031,23 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, > > else > > ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, > > dev->intfnum, info->selector, data, 1); > > - if (!ret) > > - info->flags |= (data[0] & UVC_CONTROL_CAP_GET ? > > - UVC_CTRL_FLAG_GET_CUR : 0) > > - | (data[0] & UVC_CONTROL_CAP_SET ? > > - UVC_CTRL_FLAG_SET_CUR : 0) > > - | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ? > > - UVC_CTRL_FLAG_AUTO_UPDATE : 0) > > - | (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS ? > > - UVC_CTRL_FLAG_ASYNCHRONOUS : 0); > > + if (!ret) { > > + info->flags = (data[0] & UVC_CONTROL_CAP_GET) > > + ? (info->flags | UVC_CTRL_FLAG_GET_CUR) > > + : (info->flags & ~UVC_CTRL_FLAG_GET_CUR); > > + > > + info->flags = (data[0] & UVC_CONTROL_CAP_SET) > > + ? (info->flags | UVC_CTRL_FLAG_SET_CUR) > > + : (info->flags & ~UVC_CTRL_FLAG_SET_CUR); > > + > > + info->flags = (data[0] & UVC_CONTROL_CAP_AUTOUPDATE) > > + ? (info->flags | UVC_CTRL_FLAG_AUTO_UPDATE) > > + : (info->flags & ~UVC_CTRL_FLAG_AUTO_UPDATE); > > + > > + info->flags = (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS) > > + ? (info->flags | UVC_CTRL_FLAG_ASYNCHRONOUS) > > + : (info->flags & ~UVC_CTRL_FLAG_ASYNCHRONOUS); > > + } > > nit: I would have done it as > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 4b685f883e4d..c453a67e1407 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2031,7 +2031,12 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, > else > ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, > dev->intfnum, info->selector, data, 1); > - if (!ret) > + if (!ret) { > + info->flags &= ~(UVC_CTRL_FLAG_GET_CUR | > + UVC_CTRL_FLAG_SET_CUR | > + UVC_CTRL_FLAG_AUTO_UPDATE | > + UVC_CTRL_FLAG_ASYNCHRONOUS); > + > info->flags |= (data[0] & UVC_CONTROL_CAP_GET ? > UVC_CTRL_FLAG_GET_CUR : 0) > | (data[0] & UVC_CONTROL_CAP_SET ? > @@ -2040,6 +2045,7 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev, > UVC_CTRL_FLAG_AUTO_UPDATE : 0) > | (data[0] & UVC_CONTROL_CAP_ASYNCHRONOUS ? > UVC_CTRL_FLAG_ASYNCHRONOUS : 0); > + } I prefer that slightly too. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'll make the change in my tree, no need to send a v2. > > > > kfree(data); > > return ret; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-16 23:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-02 6:50 [PATCH] media: uvcvideo: Override default flags Daniel Schaefer 2024-06-03 11:49 ` Ricardo Ribalda 2024-06-05 7:12 ` Daniel Schaefer 2024-06-16 23:20 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox