| Message ID | 20220720105646.716398-4-emil.l.velikov@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | hwcontext_vaapi: dlopen libva-x11 and libva-drm | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure_x86 | warning | Failed to run configure |
| yinshiyou/configure_loongarch64 | warning | Failed to run configure |
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
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
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.
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
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;