[FFmpeg-devel] libavutil/hwcontext_qsv: Command line using hwaccel 'QSV' doesn't work

From 687ce9c804b2618f021100235c46a33b48fa522c Mon Sep 17 00:00:00 2001
From: Zhengxu <zhengxu.maxwell@gmail.com>
Date: Wed, 14 Dec 2016 11:55:31 +0800
Subject: [PATCH] libavutil/hwcontext_qsv: Command line using hwaccel 'QSV'
 doesn't work.

Command: ffmpeg -hwaccel qsv -c:v h264_qsv -i test.264 -c:v h264_qsv out.264

Reason: hwcontext_qsv will create a child hwcontext_vaapi. VAAPI will
 open X11 display ":0.0" defaultly. However, MSDK doesn't support X11 so
 far. This results in the failure of this command.

Fix: When using VAAPI, let VAAPI try to create DRM display handle by scanning
 device nodes under '/dev/dri/'.

Signed-off-by: ChaoX A Liu <chaox.a.liu@gmail.com>
Signed-off-by: Huang, Zhengxu <zhengxu.maxwell@gmail.com>
Signed-off-by: Andrew, Zhang <huazh407@gmail.com>
---
 libavutil/hwcontext_qsv.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

On 03/01/17 07:13, Huang, Zhengxu wrote:
> From 687ce9c804b2618f021100235c46a33b48fa522c Mon Sep 17 00:00:00 2001
> From: Zhengxu <zhengxu.maxwell@gmail.com>
> Date: Wed, 14 Dec 2016 11:55:31 +0800
> Subject: [PATCH] libavutil/hwcontext_qsv: Command line using hwaccel 'QSV'
>  doesn't work.
> 
> Command: ffmpeg -hwaccel qsv -c:v h264_qsv -i test.264 -c:v h264_qsv out.264
> 
> Reason: hwcontext_qsv will create a child hwcontext_vaapi. VAAPI will
>  open X11 display ":0.0" defaultly. However, MSDK doesn't support X11 so
>  far. This results in the failure of this command.
> 
> Fix: When using VAAPI, let VAAPI try to create DRM display handle by scanning
>  device nodes under '/dev/dri/'.

We already default to attempting to open the first render node inside hwcontext_vaapi (if opening via X11 fails):

http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/hwcontext_vaapi.c;h=6176bdc880c81dec7cac0b214a8d55f3b1160abc;hb=HEAD#l936

I think if you want this behaviour it would be better to add code there rather than in hwcontext_qsv (which doesn't really care about this aspect at all, it just wants a usable subdevice for the platform).

Can you explain your case which hits this?  Do you have some other external graphics card(s) along with the on-chip Intel graphics?  For that case, I don't like the idea of scanning for a device node because it is perfectly possible to get a valid VADisplay handle for a non-QSV device (an AMD or Nvidia card with mesa, most obviously) which will then fail opaquely later when libmfx tries to use it because the Intel proprietary driver is required.  This may even fail randomly, because device nodes associated with independent drivers are not ordered.

> Signed-off-by: ChaoX A Liu <chaox.a.liu@gmail.com>
> Signed-off-by: Huang, Zhengxu <zhengxu.maxwell@gmail.com>
> Signed-off-by: Andrew, Zhang <huazh407@gmail.com>
> ---
>  libavutil/hwcontext_qsv.c | 44 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index 2dc9aca..2701b5a 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -707,12 +707,41 @@ static mfxIMPL choose_implementation(const char *device)
>      return impl;
>  }
>  
> +static int create_proper_child_device(AVBufferRef **ctx, const char *device, int flags)
> +{
> +    enum AVHWDeviceType child_device_type;
> +    char adapter[256];
> +    int  adapter_num;
> +
> +    if (CONFIG_VAAPI)
> +        child_device_type = AV_HWDEVICE_TYPE_VAAPI;
> +    else if (CONFIG_DXVA2)
> +        child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> +    else
> +        return AVERROR(ENOSYS);
> +
> +    if (device || CONFIG_DXVA2)
> +        return av_hwdevice_ctx_create(ctx, child_device_type, device, NULL, flags);
> +
> +    for (adapter_num = 0; adapter_num < 6; adapter_num++) {
> +        if (adapter_num < 3)
> +            snprintf(adapter,sizeof(adapter),
> +                "/dev/dri/renderD%d", adapter_num+128);
> +        else
> +            snprintf(adapter,sizeof(adapter),
> +                "/dev/dri/card%d", adapter_num-3);

I would prefer not to open the DRM master device (/dev/dri/card*) by default - until very recent kernels it was exclusive-access-only, so nothing else can use the graphics at the same time (most obviously another ffmpeg instance).

> +        if (av_hwdevice_ctx_create(ctx, child_device_type, adapter, NULL, flags) == 0)
> +            return 0;
> +    }
> +
> +    return AVERROR(ENOSYS);
> +}
> +
>  static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
>                               AVDictionary *opts, int flags)
>  {
>      AVQSVDeviceContext *hwctx = ctx->hwctx;
>      QSVDevicePriv *priv;
> -    enum AVHWDeviceType child_device_type;
>      AVDictionaryEntry *e;
>  
>      mfxVersion    ver = { { 3, 1 } };
> @@ -730,18 +759,7 @@ static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
>      ctx->free        = qsv_device_free;
>  
>      e = av_dict_get(opts, "child_device", NULL, 0);
> -
> -    if (CONFIG_VAAPI)
> -        child_device_type = AV_HWDEVICE_TYPE_VAAPI;
> -    else if (CONFIG_DXVA2)
> -        child_device_type = AV_HWDEVICE_TYPE_DXVA2;
> -    else {
> -        av_log(ctx, AV_LOG_ERROR, "No supported child device type is enabled\n");
> -        return AVERROR(ENOSYS);
> -    }
> -
> -    ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
> -                                 e ? e->value : NULL, NULL, 0);
> +    ret = create_proper_child_device(&priv->child_device_ctx, e ? e->value : NULL, 0);
>      if (ret < 0)
>          return ret;
>  
> -- 
> 1.8.3.1
> 

For your specific case in the ffmpeg utility it might be best to add a new command-line option (-qsv_device?) and then pass it as the "child_device" in the options dictionary?  I admit this isn't necessarily helpful for other users, but it would fix your command above without running into the other problems above.

Thanks,

- Mark

在 2017/1/3 21:14, Mark Thompson 写道:

> On 03/01/17 07:13, Huang, Zhengxu wrote:
>>  From 687ce9c804b2618f021100235c46a33b48fa522c Mon Sep 17 00:00:00 2001
>> From: Zhengxu <zhengxu.maxwell@gmail.com>
>> Date: Wed, 14 Dec 2016 11:55:31 +0800
>> Subject: [PATCH] libavutil/hwcontext_qsv: Command line using hwaccel 'QSV'
>>   doesn't work.
>>
>> Command: ffmpeg -hwaccel qsv -c:v h264_qsv -i test.264 -c:v h264_qsv out.264
>>
>> Reason: hwcontext_qsv will create a child hwcontext_vaapi. VAAPI will
>>   open X11 display ":0.0" defaultly. However, MSDK doesn't support X11 so
>>   far. This results in the failure of this command.
>>
>> Fix: When using VAAPI, let VAAPI try to create DRM display handle by scanning
>>   device nodes under '/dev/dri/'.
> We already default to attempting to open the first render node inside hwcontext_vaapi (if opening via X11 fails):
>
> http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/hwcontext_vaapi.c;h=6176bdc880c81dec7cac0b214a8d55f3b1160abc;hb=HEAD#l936
>
> I think if you want this behaviour it would be better to add code there rather than in hwcontext_qsv (which doesn't really care about this aspect at all, it just wants a usable subdevice for the platform).
your concern makes sense and this fix does may introduce some other problem.
> Can you explain your case which hits this?  Do you have some other external graphics card(s) along with the on-chip Intel graphics?  For that case, I don't like the idea of scanning for a device node because it is perfectly possible to get a valid VADisplay handle for a non-QSV device (an AMD or Nvidia card with mesa, most obviously) which will then fail opaquely later when libmfx tries to use it because the Intel proprietary driver is required.  This may even fail randomly, because device nodes associated with independent drivers are not ordered.
The reason that we do like this is that we met some special case before. 
Under the /dev/dri node there is no dev/dri/renderD128 and only
the card0 and card1. The card0 can't work and the card1 can work well. 
So we add the scanning device node code.
>
>> Signed-off-by: ChaoX A Liu <chaox.a.liu@gmail.com>
>> Signed-off-by: Huang, Zhengxu <zhengxu.maxwell@gmail.com>
>> Signed-off-by: Andrew, Zhang <huazh407@gmail.com>
>> ---
>>   libavutil/hwcontext_qsv.c | 44 +++++++++++++++++++++++++++++++-------------
>>   1 file changed, 31 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
>> index 2dc9aca..2701b5a 100644
>> --- a/libavutil/hwcontext_qsv.c
>> +++ b/libavutil/hwcontext_qsv.c
>> @@ -707,12 +707,41 @@ static mfxIMPL choose_implementation(const char *device)
>>       return impl;
>>   }
>>   
>> +static int create_proper_child_device(AVBufferRef **ctx, const char *device, int flags)
>> +{
>> +    enum AVHWDeviceType child_device_type;
>> +    char adapter[256];
>> +    int  adapter_num;
>> +
>> +    if (CONFIG_VAAPI)
>> +        child_device_type = AV_HWDEVICE_TYPE_VAAPI;
>> +    else if (CONFIG_DXVA2)
>> +        child_device_type = AV_HWDEVICE_TYPE_DXVA2;
>> +    else
>> +        return AVERROR(ENOSYS);
>> +
>> +    if (device || CONFIG_DXVA2)
>> +        return av_hwdevice_ctx_create(ctx, child_device_type, device, NULL, flags);
>> +
>> +    for (adapter_num = 0; adapter_num < 6; adapter_num++) {
>> +        if (adapter_num < 3)
>> +            snprintf(adapter,sizeof(adapter),
>> +                "/dev/dri/renderD%d", adapter_num+128);
>> +        else
>> +            snprintf(adapter,sizeof(adapter),
>> +                "/dev/dri/card%d", adapter_num-3);
> I would prefer not to open the DRM master device (/dev/dri/card*) by default - until very recent kernels it was exclusive-access-only, so nothing else can use the graphics at the same time (most obviously another ffmpeg instance).
>
>> +        if (av_hwdevice_ctx_create(ctx, child_device_type, adapter, NULL, flags) == 0)
>> +            return 0;
>> +    }
>> +
>> +    return AVERROR(ENOSYS);
>> +}
>> +
>>   static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
>>                                AVDictionary *opts, int flags)
>>   {
>>       AVQSVDeviceContext *hwctx = ctx->hwctx;
>>       QSVDevicePriv *priv;
>> -    enum AVHWDeviceType child_device_type;
>>       AVDictionaryEntry *e;
>>   
>>       mfxVersion    ver = { { 3, 1 } };
>> @@ -730,18 +759,7 @@ static int qsv_device_create(AVHWDeviceContext *ctx, const char *device,
>>       ctx->free        = qsv_device_free;
>>   
>>       e = av_dict_get(opts, "child_device", NULL, 0);
>> -
>> -    if (CONFIG_VAAPI)
>> -        child_device_type = AV_HWDEVICE_TYPE_VAAPI;
>> -    else if (CONFIG_DXVA2)
>> -        child_device_type = AV_HWDEVICE_TYPE_DXVA2;
>> -    else {
>> -        av_log(ctx, AV_LOG_ERROR, "No supported child device type is enabled\n");
>> -        return AVERROR(ENOSYS);
>> -    }
>> -
>> -    ret = av_hwdevice_ctx_create(&priv->child_device_ctx, child_device_type,
>> -                                 e ? e->value : NULL, NULL, 0);
>> +    ret = create_proper_child_device(&priv->child_device_ctx, e ? e->value : NULL, 0);
>>       if (ret < 0)
>>           return ret;
>>   
>> -- 
>> 1.8.3.1
>>
> For your specific case in the ffmpeg utility it might be best to add a new command-line option (-qsv_device?) and then pass it as the "child_device" in the options dictionary?  I admit this isn't necessarily helpful for other users, but it would fix your command above without running into the other problems above.
I totally agree with your suggestion that adding a commad-line option. 
Since the issue when opening via the X11 failed has been fixed on
the latest code, we only need to add  the code about option.  I will 
commit a new patch according to your suggestion. Thanks.
>
> Thanks,
>
> - Mark
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel