Hi Hans, On Wednesday, 25 October 2017 19:19:27 EEST Hans Verkuil wrote: > On 10/25/2017 05:48 PM, Laurent Pinchart 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: > >>>> Hi everyone, > >>>> > >>>> Here is a new attempt at the "request" (which I propose to rename > >>>> "jobs") > >>>> API for V4L2, hopefully in a manner that can converge to something that > >>>> will be merged. The core ideas should be easy to grasp for those > >>>> familiar with the previous attemps, yet there are a few important > >>>> differences. > >>>> > >>>> Most notably, user-space does not need to explicitly allocate and > >>>> manage > >>>> requests/jobs (but still can if this makes sense). We noticed that only > >>>> specific use-cases require such an explicit management, and opted for a > >>>> jobs queue that controls the flow of work over a set of opened devices. > >>>> This should simplify user-space code quite a bit, while still retaining > >>>> the ability to manage states explicitly like the previous request API > >>>> proposals allowed to do. > >>>> > >>>> The jobs API defines a few new concepts that user-space can use to > >>>> control the workflow on a set of opened V4L2 devices: > >>>> > >>>> A JOB QUEUE can be created from a set of opened FDs that are part of a > >>>> pipeline and need to cooperate (be it capture, m2m, or media controller > >>>> devices). > >>>> > >>>> A JOB can then be set up with regular (if slightly modified) V4L2 > >>>> ioctls, > >>>> and then submitted to the job queue. Once the job queue schedules the > >>>> job, its parameters (controls, etc) are applied to the devices of the > >>>> queue, and itsd buffers are processed. Immediately after a job is > >>>> submitted, the next job is ready to be set up without further user > >>>> action. > >>>> > >>>> Once a job completes, it must be dequeued and user-space can then read > >>>> back its properties (notably controls) at completion time. > >>>> > >>>> Internally, the state of jobs is managed through STATE HANDLERS. Each > >>>> driver supporting the jobs API needs to specify an implementation of a > >>>> state handler. Fortunately, most drivers can rely on the generic state > >>>> handler implementation that simply records and replays a job's > >>>> parameter > >>>> using standard V4L2 functions. Thanks to this, adding jobs API support > >>>> to a driver relying on the control framework and vb2 only requires a > >>>> dozen lines of codes. > >>>> > >>>> Drivers with specific needs or opportunities for optimization can > >>>> however > >>>> provide their own implementation of a state handler. This may in > >>>> particular be beneficial for hardware that supports configuration or > >>>> command buffers (thinking about VSP1 here). > >>>> > >>>> This is still very early work, and focus has been on the following > >>>> points: > >>>> > >>>> * Provide something that anybody can test (currently using vim2m and > >>>> vivid), > >>>> * Reuse the current V4L2 APIs as much as possible, > >>>> * Remain flexible enough to accomodate the inevitable changes that will > >>>> be requested, > >>>> * Keep line count low, even if functionality is missing at the moment. > >>>> > >>>> Please keep this in mind while going through the patches. In > >>>> particular, > >>>> at the moment the parameters of a job are limited to integer controls. > >>>> I > >>>> know that much more is expected, but V4L2 has quite a learning curve > >>>> and > >>>> I preferred to focus on the general concepts for now. More is coming > >>>> though! :) > >>>> > >>>> I have written two small example programs that demonstrate the use of > >>>> this API: > >>>> > >>>> - With a codec device (vim2m): > >>>> https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42 > >>>> > >>>> - With a capture device (vivid): > >>>> https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04 > >>>> > >>>> Considering the history with the request API, I don't expect everything > >>>> proposed here to be welcome or understood immediately. In particular I > >>>> apologize for not reusing any of the previous attempts - I was just > >>>> more > >>>> comfortable laying down my ideas from scratch. > >>>> > >>>> If this proposal is not dismissed as complete garbage I will also be > >>>> happy to discuss it in-person at the mini-summit in Prague. :) > >>> > >>> Thank you for the initiative and the patchset. > >>> > >>> While reviewing this patchset, I'm concentrating primarily on the > >>> approach > >>> taken and the design, not so much in the actual implementation which I > >>> don't think matters much at this moment. > >> > >> Thanks, that is exactly how I hoped things would go for the moment. > >> > >>> It's difficult to avoid seeing many similarities with the Request API > >>> patches posted earlier on. And not only that, rather you have to start > >>> looking for the differences in what I could call details, while > >>> important > >>> design decisions could sometimes be only visible in what appear details > >>> at > >>> this point. > >> > >> I was not quite sure whether I should base this work on one of the > >> existing patchsets (and in this case, which one) or start from > >> scratch. This being my first contribution to a new area of the kernel > >> for me, I decided to start from scratch as it would yield more > >> educative value. > > > > What bothers me here is that we had a full day face to face meeting in > > Tokyo back in June about this, where you presented this idea of how the > > userspace API could look like. We went through the proposal point by > > point to discuss potential issues, and for most points I recall agreeing > > that the changes proposed compared to the previous request API RFC were > > introducing problems that couldn't be easily solved. I walked out of the > > meeting understanding we had an agreement to go back to an API quite > > similar to the previous RFC, in particular with explicit request object > > management from userspace, and I now see several months later an RFC that > > ignores all the conclusions of our meeting. > > Laurent, can you give a link to that RFC? Just so we all refer to the same > request API RFC. That will help with the discussion tomorrow. Sure, it's available at https://www.spinics.net/lists/linux-sh/msg48289.html. Sakari has posted two newer versions based on that original series. > Two notes: my hope is that by the end of Friday we have a public API that is > good enough for the codec use case and can be extended for the more generic > use case. We have codec drivers waiting to be mainlined for years now that > are blocked because of this. It's getting ridiculous. That also means that > I don't care all that much about the kernel API: ideally it will be > suitable or extensible for the generic use case, but if it isn't and needs > to be reworked substantially for the generic use case, then so be it. > Obviously this is something I hope we can avoid, but it would be much worse > if the codec vendors would move away from V4L2 and start using other > suboptimal solutions just because we can't reach an agreement. > > As I said in the beginning: we don't have that option for the public API: > that does need some careful thought as we cannot change that later, we can > only extend it. I agree with your overall, we need to move forward on this. Focussing on the userspace API first is the way to go according to me as well. I wouldn't say that I don't care about the kernel API, but that's indeed easier to change. I'd like to share your optimism about what we will achieve by the end of Friday, and I would certainly love to be proven wrong when I'm pessimistic :) Let's try our best. > Second note (just to throw it in the discussion): I've always thought that > controls are a good way to store state, including things like formats. > > Having a single framework taking care of that would simplify things > substantially. With VIDIOC_S_EXT_CTRLS you can already set a large number > of controls in one ioctl. To some extent I agree with you, but I don't think we should base the API on VIDIOC_S_EXT_CTRLS, nor the implementation on the control framework as it exists today. That's a topic to be discussed tomorrow :-) > This does require some (or a lot?) refactoring of the control framework as > you rightly mentioned in your email, but I'm willing to do that work. Thank you. -- Regards, Laurent Pinchart
