[vlc-commits] hw: vaapi: release the native display from the instance holder
Thomas Guillem
git at videolan.org
Wed Jul 5 11:10:49 CEST 2017
vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | 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;
}
More information about the vlc-commits
mailing list