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

[Alexandre Courbot <acourbot@chromium.org>]

On Thu, May 11, 2023 at 6:00âPM Alexander Gordeev
<alexander.gordeev@opensynergy.com> wrote:
>
> On 11.05.23 10:50, Alexander Gordeev wrote:
> > On 08.05.23 06:55, Alexandre Courbot wrote:
> >> On Fri, May 5, 2023 at 8:55âPM Alexander Gordeev
> >> <alexander.gordeev@opensynergy.com> wrote:
> >>>
> >>> On 03.05.23 16:04, Cornelia Huck wrote:
> >>>> On Fri, Apr 28 2023, Alexander Gordeev
> >>>> <alexander.gordeev@opensynergy.com> wrote:
> >>>>
> >>>>> On 27.04.23 15:16, Alexandre Courbot wrote:
> >>>>>> But in any case, that's irrelevant to the guest-host interface, and I
> >>>>>> think a big part of the disagreement stems from the misconception
> >>>>>> that
> >>>>>> V4L2 absolutely needs to be used on either the guest or the host,
> >>>>>> which is absolutely not the case.
> >>>>>
> >>>>> I understand this, of course. I'm arguing, that it is harder to
> >>>>> implement it, get it straight and then maintain it over years. Also it
> >>>>> brings limitations, that sometimes can be workarounded in the virtio
> >>>>> spec, but this always comes at a cost of decreased readability and
> >>>>> increased complexity. Overall it looks clearly as a downgrade compared
> >>>>> to virtio-video for our use-case. And I believe it would be the
> >>>>> same for
> >>>>> every developer, that has to actually implement the spec, not just do
> >>>>> the pass through. So if we think of V4L2 UAPI pass through as a
> >>>>> compatibility device (which I believe it is), then it is fine to have
> >>>>> both and keep improving the virtio-video, including taking the best
> >>>>> ideas from the V4L2 and overall using it as a reference to make
> >>>>> writing
> >>>>> the driver simpler.
> >>>>
> >>>> Let me jump in here and ask another question:
> >>>>
> >>>> Imagine that, some years in the future, somebody wants to add a virtio
> >>>> device for handling video encoding/decoding to their hypervisor.
> >>>>
> >>>> Option 1: There are different devices to chose from. How is the person
> >>>> implementing this supposed to pick a device? They might have a narrow
> >>>> use case, where it is clear which of the devices is the one that
> >>>> needs to
> >>>> be supported; but they also might have multiple, diverse use cases, and
> >>>> end up needing to implement all of the devices.
> >>>
> >>> I think in this case virtio-v4l2 should be used as a compatibility
> >>> device exclusively. This means discouraging increasing its complexity
> >>> even more with more patches in the spec. virtio-video should eventually
> >>> cover all the use-cases of V4L2, so I think it is reasonable to use it
> >>> in both complex use-cases and in simple use-cases, where there is no
> >>> decoder/encoder V4L2 device on the host.
> >>>
> >>>> Option 2: There is one device with various optional features. The
> >>>> person
> >>>> implementing this can start off with a certain subset of features
> >>>> depending on their expected use cases, and add to it later, if needed;
> >>>> but the upfront complexity might be too high for specialized use cases.
> >>
> >> I don't see that many negociable features we can provide for a
> >> decoder/encoder device - at least not many that are not considered
> >> basic (like guest buffers). In terms of provided features for codecs
> >> virtio-video and virtio-v4l2 are essentially equivalent.
> >
> > Actually I see a lot of potential in using the virtio feature flag
> > negotiation for virtio-video:
> >
> > 1. We already have some feature flags related to memory management.
> >
> > 2. I think it would be great to take V4L2 controls negotiation and turn
> > it into the feature flags negotiation. I really like this idea and I'd
> > like to implement it. Not all the controls at once, of course. Still it
> > would be very easy to port more given the existing process. They
> > correspond well enough to each other, I think. This way we don't need to
> > introduce something like the VIDIOC_QUERYCTRL/VIDIOC_QUERY_EXT_CTRL, we
> > don't need two mechanisms for feature negotiations (like it would be
> > with virtio-v4l2, right?), also all the features would be in one place.
> > Then we can directly reference some enums from V4L2, like
> > v4l2_mpeg_video_h264_profile or v4l2_mpeg_video_h264_level. That's what
> > I call taking the best from V4L2.
> >
> > 3. We can have things, that V4L2 doesn't support in their stateful UAPI.
> > For example, dequeuing output buffers in decoder order.
>
> I'd like to also share my current roadmap for the draft v7 of
> virtio-video (or maybe the draft v8 in some cases). The significant
> changes would be:
>
> 1. Making querying the capabilities fully compatible with V4L2 but in 1
> round-trip over virtio, not 10+. This is what I'm actively working on
> right now.

That sounds good, while not essential since the capability negotiation
occurs before we start streaming so these extra trips should not be
perceptible by the user. But the current capability command was not
suitable and needs to be improved anyway.

> 2. Making all the commands non-blocking by providing completion events
> over the event queue.

+1 on that too, as the experience with virtio-v4l2 suggests this
eliminates some headaches down the line.

> 3. Adding back the controls from v5 and adding the corresponding feature
> flags as I wrote in the quote above.

Beware of not repeating the same mistake as v5 (all controls under one
big structure). If you mean extending the parameters mechanisms with
the things that are missing from v5, then yes that should be done
anyway.