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

[Alexander Gordeev <alexander.gordeev@opensynergy.com>]

On 02.05.23 03:07, Alexandre Courbot wrote:
> On Fri, Apr 28, 2023 at 5:55âPM Alexander Gordeev
> <alexander.gordeev@opensynergy.com> wrote:
> > On 28.04.23 06:02, Alexandre Courbot wrote:
> > > This is going to be my last answer to this thread ; I don't think I
> > > have more technical arguments to give than I already have and the
> > > discussion is drifting into territory I am not interested in engaging.
> > > At the end of the day it's up to the virtio folks to make a decision
> > > about what the best course of action is. If we end up with
> > > fragmentation, so be it, it will still be better than the current
> > > situation anyway.
> >
> > I agree with the summary. I think we have discussed all the things in
> > depth and even reached some understanding.
> > Could you please at least share your feedback on my proposal for the
> > QUEUE/DRAIN completion handling?
>
> Well I've re-read your email once more and again I confirm, this is
> exactly what virtio-v4l2 does. Here is the code that sends the dequeue
> buffer event on the event queue after a buffer is dequeued on the
> host: https://github.com/Gnurou/crosvm/blob/virtio-v4l2/devices/src/virtio/v4l2/worker.rs#L1114.
>
> IOW, there are no VIDIOC_DQBUF or VIDIOC_DQEVENT (or any command that
> can block) taking place between the guest and host, these are sent to
> the guest as events.
>
> And yes, this design simplifies things quite a bit vs. retaining
> command descriptors until some operation completes. Beyond the
> increased potential for deadlock, there is also added complexity as
> the descriptors must eventually be returned to the guest, even if the
> requested operation is interrupted. This increases the amount of state
> on the host which must keep track of pending operations and cancel
> some on occasion (which is pretty error-prone), whereas having all
> commands complete without delay could theoretically let us function
> even with a single command descriptor. Retaining the descriptors was
> supposed to reduce host/guest traffic a bit, but at that level of
> interaction (60 ops/second for a high framerate video) I don't think
> it's worth the cost.

Thanks for the clarification! So this was a misunderstanding on my side.
Indeed I'm still thinking of virtio-v4l2 as a V4L2 UAPI pass through. So
of course I thought you have VIDIOC_DQBUF and VIDIOC_DQEVENT. The
design, that you have in fact, indeed looks better to me compared to
having these ioctls passed through on the commandq. So doing the same
thing in virtio-video shouldn't break anything. I'm happy we agree on
that. I totally agree with you arguments in favor of the events.
So virtio-v4l2 is going to have quite a few patches on top of the V4L2
UAPI: the memory types, the way events are passed, maybe also more
restrictions on the data types (this can be upstreamed hopefully). Did I
miss something else? So probably it is better to present and think of
virtio-v4l2 as something like "patched V4L2 UAPI" to avoid confusion.
Now I'm really curious what the virtio-v4l2 spec would look like. I
don't say, that I'd like to give up on virtio-video. Please don't get me
wrong. I still think it is much better to have a readable clean spec and
to avoid all the patches, because they will certainly cause a lot of
confusion (like what we can see right now). I'm just glad, that we're
going in the same direction from different starting points. This
probably means, that we both do the right thing. Good!

--
Alexander Gordeev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

Phone: +49 30 60 98 54 0 - 88
Fax: +49 (30) 60 98 54 0 - 99
EMail: alexander.gordeev@opensynergy.com

www.opensynergy.com

Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB 108616B
GeschÃftsfÃhrer/Managing Director: RÃgis Adjamah

Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/>