Re: [PATCH v2 0/1] Virtio Video V4L2 driver

Keiichi Watanabe
2020-03-12T09:49:26Z

Hi Hans,

On Wed, Mar 11, 2020 at 10:26 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Dmitry,
>
> On 2/18/20 9:27 PM, Dmitry Sepp wrote:
> > Hi all,
> >
> > This is a v4l2 virtio video driver for the virtio-video device
> > specification v3 [1].
> >
> > The first version of the driver was introduced here [2].
> >
> > Changes v1 -> v2:
> > * support the v3 spec (mostly)
> > * add a module parameter to ask for pages from ZONE_DMA
> >
> > What is not implemented:
> > * Plane layout flags should be used to propagate number of planes to
> > user-space
> > * There is no real use of stream creation with bitstream format in the
> > parameter list. The driver just uses the first bitstream format from
> > the list.
> > * Setting bitrate is done in a different way compared to the spec. This
> > is because it has been already agreed on that the way the spec
> > currently describes it requires changes.
> >
> > Potential improvements:
> > * Do not send stream_create from open. Use corresponding state machine
> > condition to do this.
> > * Do not send stream_destroy from close. Do it in reqbufs(0).
> > * Cache format and control settings. Reduce calls to the device.
>
> Some general notes:
>
> Before this can be merged it needs to pass v4l2-compliance.
>
> I also strongly recommend adding support for V4L2_PIX_FMT_FWHT to
> allow testing with the vicodec emulation driver. This will also
> allow testing all sorts of corner cases without requiring special
> hardware.

I agree that it's great if we could test virtio-video with vicodec,
but I wonder if supporting FWHT is actually needed for the initial
patch.
Though it wouldn't be difficult to support FWHT in the driver, we also
needs to support it in the host's hypervisor to test it. It's not easy
for our hypervisor to support FWHT, as it doesn't talk to v4l2 driver
directly.
Without the host-side implementation, it makes no sense to support it.
Also, if we support FWHT, we should have the format in a list of
supported formats in the virtio specification. However, I'm not sure
if FWHT is a general codec enough to be added in the spec, which
shouldn't be specific to Linux.

Best regards,
Keiichi

>
> Regards,
>
> Hans
>
> >
> > Best regards,
> > Dmitry.
> >
> > [1] https://markmail.org/message/dmw3pr4fuajvarth
> > [2] https://markmail.org/message/wnnv6r6myvgb5at6
> >
> >
>