Hi Ricardo,
On 18-Nov-24 5:47 PM, Ricardo Ribalda wrote:
Hi Hans
On Mon, 18 Nov 2024 at 16:43, Hans de Goede hdegoede@redhat.com wrote:
Hi All,
On 15-Nov-24 9:20 AM, Ricardo Ribalda wrote:
On Fri, 15 Nov 2024 at 00:06, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
<snip>
How do you handle cameras that suffer from UVC_QUIRK_PRIVACY_DURING_STREAM ?
For those b) does not work.
I already suspected as much, but it is good to have this confirmed.
I'm afraid that from a userspace API pov cameras with a GPIO which only works when powered-on need to be treated the same as cameras which only have UVC_CT_PRIVACY_CONTROL IOW in this case keep exporting V4L2_CID_PRIVACY instead of switching to evdev with SW_CAMERA_LENS_COVER.
Unfortunately this will make the GPIO handling code in the UVC driver somewhat more involved since now we have both uAPI-s for GPIOs depending on UVC_QUIRK_PRIVACY_DURING_STREAM.
But I think that this makes sense, this way we end up offering 2 uAPIs depending on the hw capabilities:
- evdev with SW_CAMERA_LENS_COVER which always reports a reliable
state + events on the state changing without needing to power-up the camera.
- V4L2_CID_PRIVACY for the case where the camera needs to be
powered-on (/dev/video opened) and where the ctrl possibly needs to be polled.
Assuming we can all agree on this split based on hw capabilities I think that we must document this somewhere in the media subsystem documentation. We can then also write down there that SW_CAMERA_LENS_COVER only applies to internal cameras.
I do not think that it is worth it to keep UVC_CT_PRIVACY_CONTROL for the two devices that have connected the GPIO's pull up to the wrong power rail. Now that the GPIO can be used from userspace, I expect that those errors will be found early in the design process and never reach production stage.
If we use UVC_CT_PRIVACY_CONTROL for thes two devices:
- userspace will have to implement two different APIs
- the driver will have to duplicate the code.
- all that code will be very difficult to test: there are only 2
devices affected and it requires manual intervention to properly test it.
I think that UVC_QUIRK_PRIVACY_DURING_STREAM is a good compromise and the main user handles it properly.
Ok, as you wish. Lets go with using SW_CAMERA_LENS_COVER for the 2 models with UVC_QUIRK_PRIVACY_DURING_STREAM too.
<snip>
Is there any ACPI- or WMI-provided information that could assist with associating a privacy GPIO with a camera ?
I just realized I did not answer this question from Laurent in my previous reply.
No unfortunately there is no ACPI- or WMI-provided information that could assist with associating ACPI/WMI camera privacy controls with a specific camera. Note that these are typically not exposed as a GPIO, but rather as some vendor firmware interface.
Thinking more about this I'm starting to believe more and more that the privacy-control stuff should be handled by libcamera and then specifically by the pipeline-handler, with some helper code to share functionality where possible.
E.g. on IPU6 equipped Windows laptops there may be some ACPI/WMI driver which provides a /dev/input/event# SW_CAMERA_LENS_COVER node.
So I would expect the IPU6 pipeline-handler to search for such a /dev/input/event# node and then expose that to users of the camera through a to-be-defined API (I'm thinking a read-only control).
The code to find the event node can be shared, because this would e.g. likely also apply to some IPU3 designs as well as upcoming IPU7 designs.
<snip>
We could include the evdev in the MC graph. That will of course only be possible if the kernel knows about that association in the first place. At least the 1st category of devices would benefit from this.
Yes I was thinking about adding a link to the MC graph for this too.
Ricardo I notice that in this v3 series you still create a v4l2-subdev for the GPIO handling and then add an ancillary link for the GPIO subdev to the mc-graph. But I'm not sure how that is helpful. Userspace would still need to do parent matching, but then match the evdev parent to the subdev after getting the subdev from the mc. In that case it might as well look at the physical (USB-interface) parent of the MC/video node and do parent matching on that avoiding the need to go through the MC at all.
I think using the MC could still be useful by adding a new type of ancillary link to the MC API which provides a file-path as info to userspace rather then a mc-link and then just directly provide the /dev/input/event# path through this new API?
I guess that extending the MC API like this might be a bit of a discussion. But it would already make sense to have this for the existing input device for the snapshot button.
The driver creates a v4l2-subdevice for every entity, and the gpio today is modeled as an entity.
Ok I see that explains why the subdevice is there, thank you.
The patchset just adds an ancillary link as Sakari suggested. I am not against removing the gpio entity all together if it is not needed.
Right unlike other entities which are really part of the UVC specification, the GPIO is not a "real" UVC entity.
So I wonder if, after switching to SW_CAMERA_LENS_COVER, having this as a v4l2-subdevice buys us anything ? If not I think removing it might be a good idea.
As for the ancillary link, that was useful to have when the API was a v4l2-ctrl on the subdevice. Just like I doubt if having the subdevice at all gives us any added value, I also doubt if having the ancillary link gives us any added value.
Now that we are brainstorming here... what about adding a control that contains the name of the input device (eventX)? Is that a horrible idea?
I don't know, my initial reaction is that does not feel right to me.
We can specify that SW_CAMERA_LENS_COVER only applies to device internal cameras, but then it is up to userspace to determine which cameras that are.
I am working on wiring up this to userspace right now.. I will report back if it cannot do it.
Ricardo, great, thank you!
Ricardo, any status update on this ?
<snip>
Assuming the kernel could report the association between an evdev and camera, we would need to report which evdev SW_CAMERA_LENS_COVER originates from all the way from the evdev to the consumer of the event. How well is that supported in standard Linux system architectures ? If I'm not mistaken libinput will report the originating device, but how far up the stack is it propagated ? And which component would we expect to consume those events, should the camera evdev be managed by e.g. libcamera ?
Good questions. Looking back at our 2 primary use-cases:
a) Having an app which is using (trying to use) the camera show a notification to the user that the camera is turned-off by a privacy switch .
b) Showing on on-screen-display (OSD) with a camera / crossed-out-camera icon when the switch is toggled, similar to how muting speakers/mic show an OSD . Laptop vendor Windows add-on software does this and I know that some users have been asking for this.
I think we have everything to do b) in current compositors like gnome-shell. Using an evdev with SW_CAMERA_LENS_COVER would even be a lot easier for b) then the current V4L2_CID_PRIVACY API.
a) though is a lot harder. We could open up access to the relevant /dev/input/event# node using a udev uaccess tag so that users who can access /dev/video# nodes also get raw access to that /dev/input/event# node and then libcamera could indeed provide this information that way. I think that is probably the best option.
At least for the cases where the camera on/off switch does not simply make the camera completely disappear.
That case is harder. atm that case is not handled at all though. So even just getting b) to work for that case would be nice / an improvement.
Eventually if we give libcamera access to event# nodes which advertise SW_CAMERA_LENS_COVER (and no other privacy sensitive information) then libcamera could even separately offer some API for apps to just get that value if there is no camera to associate it with.
Actually thinking more about it libcamera probably might be the right place for some sort of "no cameras found have you tried hitting your camera privacy-switch" API. That is some API to query if such a message should be shown to the user. But that is very much future work.
Are standard apps expected to use libcamera directly or they should use pipewire? Maybe a) Should be pipewire's task?
Standard apps are supposed to use pipewire, but IMHO this is really too low-level for pipewire to handle itself.
Also see my remarks above about how I think this needs to be part of the pipeline handler. Since e.g. associating a /dev/input/event# SW_CAMERA_LENS_COVER node with a specific UVC camera is going to be UVC specific solution.
For other pipeline-handlers combined with vendor fw-interfaces offering SW_CAMERA_LENS_COVER support I do not think that there is going to be a way to actually associate the 2. So we will likely simply have the pipeline handler for e.g. IPU6 simply associate any SW_CAMERA_LENS_COVER with the normal (non IR) user facing camera.
Since we need this different ways to map a /dev/input/event# SW_CAMERA_LENS_COVER node to a specific camera this really needs to be done in libcamera IMHO.
And I think this also solves the question about needing a kernel API to associate the /dev/input/event# with a specific /dev/video#. At least for now I think we don't need an API and instead we can simply walk sysfs to find the common USB-interface parent to associate the 2.
See how xawtv associates the alsa and /dev/video# parts of tv-grabber cards for an example.
Regards,
Hans