[FFmpeg-devel] [PATCH 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm

[FFmpeg-devel] [PATCH 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm

[Emil Velikov]

Greetings everyone,

As you may know the libva* set of libraries share an internal ABI
between them. In a resent libva commit, the va_fool API was removed.

Thus if one is to mix different versions of libva.so and libva-x11.so
they will get an error, leading to a crash of the whole stack.

The simple solution is to dlopen() the winsys components, like libva-x11
and libva-drm. The changes are pretty minor and allow us to handle most
of the issues.

Comments and suggestions are welcome, but please me gentle it's my first
time hacking on ffmpeg.

Thanks
Emil

Aside:
 - Please consider backporting it to the stable branches in due time.
 - My TODO includes to reducing the massive ABI between libva* and
backend drivers, which is likely to involve adding a "registration" API.

Emil Velikov (3):
  hwcontext_vaapi: do not link against libva-x11.so
  hwcontext_vaapi: do not link against libva-drm.so
  hwcontext_vaapi: #if guard VAAPI_DRM specifics

 configure                   |  2 +-
 libavutil/hwcontext_vaapi.c | 92 +++++++++++++++++++++++++++++++++++--
 2 files changed, 90 insertions(+), 4 deletions(-)

-- 
2.37.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Emil Velikov]

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(-)

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;
-- 
2.37.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1663 bytes --]

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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] hwcontext_vaapi: do not link against libva-drm.so

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

There is an internal ABI between libva.so and libva-drm.so. So having
mismatched versions can cause all sorts of issues.

We had the breakage between libva.so and libva-x11.so addressed with
earlier commit. There's no point in waiting for things to break wrt
libva-drm.so so pre-emptively, switch to dlopen()-ing the library.

Cc: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 configure                   |  2 +-
 libavutil/hwcontext_vaapi.c | 48 +++++++++++++++++++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ab2a29544c..3d0fe0df61 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 vdpau_x11_extralibs"
+avutil_extralibs="d3d11va_extralibs nanosleep_extralibs pthreads_extralibs vdpau_x11_extralibs"
 
 # programs
 ffmpeg_deps="avcodec avfilter avformat"
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index e44d324928..7734a50fc0 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -31,6 +31,8 @@
 #endif
 #if HAVE_VAAPI_DRM
 #   include <va/va_drm.h>
+#   include <dlfcn.h>
+#   define VA_DRM_LIB "libva-drm.so" VA_ABI
 #endif
 
 #if CONFIG_LIBDRM
@@ -66,6 +68,7 @@ typedef struct VAAPIDevicePriv {
     Display *x11_display;
 #endif
 
+    void *libva_drm;
     int drm_fd;
 } VAAPIDevicePriv;
 
@@ -1582,6 +1585,8 @@ static void vaapi_device_free(AVHWDeviceContext *ctx)
 
     if (priv->drm_fd >= 0)
         close(priv->drm_fd);
+    if (priv->libva_drm)
+        dlclose(priv->libva_drm);
 
     av_freep(&priv);
 }
@@ -1665,6 +1670,8 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
 
 #if HAVE_VAAPI_DRM
     while (!display && try_drm) {
+        VADisplay (*GetDisplayDRM)(int fd);
+
         // If the device is specified, try to open it as a DRM device node.
         // If not, look for a usable render node, possibly restricted to those
         // using a specified kernel driver.
@@ -1722,8 +1729,26 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
                 break;
         }
 
-        display = vaGetDisplayDRM(priv->drm_fd);
+        priv->libva_drm = dlopen(VA_DRM_LIB, RTLD_NOW | RTLD_LOCAL);
+        if (!priv->libva_drm) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot open %s library %s.\n",
+                   VA_DRM_LIB, dlerror());
+            return AVERROR_UNKNOWN;
+        }
+
+        GetDisplayDRM = dlsym(priv->libva_drm, "vaGetDisplayDRM");
+        if (!GetDisplayDRM) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot retrieve %s entrypoint %s.\n",
+                   "vaGetDisplayDRM", dlerror());
+            // Always dlclose after the dlerror(). The former can alter the
+            // error string returned by the latter.
+            dlclose(priv->libva_drm);
+            return AVERROR_UNKNOWN;
+        }
+
+        display = GetDisplayDRM(priv->drm_fd);
         if (!display) {
+            dlclose(priv->libva_drm);
             av_log(ctx, AV_LOG_VERBOSE, "Cannot open a VA display "
                    "from DRM device %s.\n", device);
             return AVERROR_EXTERNAL;
@@ -1811,6 +1836,7 @@ static int vaapi_device_derive(AVHWDeviceContext *ctx,
 #if HAVE_VAAPI_DRM
     if (src_ctx->type == AV_HWDEVICE_TYPE_DRM) {
         AVDRMDeviceContext *src_hwctx = src_ctx->hwctx;
+        VADisplay (*GetDisplayDRM)(int fd);
         VADisplay *display;
         VAAPIDevicePriv *priv;
         int fd;
@@ -1879,8 +1905,26 @@ static int vaapi_device_derive(AVHWDeviceContext *ctx,
         ctx->user_opaque = priv;
         ctx->free        = &vaapi_device_free;
 
-        display = vaGetDisplayDRM(fd);
+        priv->libva_drm = dlopen(VA_DRM_LIB, RTLD_NOW | RTLD_LOCAL);
+        if (!priv->libva_drm) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot open %s library %s.\n",
+                   VA_DRM_LIB, dlerror());
+            return AVERROR_UNKNOWN;
+        }
+
+        GetDisplayDRM = dlsym(priv->libva_drm, "vaGetDisplayDRM");
+        if (!GetDisplayDRM) {
+            av_log(ctx, AV_LOG_ERROR, "Cannot retrieve %s entrypoint %s.\n",
+                   "vaGetDisplayDRM", dlerror());
+            // Always dlclose after the dlerror(). The former can alter the
+            // error string returned by the latter.
+            dlclose(priv->libva_drm);
+            return AVERROR_UNKNOWN;
+        }
+
+        display = GetDisplayDRM(fd);
         if (!display) {
+            dlclose(priv->libva_drm);
             av_log(ctx, AV_LOG_ERROR, "Failed to open a VA display from "
                    "DRM device.\n");
             return AVERROR(EIO);
-- 
2.37.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics

[Emil Velikov]

From: Emil Velikov <emil.velikov@collabora.com>

Similar to the VAAPI_X11 bits, guard all the VAAPI_DRM parts behind an
disabled.

Cc: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 libavutil/hwcontext_vaapi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 7734a50fc0..7aea3e7b96 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -18,6 +18,10 @@
 
 #include "config.h"
 
+#if !HAVE_VAAPI_X11 && !HAVE_VAAPI_DRM
+#error "At least one VAAPI winsys is required X11 or DRM"
+#endif
+
 #if CONFIG_VAAPI_1
 #   define VA_ABI ".2"
 #else
@@ -68,8 +72,10 @@ typedef struct VAAPIDevicePriv {
     Display *x11_display;
 #endif
 
+#if HAVE_VAAPI_DRM
     void *libva_drm;
     int drm_fd;
+#endif
 } VAAPIDevicePriv;
 
 typedef struct VAAPISurfaceFormat {
@@ -1583,10 +1589,12 @@ static void vaapi_device_free(AVHWDeviceContext *ctx)
         XCloseDisplay(priv->x11_display);
 #endif
 
+#if HAVE_VAAPI_DRM
     if (priv->drm_fd >= 0)
         close(priv->drm_fd);
     if (priv->libva_drm)
         dlclose(priv->libva_drm);
+#endif
 
     av_freep(&priv);
 }
@@ -1645,7 +1653,9 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
     if (!priv)
         return AVERROR(ENOMEM);
 
+#if HAVE_VAAPI_DRM
     priv->drm_fd = -1;
+#endif
 
     ctx->user_opaque = priv;
     ctx->free        = vaapi_device_free;
-- 
2.37.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm

[Emil Velikov]

On Tue, 19 Jul 2022 at 17:56, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> Greetings everyone,
>

Sincere apologies for accidentally sending this out multiple times. I
may have found a bug where `git send-email --dry-run` does not work
correctly.

Thanks
Emil
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Emil Velikov]

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(-)

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;
-- 
2.37.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".