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

[Alexandre Courbot <acourbot@chromium.org>]

Thanks for the feedback Albert!

On Mon, Mar 27, 2023 at 10:00âPM Albert Esteve <aesteve@redhat.com> wrote:
>
>
>
>
> On Tue, Mar 14, 2023 at 6:06âAM Alexandre Courbot <acourbot@chromium.org> wrote:
>>
>> Hi Cornelia,
>>
>> On Fri, Mar 10, 2023 at 11:20âPM Cornelia Huck <cohuck@redhat.com> wrote:
>> >
>> > 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!
>>
>> I am delighted that you seem to like it!
>>
>> >
>> > >
>> > > 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.)
>>
>> For non-Linux systems (host or guest, we may need to copy/paste or
>> reproduce the UAPI structures. Thankfully they are unambiguously
>> described in the UAPI documentation, see for example
>> https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-enum-fmt.html
>> for the formats structure.
>>
>> >
>> > >
>> > > 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.
>>
>> Turns out that ffmpeg can also be used to capture video and encode it
>> in a more convenient format, e.g:
>>
>> ffmpeg -f v4l2 -i /dev/video0 output.mkv
>>
>> This is a very good sign with respect to compliance with V4L2
>> protocols. We had a very hard time getting the original virtio-video
>> to be used with common tools and passing the V4L2 compliance test in
>> general, but with this approach it seems like it will be much simpler
>> to achieve.
>
>
> I was curious, and I have tried the prototype. I was gladly surprised at how nice
> it behaves in this state. Really nice work.
>
> I have managed to expose different devices to the host and run compliance tests,
> capture frames from a camera, and encode by doing:
>
> # v4l2-ctl -d0 -x width=640,height=480 -v width=640,height=480,pixelformat=FWHT \
>                 --stream-mmap --stream-out-mmap \
>                 --stream-from in-640-480.YU12 --stream-to output.fwht

Just a small update: I have updated the device and driver to handle
V4L2 events and polling and the vicodec decoder is now also working.

Instructions are mostly unchanged:

* Insert the vicodec module on your host (sudo modprobe vicodec multiplanar=1)
* Check that it is working properly:
** Encode 30 frames in FWHT format:
v4l2-ctl -d/dev/video0 -x pixelformat=NV12 --stream-mmap
--stream-out-mmap --stream-to-hdr out.fwht --stream-count 30
** Decode them back to NV12:
v4l2-ctl -d/dev/video1 -v pixelformat=NV12 --stream-mmap
--stream-out-mmap --stream-from-hdr out.fwht --stream-to out.nv12
(You can check the result with YUView, frame size is 1280x720).
* Get the kernel at https://github.com/Gnurou/linux/tree/virtio-v4l2
and build with CONFIG_VIRTIO_V4L2,
* Get crosvm at https://github.com/Gnurou/crosvm/tree/virtio-v4l2 and
run it with the "--virtio-v4l2=/dev/video0 --virtio-v4l2=/dev/video1"
options to virtualize the encoder and decoder devices,
* Run the same commands as the second step in the guest, you should
get the exact same result as when they were run on the host.

This demonstrates that a decoder device can be virtualized quite
simply by this solution. Next let me enable controls (so
v4l2-compliance passes in the guest) and the remaining types of buffer
memories and we should have something rather complete, at least on the
guest side.

I have also chatted a bit more with a few camera folks at Google, and
they agreed that V4L2 with requests should be enough to support even
modern camera pipelines. That is, the whole complexity of the host
pipeline would not and should not be entirely exposed to the guest,
only a simpler model that makes sense for what the camera is used for,
and for that V4L2 is more than enough. So a stronger confidence that
this will be suitable for the camera use-case as well.

If nobody strongly objects, I think this can be pushed a bit more
officially. Cornelia, would you consider it for inclusion if I
switched the next version of the specification to use V4L2 as the
host/guest protocol? This may take some more time as I want to confirm
the last details with code, but it should definitely be faster to
merge and to test with a real implementation than our previous
virtio-video attempts.

Cheers,
Alex.




>
> Some results for the compliance tests using vicodec:
> *  Enconder device
>     * Host
> Total for vicodec device /dev/video4: 46, Succeeded: 45, Failed: 1, Warnings: 0
>     * Guest
> Total for virtio_v4l2 device /dev/video0: 45, Succeeded: 34, Failed: 11, Warnings: 0
>
> *  Decoder device
>     * Host
> Total for vicodec device /dev/video5: 46, Succeeded: 42, Failed: 4, Warnings: 0
>     * Guest
> Total for virtio_v4l2 device /dev/video0: 45, Succeeded: 36, Failed: 9, Warnings: 0
>
>>
>>
>> >
>> > >
>> > > 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.
>>
>> The spec should indeed be considerably lighter. I'll wait for more
>> feedback, but if the concept appeals to other people as well, I may
>> give the spec a try soon.
>>
>
> From my perspective, I think we gain a simpler backend, simpler
> driver, and potential support for more devices for free. It may
> not cover all usecases, but that has already been discussed in this
> thread. For that purpose, virtio-camera will still be a possibility.
>
> FWIW, I think this is a great step in the right direction, as it will make both
> drivers and devices easier to implement and debug.
>
> BR,
> Albert
>
>>
>> Meanwhile I'll also try to add support for stateful decoders and
>> guest-allocated buffers to the prototype so it can be considered more
>> complete.
>>
>> Cheers,
>> Alex.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>>