[PATCH v8 0/6] dcc: Create a stream for non-gl/remote clients that want to use dmabuf (v8)
Michael Scherle
michael.scherle at rz.uni-freiburg.de
Mon Sep 9 08:42:53 UTC 2024
Hi,
> Hi,
> now I start to understand. You are only testing what you want to
> achieve disregarding any compatibility. For instance you didn't test
> any system which is not Linux (yes, both Mac and Windows are
> supported, obviously with different features) or does have a non Intel
> card.
I am sorry if I have made mistakes here, I am still new to this. To
avoid further confusion, I am not the author, I just found the merge
request and it is exactly what I need and tested it first as the author
described with -device virtio-vga.
Then I tested it the way I needed it, with partitioned gpu's that are
passed through to the VM in qemu. I am aware that I have only tested
this on Intel GPU's, I thought this would not be bad at first, as I have
not seen it tested at all with gpus passed through.
Other reasons were: For AMD I only have the AMD Instinct MI100, which is
only an accelerator and has no graphics functionality. For Nvidia I
would have an A30 and could possibly get an Nvidia L40S for testing.
However, since licenses are required for partitioning, I haven't looked
into it yet.
Also I wasn't aware that windows and mac are also supported on the
server side, so I should have definitely mentioned that I only tested it
under linux.
> If we suppose system S with libraries L1, L2, L3 respectively with
> versions V1, V2 and V3 provides features F1 and F2 the same system
> should continue to provide the same features with these libraries and
> versions. That does not mean they need to support new features.
Ok I think now it is clear what is stopping the merge request.
> I also bet that to make the code compile you had to update some system
> libraries. Not an issue but Spice should continue to compile with
> stuck distro libraries (still, don't need to support new features).
>
> To limit the back and forth of series versions I suggest you also set
> up the CI so you can see some of the failures yourself.
Ok I will do it.
> I already have some patches.
>
> Regards,
> Frediano
>
Greetings
Michael
On 08.09.24 18:43, Frediano Ziglio wrote:
> Il giorno ven 6 set 2024 alle ore 10:43 Michael Scherle
> <michael.scherle at rz.uni-freiburg.de> ha scritto:
>>
>> Hi,
>>
>> thank you very much for your reply. Of course I understand that
>> non-compiling/crashing code should not be merged into main.
>> Unfortunately I have not been able to reproduce this so far. Here's a
>> list of the systems I have tested on:
>>
>> - Ubuntu server 20.04 LTS
>> AMD EPYC 7742
>> Intel Flex 140 with SR-IOV passthrough and without passthrough
>>
>> - CentOS Stream 9
>> AMD EPYC 7742
>> Intel Flex 170 (SR-IOV)
>>
>> - Ubuntu 22.04
>> 8th Gen Intel with GVT-g and without passthrough
>>
>> - Fedora 39
>> 12th Gen Intel (SR-IOV)
>>
>> Do you have an example system on which it does not compile/crash or what
>> the error messages are? Then I could possibly reproduce it and
>> contribute to a fix.
>>
>> Greetings,
>> Michael
>>
>
> Hi,
> now I start to understand. You are only testing what you want to
> achieve disregarding any compatibility. For instance you didn't test
> any system which is not Linux (yes, both Mac and Windows are
> supported, obviously with different features) or does have a non Intel
> card.
>
> If we suppose system S with libraries L1, L2, L3 respectively with
> versions V1, V2 and V3 provides features F1 and F2 the same system
> should continue to provide the same features with these libraries and
> versions. That does not mean they need to support new features.
>
> I also bet that to make the code compile you had to update some system
> libraries. Not an issue but Spice should continue to compile with
> stuck distro libraries (still, don't need to support new features).
>
> To limit the back and forth of series versions I suggest you also set
> up the CI so you can see some of the failures yourself.
>
> I already have some patches.
>
> Regards,
> Frediano
>
>
>> On 05.09.24 08:13, Frediano Ziglio wrote:
>>> Hi,
>>> I surely should get back to this.
>>>
>>> At the moment the series does not even compile on most of the supported systems.
>>> I understand that the feature works for some but merging in master not
>>> compiling code does not seem really nice to me.
>>> I think before merging code should compile and run, maybe with
>>> disabled features due to detected limitations but surely not crash if
>>> any unwritten and untested dependencies are not met.
>>>
>>> Regards,
>>> Frediano
>>>
>>> Il giorno ven 30 ago 2024 alle ore 12:53 Michael Scherle
>>> <michael.scherle at rz.uni-freiburg.de> ha scritto:
>>>>
>>>>
>>>>
>>>> On 10.06.24 20:34, Vivek Kasireddy wrote:
>>>>> For clients that cannot accept a dmabuf fd directly (such as those
>>>>> running on a remote system), this patch series provides a way for
>>>>> the Spice server to stream the gl/dmabuf data/buffer instead. This
>>>>> is mostly done by enabling the creation of Gst memory using a dmabuf
>>>>> fd as the source. This ability is useful given that dmabuf is the
>>>>> standard mechanism for sharing buffers between various drivers and
>>>>> userspace in many Graphics and Media usecases. Currently, this is
>>>>> tested with Qemu and remote-viewer using the x264enc/avdec_h264
>>>>> and msdkh264enc/dec plugins to stream the Guest/VM desktop but it
>>>>> can be easily extended to other plugins and applications.
>>>>>
>>>>> Here is roughly how things work:
>>>>> - The application (e.g, Qemu) chooses its preferred codec (a Gstreamer
>>>>> one) and calls gl_scanout (to update the fd) followed by gl_draw.
>>>>> - In response, the Spice server checks to see if the client is capable
>>>>> of accepting a dmabuf fd directly or not. If yes, the fd is forwarded
>>>>> directly to the client; otherwise, a new stream is created.
>>>>> - The Spice server then sends the dmabuf fd to the Gstreamer encoder
>>>>> which uses it as an input for creating an encoded buffer which is then
>>>>> sent to the client.
>>>>> - Once the encoding process is done, an async completion cookie is sent
>>>>> to the application.
>>>>>
>>>>> Here is a link to the previous version that used a drawable to share
>>>>> the dmabuf fd with the Gstreamer encoder:
>>>>> https://lists.freedesktop.org/archives/spice-devel/2023-January/052948.html
>>>>>
>>>>> This version is tested together with following (required) patches in qemu:
>>>>> https://gitlab.freedesktop.org/Vivek/qemu/-/commits/spice_gl_on_v4
>>>>>
>>>>> Changelog:
>>>>>
>>>>> v8:
>>>>> - Added a new gstreamer-encoder patch to this series to convert drm format
>>>>> shared by the VMM to the appropriate Gstreamer format.
>>>>>
>>>>> v7:
>>>>> - Revert back to the previous design where we do not share fd with the stream
>>>>> and scanout is the sole owner of the fd. This is because sharing fd ownership
>>>>> opens up a lot of corner cases.
>>>>>
>>>>> v6: (Frediano)
>>>>> - Properly share ownership of the dmabuf fd between stream and scanout
>>>>> - Ensure that a newly created stream is associated with all connected clients
>>>>>
>>>>> v5:
>>>>> - Addressed review comments from Frediano mainly regarding adding autoconf
>>>>> support for gstreamer-allocators dependency and not needing to access
>>>>> scanout as part of gl draw operation
>>>>>
>>>>> v4:
>>>>> - Test with Virgl enabled
>>>>> - Associate dmabuf's y0_top with stream's top_down variable
>>>>>
>>>>> v3:
>>>>> - Updated the second patch to have a new primary surface created
>>>>> whenever a new stream gets created. This will avoid having to
>>>>> trigger primary surface creation from Qemu. And, this change
>>>>> also fixes the following error seen with v2:
>>>>> ../server/display-channel.cpp:2074:display_channel_create_surface:
>>>>> condition `!display->priv->surfaces[surface_id]' failed
>>>>> - Rebase all patches on master
>>>>>
>>>>> v2:
>>>>> - Update all patches to address review comments from Frediano
>>>>> - Tested this series with msdkh264enc/dec plugins that will be added
>>>>> in another patch series
>>>>>
>>>>> Cc: Frediano Ziglio <freddy77 at gmail.com>
>>>>> Cc: Gerd Hoffmann <kraxel at redhat.com>
>>>>> Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>>> Cc: Dongwon Kim <dongwon.kim at intel.com>
>>>>>
>>>>> Vivek Kasireddy (6):
>>>>> dcc: Check to see if the client supports multiple codecs (v2)
>>>>> dcc: Create a stream associated with gl_draw for non-gl clients (v6)
>>>>> dcc-send: Encode and send gl_draw stream data to the remote client
>>>>> (v4)
>>>>> gstreamer-encoder: Add an encoder function that takes dmabuf fd as
>>>>> input (v3)
>>>>> gstreamer-encoder: Map the drm format to appropriate Gstreamer format
>>>>> video-stream: Don't stop a stream associated with gl_draw (v2)
>>>>>
>>>>> configure.ac | 2 +-
>>>>> meson.build | 2 +-
>>>>> server/dcc-send.cpp | 159 ++++++++++++++++----
>>>>> server/dcc.cpp | 31 ++--
>>>>> server/dcc.h | 6 +
>>>>> server/display-channel-private.h | 1 +
>>>>> server/display-channel.cpp | 1 +
>>>>> server/gstreamer-encoder.c | 246 ++++++++++++++++++++++++++-----
>>>>> server/video-encoder.h | 14 ++
>>>>> server/video-stream.cpp | 205 ++++++++++++++++++++++----
>>>>> server/video-stream.h | 4 +
>>>>> 11 files changed, 563 insertions(+), 108 deletions(-)
>>>>>
>>>>
>>>> I tested this patchset with several configurations:
>>>>
>>>> - With a passthrough intel gvt-g virtual gpu it works.
>>>>
>>>> - With virtio-vga it works.
>>>>
>>>> - With some extra patches to qemu and an virtualization driver it even
>>>> works with an virtual GPU from an SR-IOV-partitioned Intel Flex
>>>> 140 GPU. Note this extra patches are related to the gpu and not this
>>>> patchset.
>>>>
>>>> This patch is a significant improvement for handling graphically
>>>> demanding tasks or rapidly changing image content and is crucial for
>>>> SPICE to be a component of a competitive virtual desktop infrastructure.
>>>>
>>>> Tested-by: Michael Scherle <michael.scherle at rz.uni-freiburg.de>
>>>>
>>>> Greetings,
>>>> Michael
>>>>
More information about the Spice-devel
mailing list