| Message ID | 20210430112611.475039-1-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: uvcvideo: implement UVC 1.5 ROI | expand |
Hi Sergey, On 30/04/2021 13:26, Sergey Senozhatsky wrote: > Hello, > > This patch set implements UVC 1.5 ROI using v4l2_selection API. Is the selection API the right approach for this? Wouldn't it make sense to use controls instead? That would interface easily with the Request API allowing per-frame changes to the ROI, and with dynamic array controls (1) it allows you to provide multiple ROIs as well. The only missing piece would be MIN/MAX for compound controls, but that can easily be added to the control framework. If this was discussed before, then can you give a me pointer to that discussion? I couldn't find anything for that, but I didn't look very long for it :-) In any case, it doesn't really feel like it is the right API for this job. I really need to review this series when I have a bit more time :-( Regards, Hans (1) https://patchwork.linuxtv.org/project/linux-media/cover/20210428101841.696059-1-hverkuil-cisco@xs4all.nl/ > > v4: > -- split ROI rect selection API and auto-controls > -- handle very large ROI rectangles: limit to frame dimensions > > Sergey Senozhatsky (5): > media: v4l UAPI: add ROI selection targets > media: v4l UAPI: document ROI selection targets > media: uvcvideo: add ROI auto controls > media: v4l UAPI: document ROI auto_controls > media: uvcvideo: add UVC 1.5 ROI control > > .../media/v4l/ext-ctrls-camera.rst | 23 +++ > .../media/v4l/selection-api-configuration.rst | 22 +++ > .../media/v4l/selection-api-examples.rst | 27 +++ > .../media/v4l/v4l2-selection-targets.rst | 24 +++ > drivers/media/usb/uvc/uvc_ctrl.c | 19 ++ > drivers/media/usb/uvc/uvc_v4l2.c | 185 +++++++++++++++++- > include/uapi/linux/usb/video.h | 1 + > include/uapi/linux/v4l2-common.h | 8 + > include/uapi/linux/v4l2-controls.h | 9 + > 9 files changed, 315 insertions(+), 3 deletions(-) >
Hi Hans, On (21/04/30 14:49), Hans Verkuil wrote: > Hi Sergey, > > On 30/04/2021 13:26, Sergey Senozhatsky wrote: > > Hello, > > > > This patch set implements UVC 1.5 ROI using v4l2_selection API. > > Is the selection API the right approach for this? Wouldn't it make > sense to use controls instead? [..] > If this was discussed before, then can you give a me pointer to that discussion? > I couldn't find anything for that, but I didn't look very long for it :-) I believe Tomasz raised this question over IRC back in the days and there was no clear conclusion at the end: selection API vs control - 50/50 split. After internal discussions we decided to go with the selection API. > In any case, it doesn't really feel like it is the right API for this job. Well, we pass a rectangle to the driver. The driver already knows what to do with some of those rectangles, we teach it to handle one more. So we don't introduce anything new, but use the existing API instead.
On (21/04/30 20:26), Sergey Senozhatsky wrote: > v4: > -- split ROI rect selection API and auto-controls > -- handle very large ROI rectangles: limit to frame dimensions Please ignore this series. I just sent out v5 with UAPI fixes.
On 01/05/2021 03:58, Sergey Senozhatsky wrote: > Hi Hans, > > On (21/04/30 14:49), Hans Verkuil wrote: >> Hi Sergey, >> >> On 30/04/2021 13:26, Sergey Senozhatsky wrote: >>> Hello, >>> >>> This patch set implements UVC 1.5 ROI using v4l2_selection API. >> >> Is the selection API the right approach for this? Wouldn't it make >> sense to use controls instead? > > [..] > >> If this was discussed before, then can you give a me pointer to that discussion? >> I couldn't find anything for that, but I didn't look very long for it :-) > > I believe Tomasz raised this question over IRC back in the days and there > was no clear conclusion at the end: selection API vs control - 50/50 split. > After internal discussions we decided to go with the selection API. > >> In any case, it doesn't really feel like it is the right API for this job. > > Well, we pass a rectangle to the driver. The driver already knows what > to do with some of those rectangles, we teach it to handle one more. So > we don't introduce anything new, but use the existing API instead. > Yes, but this works for only one ROI since the Selection API has no provision for rectangle arrays, but with the upcoming dynamic array control support this is trivial for controls. In addition, controls are already integrated in the Request API, so this will automatically work with requests as well. I don't remember being involved in the irc discussion (if I was, I don't remember it), and that discussion definitely did not know about dynamic arrays since that's brand new and not even merged yet and may even precede the Request API depending on how long ago the irc discussion was. I think being able to provide multiple ROI rectangles is an obvious feature in general, even if UVC currently supports only a single rectangle. Ditto for being able to use them in a request. You get both for free when using controls. Before I can accept this series I think I need to have feedback from others why they believe the Selection API is the right API to use. Regards, Hans