[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Tue, Feb 07 2023, Alexandre Courbot <acourbot@chromium.org> wrote: > On Mon, Feb 6, 2023 at 11:13 PM Cornelia Huck <cohuck@redhat.com> wrote: >> >> On Thu, Jan 19 2023, Alexander Gordeev <alexander.gordeev@opensynergy.com> wrote: >> >> > Hi Alexandre, >> > >> > On 12.01.23 07:39, Alexandre Courbot wrote: >> >> On Thu, Jan 12, 2023 at 3:42 AM Alexander Gordeev >> >> <alexander.gordeev@opensynergy.com> wrote: >> >> >>> Well, on the one hand mimicking v4l2 looks like an easy solution from >> >>> virtio-video spec writing perspective. (But the implementers will have >> >>> to read the V4L2 API instead AFAIU, which is probably longer...) >> >> It should not necessarily be much longer as the parts we are >> >> interested in have their own dedicated pages: >> >> >> >> https://docs.kernel.org/userspace-api/media/v4l/dev-decoder.html <https://docs.kernel.org/userspace-api/media/v4l/dev-decoder.html>https://docs.kernel.org/userspace-api/media/v4l/dev-encoder.html <https://docs.kernel.org/userspace-api/media/v4l/dev-encoder.html> >> >> >> >> Besides, the decoding and encoding processes are described with more >> >> precision, not that we couldn't do that here but it would make the >> >> spec grow longer than I am comfortable with... >> > >> > I read the references carefully, thanks. I am somewhat familiar with the >> > stateful decoder API, but the stateless one still needs exploring. >> > >> > There is one serious issue with these references IMO: they represent >> > guest user-space <-> v4l2 subsystem API, not v4l2 subsystem <-> >> > virtio-video driver API. Just to make sure we're on the same page: >> > >> > guest user-space <-> v4l2 kernel subsystem <-> virtio-video driver <- >> > virtio-video protocol -> virtio-video device. >> > >> > I believe this is how it is supposed to work, right? So I thought, that >> > your intention is to simplify virtio-video driver and virtio-video >> > protocol by reusing the v4l2 subsystem <-> v4l2 driver API. But having >> > these references I can assume, that you want to use user-space <-> v4l2 >> > subsystem API, right? Well, I think this cannot happen and therefore >> > these references cannot be used directly unless: >> > >> > 1. You suggest that virtio-video driver should not use v4l2 subsystem, >> > but should mimic its user-space API in every detail. Probably not a good >> > idea. >> > >> > 2. There is already a way to bypass the subsystem completely. I'm not >> > aware of that. >> > >> > 3. user-space <-> v4l2 subsystem API is already the same or very close >> > to v4l2 subsystem <-> v4l2 driver API. I believe this is not the case >> > even with stateful decoder/encoder. Even more with stateless decoders >> > because I can see, that v4l2 subsystem actually stores some state in >> > this case as well. Which is quite reasonable I think. >> > >> > So I think what we need to reference here is v4l2 subsystem <-> v4l2 >> > driver API. Do you have this reference? Well, I know there is some >> > documentation, but still I doubt that. AFAIR kernel internal APIs are >> > never fixed. Right? >> >> So, I'm not that familiar with v4l2, but if that's indeed the case, >> depending on some kernel internal APIs is a no-go. First, because >> in-kernel APIs are not stable, and second, because we want something >> that's BSD-licenced (as opposed to GPLv2-licenced) to point to. The >> kernel<->userspace API would work (BSD-licenced header and stable); I >> had the impression that we wanted to reuse the various #defines in >> there -- did I misunderstand? > > Sorry, I should have replied earlier to lift any misunderstanding. I > am not suggesting to use any kernel internal API as reference. My > suggestion is to stick strictly to the UAPI which is stable (as in, > guaranteed to be backward-compatible) and well documented. Here is for > instance the part documenting buffer queuing/dequeuing: > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-qbuf.html > > Changing anything in this documentation in a backward-incompatible way > would break user-space, and everyone familiar with the kernel > community knows what happens when someone breaks user-space. ;) So > this can be a reliable source for virtio-video (and if you look > closely, you will also notice many similarities between the two). Yes, that sounds good to me :) > >> >> (...) >> >> >> Let me try to summarize the case for using V4L2 over Virtio (I'll call >> >> it virtio-v4l2 to differentiate it from the current spec). >> >> >> >> There is the argument that virtio-video turns out to be a recreation >> >> of the stateful V4L2 decoder API, which itself works similarly to >> >> other high-level decoder APIs. So it's not like we could or should >> >> come with something very different. In parallel, virtio-camera is also >> >> currently using V4L2 as its model. While this is subject to change, I >> >> am starting to see a pattern here. :) >> >> >> >> Transporting V4L2 over virtio would considerably shorten the length of >> >> this spec, as we would just need to care about the transport aspect >> >> and minor amendments to the meaning of some V4L2 structure members, >> >> and leave the rest to V4L2 which is properly documented and for which >> >> there is a large collection of working examples. >> >> >> >> This would work very well for codec devices, but as a side-effect >> >> would also enable other kinds of devices that may be useful to >> >> virtualize, like image processors, DVB cards, and cameras. This >> >> doesn't mean virtio-v4l2 should be the *only* way to support cameras >> >> over virtio. It is a nice bonus of encapsulating V4L2, it may be >> >> sufficient for simple (most?) use-cases, but also doesn't forbid more >> >> specialized virtual devices for complex camera pipelines to be added >> >> later. virtio-v4l2 would just be the generic virtual video device that >> >> happens to be sufficient for our accelerated video needs - and if your >> >> host camera is a USB UVC one, well feel free to use that too. >> >> >> >> In other words, I see an opportunity to enable a whole class of >> >> devices instead of a single type for the same effort and think we >> >> should seriously consider this. >> >> >> >> I have started to put down what a virtio-v4l2 transport might look >> >> like, and am also planning on putting together a small >> >> proof-of-concept. If I can get folks here to warm up to the idea, I >> >> believe we should be able to share a spec and prototype in a month or >> >> so. >> > >> > Thanks for the detailed explanation. Please check my comments above. I'd >> > like to resolve the mentioned issue first. >> >> I hope we can sort this out soon -- I guess I'm not the only one who is >> anxious about this spec moving forward :) Please let me know if I can >> help in any way. > > I'll try to address Alexander's points in more detail, but I am not > seeing any blocking issue with using the V4L2 UAPI as the basis for > virtio-video (we are working on a small proof-of-concept and things > are going smoothly so far). Great to hear, looking forward to it!
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]