Re: [PATCH v2 0/1] virtio-media: Add device specification

Hi Albert,

On 07.06.24 10:00, Albert Esteve wrote:
> Hi,
> 
> This a formal attempt of including virtio-media
> device specification.
> 
> Virtio-media came from a discussion on virtio-dev
> mailing list, which lead to presenting virtio-v4l2[1]
> specification as an alternative to virtio-video.
> 
> Later, virtio-v4l2 was renamed to virtio-media[2]
> and published through:
> 
> https://github.com/chromeos/virtio-media
> 
> The repository above includes a virtio-media driver able
> to pass v4l2-compliance when proxying the vivid/vicodec
> virtual devices or an actual UVC camera using the
> V4L2 vhost device (available in the repository).
> Steps to reproduce are also detailed[3].
> 
> There is some overlap with virtio-video in regards
> to which devices it can handle. However,
> as virtio-media will likely be the virtualization
> solution for ChromeOS (already landed into the chromeos
> organization) and possibly other Google projects for
> media devices, it would be desirable to include the
> specification in the next virtio release despite
> the aforementioned overlap.
> 
> The device ID in this document differs from
> the ID in the virtio-media project repository.
> And it will probably need some discussion on which
> would be the correct definitive ID.
> 
> Full PDF: https://drive.google.com/file/d/1pNCFP06VoV8Zuyx0aDVQ7HAT4xp-pZ0a/view?usp=sharing
> PDF with the media section only: https://drive.google.com/file/d/1sn3NUUeCm46zVyJKHkpytTIgGw1fUt5T/view?usp=sharing
> 
> [1] https://mail.google.com/mail/u/0?ui=2&ik=73ebd65ebd&attid=0.1&permmsgid=msg-f:1767388565327924962&th=1887068940754ee2&view=att&disp=inline&realattid=f_libalimc0
> [2] https://www.mail-archive.com/[email protected]/msg12665.html
> [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
> 
> Albert Esteve (1):
>    virtio-media: Add virtio media device specification
> 
>   conformance.tex                           |  13 +-
>   content.tex                               |   1 +
>   device-types/media/description.tex        | 578 ++++++++++++++++++++++
>   device-types/media/device-conformance.tex |  11 +
>   device-types/media/driver-conformance.tex |   9 +
>   5 files changed, 608 insertions(+), 4 deletions(-)
>   create mode 100644 device-types/media/description.tex
>   create mode 100644 device-types/media/device-conformance.tex
>   create mode 100644 device-types/media/driver-conformance.tex

I'd like to add some general considerations:

1. virtio-media device capability discovery is very chatty

V4L2 requires potentially hundreds of system calls to get the full view 
of the device capabilities. This is inherited by virtio-media.
AFAIU V4L2 developers agree there is room for enhancement here, see [1], 
[2]. Also I've been told VIDIOC_G_EXT_CTRLS/VIDIOC_S_EXT_CTRLS were 
added for similar reasons.

 From the point of view of virtualization developers like my colleagues 
and me hundreds of hypervisor "exits" are excessive and costly. It can 
noticeably increase boot times, which is something we fight against in 
automotive. AFAIU other virtio developers agree with this, see [3].

In contrast virtio-video has been doing this in one command from day 
one. Yes, the data was incomplete. That's why in draft v9 I added TLV as 
a way to store all possible capabilities. I hope the linux-media folks 
could take a look on it. Maybe this is something, that can be adopted in 
V4L2?

2. virtio-media has a hard dependency on V4L2

There are certainly some "patches" on top of V4L2 in virtio-media, like 
the representation of the extended controls (which actually looks 
similar to the representation of the controls in virtio-video) or 
VIRTIO_MEDIA_EVT_DQBUF. But how far can this go? Is it OK to add your 
own ioctl ids in virtio-media?

AFAIK the linux-media maintainers have been overloaded for years, see 
[4]. Would they be happy to deal with the additional requirements? Would 
virtio community like to have a dependency here?

2.1. an example: format modifiers

There is a patchset aiming at unifying V4L2 pixel formats and extending 
them with DRM format modifiers. It is now at version 7 submitted in 
2023, see [5]. The first version was submitted in 2019, see [6]. Not 
merged yet AFAIK.

In virtio-video I just added them in draft v9.

I'm absolutely not trying to criticize here. I just try to highlight 
that there is a lot of legacy and the process is painful. Right now we 
have an opportunity to design a new API according to the current state 
of the art of the stateful codecs.

3. uncertainty with cameras

AFAIK there is still no agreement about how cameras should be 
virtualized, see [3], [7], [8], [9]. virtio-media provides support for 
cameras in a specific way, which might not be desirable.

4. (minor) is it possible/hard to implement the device in hardware/on a 
micro controller?

This is something I thought about recently, there might be a use-case 
for it in the future. One of the concerns is that dynamic memory 
allocations are IMHO inevitable in virtio-media or virtio-video up to 
draft v8. I think multiple virtqueues in virtio-video draft v9 would 
help here. Not sure yet...

There are also other minor concerns, that are probably tolerable.

[1] 
https://lore.kernel.org/linux-media/[email protected]/
[2] Page 6 in 
https://hverkuil.home.xs4all.nl/mediasummit2023-pdfs/Hsia-Jun%20Li%20-%20V4L2%20M2M%20EXT%20API%20enhancement.pdf
[3] https://old.linaro.org/blog/the-challenges-of-abstracting-virtio/
[4] 
https://lore.kernel.org/linux-media/[email protected]/
[5] 
https://lore.kernel.org/linux-media/[email protected]/
[6] 
https://lore.kernel.org/linux-media/[email protected]/
[7] https://lore.kernel.org/virtio-dev/[email protected]/
[8] 
https://lore.kernel.org/linux-media/[email protected]/
[9] 
https://lore.kernel.org/virtio-dev/CAAFQd5BrhDZtFX3vdhBVSLXthe8CykYsZzVQ9ddZxVNvNS3ArA@mail.gmail.com/

-- 
Alexander Gordeev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
www.opensynergy.com