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