Hi Alex,

Sorry for the late reply. I think this is a nice solution, specially for
the host-guest
memory management part. I was intrigued with how would you solve this, as
the v4l2 struct memory fields can't be directly used.

In that sense, sending a field that is going to be ignored by both driver
and device,
feels like kindof a waste. But I guess there is not a good solution to that.
Have you considered avoiding using struct v4l2_buffer for QBUF and DQBUF
ioctls,
and having virtio-v4l2 specific struct for them? The device would have the
burden to
copy the all the fields and leave the `m` field out, so it may have its own
downsides.

In any case, the specs are short and clear to follow. Great work!

I just have a couple comments/questions:

- The text in `0.1.6.3` and `0.1.6.3.1` is saying the same thing twice (?).
Maybe would be
  clearer to unify?

- If I understood correctly, the `stream_id` is assigned by the device
after receiving
  a VIRTIO_V4L2_CMD_OPEN? Is it supposed to be a correlative natural number
(1, 2, etc.)?
  I assume that after (successfully) closing the stream, its stream_id can
be reused?

- For section `0.1.6.7` I think it would be good to refer (again) to the
section `0.1.6.5` so that
  we can quickly navigate to the section that explains the memory fields in
the presented events.

Best regards,
Albert

On Wed, May 31, 2023 at 8:08 AM Michael S. Tsirkin <[email protected]> wrote:

> On Wed, May 31, 2023 at 03:02:04PM +0900, Alexandre Courbot wrote:
> > Hello everyone,
> >
> > With the virtio-video call taking place soon, I thought it would help
> > everyone understand both proposals if I sent a more formal
> > specification of what virtio-v4l2 looks like. Please find it attached
> > to this email.
> >
> > I apologize for not finishing and sending this earlier, but hopefully
> > at 7 and a half pages it should be rather quick to skim through. :)
> >
> > Despite its short size, this spec is capable of supporting camera and
> > image processor devices (on top of decoder/encoders), and also allows
> > for another kind of memory backing for buffers (host-managed) that is
> > not supported by virtio-video.
> >
> > For convenience I have made use of clickable links to the relevant
> > parts of the V4L2 documentation, since it is supposed to be used
> > alongside this spec.
> >
> > Looking forward to a fruitful discussion tomorrow!
> >
> > Cheers,
> > Alex.
>
>
> Can you please post patch with tex source inline as opposed as
> pdf as an attachment? Thanks!
>
> --
> MST
>
>

Reply via email to