> > 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); > > > }