[FFmpeg-devel] [PATCH 5/6] ffmpeg: Always make the qsv device if the option is set

[wm4]

On Tue, 17 Jan 2017 22:31:48 +0000
Mark Thompson <sw at jkqxz.net> wrote:

> Previously it was only created if the decode hwaccel was requested;
> this makes it unconditionally so we can use it without the hwaccel.
> ---
>  ffmpeg.h     |  4 +---
>  ffmpeg_opt.c | 13 ++++++++++++-
>  ffmpeg_qsv.c | 14 ++++++--------
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/ffmpeg.h b/ffmpeg.h
> index 75bf50e..7362c2d 100644
> --- a/ffmpeg.h
> +++ b/ffmpeg.h
> @@ -604,9 +604,6 @@ extern const OptionDef options[];
>  extern const HWAccel hwaccels[];
>  extern int hwaccel_lax_profile_check;
>  extern AVBufferRef *hw_device_ctx;
> -#if CONFIG_QSV
> -extern char *qsv_device;
> -#endif
>  
>  
>  void term_init(void);
> @@ -641,6 +638,7 @@ int vdpau_init(AVCodecContext *s);
>  int dxva2_init(AVCodecContext *s);
>  int vda_init(AVCodecContext *s);
>  int videotoolbox_init(AVCodecContext *s);
> +int qsv_device_init(const char *device);
>  int qsv_init(AVCodecContext *s);
>  int qsv_transcode_init(OutputStream *ost);
>  int vaapi_decode_init(AVCodecContext *avctx);
> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> index a1c02fc..49397be 100644
> --- a/ffmpeg_opt.c
> +++ b/ffmpeg_opt.c
> @@ -462,6 +462,17 @@ static int opt_vaapi_device(void *optctx, const char *opt, const char *arg)
>  }
>  #endif
>  
> +#if CONFIG_QSV
> +static int opt_qsv_device(void *optctx, const char *opt, const char *arg)
> +{
> +    int err;
> +    err = qsv_device_init(arg);
> +    if (err < 0)
> +        exit_program(1);
> +    return 0;
> +}
> +#endif
> +
>  /**
>   * Parse a metadata specifier passed as 'arg' parameter.
>   * @param arg  metadata string to parse
> @@ -3694,7 +3705,7 @@ const OptionDef options[] = {
>  #endif
>  
>  #if CONFIG_QSV
> -    { "qsv_device", HAS_ARG | OPT_STRING | OPT_EXPERT, { &qsv_device },
> +    { "qsv_device", HAS_ARG | OPT_EXPERT, { .func_arg = opt_qsv_device },
>          "set QSV hardware device (DirectX adapter index, DRM path or X11 display name)", "device"},
>  #endif
>  
> diff --git a/ffmpeg_qsv.c b/ffmpeg_qsv.c
> index 86824b6..9356f05 100644
> --- a/ffmpeg_qsv.c
> +++ b/ffmpeg_qsv.c
> @@ -28,8 +28,6 @@
>  
>  #include "ffmpeg.h"
>  
> -char *qsv_device = NULL;
> -
>  static int qsv_get_buffer(AVCodecContext *s, AVFrame *frame, int flags)
>  {
>      InputStream *ist = s->opaque;
> @@ -43,19 +41,19 @@ static void qsv_uninit(AVCodecContext *s)
>      av_buffer_unref(&ist->hw_frames_ctx);
>  }
>  
> -static int qsv_device_init(InputStream *ist)
> +int qsv_device_init(const char *device)
>  {
>      int err;
>      AVDictionary *dict = NULL;
>  
> -    if (qsv_device) {
> -        err = av_dict_set(&dict, "child_device", qsv_device, 0);
> +    if (device) {
> +        err = av_dict_set(&dict, "child_device", device, 0);
>          if (err < 0)
>              return err;
>      }
>  
>      err = av_hwdevice_ctx_create(&hw_device_ctx, AV_HWDEVICE_TYPE_QSV,
> -                                 ist->hwaccel_device, dict, 0);
> +                                 device, dict, 0);
>      if (err < 0) {
>          av_log(NULL, AV_LOG_ERROR, "Error creating a QSV device\n");
>          goto err_out;
> @@ -76,7 +74,7 @@ int qsv_init(AVCodecContext *s)
>      int ret;
>  
>      if (!hw_device_ctx) {
> -        ret = qsv_device_init(ist);
> +        ret = qsv_device_init(ist->hwaccel_device);
>          if (ret < 0)
>              return ret;
>      }
> @@ -148,7 +146,7 @@ int qsv_transcode_init(OutputStream *ost)
>      av_log(NULL, AV_LOG_VERBOSE, "Setting up QSV transcoding\n");
>  
>      if (!hw_device_ctx) {
> -        err = qsv_device_init(ist);
> +        err = qsv_device_init(ist->hwaccel_device);
>          if (err < 0)
>              goto fail;
>      }

Like the vaapi case, this leaks a device context each time the option
is used after the first time.

Also, shouldn't this maybe only set the hwcontext type, and ffmpeg.c
should call av_hwdevice_ctx_create() in a common code path later?