diff mbox series

[FFmpeg-devel,1/3] hwcontext_vaapi: do not link against libva-x11.so

Message ID 20220719165656.640869-2-emil.l.velikov@gmail.com
State New
Headers show
Series hwcontext_vaapi: dlopen libva-x11 and libva-drm | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

Emil Velikov July 19, 2022, 4:56 p.m. UTC 
From: Emil Velikov <emil.velikov@collabora.com>

There is an internal ABI between libva.so the libva-XXX.so libraries.

With a recent change, the internal va_fool API was removed breaking the
ABI. So if libva.so and libva-x11.so are from different version, the
whole stack will crash.

Instead we can dlopen() the libva-x11 library and gracefully error out.

Cc: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Greetings team, please consider backporting this for the stable
releases.

I've noticed that we've got plenty of pre-existing memory and state
leaks in vaapi_device_create(). Would it make sense to fix those up? If
so I can follow-up with a patch.
---
 configure                   |  2 +-
 libavutil/hwcontext_vaapi.c | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer July 19, 2022, 10:11 p.m. UTC | #1 
On Tue, Jul 19, 2022 at 05:56:54PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> There is an internal ABI between libva.so the libva-XXX.so libraries.
> 
> With a recent change, the internal va_fool API was removed breaking the
> ABI. So if libva.so and libva-x11.so are from different version, the
> whole stack will crash.
> 
> Instead we can dlopen() the libva-x11 library and gracefully error out.
> 
> Cc: Mark Thompson <sw@jkqxz.net>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Greetings team, please consider backporting this for the stable
> releases.
> 
> I've noticed that we've got plenty of pre-existing memory and state
> leaks in vaapi_device_create(). Would it make sense to fix those up? If
> so I can follow-up with a patch.
> ---
>  configure                   |  2 +-
>  libavutil/hwcontext_vaapi.c | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 2 deletions(-)

../configure --enable-gpl --cc='ccache gcc'  --enable-version3    --enable-vdpau
make -j32
...
LD	ffmpeg_g
/usr/bin/ld: libavutil/libavutil.a(hwcontext_vaapi.o): undefined reference to symbol 'dlclose@@GLIBC_2.2.5'
//lib/x86_64-linux-gnu/libdl.so.2: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
/home/michael/ffmpeg-git/ffmpeg/Makefile:131: recipe for target 'ffmpeg_g' failed
make: *** [ffmpeg_g] Error 1


[...]
diff mbox series

Patch

diff --git a/configure b/configure
index 18d9b61a99..ab2a29544c 100755
--- a/configure
+++ b/configure
@@ -3816,7 +3816,7 @@  swscale_suggest="libm stdatomic"
 
 avcodec_extralibs="pthreads_extralibs iconv_extralibs dxva2_extralibs"
 avfilter_extralibs="pthreads_extralibs"
-avutil_extralibs="d3d11va_extralibs nanosleep_extralibs pthreads_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs"
+avutil_extralibs="d3d11va_extralibs nanosleep_extralibs pthreads_extralibs vaapi_drm_extralibs vdpau_x11_extralibs"
 
 # programs
 ffmpeg_deps="avcodec avfilter avformat"
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index c3a98bc4b1..e44d324928 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -18,8 +18,16 @@ 
 
 #include "config.h"
 
+#if CONFIG_VAAPI_1
+#   define VA_ABI ".2"
+#else
+#   define VA_ABI ".1"
+#endif
+
 #if HAVE_VAAPI_X11
 #   include <va/va_x11.h>
+#   include <dlfcn.h>
+#   define VA_X11_LIB "libva-x11.so" VA_ABI
 #endif
 #if HAVE_VAAPI_DRM
 #   include <va/va_drm.h>
@@ -54,6 +62,7 @@ 
 
 typedef struct VAAPIDevicePriv {
 #if HAVE_VAAPI_X11
+    void *libva_x11;
     Display *x11_display;
 #endif
 
@@ -1565,6 +1574,8 @@  static void vaapi_device_free(AVHWDeviceContext *ctx)
         vaTerminate(hwctx->display);
 
 #if HAVE_VAAPI_X11
+    if (priv->libva_x11)
+        dlclose(priv->libva_x11);
     if (priv->x11_display)
         XCloseDisplay(priv->x11_display);
 #endif
@@ -1723,14 +1734,35 @@  static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
 
 #if HAVE_VAAPI_X11
     if (!display && try_x11) {
+        VADisplay (*GetDisplay)(Display *dpy);
+
+        priv->libva_x11 = dlopen(VA_X11_LIB, RTLD_NOW | RTLD_LOCAL);
+        if (!priv->libva_x11) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot open %s library %s.\n",
+                   VA_X11_LIB, dlerror());
+            return AVERROR_UNKNOWN;
+        }
+
+        GetDisplay = dlsym(priv->libva_x11, "vaGetDisplay");
+        if (!GetDisplay) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot retrieve %s entrypoint %s.\n",
+                   "vaGetDisplay", dlerror());
+            // Always dlclose after the dlerror(). The former can alter the
+            // error string returned by the latter.
+            dlclose(priv->libva_x11);
+            return AVERROR_UNKNOWN;
+        }
+
         // Try to open the device as an X11 display.
         priv->x11_display = XOpenDisplay(device);
         if (!priv->x11_display) {
+            dlclose(priv->libva_x11);
             av_log(ctx, AV_LOG_VERBOSE, "Cannot open X11 display "
                    "%s.\n", XDisplayName(device));
         } else {
-            display = vaGetDisplay(priv->x11_display);
+            display = GetDisplay(priv->x11_display);
             if (!display) {
+                dlclose(priv->libva_x11);
                 av_log(ctx, AV_LOG_ERROR, "Cannot open a VA display "
                        "from X11 display %s.\n", XDisplayName(device));
                 return AVERROR_UNKNOWN;