Re: [PATCH RFC v4 0/1] Virtio Video Device Specification

[Alexandre Courbot <acourbot@chromium.org>]

On Fri, Jul 3, 2020 at 6:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jul 03, 2020 at 02:45:15PM +0900, Alexandre Courbot wrote:
> > Hi Dmitry,
> >
> > On Thu, Jul 2, 2020 at 10:47 PM Dmitry Sepp <dmitry.sepp@opensynergy.com> wrote:
> > >
> > > Hi Keiichi,
> > >
> > > Thanks for the clarification. I believe we should explicitly describe this in
> > > the VIRTIO_VIDEO_CMD_RESOURCE_ATTACH section. And I also still see a problem
> > > there. If it is a guest allocated resource, we cannot consider it to be free
> > > until the device really releases it. And it won't happen until we issue the
> > > next ATTACH call. Also, as we discussed before, it might be not possible to
> > > free individual buffers, but the whole queue only.
> >
> > In the case of the encoder, a V4L2 driver is not supposed to let
> > user-space dequeue an input frame while it is used as reference for
> > the encoding process. So if we add a similar rule in the virtio-video
> > specification, I suppose this would solve the problem?
> >
> > For the decoder case, we have a bigger problem though. Since DMABUFs
> > can be arbitrarily attached to any V4L2 buffer ID, we may end up
> > re-queueing the DMABUF of a decoded frame that is still used as
> > reference under a different V4L2 buffer ID. In this case I don't think
> > the driver has a way to know that the memory resource should not be
> > overwritten, and it will thus happily use it as a decode target. The
> > easiest fix is probably to update the V4L2 stateful specification to
> > require that reused DMABUFs must always be assigned to the same V4L2
> > buffer ID, and must be kept alive as long as decoding is in progress,
> > or until a resolution change event is received. We can then apply a
> > similar rule to the virtio device.
>
>
> Sounds like a generic kind of problem - how do other devices solve it?

Most users of V4L2 drivers use MMAP buffers, which don't suffer from
this problem: since MMAP buffers are managed by V4L2 and the same V4L2
buffer ID always corresponds to the same backing memory, the driver
just needs to refrain from decoding into a given V4L2 buffer as long
as it is used as a reference frames. This is something that all
drivers currently do AFAICT.

The problem only occurs if you let userspace map anything to V4L2
buffers (USERPTR or DMABUF buffers). In order to guarantee the same
reliable behavior as with MMAP buffers, you must enforce the same
rule: always the same backing memory for a given V4L2 buffer.

... or you can rotate between a large enough number of buffers to
leave enough space for the reference tag to expire on your frames, but
that's clearly not a good way to address the problem.