Le samedi 04 août 2018 à 15:50 +0200, Hans Verkuil a écrit :
> 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?
> 
> 3) Some controls may be required in each request, how to let userspace know this?
>    Is it even necessary to inform userspace?

For state-less codec, there is a very strict set of controls that must
be supported / filled. The data format pretty much dictate this.

For complex camera's and video transformation m2m devices, there is a
gap indeed. Duplicating the formats for this case does not seem like
the right approach.

> 
> 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.

In GStreamer, we call REQBUFS(type, count=0) for each types we support.
This call should never fail, unless the type is not supported. We build
a list of supported I/O mode this way. It's also a no-op, because we
didn't allocate any buffers yet.

> 
> 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
> #define V4L2_BUF_CAP_REQUIRES_REQUESTS	0x00000002
> #define V4L2_BUF_CAP_HAS_MMAP		0x00000100
> #define V4L2_BUF_CAP_HAS_USERPTR	0x00000200
> #define V4L2_BUF_CAP_HAS_DMABUF		0x00000400

Looks similar to the bit map we create inside GStreamer using the
described technique. Though we also add HAS_CREATE_BUFS to the lot.

My main concern is in userspace like GStreamer, the difficulty is to
sort drivers that we support, from the ones that we don't. So if we
don't support requests yet, we would like to detect this early. As
CODEC don't really have an initial format, I believe that before S_FMT,
any kind of call to REQBUFS might fail at the moment. So detection
would be very late.

Thoughm be aware this is a totally artificial issue in the short term
since state-less CODEC uses dedicated formats.

> 
> 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.
> 
> 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 :-) ).

Just a clarification, the doc is pretty clear the MMAP is supported if
the device capability have STREAMING in 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.
> 
> 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.
> 
> I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed here.
> 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.

Right, I don't think this is needed in the short term, as we target
only stateless CODEC. But this is important use case for let's say
request to cameras. When we get there, we will need a mechnism to
list all the controls that can be included in a request, and also all
the only the must be present (if any).

> 
> 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.

I'd prefer to say it's not urgent.

> 
> Comments? Ideas?
> 
> Regards,
> 
> 	Hans