Re: [virtio-dev] Re: [PATCH v2 1/1] virtio-video: Add virtio video device specification

Tomasz Figa
2020-01-09T14:21:00Z

On Wed, Jan 8, 2020 at 10:52 PM Keiichi Watanabe <keiichiw@chromium.org> wrote:
>
> Hi Gerd,
>
> On Thu, Dec 19, 2019 at 10:12 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > Hi,
> >
> > > > However that still doesn't let the driver know which buffers will be
> > > > dequeued when. A simple example of this scenario is when the guest is
> > > > done displaying a frame and requeues the buffer back to the decoder.
> > > > Then the decoder will not choose it for decoding next frames into as
> > > > long as the frame in that buffer is still used as a reference frame,
> > > > even if one sends the drain request.
> > > It might be that I'm getting your point wrong, but do you mean some hardware
> > > can mark a buffer as ready to be displayed yet still using the underlying
> > > memory to decode other frames?
> >
> > Yes, this is how I understand Tomasz Figa.
> >
> > > This means, if you occasionally/intentionally
> > > write to the buffer you mess up the whole decoding pipeline.
> >
> > And to avoid this the buffer handling aspect must be clarified in the
> > specification. Is the device allowed to continue using the buffer after
> > finishing decoding and completing the queue request? If so, how do we
> > hand over buffer ownership back to the driver so it can free the pages?
> > drain request? How do we handle re-using buffers? Can the driver
> > simply re-queue them and expect the device figures by itself whenever it
> > can use the buffer or whenever it is still needed as reference frame?
>
> The device shouldn't be able to access buffers after it completes a
> queue request.
> The device can touch buffer contents from when a queue request is sent
> until the device responds it.
> In contrast, the driver must not modify buffer contents in that period.
>
> Regarding re-using, the driver can simply re-queue buffers returned by
> the device. If the device needs a buffer as reference frame, it must
> not return the buffer.

I think that might not be what we expect. We want the decoder to
return a decoded frame as soon as possible, but that decoded frame may
be also needed for decoding next frames. In V4L2 stateful decoder, the
API is defined that the client must not modify the decoded
framebuffer, otherwise decoding next frames may not be correct. We may
need something similar, with an explicit operation that makes the
buffers not accessible to the host anymore. I think the queue flush
operation could work as such.

> I'll describe this rule in the next version of the patch.
>
> Best regards,
> Keiichi
>
> >
> > cheers,
> > Gerd
> >