From: Mauro Carvalho Chehab <mchehab+huawei@kernel•org>
To: Alexandre Courbot <gnurou@gmail•com>
Cc: "Albert Esteve" <aesteve@redhat•com>,
"Michael S. Tsirkin" <mst@redhat•com>,
"Mauro Carvalho Chehab" <mchehab@kernel•org>,
"Hans Verkuil" <hverkuil@xs4all•nl>,
"Jason Wang" <jasowang@redhat•com>,
"Xuan Zhuo" <xuanzhuo@linux•alibaba.com>,
"Eugenio Pérez" <eperezma@redhat•com>,
gurchetansingh@google•com, daniel.almeida@collabora•com,
adelva@google•com, changyeon@google•com,
nicolas.dufresne@collabora•com, linux-kernel@vger•kernel.org,
linux-media@vger•kernel.org, virtualization@lists•linux.dev
Subject: Re: [PATCH v3] media: add virtio-media driver
Date: Tue, 27 May 2025 16:42:43 +0200 [thread overview]
Message-ID: <20250527164243.092e8aae@sal.lan> (raw)
In-Reply-To: <CAAVeFuJtp=UEEULeMSVpmYDmH81Y6OQgj6NCeuPUhabSRHw4dA@mail.gmail.com>
Em Tue, 27 May 2025 23:03:39 +0900
Alexandre Courbot <gnurou@gmail•com> escreveu:
> On Tue, May 27, 2025 at 10:35 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel•org> wrote:
> >
> > Em Tue, 27 May 2025 22:21:42 +0900
> > Alexandre Courbot <gnurou@gmail•com> escreveu:
> >
> > > On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel•org> wrote:
> > > >
> > > > Em Tue, 27 May 2025 15:14:50 +0900
> > > > "Alexandre Courbot" <gnurou@gmail•com> escreveu:
> > > >
> > > > > Hi Mauro,
> > > > >
> > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote:
> > > > > > Hi Michael,
> > > > > >
> > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900
> > > > > > Alexandre Courbot <gnurou@gmail•com> escreveu:
> > > > > >
> > > > > >> Add the first version of the virtio-media driver.
> > > > > >>
> > > > > >> This driver acts roughly as a V4L2 relay between user-space and the
> > > > > >> virtio virtual device on the host, so it is relatively simple, yet
> > > > > >> unconventional. It doesn't use VB2 or other frameworks typically used in
> > > > > >> a V4L2 driver, and most of its complexity resides in correctly and
> > > > > >> efficiently building the virtio descriptor chain to pass to the host,
> > > > > >> avoiding copies whenever possible. This is done by
> > > > > >> scatterlist_builder.[ch].
> > > > > >>
> > > > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using
> > > > > >> code generated through macros for ioctls that can be forwarded directly,
> > > > > >> which is most of them.
> > > > > >>
> > > > > >> virtio_media_driver.c provides the expected driver hooks, and support
> > > > > >> for mmapping and polling.
> > > > > >>
> > > > > >> This version supports MMAP buffers, while USERPTR buffers can also be
> > > > > >> enabled through a driver option. DMABUF support is still pending.
> > > > > >
> > > > > > It sounds that you applied this one at the virtio tree, but it hasn't
> > > > > > being reviewed or acked by media maintainers.
> > > > > >
> > > > > > Please drop it.
> > > > > >
> > > > > > Alexandre,
> > > > > >
> > > > > > Please send media patches to media maintainers, c/c other subsystem
> > > > > > maintainers, as otherwise they might end being merged without a
> > > > > > proper review.
> > > > >
> > > > > Sorry about that, I put everyone in "To:" without giving it a second
> > > > > thought.
> > > > >
> > > > > >
> > > > > > In this particular case, we need to double-check if this won't cause
> > > > > > any issues, in special with regards to media locks and mutexes.
> > > > >
> > > > > Agreed, I am not 100% confident about that part myself.
> > > > >
> > > > > >
> > > > > > I'll try to look on it after this merge window, as it is too late
> > > > > > for it to be applied during this one.
> > > > >
> > > > > Appreciate that - given the high traffic on the list I was worried that
> > > > > this patch would eventually be overlooked. Not making it for this merge
> > > > > window should not be a problem, so please take the time you need.
> > > >
> > > > Provided that your patch was caught by patchwork, it won't be lost:
> > > > https://patchwork.linuxtv.org/project/linux-media/patch/20250412-virtio-media-v3-1-97dc94c18398@gmail.com/
> > > >
> > > > Please notice that our CI got a number of checkpatch issues there.
> > > > Please check and fix the non-false-positive ones.
> > >
> > > Will do. The macro-related ones are false-positives AFAICT. Some of
> > > the "lines should not end with a '('" are actually the result of
> > > applying clang-format with the kernel-provided style...
> >
> > I don't know any lint tool that honors kernel style. The best one
> > is checkpatch with the auto-correcting parameter in pedantic mode,
> > but still one needs to manually review its output.
> >
> > >
> > > >
> > > > Btw, I was looking at:
> > > >
> > > > https://github.com/chromeos/virtio-media
> > > >
> > > > (I'm assuming that this is the QEMU counterpart, right?)
> > >
> > > crosvm actually, but QEMU support is also being worked on.
> >
> > Do you have already QEMU patches? The best is to have the Kernel driver
> > submitted altogether with QEMU, as Kernel developers need it to do the
> > tests. In my case, I never use crosvm, and I don't have any Chromebook
> > anymore.
>
> IIRC Albert Esteve was working on this, maybe he can share the current status.
That would be nice.
>
> Note that crosvm does not require a Chromebook, you can build and run
> it pretty easily on a regular PC. I have put together a document to
> help with that:
>
> https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md
>
> It also shows how to proxy a host UVC camera into the guest and use
> some of the "fake" devices I talked about earlier.
Ok, I'll look on it, thanks for the hint!
> If following these
> steps is too much friction, just reading through the document should
> give you a decent idea of what virtio-media can do.
>
> >
> > > > And I noticed something weird there:
> > > >
> > > > "Unsupported ioctls
> > > >
> > > > A few ioctls are replaced by other, more suitable mechanisms. If being requested these ioctls, the device must return the same response as it would for an unknown ioctl, i.e. ENOTTY.
> > > >
> > > > VIDIOC_QUERYCAP is replaced by reading the configuration area.
> > > > VIDIOC_DQBUF is replaced by a dedicated event.
> > > > VIDIOC_DQEVENT is replaced by a dedicated event."
> > > >
> > > > While this could be ok for cromeOS, this will be broken for guests with
> > > > Linux, as all Linux applications rely on VIDIOC_QUERYCAP and VIDIOC_DQBUF
> > > > to work. Please implement support for it, as otherwise we won't even be
> > > > able to test the driver with the v4l2-compliance tool (*).
> > >
> > > The phrasing was a bit confusing. The guest driver does support these
> > > ioctls, and passes v4l2-compliance. So there is no problem here.
> >
> > Please add v4l2-compliance output on the next patch series.
>
> Certainly. Note that the output is dependent on what the host provides
> for a device. If it e.g. proxies something that is not fully
> compliant, the guest will reflect the same errors.
Makes sense. In this case, the best is to add at the PR the v4l2-compliance
output for both the host real V4L2 driver and the guest one.
>
> >
> > > Where these ioctls are not supported is between the guest and the
> > > host, i.e. as ioctls encapsulated into a virtio request. For QUERYCAP,
> > > that's because the virtio configuration area already provides the same
> > > information. For DQBUF and DQEVENT, that's because we want to serve
> > > these asynchronously for performance reasons (hence the dedicated
> > > events).
> > >
> > > But these differences don't affect guest user applications which will
> > > be able to perform these ioctls exactly as they expect.
> >
> > OK. Better to let it clear then at the documentation.
> >
> > > >
> > > > (*) Passing at v4l2-compliance is a requirement for any media driver
> > > > to be merged.
> > > >
> > > > With regards to testing, what's the expected testing scenario?
> > > > My guess is that, as a virtio device, a possible test scenario would be
> > > > to have the UVC camera from the host OS mapped using virtio-camera into
> > > > the guest OS, allowing a V4L2 application running at the guest to map the
> > > > camera from the host, right?
> > >
> > > That's one of the scenarios, yes.
> >
> > Ok, this is the easiest test scenario for media developers.
> >
> > > Another one is to expose an accelerated decoder on the host to the guest.
> >
> > > One can also create
> > > "fake" devices backed by software on the host for testing purposes.
> >
> > That sounds interesting. It probably makes sense to have some test
> > scenario using such fake devices plus v4l2-compliance test to check
> > for regressions running on some CI engine.
>
> Yes, regression catching in a CI is one of the use-cases we thought about.
>
> >
> > > That's actually the benefit of using V4L2 as a guest/host protocol:
> > > the same guest and the same software stack on the host can be used to
>
> I made a typo here: "the same guest DRIVER". That's an important point. :)
It was clear enough to me ;-)
>
> Cheers,
> Alex.
next prev parent reply other threads:[~2025-05-27 14:42 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-12 4:08 [PATCH v3] media: add virtio-media driver Alexandre Courbot
2025-04-12 14:27 ` Markus Elfring
2025-05-26 12:13 ` Mauro Carvalho Chehab
2025-05-27 6:14 ` Alexandre Courbot
2025-05-27 9:13 ` Mauro Carvalho Chehab
2025-05-27 13:21 ` Alexandre Courbot
2025-05-27 13:35 ` Mauro Carvalho Chehab
2025-05-27 14:03 ` Alexandre Courbot
2025-05-27 14:42 ` Mauro Carvalho Chehab [this message]
2025-06-17 8:49 ` Mauro Carvalho Chehab
2025-06-17 9:03 ` Mauro Carvalho Chehab
2025-06-17 10:20 ` Mauro Carvalho Chehab
2025-06-18 14:27 ` Alexandre Courbot
2025-06-18 15:05 ` Mauro Carvalho Chehab
2025-06-20 12:03 ` Alexandre Courbot
2025-09-08 10:03 ` Mauro Carvalho Chehab
2025-06-18 14:16 ` Alexandre Courbot
2025-06-18 14:40 ` Mauro Carvalho Chehab
2025-05-27 14:23 ` Michael S. Tsirkin
2025-05-27 14:39 ` Mauro Carvalho Chehab
2025-05-27 15:06 ` Michael S. Tsirkin
2025-05-28 11:07 ` Alexandre Courbot
2025-05-28 16:23 ` Ricardo Ribalda
2025-06-01 9:34 ` Mauro Carvalho Chehab
2025-06-01 10:01 ` Ricardo Ribalda
2025-07-24 17:24 ` Mauro Carvalho Chehab
2025-07-28 11:51 ` Alexandre Courbot
2025-09-09 9:12 ` Mauro Carvalho Chehab
2026-05-29 16:03 ` Brian Daniels
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250527164243.092e8aae@sal.lan \
--to=mchehab+huawei@kernel$(echo .)org \
--cc=adelva@google$(echo .)com \
--cc=aesteve@redhat$(echo .)com \
--cc=changyeon@google$(echo .)com \
--cc=daniel.almeida@collabora$(echo .)com \
--cc=eperezma@redhat$(echo .)com \
--cc=gnurou@gmail$(echo .)com \
--cc=gurchetansingh@google$(echo .)com \
--cc=hverkuil@xs4all$(echo .)nl \
--cc=jasowang@redhat$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-media@vger$(echo .)kernel.org \
--cc=mchehab@kernel$(echo .)org \
--cc=mst@redhat$(echo .)com \
--cc=nicolas.dufresne@collabora$(echo .)com \
--cc=virtualization@lists$(echo .)linux.dev \
--cc=xuanzhuo@linux$(echo .)alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox