Hi Marc-André, > Subject: Re: [PATCH v5 0/7] ui/spice: Enable gl=on option for non-local or > remote clients > > > > > To address the limitation that this option is incompatible with > > remote clients, this patch series adds an option to select a > > preferred codec and also enable gl=on option for clients that > > are connected via the network. In other words, with this option > > enabled (and the below linked Spice series merged), it would be > > possible to have Qemu share a dmabuf fd with Spice, which would > > then forward it to a hardware or software based encoder and > > eventually send the data associated with the fd to a client that > > could be located on a different machine. > > > > Essentially, this patch series provides a hardware accelerated, > > opensource VDI option for users using Qemu and Spice by leveraging > > the iGPU/dGPU on the host machine to encode the Guest FB via the > > Gstreamer framework. > > > > I tested the series on fedora with intel-media-driver installed from > rpmfusion. > > Without explicit video-codecs argument I get: > qemu-system-x86_64: warning: Spice: > ../server/dcc-send.cpp:1780:red_marshall_gl_draw_stream: No video > encoder available for this stream > qemu-system-x86_64: warning: spice: no gl-draw-done within one second > > If I specify video-codecs=gstreamer:h264, then it seems to work fine. > > I wish all of this would be better documented or more explicit, as Do you think it makes sense to use "gstreamer:h264" as default if the user chose -spice gl=on but did not specify any video-codecs explicitly?
> each step took me a while to figure out (why it didn't pick a > compatible encoder, what was the argument for video-codecs, why gst > didn't support h264enc, where to find the encoder, ...). I am not even > sure I am doing all this correctly. Maybe there should be some > docs/interop/spice.rst to document qemu -spice and video-codecs > usages? (and move docs/spice-port-fqdn.txt content there too) The codecs negotiation between Qemu and Spice can definitely be improved as we test/add newer codecs such as AV1, etc. As far as documentation is concerned, I'd like to do it in a follow up series if it is ok with you since it might delay this series given that it would probably require few more revisions/versions to get it right. > > I suppose the issue with better default for video-codecs is on > spice-server side, so for this series: > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> Thank you so much! Thanks, Vivek > > > > v4 -> v5 (suggestions from Marc-André): > > - Fix the errors (mostly 80 chars limit violations) identified by > > scripts/checkpatch.pl > > - Rename the globals to have a spice_ prefix for consistency > > - Rename MAX_REFRESH_RATE to DEFAULT_MAX_REFRESH_RATE > > - Added comments to explain how/when the gl_draw request is submitted > > to spice server in the remote clients case > > - Fix the mem_obj leak that would occur when the associated texture > > is destroyed or when an error is encountered while creating a > > texture from an fd (Dmitry and Michael) > > - Merged Michael's patch to fix the mem_obj leak into this series and > > added his Co-developed-by tag to the relevant patches > > > > v3 -> v4 (suggestions from Marc-André): > > - Add a new parameter to make max_refresh_rate configurable > > - Have surface_gl_create_texture_from_fd() return bool after checking > > for errors > > - Remove the check for PIXMAN_r5g6b5() in spice_gl_replace_fd_texture() > > - Report errors in spice_gl_replace_fd_texture() when someting fails > > - Use glGetError() correctly by adding an additional (dummy) call > > before checking for actual errors (Dmitry) > > - Add a new patch to check fd values in egl_dmabuf_export_texture() > > - Rebase on Qemu master > > > > v2 -> v3: > > - Check for errors after invoking glImportMemoryFdEXT() using > > glGetError() and report the error to user (Dmitry) > > > > v1 -> v2: > > - Replace the option name preferred-codec with video-codecs (Marc-André) > > - Add a warning when an fd cannot be created from texture (Marc-André) > > - Add a new patch to blit the scanout texture into a linear one to > > make it work with virgl > > - Rebased and tested against the latest Spice master > > > > Tested with the following Qemu parameters: > > -device virtio-vga,max_outputs=1,xres=1920,yres=1080,blob=true > > -spice port=3001,gl=on,disable-ticketing=on,video-codecs=gstreamer:h264 > > > > and remote-viewer --spice-debug spice://x.x.x.x:3001 on the client side. > > > > Associated Spice server MR (merged): > > https://gitlab.freedesktop.org/spice/spice/-/merge_requests/229 > > > > --- > > Cc: Gerd Hoffmann <kra...@redhat.com> > > Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > > Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com> > > Cc: Frediano Ziglio <fredd...@gmail.com> > > Cc: Michael Scherle <michael.sche...@rz.uni-freiburg.de> > > Cc: Dongwon Kim <dongwon....@intel.com> > > Cc: Alex Bennée <alex.ben...@linaro.org> > > > > Vivek Kasireddy (7): > > ui/egl-helpers: Error check the fds in egl_dmabuf_export_texture() > > ui/spice: Add an option for users to provide a preferred codec > > ui/spice: Enable gl=on option for non-local or remote clients > > ui/spice: Add an option to submit gl_draw requests at fixed rate > > ui/console-gl: Add a helper to create a texture with linear memory > > layout > > ui/spice: Create a new texture with linear layout when gl=on is > > enabled > > ui/spice: Blit the scanout texture if its memory layout is not linear > > > > include/ui/console.h | 3 + > > include/ui/spice-display.h | 5 + > > include/ui/surface.h | 1 + > > qemu-options.hx | 10 ++ > > ui/console-gl.c | 54 +++++++++ > > ui/egl-helpers.c | 6 + > > ui/spice-core.c | 28 +++++ > > ui/spice-display.c | 226 ++++++++++++++++++++++++++++++++++--- > > 8 files changed, 317 insertions(+), 16 deletions(-) > > > > -- > > 2.49.0 > > > > > > > -- > Marc-André Lureau
