Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification

[Cornelia Huck <cohuck@redhat.com>]

On Fri, Mar 10 2023, Alexandre Courbot <acourbot@chromium.org> wrote:

> Hi Cornelia,
>
> On Fri, Mar 10, 2023 at 7:51âPM Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> On Tue, Feb 07 2023, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> > 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:
>> >>> 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!
>>
>> Quick question: Is there any git repo or similar where interested
>> parties can follow along? It would be great to have virtio-video in 1.3;
>> if you have some idea on when it might be ready, we could come up with a
>> schedule to accommodate that.
>
> I'm glad you asked, as a matter of fact I have just finished the
> virtio-v4l2 proof of concept today! It is capable of exposing a camera
> or encoder V4L2 device from the host to the guest, by encapsulating
> V4L2 commands into virtio.

\o/ Excellent news!

>
> The guest driver code (single file for simplicity):
> https://github.com/Gnurou/linux/blob/virtio-v4l2/drivers/media/virtio-v4l2/virtio_v4l2_driver.c
>
> Bulk of the host-side crosvm device code:
> https://github.com/Gnurou/crosvm/blob/virtio-v4l2/devices/src/virtio/v4l2/protocol.rs
> https://github.com/Gnurou/crosvm/blob/virtio-v4l2/devices/src/virtio/v4l2/worker.rs
>
> Neither are works of art, so please forgive the few inefficiencies
> here and there - the goal was to make them easy to understand. Still,
> the guest driver is probably closer to what a final driver would look
> like. It fits in around 1,000 LoCs (comments excluded), which is
> enough to support stateful video encoders as well as USB camera
> devices. Decoders cannot be run yet because they require support for
> V4L2 events and polling - I will try to enable these features next.
> But even in its current state this driver shows one interesting aspect
> of virtio-v4l2, at least for Linux guests: a single and relatively
> simple driver is able to drive a wide range of devices.

I had a quick look at the driver; it indeed looks like a big win on
Linux systems. (The one thing I'm missing is how easy it would be to
replicate the used v4l2 parts on non-Linux systems.)

>
> The crosvm device code proxies a V4L2 device on the host, again using
> roughly 1,200 lines of non-comment code. This design does not intend
> to reflect what an actual host device will look like - in effect they
> should be much more specialized since they are unlikely to also call
> into V4L2 on the host side. However, if the host is Linux and we just
> want to expose a USB camera or other V4L2 device almost as-is, then
> this could actually be a good fit.
>
> The protocol should be easy to understand by looking at the code - we
> only have 5 virtio commands to open/close a session, map/unmap a
> host-allocated buffer into the guest PAS, and the IOCTL command which
> sends V4L2 ioctl structures to the host and waits for its reply. All
> ioctls are synchronous per-session, meaning that a session only sends
> one ioctl at a time and waits for its response before it can send the
> next (as this is what user-space does too). Ioctls, however, never
> block on the host side and the ones that would do (DQBUF and DQEVENT)
> are replaced by host-initiated events. On top of being familiar to
> people who have worked with V4L2 (i.e. a large portion of the media
> folks), this simple design seems to be efficient as I have observed
> identical performance on both host and guest with the vicodec virtual
> encoder. Since this device generates frames using the CPU and keeps
> one core 100% busy, any overhead introduced by virtualization should
> be noticeable - yet I got nearly identical framerates on both host and
> guest.

I haven't worked with v4l2, but this approach sounds reasonable to me.

>
> Things that still need to be implemented before this can be considered
> more complete:
>
> * Controls. This should not be particularly difficult but I left it
> for now as they are not necessary to demonstrate the viability of this
> project.
> * Guest-allocated memory buffers and virtio objects. Based on our
> previous experience with virtio-video these should not be difficult to
> implement. Currently all video buffers are allocated by the host, and
> mapped into the guest if needed.
> * Events and polling, required to use a decoder. Again these were not
> strictly necessary for the proof of concept, but since we've gone this
> far I will try to get them to work as the next step.
> * Requests and multi-part media devices. This will be necessary in
> order to support more modern camera pipelines. I haven't made up my
> mind yet about whether we should support this, but if we want to it
> should not be too hard (describe several devices in the configuration
> space and enable the request-related commands). I need to talk to
> camera folks to know whether there is an actual interest in this.
> * Support for more ioctls, in case we want to support tuners and radios.
>
> If you want to try this code, you need to build the guest kernel with
> CONFIG_VIRTIO_V4L2 and enable the `v4l2` feature when building crosvm
> (check out the Book of Crosvm if you need instructions on how to build
> and use it). Then pass --virtio-v4l2=/dev/videoX to crosvm in order to
> expose the /dev/videoX host V4L2 device to the guest.
>
> I have successfully captured frames (and verified their validity)
> using the following devices:
>
> * A simple USB camera using the `uvcvideo` driver. Both host and guest
> could capture a MJPEG stream with the following command:
> v4l2-ctl -d0 -v pixelformat=MJPG --stream-mmap --stream-to=test.mjpg
>
> * The vivid virtual camera driver. I could capture a valid YUV stream
> using the following command:
> v4l2-ctl -d0 -v pixelformat=NV12 --stream-mmap --stream-to test.yuv
>
> * The encoder device of the vicodec virtual codec driver. On both host
> and guest, the following command produces a valid FWHT stream in
> `test.fwht`:
> v4l2-ctl -x pixelformat=NV12 --stream-mmap --stream-out-mmap
> --stream-to-hdr test.fwht

This looks very good already.

>
> By this work I hope to demonstrate to people interested in video
> virtualization that encapsulating V4L2 in virtio is not only a viable
> solution, it is a huge shortcut in terms of specification crafting,
> driver writing, and overall headaches involved in specifying something
> as complex as a video device. Not only could we support video decoders
> and encoders, which was the goal of virtio-video, we would also get
> image processors, video overlays and simple cameras for free, and
> potentially more complex cameras if we decide to.
>
> After writing this prototype (and a couple attempts at the
> virtio-video specification) I don't see any reason not to rely on a
> battle-tested protocol instead of designing our own that does
> basically the same thing. The genericity of V4L2 may mean that
> sometimes we will need 2 commands where virtio-video would require
> only one, but we are talking about a low frequency of virtio commands
> (60 fps for video playback typically) and that genericity comes with
> the benefit of a single Linux guest driver.
>
> If there is an agreement to move forward with this, I guess the next
> step for me will be to write a proper spec so the protocol can be
> understood and discussed in detail. Then why not try and upstream the
> kernel driver and make ChromeOS use this too in place of our
> heavily-patched virtio-video. :) We might even make it for virtio 1.3.
>
> Looking forward to your feedback. Please don't hesitate to ask
> questions, especially if you are not familiar with V4L2. I can also
> help folks interested in running this with the setup if needed.

Thank you for sharing your work! I think this looks very promising, and
I'd like to hear feedback from others as well. I assume that would make
the spec change more digestible than earlier versions.