Em Sat, 4 Aug 2018 15:50:04 +0200 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > Hi all, > > While the Request API patch series addresses all the core API issues, there > are some high-level considerations as well: > > 1) How can the application tell that the Request API is supported and for > which buffer types (capture/output) and pixel formats? > > 2) How can the application tell if the Request API is required as opposed to being > optional? Huh? Why would it be mandatory? > > 3) Some controls may be required in each request, how to let userspace know this? > Is it even necessary to inform userspace? Again, why would it need to have a set of mandatory controls for requests to work? If this is really required, it should have a way to send such list to userspace. > > 4) (For bonus points): How to let the application know which streaming I/O modes > are available? That's never been possible before, but it would be very nice > indeed if that's made explicit. > > Since the Request API associates data with frame buffers it makes sense to expose > this as a new capability field in struct v4l2_requestbuffers and struct v4l2_create_buffers. > > The first struct has 2 reserved fields, the second has 8, so it's not a problem to > take one for a capability field. Both structs also have a buffer type, so we know > if this is requested for a capture or output buffer type. The pixel format is known > in the driver, so HAS/REQUIRES_REQUESTS can be set based on that. I doubt we'll have > drivers where the request caps would actually depend on the pixel format, but it > theoretically possible. For both ioctls you can call them with count=0 at the start > of the application. REQBUFS has of course the side-effect of deleting all buffers, > but at the start of your application you don't have any yet. CREATE_BUFS has no > side-effects. > > I propose adding these capabilities: > > #define V4L2_BUF_CAP_HAS_REQUESTS 0x00000001 I'm OK with that. > #define V4L2_BUF_CAP_REQUIRES_REQUESTS 0x00000002 But I'm not ok with breaking even more userspace support by forcing requests. > #define V4L2_BUF_CAP_HAS_MMAP 0x00000100 > #define V4L2_BUF_CAP_HAS_USERPTR 0x00000200 > #define V4L2_BUF_CAP_HAS_DMABUF 0x00000400 Those sounds ok to me too. > > If REQUIRES_REQUESTS is set, then HAS_REQUESTS is also set. > > At this time I think that REQUIRES_REQUESTS would only need to be set for the > output queue of stateless codecs. Same as before: I don't see the need of support a request-only driver. > > If capabilities is 0, then it's from an old kernel and all you know is that > requests are certainly not supported, and that MMAP is supported. Whether USERPTR > or DMABUF are supported isn't known in that case (just try it :-) ). > > Strictly speaking we do not need these HAS_MMAP/USERPTR/DMABUF caps, but it is very > easy to add if we create a new capability field anyway, and it has always annoyed > the hell out of me that we didn't have a good way to let userspace know what > streaming I/O modes we support. And with vb2 it's easy to implement. Yeah, that sounds a bonus to me too. > Regarding point 3: I think this should be documented next to the pixel format. I.e. > the MPEG-2 Slice format used by the stateless cedrus codec requires the request API > and that two MPEG-2 controls (slice params and quantization matrices) must be present > in each request. Makes sense to document with the pixel format... > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed here. but it sounds worth to also have a flag. > It's really implied by the fact that you use a stateless codec. It doesn't help > generic applications like v4l2-ctl or qv4l2 either since in order to support > stateless codecs they will have to know about the details of these controls anyway. Yeah, but they could skip enum those ioctls if they see one marked with V4L2_CTRL_FLAG_REQUIRED_IN_REQ and don't know how to use. Then, default to not use request API. Then, the driver would use a default that would work (even not providing the best possible compression). > So I am inclined to say that it is not necessary to expose this information in > the API, but it has to be documented together with the pixel format documentation. > > Comments? Ideas? > > Regards, > > Hans Thanks, Mauro
