Re: [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a subdevice

Ricardo Ribalda
2024-11-10T10:32:31Z

Hi Mauro

On Sun, 10 Nov 2024 at 11:03, Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Sat, 9 Nov 2024 17:29:54 +0100
> Ricardo Ribalda <ribalda@chromium.org> escreveu:
>
> > >
> > > I think that should sort the issue, assuming that 1. above holds true.
> > >
> > > One downside is that this stops UVC button presses from working when
> > > not streaming. But userspace will typically only open the /dev/video#
> > > node if it plans to stream anyways so there should not be much of
> > > a difference wrt button press behavior.
> >
> > I do not personally use the button, but it is currently implemented as
> > a standard HID device.
>
> IMO, controlling the privacy via evdev is the best approach then. There's
> no need for a RW control neither at subdev or at device level. It could
> make sense a Read only to allow apps to read, but still it shall be up to
> the Kernel to protect the stream if the button is pressed.
>
> > Making it only work during streamon() might be
> > a bit weird.
> > I am afraid that if there is a button we should keep the current behaviour.
>
> Privacy matters only when streaming. IMO the Kernel check for it needs to
> be done at DQBUF time and at read() calls, as one can enable/disable the
> camera while doing videoconf calls. I do that a lot with app "soft" buttons,
> and on devices that physically support cutting the video.
>
> I don't trust myself privacy soft buttons, specially when handled in userspace,
> so what I have are webcam covers (and a small stick glued at a laptop camera
> that has a too small sensor for a webcam cover). I only remove the cover/stick
> when I want to participate on videoconf with video enabled with the builtin
> camera.
>
> Regards

I think we are mixing up concepts here.

On one side we have the uvc button. You can see one here
https://www.sellpy.dk/item/2Yk1ZULbki?utm_source=google&utm_medium=cpc&utm_campaign=17610409619&gad_source=1&gclid=Cj0KCQiA0MG5BhD1ARIsAEcZtwR9-09ZtTIVNbVknrZCtCd7ezVM8YFw1yQXfs81FWhofg9eW-iBrsIaAopVEALw_wcB
That button is not represented as a hid device. We do not know how the
user will use this button. They could even use it to start an app when
pressed.

On the other side we have the privacy gpio. The chassis has a switch
that is connected to the camera and to the SOC. You can see one here:
https://support.hp.com/ie-en/document/ish_3960099-3335046-16 .We link
the camera with a gpio via the acpi table. When the user flips the
button, the camera produces black frames and the SOC gets an IRQ. The
IRQ is used to display a message like "Camera off" and the value of
the GPIO can be checked when an app is running to tell the user:
"Camera not available, flip the privacy button if you want to use it."
Userspace cannot change the value of the gpio. It is read-only,
userspace cannot override the privacy switch. The privacy gpio is
represented with a control in /dev/videoX This patchset wants to move
it to /dev/v4l2-subdevX

To make things more complicated. Recently some cameras are starting to
have their own privacy control without the need of an external gpio.
This is also represented as a control in /dev/videoX.

Now that we have these 3 concepts in place:

Today a uvc camera is powered up when /dev/videoX is open(), not when
it is streaming. This means that if we want to get an event for the
privacy gpio we have to powerup the camera, which results in power
consumption. This can be fixed by moving the control to a subdevice
(technically the gpio is not part of the camera, so it makes sense).

If we only powerup the camera during streamon we will break the uvc
button, and the async controls.

>
> Thanks,
> Mauro

--
Ricardo Ribalda