vlc | branch: master | Thomas Guillem <[email protected]> | Wed Jul 5 10:54:13 2017 +0200| [040316e977fb62c18855bf28a512a50f68f15202] | committer: Thomas Guillem
hw: vaapi: release the native display from the instance holder This fixes undefined behaviors when the native display is destroyed before the VADisplay is terminated. This caused a crash or invalid surfaces with the DRM native display when changing between 2 vouts. For X11, no crashes were observed, probably because the native display was already hold by the window. > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=040316e977fb62c18855bf28a512a50f68f15202 --- modules/codec/avcodec/vaapi.c | 59 ++++++++++++++------------- modules/hw/vaapi/vlc_vaapi.c | 15 ++++++- modules/hw/vaapi/vlc_vaapi.h | 10 ++++- modules/video_output/opengl/converter_vaapi.c | 56 +++++++++++++------------ 4 files changed, 81 insertions(+), 59 deletions(-) diff --git a/modules/codec/avcodec/vaapi.c b/modules/codec/avcodec/vaapi.c index 8c00f216c5..a9609b5838 100644 --- a/modules/codec/avcodec/vaapi.c +++ b/modules/codec/avcodec/vaapi.c @@ -55,12 +55,6 @@ struct vlc_va_sys_t { struct vlc_vaapi_instance *va_inst; -#ifdef VLC_VA_BACKEND_XLIB - Display *p_display_x11; -#endif -#ifdef VLC_VA_BACKEND_DRM - int drm_fd; -#endif struct vaapi_context hw_ctx; #ifndef VLC_VA_BACKEND_DR /* XLIB or DRM */ @@ -257,14 +251,21 @@ static void Delete(vlc_va_t *va, void **hwctx) vlc_vaapi_DestroyContext(o, sys->hw_ctx.display, sys->hw_ctx.context_id); vlc_vaapi_DestroyConfig(o, sys->hw_ctx.display, sys->hw_ctx.config_id); vlc_vaapi_ReleaseInstance(sys->va_inst); + free(sys); +} + #ifdef VLC_VA_BACKEND_XLIB - XCloseDisplay(sys->p_display_x11); +static void X11NativeDestroy(VANativeDisplay native) +{ + XCloseDisplay(native); +} #endif #ifdef VLC_VA_BACKEND_DRM - vlc_close(sys->drm_fd); -#endif - free(sys); +static void DRMNativeDestroy(VANativeDisplay native) +{ + vlc_close((intptr_t) native); } +#endif static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt, const es_format_t *fmt, picture_sys_t *p_sys) @@ -304,17 +305,22 @@ static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt, sys->va_inst = NULL; /* Create a VA display */ + VANativeDisplay native = NULL; + vlc_vaapi_native_destroy_cb native_destroy_cb = NULL; #ifdef VLC_VA_BACKEND_XLIB - sys->p_display_x11 = XOpenDisplay(NULL); - if (!sys->p_display_x11) + Display *p_display_x11 = XOpenDisplay(NULL); + if (!p_display_x11) { msg_Err(va, "Could not connect to X server"); goto error; } - sys->hw_ctx.display = vaGetDisplay(sys->p_display_x11); + native = p_display_x11; + native_destroy_cb = X11NativeDestroy; + sys->hw_ctx.display = vaGetDisplay(p_display_x11); #endif #ifdef VLC_VA_BACKEND_DRM + int drm_fd = -1; static const char drm_device_paths[][20] = { "/dev/dri/renderD128", "/dev/dri/card0" @@ -322,16 +328,20 @@ static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt, for (int i = 0; ARRAY_SIZE(drm_device_paths); i++) { - sys->drm_fd = vlc_open(drm_device_paths[i], O_RDWR); - if (sys->drm_fd < 0) + drm_fd = vlc_open(drm_device_paths[i], O_RDWR); + if (drm_fd < 0) continue; - sys->hw_ctx.display = vaGetDisplayDRM(sys->drm_fd); + sys->hw_ctx.display = vaGetDisplayDRM(drm_fd); if (sys->hw_ctx.display) + { + native_destroy_cb = DRMNativeDestroy; + native = (VANativeDisplay *)(intptr_t) drm_fd; break; + } - vlc_close(sys->drm_fd); - sys->drm_fd = -1; + vlc_close(drm_fd); + drm_fd = -1; } #endif if (sys->hw_ctx.display == NULL) @@ -340,7 +350,8 @@ static int Create(vlc_va_t *va, AVCodecContext *ctx, enum PixelFormat pix_fmt, goto error; } - sys->va_inst = vlc_vaapi_InitializeInstance(o, sys->hw_ctx.display); + sys->va_inst = vlc_vaapi_InitializeInstance(o, sys->hw_ctx.display, native, + native_destroy_cb); if (!sys->va_inst) goto error; @@ -387,16 +398,6 @@ error: vlc_vaapi_DestroyConfig(o, sys->hw_ctx.display, sys->hw_ctx.config_id); if (sys->va_inst != NULL) vlc_vaapi_ReleaseInstance(sys->va_inst); - else if (sys->hw_ctx.display != NULL) - vaTerminate(sys->hw_ctx.display); -#ifdef VLC_VA_BACKEND_XLIB - if( sys->p_display_x11 != NULL) - XCloseDisplay(sys->p_display_x11); -#endif -#ifdef VLC_VA_BACKEND_DRM - if( sys->drm_fd != -1 ) - vlc_close(sys->drm_fd); -#endif free(sys); return VLC_EGENERIC; } diff --git a/modules/hw/vaapi/vlc_vaapi.c b/modules/hw/vaapi/vlc_vaapi.c index f376ddd713..7dfe2aea45 100644 --- a/modules/hw/vaapi/vlc_vaapi.c +++ b/modules/hw/vaapi/vlc_vaapi.c @@ -61,23 +61,32 @@ struct vlc_vaapi_instance { VADisplay dpy; + VANativeDisplay native; + vlc_vaapi_native_destroy_cb native_destroy_cb; atomic_uint pic_refcount; }; struct vlc_vaapi_instance * -vlc_vaapi_InitializeInstance(vlc_object_t *o, VADisplay dpy) +vlc_vaapi_InitializeInstance(vlc_object_t *o, VADisplay dpy, + VANativeDisplay native, + vlc_vaapi_native_destroy_cb native_destroy_cb) { int major = 0, minor = 0; VA_CALL(o, vaInitialize, dpy, &major, &minor); struct vlc_vaapi_instance *inst = malloc(sizeof(*inst)); if (unlikely(inst == NULL)) - return NULL; + goto error; inst->dpy = dpy; + inst->native = native; + inst->native_destroy_cb = native_destroy_cb; atomic_init(&inst->pic_refcount, 1); return inst; error: + vaTerminate(dpy); + if (native != NULL && native_destroy_cb != NULL) + native_destroy_cb(native); return NULL; } @@ -94,6 +103,8 @@ vlc_vaapi_ReleaseInstance(struct vlc_vaapi_instance *inst) if (atomic_fetch_sub(&inst->pic_refcount, 1) == 1) { vaTerminate(inst->dpy); + if (inst->native != NULL && inst->native_destroy_cb != NULL) + inst->native_destroy_cb(inst->native); free(inst); } } diff --git a/modules/hw/vaapi/vlc_vaapi.h b/modules/hw/vaapi/vlc_vaapi.h index 0efd3370b5..a497a12746 100644 --- a/modules/hw/vaapi/vlc_vaapi.h +++ b/modules/hw/vaapi/vlc_vaapi.h @@ -35,11 +35,17 @@ * VA instance management * **************************/ +typedef void (*vlc_vaapi_native_destroy_cb)(VANativeDisplay); struct vlc_vaapi_instance; -/* Initializes the VADisplay and sets the reference counter to 1. */ +/* Initializes the VADisplay and sets the reference counter to 1. If not NULL, + * native_destroy_cb will be called when the instance is released in order to + * destroy the native holder (that can be a drm/x11/wl). On error, dpy is + * terminated and the destroy callback is called. */ struct vlc_vaapi_instance * -vlc_vaapi_InitializeInstance(vlc_object_t *o, VADisplay dpy); +vlc_vaapi_InitializeInstance(vlc_object_t *o, VADisplay dpy, + VANativeDisplay native, + vlc_vaapi_native_destroy_cb native_destroy_cb); /* Increments the VAAPI instance refcount */ VADisplay diff --git a/modules/video_output/opengl/converter_vaapi.c b/modules/video_output/opengl/converter_vaapi.c index 567f0c5c75..7c8129a48b 100644 --- a/modules/video_output/opengl/converter_vaapi.c +++ b/modules/video_output/opengl/converter_vaapi.c @@ -51,9 +51,6 @@ struct priv VADisplay vadpy; VASurfaceID *va_surface_ids; PFNGLEGLIMAGETARGETTEXTURE2DOESPROC glEGLImageTargetTexture2DOES; -#ifdef HAVE_VA_X11 - Display *x11dpy; -#endif unsigned fourcc; EGLint drm_fourccs[3]; @@ -255,11 +252,6 @@ tc_vaegl_release(const opengl_tex_converter_t *tc) vlc_vaapi_ReleaseInstance(priv->vainst); -#ifdef HAVE_VA_X11 - if (priv->x11dpy != NULL) - XCloseDisplay(priv->x11dpy); -#endif - free(tc->priv); } @@ -293,10 +285,12 @@ tc_va_check_interop_blacklist(opengl_tex_converter_t *tc, VADisplay *vadpy) } static int -tc_vaegl_init(opengl_tex_converter_t *tc, VADisplay *vadpy) +tc_vaegl_init(opengl_tex_converter_t *tc, VADisplay *vadpy, + VANativeDisplay native, + vlc_vaapi_native_destroy_cb native_destroy_cb) { if (vadpy == NULL) - return VLC_EGENERIC; + goto error; int ret = VLC_ENOMEM; struct priv *priv = tc->priv = calloc(1, sizeof(struct priv)); @@ -319,9 +313,15 @@ tc_vaegl_init(opengl_tex_converter_t *tc, VADisplay *vadpy) tc->pf_release = tc_vaegl_release; tc->pf_get_pool = tc_vaegl_get_pool; - priv->vainst = vlc_vaapi_InitializeInstance(VLC_OBJECT(tc->gl), priv->vadpy); + priv->vainst = vlc_vaapi_InitializeInstance(VLC_OBJECT(tc->gl), priv->vadpy, + native, native_destroy_cb); if (priv->vainst == NULL) + { + /* Already released by vlc_vaapi_InitializeInstance */ + vadpy = NULL; + native_destroy_cb = NULL; goto error; + } if (tc_va_check_interop_blacklist(tc, priv->vadpy)) goto error; @@ -337,10 +337,23 @@ error: if (priv->vainst) vlc_vaapi_ReleaseInstance(priv->vainst); else - vaTerminate(vadpy); - free(tc->priv); + { + if (vadpy != NULL) + vaTerminate(vadpy); + if (native != NULL && native_destroy_cb != NULL) + native_destroy_cb(native); + } + if (tc->priv) + free(tc->priv); return ret; } +#ifdef HAVE_VA_X11 +static void +x11_native_destroy_cb(VANativeDisplay native) +{ + XCloseDisplay(native); +} +#endif int opengl_tex_converter_vaapi_init(opengl_tex_converter_t *tc) @@ -368,25 +381,16 @@ opengl_tex_converter_vaapi_init(opengl_tex_converter_t *tc) if (x11dpy == NULL) return VLC_EGENERIC; - if (tc_vaegl_init(tc, vaGetDisplay(x11dpy))) - { - XCloseDisplay(x11dpy); - return VLC_EGENERIC; - } - struct priv *priv = tc->priv; - priv->x11dpy = x11dpy; - break; + return tc_vaegl_init(tc, vaGetDisplay(x11dpy), x11dpy, + x11_native_destroy_cb); } #endif #ifdef HAVE_VA_WL case VOUT_WINDOW_TYPE_WAYLAND: - if (tc_vaegl_init(tc, vaGetDisplayWl(tc->gl->surface->display.wl))) - return VLC_EGENERIC; - break; + return tc_vaegl_init(tc, vaGetDisplayWl(tc->gl->surface->display.wl), + NULL, NULL); #endif default: return VLC_EGENERIC; } - - return VLC_SUCCESS; } _______________________________________________ vlc-commits mailing list [email protected] https://mailman.videolan.org/listinfo/vlc-commits
