diff mbox series

[FFmpeg-devel] hwcontext_vaapi: Only accept a render node when deriving from DRM device

Message ID ada93619-8fc2-717f-9f45-f1b35208b76a@jkqxz.net
State Accepted
Headers show
Series [FFmpeg-devel] hwcontext_vaapi: Only accept a render node when deriving from DRM device | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Mark Thompson Feb. 16, 2020, 8:59 p.m. UTC 
If we are given a non-render node, try to find the matching render node and
fail if that isn't possible.

libva will not accept a non-render device which is not DRM master, because
it requires legacy DRM authentication to succeed in that case:
<https://github.com/intel/libva/blob/master/va/drm/va_drm.c#L68-L75>.  This
is annoying for kmsgrab because in most recording situations DRM master is
already held by something else (such as a windowing system), leading to
device derivation not working and forcing the user to create the target
VAAPI device separately.
---
Fixes a longstanding annoyance with -vf hwmap=derive_device=vaapi.

(E.g. <http://ffmpeg.org/pipermail/ffmpeg-user/2020-February/046660.html>.)


 libavutil/hwcontext_vaapi.c | 46 ++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

Comments

Anton Khirnov Feb. 18, 2020, 2:23 p.m. UTC | #1 
Quoting Mark Thompson (2020-02-16 21:59:54)
> If we are given a non-render node, try to find the matching render node and
> fail if that isn't possible.
> 
> libva will not accept a non-render device which is not DRM master, because
> it requires legacy DRM authentication to succeed in that case:
> <https://github.com/intel/libva/blob/master/va/drm/va_drm.c#L68-L75>.  This
> is annoying for kmsgrab because in most recording situations DRM master is
> already held by something else (such as a windowing system), leading to
> device derivation not working and forcing the user to create the target
> VAAPI device separately.
> ---
> Fixes a longstanding annoyance with -vf hwmap=derive_device=vaapi.
> 
> (E.g. <http://ffmpeg.org/pipermail/ffmpeg-user/2020-February/046660.html>.)

Looks reasonable.
Mark Thompson Feb. 24, 2020, 12:25 a.m. UTC | #2 
On 18/02/2020 14:23, Anton Khirnov wrote:
> Quoting Mark Thompson (2020-02-16 21:59:54)
>> If we are given a non-render node, try to find the matching render node and
>> fail if that isn't possible.
>>
>> libva will not accept a non-render device which is not DRM master, because
>> it requires legacy DRM authentication to succeed in that case:
>> <https://github.com/intel/libva/blob/master/va/drm/va_drm.c#L68-L75>.  This
>> is annoying for kmsgrab because in most recording situations DRM master is
>> already held by something else (such as a windowing system), leading to
>> device derivation not working and forcing the user to create the target
>> VAAPI device separately.
>> ---
>> Fixes a longstanding annoyance with -vf hwmap=derive_device=vaapi.
>>
>> (E.g. <http://ffmpeg.org/pipermail/ffmpeg-user/2020-February/046660.html>.)
> 
> Looks reasonable.

Applied.

Thanks,

- Mark
diff mbox series

Patch

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index cf117640f2..d3bea3070b 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -1628,6 +1628,7 @@  static int vaapi_device_derive(AVHWDeviceContext *ctx,
         AVDRMDeviceContext *src_hwctx = src_ctx->hwctx;
         VADisplay *display;
         VAAPIDevicePriv *priv;
+        int fd;
 
         if (src_hwctx->fd < 0) {
             av_log(ctx, AV_LOG_ERROR, "DRM instance requires an associated "
@@ -1635,17 +1636,56 @@  static int vaapi_device_derive(AVHWDeviceContext *ctx,
             return AVERROR(EINVAL);
         }
 
+#if CONFIG_LIBDRM
+        {
+            int node_type = drmGetNodeTypeFromFd(src_hwctx->fd);
+            char *render_node;
+            if (node_type < 0) {
+                av_log(ctx, AV_LOG_ERROR, "DRM instance fd does not appear "
+                       "to refer to a DRM device.\n");
+                return AVERROR(EINVAL);
+            }
+            if (node_type == DRM_NODE_RENDER) {
+                fd = src_hwctx->fd;
+            } else {
+                render_node = drmGetRenderDeviceNameFromFd(src_hwctx->fd);
+                if (!render_node) {
+                    av_log(ctx, AV_LOG_ERROR, "Failed to find a render node "
+                           "matching the DRM device.\n");
+                    return AVERROR(ENODEV);
+                }
+                fd = open(render_node, O_RDWR);
+                if (fd < 0) {
+                    av_log(ctx, AV_LOG_ERROR, "Failed to open render node %s"
+                           "matching the DRM device.\n", render_node);
+                    free(render_node);
+                    return AVERROR(errno);
+                }
+                av_log(ctx, AV_LOG_VERBOSE, "Using render node %s in place "
+                       "of non-render DRM device.\n", render_node);
+                free(render_node);
+            }
+        }
+#else
+        fd = src_hwctx->fd;
+#endif
+
         priv = av_mallocz(sizeof(*priv));
         if (!priv)
             return AVERROR(ENOMEM);
 
-        // Inherits the fd from the source context, which will close it.
-        priv->drm_fd = -1;
+        if (fd == src_hwctx->fd) {
+            // The fd is inherited from the source context and we are holding
+            // a reference to that, we don't want to close it from here.
+            priv->drm_fd = -1;
+        } else {
+            priv->drm_fd = fd;
+        }
 
         ctx->user_opaque = priv;
         ctx->free        = &vaapi_device_free;
 
-        display = vaGetDisplayDRM(src_hwctx->fd);
+        display = vaGetDisplayDRM(fd);
         if (!display) {
             av_log(ctx, AV_LOG_ERROR, "Failed to open a VA display from "
                    "DRM device.\n");