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

[Tomasz Figa <tfiga@chromium.org>]

On Mon, Dec 16, 2019 at 7:32 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > Hmm, modern GPUs support both encoding and decoding ...
> >
> > Many SoC architectures have completely separate IP blocks for encoding
> > and decoding. Similarly, in GPUs those are usually completely separate
> > parts of the pipeline.
>
> In the OS there is one driver per GPU handling both ...
>

That's usually only the case for the desktop PC-style GPUs. That said,
it probably doesn't matter from the protocol point of view how it's
handled in the host. At least it is definitely safe to assume that the
host provides functionality for independently decoding and encoding
things from different processes, which is enough to implement virtio
decoder and encoder as separate devices.

> > > I don't think we should bake that restriction into the specification.
> > > It probably makes sense to use one virtqueue per function though, that
> > > will simplify dispatching in both host and guest.
> > >
> >
> > Wouldn't it make the handling easier if we had one virtio device per function?
>
> I don't think there is much of a difference between one device per
> function and one virtqueue per function.  You'll probably have a single
> driver for both anyway, to share common code for buffer management etc.
>

The semantics of the encoder and decoder on the driver side, at least
on Linux where V4L2 is used, are different enough for much of the code
to be separated.

That said, even with separate devices, the same driver can be
instantiated multiple times, which is a common case in Linux, handled
by the driver core. That basically gives us the ability to have as
many instances of whatever function we want for free, without the need
to explicitly handle multiple functions in one device in the driver.
So that clearly equals simpler driver code.

On the host side, the encode and decode APIs are different as well, so
having separate implementation decoder and encoder, possibly just
sharing some helper code, would make much more sense.

> > > Use separate pixel_format (fourcc) and stream_format (H.264 etc.) enums?
> >
> > I'd specifically avoid FourCC here, as it's very loosely defined and
> > could introduce confusion.
>
> I don't think using fourcc is a problem, and given that both drm and
> v4l2 use fourcc already this would be a good choice I think.

Both DRM and V4L2 use two mutually incompatible sets of FourCCs, so
I'm not sure how it could be a good choice. At least unless we decide
to pick a specific set of FourCC. It doesn't help that Windows/DirectX
has its own set of FourCCs that's again slightly different than the
two mentioned before.

>
> But the definition should be more specific than just "fourcc".  Best
> would be to explicitly list and define each format supported by the
> spec.

Why not be consistent with virtio-gpu and just define new formats as
we add support for them as sequential integers?

Best regards,
Tomasz