Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Tomasz Figa]

Hi Laurent,

On Thu, Oct 26, 2017 at 12:48 AM, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> Hello,
>
> On Monday, 23 October 2017 11:45:01 EEST Alexandre Courbot wrote:
>> On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@xxxxxx> wrote:
>> > On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
[snip]
>> > Both request and jobs APIs have the concept of a request, or a job, which
>> > is created by the user and then different buffers or controls can be bound
>> > to that request. (Other configuration isn't really excluded but it's
>> > non-trivial to implement in practice.) This is common for both.
>>
>> Yes, the main difference being that the current proposal manages the
>> jobs flow implicitly by default, to ease the most common uses of this
>> API (codecs & camera). It still maintains the ability to control them
>> more finely similarly to the previous request API proposals.
>
> Implicit job handling might make your codec use-cases simpler, but it does
> *not* ease camera support.

I'd appreciate some example use cases for explicit job handling that
would benefit the camera.

[snip]
>> >   Also --- when an association with a video devnode file descriptor and a
>> >   job with a request is made, when does it cease to exist? When the job is
>> >   released? When the job is done?
>>
>> Association is made between a job queue (to which an undefined number
>> of jobs can be queued) and a set of device nodes. A job queue remains
>> active as long as its file descriptor is not closed. So the short
>> answer to your question is that the devnode remains part of the queue
>> until the file descriptor obtained by opening /dev/v4l2_jobqueue (and
>> initialized using VIDIOC_JOBQUEUE_INIT) is opened. This is of course
>> subject to change if /dev/v4l2_jobqueue disappears, but I would like
>> to retain the idea of managing the jobs queue via its own file
>> descriptor.
>
> This makes it quite inconvenient to change controls both through a request and
> directly, a userspace application would need to open subdevs and video nodes
> twice, keeping one "direct" fd and associating the other one with the queue.
> That's a complexity increase for userspace without any advantage in my
> opinion.

There is the .which field within v4l2_ext_controls struct, which could
be used to determine whether the operation applies to a request or
directly.

[snip]
>> > Another matter is making videobuf2 helpful here: we should have, if not in
>> > the videobuf2 framework itself, then around it helper function(s) to
>> > manage the submission of buffers to a driver. You can get things working
>> > pretty easily but the error handling is very painful: what do you do, for
>> > instance, with buffers queued with a request if queueing the request
>> > itself fails, possibly because the user hasn't provided enough buffers
>> > with the request? Mark the buffers errorneous and return them to the user?
>> > Probably so, but that requires the user to dequeue the buffers and gather
>> > the request again. I presume this would only happen in special
>> > circumstances though, and not typically in an application using requests.
>> > This, and many other special cases still must be handled by the kernel.
>>
>> Error handling is still pretty weak in that version. I would like to
>> get an overall agreement on the general direction before looking at
>> this more closely though, as I suppose getting things right will take
>> some time.
>
> I believe error handling would be much simpler if we passed all request
> parameters through a single ioctl, as we would have one clear point where to
> perform validation with all required information available.

Compared to fine granularity of particular ioctls, this would make
determining the error cause more difficult or at least would require
providing a special interface to userspace just to communicate the
exact reason of the failure.

Best regards,
Tomasz