Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm
@ 2022-07-20 10:56 Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so Emil Velikov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Emil Velikov @ 2022-07-20 10:56 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mark Thompson, emil.l.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so
  2022-07-20 10:56 [FFmpeg-devel] [PATCH v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm Emil Velikov
@ 2022-07-20 10:56 ` Emil Velikov
  2022-07-20 16:23   ` Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 2/3] hwcontext_vaapi: do not link against libva-drm.so Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics Emil Velikov
  2 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2022-07-20 10:56 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mark Thompson, emil.l.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [FFmpeg-devel] [PATCH v2 2/3] hwcontext_vaapi: do not link against libva-drm.so
  2022-07-20 10:56 [FFmpeg-devel] [PATCH v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so Emil Velikov
@ 2022-07-20 10:56 ` Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics Emil Velikov
  2 siblings, 0 replies; 9+ messages in thread
From: Emil Velikov @ 2022-07-20 10:56 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mark Thompson, emil.l.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics
  2022-07-20 10:56 [FFmpeg-devel] [PATCH v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so Emil Velikov
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 2/3] hwcontext_vaapi: do not link against libva-drm.so Emil Velikov
@ 2022-07-20 10:56 ` Emil Velikov
  2022-07-21 21:05   ` Mark Thompson
  2 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2022-07-20 10:56 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mark Thompson, emil.l.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so Emil Velikov
@ 2022-07-20 16:23   ` Emil Velikov
  0 siblings, 0 replies; 9+ messages in thread
From: Emil Velikov @ 2022-07-20 16:23 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Mark Thompson

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics
  2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics Emil Velikov
@ 2022-07-21 21:05   ` Mark Thompson
  2022-07-22 13:27     ` Emil Velikov
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Thompson @ 2022-07-21 21:05 UTC (permalink / raw)
  To: ffmpeg-devel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics
  2022-07-21 21:05   ` Mark Thompson
@ 2022-07-22 13:27     ` Emil Velikov
  2022-07-28 14:04       ` Anton Khirnov
  0 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2022-07-22 13:27 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics
  2022-07-22 13:27     ` Emil Velikov
@ 2022-07-28 14:04       ` Anton Khirnov
  2022-07-29 15:35         ` Emil Velikov
  0 siblings, 1 reply; 9+ messages in thread
From: Anton Khirnov @ 2022-07-28 14:04 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics
  2022-07-28 14:04       ` Anton Khirnov
@ 2022-07-29 15:35         ` Emil Velikov
  0 siblings, 0 replies; 9+ messages in thread
From: Emil Velikov @ 2022-07-29 15:35 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-07-29 15:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20 10:56 [FFmpeg-devel] [PATCH v2 0/3] hwcontext_vaapi: dlopen libva-x11 and libva-drm Emil Velikov
2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 1/3] hwcontext_vaapi: do not link against libva-x11.so Emil Velikov
2022-07-20 16:23   ` Emil Velikov
2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 2/3] hwcontext_vaapi: do not link against libva-drm.so Emil Velikov
2022-07-20 10:56 ` [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: #if guard VAAPI_DRM specifics Emil Velikov
2022-07-21 21:05   ` Mark Thompson
2022-07-22 13:27     ` Emil Velikov
2022-07-28 14:04       ` Anton Khirnov
2022-07-29 15:35         ` Emil Velikov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git