Em Tue, 27 May 2025 23:03:39 +0900 Alexandre Courbot <gnu...@gmail.com> escreveu:
> On Tue, May 27, 2025 at 10:35 PM Mauro Carvalho Chehab > <mchehab+hua...@kernel.org> wrote: > > > > Em Tue, 27 May 2025 22:21:42 +0900 > > Alexandre Courbot <gnu...@gmail.com> escreveu: > > > > > On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab > > > <mchehab+hua...@kernel.org> wrote: > > > > > > > > Em Tue, 27 May 2025 15:14:50 +0900 > > > > "Alexandre Courbot" <gnu...@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 <gnu...@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-97dc94c18...@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.
