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

[wm4]

On Sat, 21 Jan 2017 22:16:15 +0000
Mark Thompson <sw at jkqxz.net> wrote:

> On 18/01/17 08:01, wm4 wrote:
> > 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.  
> 
> Right, I can fix that.  (For VAAPI too.)
> 
> > 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?  
> 
> Is there any case where the ordering matters?
> 
> (It does have to be created unconditionally once you have the option, because it needs to be set for filters to work.  The skipped scale_qsv and subsequent deinterlace_qsv filters can be merged to work with hwupload after this.)

Probably would be a separate change at a much later time anyway.