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

[FFmpeg-devel] [PATCH v2 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 this
king of issues.

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

Thanks
Emil

Aside:
 - Please consider backporting it to the stable branches in due time.
 - I've noticed that we leak state in the error paths, happy to send
   follow-up patches if you'd like those fixed.
 - My TODO includes reducing the massive ABI between libva* and
   backend drivers, to a single extra "registration" API entrypoint.

---

Changes in v2:
 - Add libdl dependency, to address underlinking with older glibc

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                   |  3 +-
 libavutil/hwcontext_vaapi.c | 92 +++++++++++++++++++++++++++++++++++--
 2 files changed, 91 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 v2 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.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
v2: add libdl dependency
---
 configure                   |  3 ++-
 libavutil/hwcontext_vaapi.c | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 18d9b61a99..605afd58a7 100755
--- a/configure
+++ b/configure
@@ -3816,7 +3816,8 @@ 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_deps="libdl"
+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 v2 1/3] hwcontext_vaapi: do not link against libva-x11.so

[Emil Velikov]

On Wed, 20 Jul 2022 at 11:56, Emil Velikov <emil.l.velikov@gmail.com> 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.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> v2: add libdl dependency
> ---
>  configure                   |  3 ++-
>  libavutil/hwcontext_vaapi.c | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 18d9b61a99..605afd58a7 100755
> --- a/configure
> +++ b/configure
> @@ -3816,7 +3816,8 @@ 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_deps="libdl"

Hmm, using _deps seems to be causing issues - "libdl at $avdevice_deps
is not at $LIBRARY_LIST".
In particular, due to the direct dependency between avutil and
avdevice, libdl gets propagated into avdevice_deps and libdl where not
part of $LIBRARY_LIST.

On the other hand, if we use avutil_deps_any libdl does not end up in
avdevice_deps (is this intentional), yet it looks incorrect. We're
providing a single entry in an "any" list, where we do need the
functionality.
Be that explicitly from libdl.so or implicitly from the C runtime.

Looking at all the other deps - perhaps deps_select is the best option
to use here?

Aside: Kicking off `make` after configure is changed does not trigger
[config.mak] regeneration. Is that intentional? All the other projects
that I've used do so.

Thanks in advance,
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 v2 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.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
v2: rebase against the libdl fixup
---
 configure                   |  2 +-
 libavutil/hwcontext_vaapi.c | 48 +++++++++++++++++++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 605afd58a7..a941d4b927 100755
--- a/configure
+++ b/configure
@@ -3817,7 +3817,7 @@ swscale_suggest="libm stdatomic"
 avcodec_extralibs="pthreads_extralibs iconv_extralibs dxva2_extralibs"
 avfilter_extralibs="pthreads_extralibs"
 avutil_deps="libdl"
-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 v2 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 a
compiler guard.

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 v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics

[Mark Thompson]

On 20/07/2022 11:56, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> Similar to the VAAPI_X11 bits, guard all the VAAPI_DRM parts behind a
> compiler guard.
> 
> 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"

No it isn't.

Originally there wasn't a possibility to link with any winsys here - libavcodec users had to get the device themselves and pass it in.

The winsys link was added to the ffmpeg utility initially for command-line use and then moved to libavutil when it was clear that it would be useful to other library users; there isn't any requirement to use it, though.  (E.g. disable it and note that programs handling the winsys themselves like mpv and vlc still work perfectly well.)

- Mark
_______________________________________________
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 v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics

[Emil Velikov]

On Thu, 21 Jul 2022 at 22:06, Mark Thompson <sw@jkqxz.net> wrote:
>
> On 20/07/2022 11:56, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > Similar to the VAAPI_X11 bits, guard all the VAAPI_DRM parts behind a
> > compiler guard.
> >
> > 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"
>
> No it isn't.
>
> Originally there wasn't a possibility to link with any winsys here - libavcodec users had to get the device themselves and pass it in.
>
> The winsys link was added to the ffmpeg utility initially for command-line use and then moved to libavutil when it was clear that it would be useful to other library users; there isn't any requirement to use it, though.  (E.g. disable it and note that programs handling the winsys themselves like mpv and vlc still work perfectly well.)
>

Assuming I'm reading the code correctly, currently when both are
undefined vaapi_device_create() will be basically a dummy, doing some
basic malloc + opts parsing and erroring out.

So as-is functions like av_hwdevice_ctx_alloc() will return success,
although as av_hwdevice_ctx_create() is called later on it will always
fail.
My aim was to effectively omit the HWContextType vaapi instance in the
final libavutil, since it cannot work. Perhaps there's a better way to
achieve that?

In either case, I will drop that check for now.

Thank you o/
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".

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

[Anton Khirnov]

Quoting Emil Velikov (2022-07-22 15:27:26)
> 
> Assuming I'm reading the code correctly, currently when both are
> undefined vaapi_device_create() will be basically a dummy, doing some
> basic malloc + opts parsing and erroring out.
> 
> So as-is functions like av_hwdevice_ctx_alloc() will return success,
> although as av_hwdevice_ctx_create() is called later on it will always
> fail.
> My aim was to effectively omit the HWContextType vaapi instance in the
> final libavutil, since it cannot work. Perhaps there's a better way to
> achieve that?

You seem to have missed that av_hwdevice_ctx_create() is entirely
optional. The users _can_ call it to avoid some boilerplate, but they
can just as well use av_hwdevice_ctx_alloc()+av_hwdevice_ctx_init(),
while opening the device themselves using whatever other means.

-- 
Anton Khirnov
_______________________________________________
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 v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics

[Emil Velikov]

On Thu, 28 Jul 2022 at 15:04, Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Emil Velikov (2022-07-22 15:27:26)
> >
> > Assuming I'm reading the code correctly, currently when both are
> > undefined vaapi_device_create() will be basically a dummy, doing some
> > basic malloc + opts parsing and erroring out.
> >
> > So as-is functions like av_hwdevice_ctx_alloc() will return success,
> > although as av_hwdevice_ctx_create() is called later on it will always
> > fail.
> > My aim was to effectively omit the HWContextType vaapi instance in the
> > final libavutil, since it cannot work. Perhaps there's a better way to
> > achieve that?
>
> You seem to have missed that av_hwdevice_ctx_create() is entirely
> optional. The users _can_ call it to avoid some boilerplate, but they
> can just as well use av_hwdevice_ctx_alloc()+av_hwdevice_ctx_init(),
> while opening the device themselves using whatever other means.
>

Indeed, I have. Thanks for the clarification.

-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 v2 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 this
king of issues.

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

Thanks
Emil

Changes in v2:
 - add linkage against libdl

Aside:
 - Please consider backporting it to the stable branches in due time.
 - I've noticed that we leak state in the error paths, happy to send
   follow-up patches if you'd like those fixed.
 - My TODO includes reducing the massive ABI between libva* and
   backend drivers, to a single extra "registration" API entrypoint.

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".

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

[Nicolas George]

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

Emil Velikov (12022-07-19):
> 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

... a configure check.

If the person who installs replaces a library with another, it is their
responsibility to check they are compatible.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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".

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

[Emil Velikov]

On Tue, 19 Jul 2022 at 19:16, Nicolas George <george@nsup.org> wrote:
>
> Emil Velikov (12022-07-19):
> > 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
>
> ... a configure check.
>
> If the person who installs replaces a library with another, it is their
> responsibility to check they are compatible.
>

While I wholeheartedly agree, it's not so easy to enforce compile time
decisions at runtime. In the past, I have debugged and reported issues
where Linux distributions do not enforce the above.

We do have the typical Linux distribution model (where we have dozens
upon distros) and other distribution models. IMHO checking each
instance and combination doesn't scale. We could bring awareness to
the issue in say distribution/workflow X, which sadly may come as
finger-pointing and thus alienating.

Hope that makes sense and the team is willing to consider the extra 90
lines worth of code.

Thanks in advance
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".

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

[Mark Thompson]

On 20/07/2022 17:41, Emil Velikov wrote:
> On Tue, 19 Jul 2022 at 19:16, Nicolas George <george@nsup.org> wrote:
>>
>> Emil Velikov (12022-07-19):
>>> 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
>>
>> ... a configure check.
>>
>> If the person who installs replaces a library with another, it is their
>> responsibility to check they are compatible.
>>
> 
> While I wholeheartedly agree, it's not so easy to enforce compile time
> decisions at runtime. In the past, I have debugged and reported issues
> where Linux distributions do not enforce the above.
> 
> We do have the typical Linux distribution model (where we have dozens
> upon distros) and other distribution models. IMHO checking each
> instance and combination doesn't scale. We could bring awareness to
> the issue in say distribution/workflow X, which sadly may come as
> finger-pointing and thus alienating.
> 
> Hope that makes sense and the team is willing to consider the extra 90
> lines worth of code.

The argument "libfoo can be broken in some particular configuration, so lets use dlopen() to make errors happen later" seems like it applies to every library.  Why is this case so special?  Who are the users running into this specific problem and who are stuck with broken versions they can't update?

(Also, shouldn't lazy binding save people in this situation if they don't actually use the feature, as they presumably don't if barfing at runtime makes sense?)

Tbh I don't think FFmpeg libraries are the right place to be putting this sort of workaround.

Thanks,

- Mark
_______________________________________________
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 v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm

[Emil Velikov]

On Thu, 21 Jul 2022 at 21:47, Mark Thompson <sw@jkqxz.net> wrote:
>
> On 20/07/2022 17:41, Emil Velikov wrote:
> > On Tue, 19 Jul 2022 at 19:16, Nicolas George <george@nsup.org> wrote:
> >>
> >> Emil Velikov (12022-07-19):
> >>> 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
> >>
> >> ... a configure check.
> >>
> >> If the person who installs replaces a library with another, it is their
> >> responsibility to check they are compatible.
> >>
> >
> > While I wholeheartedly agree, it's not so easy to enforce compile time
> > decisions at runtime. In the past, I have debugged and reported issues
> > where Linux distributions do not enforce the above.
> >
> > We do have the typical Linux distribution model (where we have dozens
> > upon distros) and other distribution models. IMHO checking each
> > instance and combination doesn't scale. We could bring awareness to
> > the issue in say distribution/workflow X, which sadly may come as
> > finger-pointing and thus alienating.
> >
> > Hope that makes sense and the team is willing to consider the extra 90
> > lines worth of code.
>
> The argument "libfoo can be broken in some particular configuration, so lets use dlopen() to make errors happen later" seems like it applies to every library.  Why is this case so special?  Who are the users running into this specific problem and who are stuck with broken versions they can't update?
>
It's a long story, hope I don't bore you to death :-P

Even though I've been itching to hack on ffmpeg for a while, the bug
that allowed me to do that is
https://github.com/ValveSoftware/steam-for-linux/issues/8673

As a background, steam as well as some of the programs/games shipped
use libraries provided by ffmpeg. In addition, steam ships with a
steam runtime, which is effectively a partial chroot of an old Ubuntu.
For various compatibility reasons, one cannot simply update it, so the
startup scripting will try and promote a set of the host libraries (if
newer) so that they're used instead of the bundled Ubuntu ones.

What happens in the libva case is that distributions can provide only
libva.so and omit libva-x11.so. Which due to the internal ABI break
(removal of the va_fool API), means that steam and likely some games
will simply crash out.

Now let me try and draw an analogy to another set of libraries which
also share internal ABI - libdrm.so, libdrm_nouveau.so,
libdrm_amdgpu.so libdrm_intel.so, etc. To the best of my knowledge
there was no breakage in there be that internal or public ABI.

In addition, while distribution may allow you to install only some
(say libdrm.so without libdrm_intel.so), a pair of those is pulled by
the respective GL and Vulkan drivers. For example: the amdgpu GL
driver (amdgpu_dri.so) and radv Vulkand driver (libvulkan_radeon.so)
depend on libdrm_amdgpu.so and libdrm.so. Hence, in practical terms
users cannot hit a similar issue... unless they very very deliberately
try to do so.

So while one solution is to go around telling users and distributions
that they're "doing it wrong", IMHO a more pragmatic solution is to
include this brief workaround in ffmpeg. At least in the short to mid
term.
As mentioned in the cover letter (sorry again for sending the series
multiple times), I have some plans for a proper long term, which would
reside in libva. Alas as you have experienced yourself, the libva
maintainers can be rather busy, so we're looking at least a couple of
months until a new libva release is out and further X months, until it
ripples down to end-users.

> (Also, shouldn't lazy binding save people in this situation if they don't actually use the feature, as they presumably don't if barfing at runtime makes sense?)
>
I suspect you meant using RTLD_LAZY - I believe it should work. Will
test and re-spin for the next revision.

Hope the above makes sense. I tried to stay brief and on-point, but if
anyone likes further details I will be happy to provide.

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".

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

[Timo Rothenpieler]

On 27/07/2022 21:51, Emil Velikov wrote:
> On Thu, 21 Jul 2022 at 21:47, Mark Thompson <sw@jkqxz.net> wrote:
>>
>> On 20/07/2022 17:41, Emil Velikov wrote:
>>> On Tue, 19 Jul 2022 at 19:16, Nicolas George <george@nsup.org> wrote:
>>>>
>>>> Emil Velikov (12022-07-19):
>>>>> 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
>>>>
>>>> ... a configure check.
>>>>
>>>> If the person who installs replaces a library with another, it is their
>>>> responsibility to check they are compatible.
>>>>
>>>
>>> While I wholeheartedly agree, it's not so easy to enforce compile time
>>> decisions at runtime. In the past, I have debugged and reported issues
>>> where Linux distributions do not enforce the above.
>>>
>>> We do have the typical Linux distribution model (where we have dozens
>>> upon distros) and other distribution models. IMHO checking each
>>> instance and combination doesn't scale. We could bring awareness to
>>> the issue in say distribution/workflow X, which sadly may come as
>>> finger-pointing and thus alienating.
>>>
>>> Hope that makes sense and the team is willing to consider the extra 90
>>> lines worth of code.
>>
>> The argument "libfoo can be broken in some particular configuration, so lets use dlopen() to make errors happen later" seems like it applies to every library.  Why is this case so special?  Who are the users running into this specific problem and who are stuck with broken versions they can't update?
>>
> It's a long story, hope I don't bore you to death :-P
> 
> Even though I've been itching to hack on ffmpeg for a while, the bug
> that allowed me to do that is
> https://github.com/ValveSoftware/steam-for-linux/issues/8673
> 
> As a background, steam as well as some of the programs/games shipped
> use libraries provided by ffmpeg. In addition, steam ships with a
> steam runtime, which is effectively a partial chroot of an old Ubuntu.
> For various compatibility reasons, one cannot simply update it, so the
> startup scripting will try and promote a set of the host libraries (if
> newer) so that they're used instead of the bundled Ubuntu ones.

That sounds incredibly broken and will of course cause stuff to break.
I see the issue lies with that magic script, not with ffmpeg.

You could make that exact argument for literally every single external 
library, and we don't dlopen() most of them.
_______________________________________________
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 v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm

[Emil Velikov]

Greetings Timo,

On Wed, 27 Jul 2022 at 21:00, Timo Rothenpieler <timo@rothenpieler.org> wrote:

> That sounds incredibly broken and will of course cause stuff to break.
> I see the issue lies with that magic script, not with ffmpeg.
>

Apologies, never meant to imply that ffmpeg is broken - far from it.
Simply put libva broke their ABI, without bumping the major version.

Hence the libva ABI should be cleaned up and fixed - as mentioned in
my original cover letter and the hunk of my previous email that got
mysteriously removed :-P
As also mentioned, the maintainers are slow, so the odds of having a
fix that reaches end users this year are very close to zero.

What I'm asking here is for a quick short-to-mid term workaround, to
be applied in ffmpeg.

While the startup script is fragile, a similar issue can happen _even
without_ it.

> You could make that exact argument for literally every single external
> library, and we don't dlopen() most of them.

I tried to clarify why and how the libva use-case varies from other
libraries, in my email that you've trimmed out. If my analysis is
incomplete or off, kindly point me to the place that comes as such.

If any developers feel like it might be easier moving some of this
discussion to IRC, do let me know. I can hop on the ffmpeg-devel.

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".

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

[Emil Velikov]

On Wed, 27 Jul 2022 at 20:51, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Thu, 21 Jul 2022 at 21:47, Mark Thompson <sw@jkqxz.net> wrote:
> >
> > On 20/07/2022 17:41, Emil Velikov wrote:
> > > On Tue, 19 Jul 2022 at 19:16, Nicolas George <george@nsup.org> wrote:
> > >>
> > >> Emil Velikov (12022-07-19):
> > >>> 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
> > >>
> > >> ... a configure check.
> > >>
> > >> If the person who installs replaces a library with another, it is their
> > >> responsibility to check they are compatible.
> > >>
> > >
> > > While I wholeheartedly agree, it's not so easy to enforce compile time
> > > decisions at runtime. In the past, I have debugged and reported issues
> > > where Linux distributions do not enforce the above.
> > >
> > > We do have the typical Linux distribution model (where we have dozens
> > > upon distros) and other distribution models. IMHO checking each
> > > instance and combination doesn't scale. We could bring awareness to
> > > the issue in say distribution/workflow X, which sadly may come as
> > > finger-pointing and thus alienating.
> > >
> > > Hope that makes sense and the team is willing to consider the extra 90
> > > lines worth of code.
> >
> > The argument "libfoo can be broken in some particular configuration, so lets use dlopen() to make errors happen later" seems like it applies to every library.  Why is this case so special?  Who are the users running into this specific problem and who are stuck with broken versions they can't update?
> >
> It's a long story, hope I don't bore you to death :-P
>
> Even though I've been itching to hack on ffmpeg for a while, the bug
> that allowed me to do that is
> https://github.com/ValveSoftware/steam-for-linux/issues/8673
>
> As a background, steam as well as some of the programs/games shipped
> use libraries provided by ffmpeg. In addition, steam ships with a
> steam runtime, which is effectively a partial chroot of an old Ubuntu.
> For various compatibility reasons, one cannot simply update it, so the
> startup scripting will try and promote a set of the host libraries (if
> newer) so that they're used instead of the bundled Ubuntu ones.
>
> What happens in the libva case is that distributions can provide only
> libva.so and omit libva-x11.so. Which due to the internal ABI break
> (removal of the va_fool API), means that steam and likely some games
> will simply crash out.
>
> Now let me try and draw an analogy to another set of libraries which
> also share internal ABI - libdrm.so, libdrm_nouveau.so,
> libdrm_amdgpu.so libdrm_intel.so, etc. To the best of my knowledge
> there was no breakage in there be that internal or public ABI.
>
> In addition, while distribution may allow you to install only some
> (say libdrm.so without libdrm_intel.so), a pair of those is pulled by
> the respective GL and Vulkan drivers. For example: the amdgpu GL
> driver (amdgpu_dri.so) and radv Vulkand driver (libvulkan_radeon.so)
> depend on libdrm_amdgpu.so and libdrm.so. Hence, in practical terms
> users cannot hit a similar issue... unless they very very deliberately
> try to do so.
>
> So while one solution is to go around telling users and distributions
> that they're "doing it wrong", IMHO a more pragmatic solution is to
> include this brief workaround in ffmpeg. At least in the short to mid
> term.
> As mentioned in the cover letter (sorry again for sending the series
> multiple times), I have some plans for a proper long term, which would
> reside in libva. Alas as you have experienced yourself, the libva
> maintainers can be rather busy, so we're looking at least a couple of
> months until a new libva release is out and further X months, until it
> ripples down to end-users.
>

Mark, humble ping?

Can you kindly let me know if the above argument seems reasonable and
more importantly if it even makes sense. I am more than happy to
provide more details and elaborate, if anything is unclear.

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".