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@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
Cc: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
Greetings,
Michael