Re: [Qemu-devel] [Spice-devel] [PATCH Qemu] Change spice-server protocol for GL texture passing

[Frediano Ziglio <fziglio@redhat.com>]

> 
> Hi
> 
> ----- Original Message -----
> > Forgot to add RFC to the subject
> > 
> 
> What's the rationale? if you share the texture id, you must share the GL
> context too, right? Why not use a lower level dmabuf fd that can be imported
> by the server gl context (which is also what the protocol require anyway)?
> 

Yes, the display and context are shared using spice_qxl_gl_init.
Importing again into a gl context would mean that you have to export the
DRM prime and import again in a separate (not shared) context.
It's also doable, just add 2 system call and wrapping/unwrapping.
Would be good to pass the EGLDisplay then so spice-server don't have to
initialize again possibly using another physical card.

We have 4 cases:
- client not connected;
- local client;
- remote client, software encoding;
- remote client, hardware encoding.

Client not connected
Passing the texture is a no-operation, passing DRM prime require to
extract the handle and close every frame.

Local client
In this case there is no overhear, DRM prime is always extracted and
passed to the client

Remote client, software encoding
Due to different problems (DRM prime not mmap-able or data not portably
extractable) we'll need to import the DRM prime into a different EGL
context (not shared with the original one), create another texture,
extract data and free all texture/DRM prime.

Remote client, hardware encoding
It's not clear if it's better to pass the DRM prime or the texture,
some API pass the texture. I got confirmation that gst_dmabuf_allocator_new
could try to use mmap in some cases so we should check this somehow
to make sure it does not.

Taking into account that DRM prime came with "free" reference counting
creating the DRM prime from texture basically increase a counter which is
used by our implementation to make sure texture is still existing so
possibly passing texture instead of DRM prime just save a system call
in the normal case. I don't know what happens to the DRM object handle when
the texture is destroyed (in Qemu) with glDeleteTextures if bindings keep
texture "alive" or are all reset.


Could be that keeping qemu_spice_gl_scanout and spice_qxl_gl_scanout_texture
as current implementation and adding a spice_qxl_gl_init/spice_qxl_gl_setup
passing just QXLInstance and EGLDisplay is a better solution.

Does is sound reasonable?

Frediano

> > 
> > > 
> > > ---
> > >  ui/spice-core.c    |  5 -----
> > >  ui/spice-display.c | 29 ++++++++---------------------
> > >  2 files changed, 8 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/ui/spice-core.c b/ui/spice-core.c
> > > index da05054..f7647f7 100644
> > > --- a/ui/spice-core.c
> > > +++ b/ui/spice-core.c
> > > @@ -828,11 +828,6 @@ void qemu_spice_init(void)
> > >  
> > >  #ifdef HAVE_SPICE_GL
> > >      if (qemu_opt_get_bool(opts, "gl", 0)) {
> > > -        if ((port != 0) || (tls_port != 0)) {
> > > -            error_report("SPICE GL support is local-only for now and "
> > > -                         "incompatible with -spice port/tls-port");
> > > -            exit(1);
> > > -        }
> > >          if (egl_rendernode_init() != 0) {
> > >              error_report("Failed to initialize EGL render node for SPICE
> > >              GL");
> > >              exit(1);
> > > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > > index 2a77a54..72137bd 100644
> > > --- a/ui/spice-display.c
> > > +++ b/ui/spice-display.c
> > > @@ -852,6 +852,10 @@ static void qemu_spice_gl_block_timer(void *opaque)
> > >  static QEMUGLContext qemu_spice_gl_create_context(DisplayChangeListener
> > >  *dcl,
> > >                                                    QEMUGLParams *params)
> > >  {
> > > +    SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay,
> > > dcl);
> > > +
> > > +    spice_qxl_gl_init(&ssd->qxl, qemu_egl_display, qemu_egl_rn_ctx);
> > > +
> > >      eglMakeCurrent(qemu_egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE,
> > >                     qemu_egl_rn_ctx);
> > >      return qemu_egl_create_context(dcl, params);
> > > @@ -864,28 +868,11 @@ static void
> > > qemu_spice_gl_scanout(DisplayChangeListener
> > > *dcl,
> > >                                    uint32_t w, uint32_t h)
> > >  {
> > >      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay,
> > >      dcl);
> > > -    EGLint stride = 0, fourcc = 0;
> > > -    int fd = -1;
> > > -
> > > -    if (tex_id) {
> > > -        fd = egl_get_fd_for_texture(tex_id, &stride, &fourcc);
> > > -        if (fd < 0) {
> > > -            fprintf(stderr, "%s: failed to get fd for texture\n",
> > > __func__);
> > > -            return;
> > > -        }
> > > -        dprint(1, "%s: %dx%d (stride %d, fourcc 0x%x)\n", __func__,
> > > -               w, h, stride, fourcc);
> > > -    } else {
> > > -        dprint(1, "%s: no texture (no framebuffer)\n", __func__);
> > > -    }
> > > -
> > > -    assert(!tex_id || fd >= 0);
> > >  
> > > -    /* note: spice server will close the fd */
> > > -    spice_qxl_gl_scanout(&ssd->qxl, fd,
> > > -                         surface_width(ssd->ds),
> > > -                         surface_height(ssd->ds),
> > > -                         stride, fourcc, y_0_top);
> > > +    spice_qxl_gl_scanout_texture(&ssd->qxl, tex_id,
> > > +                                 surface_width(ssd->ds),
> > > +                                 surface_height(ssd->ds),
> > > +                                 y_0_top);
> > >  
> > >      qemu_spice_gl_monitor_config(ssd, x, y, w, h);
> > >  }