diff mbox series

[FFmpeg-devel,v2,1/2] qsvdec: add support for HW_DEVICE_CTX method

Message ID 20210729070412.1250-1-haihao.xiang@intel.com
State New
Headers show
Series [FFmpeg-devel,v2,1/2] qsvdec: add support for HW_DEVICE_CTX method
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Haihao Xiang July 29, 2021, 7:04 a.m. UTC
This allows user set hw_device_ctx instead of hw_frames_ctx for QSV
decoders, hence we may remove the ad-hoc libmfx setup code from FFmpeg.

"-hwaccel_output_format format" is applied to QSV decoders after
removing the ad-hoc libmfx code. To keep compatibility with old
commandlines, the default format is set to AV_PIX_FMT_QSV. User should
set "-hwaccel_output_format qsv" explicitly if AV_PIX_FMT_QSV is
expected.

User may use the normal way to set device for QSV decoders now.
"-qsv_device device" is deprecated and will be removed from FFmpeg.

For example:

$> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0 -init_hw_device
qsv=hw@va -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -

/dev/dri/renderD128 is actually open for h264_qsv decoder without this
patch. After applying this patch, /dev/dri/card0 is used.
---
v2: Rebase the patchset against the latest master branch and resolve a
conflict.

 fftools/Makefile     |   1 -
 fftools/ffmpeg.h     |   1 -
 fftools/ffmpeg_hw.c  |  12 +++++
 fftools/ffmpeg_opt.c |  54 +++++++++++++++++++--
 fftools/ffmpeg_qsv.c | 109 -------------------------------------------
 libavcodec/qsvdec.c  |  31 +++++++++++-
 6 files changed, 92 insertions(+), 116 deletions(-)
 delete mode 100644 fftools/ffmpeg_qsv.c

Comments

Soft Works July 30, 2021, 8:18 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Haihao Xiang
> Sent: Thursday, 29 July 2021 09:04
> To: ffmpeg-devel@ffmpeg.org
> Cc: Haihao Xiang <haihao.xiang@intel.com>
> Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> This allows user set hw_device_ctx instead of hw_frames_ctx for QSV
> decoders, hence we may remove the ad-hoc libmfx setup code from
> FFmpeg.
> 
> "-hwaccel_output_format format" is applied to QSV decoders after removing
> the ad-hoc libmfx code. To keep compatibility with old commandlines, the
> default format is set to AV_PIX_FMT_QSV. User should set "-
> hwaccel_output_format qsv" explicitly if AV_PIX_FMT_QSV is expected.
> 
> User may use the normal way to set device for QSV decoders now.
> "-qsv_device device" is deprecated and will be removed from FFmpeg.
> 
> For example:
> 
> $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0 -init_hw_device
> qsv=hw@va -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -

Hi Haihao,

Why so complicated with double device initialization?
The regular way is this:

ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=/dev/dri/card0" -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -

Or for Windows:

ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -

> 
> /dev/dri/renderD128 is actually open for h264_qsv decoder without this
> patch. After applying this patch, /dev/dri/card0 is used.
> ---
> v2: Rebase the patchset against the latest master branch and resolve a
> conflict.
> 
>  fftools/Makefile     |   1 -
>  fftools/ffmpeg.h     |   1 -
>  fftools/ffmpeg_hw.c  |  12 +++++
>  fftools/ffmpeg_opt.c |  54 +++++++++++++++++++--  fftools/ffmpeg_qsv.c
> | 109 -------------------------------------------
>  libavcodec/qsvdec.c  |  31 +++++++++++-
>  6 files changed, 92 insertions(+), 116 deletions(-)  delete mode 100644
> fftools/ffmpeg_qsv.c
> 
> diff --git a/fftools/Makefile b/fftools/Makefile index 5affaa3f56..5234932ab0
> 100644
> --- a/fftools/Makefile
> +++ b/fftools/Makefile
> @@ -10,7 +10,6 @@ ALLAVPROGS   =
> $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
>  ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
> 
>  OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o
> fftools/ffmpeg_hw.o
> -OBJS-ffmpeg-$(CONFIG_LIBMFX)       += fftools/ffmpeg_qsv.o
>  ifndef CONFIG_VIDEOTOOLBOX
>  OBJS-ffmpeg-$(CONFIG_VDA)          += fftools/ffmpeg_videotoolbox.o
>  endif
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index
> d9c0628657..d2dd7ca092 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -61,7 +61,6 @@ enum HWAccelID {
>      HWACCEL_AUTO,
>      HWACCEL_GENERIC,
>      HWACCEL_VIDEOTOOLBOX,
> -    HWACCEL_QSV,
>  };
> 
>  typedef struct HWAccel {
> diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c index
> fc4a5d31d6..043e4c5863 100644
> --- a/fftools/ffmpeg_hw.c
> +++ b/fftools/ffmpeg_hw.c
> @@ -339,6 +339,18 @@ int hw_device_setup_for_decode(InputStream *ist)
>          } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
>              type = ist->hwaccel_device_type;
>              dev = hw_device_get_by_type(type);
> +
> +            // When "-qsv_device device" is used, an internal QSV device named
> +            // as "__qsv_device" is created and another QSV device is created
> +            // if "-init_hw_device qsv=name@name" is used. There are 2 QSV
> devices
> +            // if both "-qsv_device device" and "-init_hw_device
> qsv=name@name"
> +            // are used, hw_device_get_by_type(AV_HWDEVICE_TYPE_QSV)
> returns NULL.
> +            // To keep back-compatibility with the removed ad-hoc libmfx setup
> code,
> +            // call hw_device_get_by_name("__qsv_device") to select the
> internal QSV
> +            // device. This will be removed once -qsv_device is no longer
> supported.
> +            if (!dev && type == AV_HWDEVICE_TYPE_QSV)
> +                dev = hw_device_get_by_name("__qsv_device");
> +
>              if (!dev)
>                  err = hw_device_init_from_type(type, NULL, &dev);
>          } else {
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index
> 1b43bab9fc..4ddc6c939b 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -137,9 +137,6 @@ static const char *const opt_name_enc_time_bases[]
> = {"enc_time_base"
>  const HWAccel hwaccels[] = {
>  #if CONFIG_VIDEOTOOLBOX
>      { "videotoolbox", videotoolbox_init, HWACCEL_VIDEOTOOLBOX,
> AV_PIX_FMT_VIDEOTOOLBOX }, -#endif -#if CONFIG_LIBMFX
> -    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
>  #endif
>      { 0 },
>  };
> @@ -571,6 +568,49 @@ 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) { #if CONFIG_VAAPI
> +    const char *prefix = "vaapi=__qsv_child_device:"; #elif
> +CONFIG_DXVA2
> +    const char *prefix = "dxva2=__qsv_child_device:"; #else
> +    const char *prefix = NULL;
> +#endif

Same like for the command line. You can directly initialize a QSV context 
with a single operation.

[...]

> +    if (avctx->hw_device_ctx && !avctx->hw_frames_ctx && ret ==
> AV_PIX_FMT_QSV) {
> +        AVHWFramesContext *hwframes_ctx;
> +        AVQSVFramesContext *frames_hwctx;
> +
> +        avctx->hw_frames_ctx =
> + av_hwframe_ctx_alloc(avctx->hw_device_ctx);
> +
> +        if (!avctx->hw_frames_ctx) {
> +            av_log(avctx, AV_LOG_ERROR, "av_hwframe_ctx_alloc failed\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        hwframes_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx-
> >data;
> +        frames_hwctx = hwframes_ctx->hwctx;
> +        hwframes_ctx->width             = FFALIGN(avctx->coded_width,  32);
> +        hwframes_ctx->height            = FFALIGN(avctx->coded_height, 32);
> +        hwframes_ctx->format            = AV_PIX_FMT_QSV;
> +        hwframes_ctx->sw_format         = avctx->sw_pix_fmt;
> +        hwframes_ctx->initial_pool_size = 64 + avctx->extra_hw_frames;


I'm not sure whether 64 is a good default choice. I had to reduce this to 32 when processing 4k video with OpenCL interop (memory sharing), otherwise I got OOM errors. (with video mem set to the maximum of 1G in the BIOS)


Besides the above comments, this looks good to me in general. It is the right step to go forward IMO.

Just the double device initialization should not be required (it isn't now) when it comes to the selection between D3D9 and D3D11. I am simply using a device parameter for this, and a command line looks like this:

ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1,dx11=1" -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -


But let's get this done done first..
softworkz
Haihao Xiang Aug. 2, 2021, 5:52 a.m. UTC | #2
On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Haihao Xiang
> > Sent: Thursday, 29 July 2021 09:04
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > HW_DEVICE_CTX method
> > 
> > This allows user set hw_device_ctx instead of hw_frames_ctx for QSV
> > decoders, hence we may remove the ad-hoc libmfx setup code from
> > FFmpeg.
> > 
> > "-hwaccel_output_format format" is applied to QSV decoders after removing
> > the ad-hoc libmfx code. To keep compatibility with old commandlines, the
> > default format is set to AV_PIX_FMT_QSV. User should set "-
> > hwaccel_output_format qsv" explicitly if AV_PIX_FMT_QSV is expected.
> > 
> > User may use the normal way to set device for QSV decoders now.
> > "-qsv_device device" is deprecated and will be removed from FFmpeg.
> > 
> > For example:
> > 
> > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0 -init_hw_device
> > qsv=hw@va -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> 
> Hi Haihao,
> 
> Why so complicated with double device initialization?
> The regular way is this:
> 
> ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=/dev/dri/card0" -hwaccel
> qsv -c:v h264_qsv -i input.h264 -f null -
> 
> Or for Windows:
> 
> ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel qsv -c:v
> h264_qsv -i input.h264 -f null -

After applying this patch, the above commands may work too, actually a child
device is created internally in this way, then a QSV device is derived from the
child device implicitly. However deriving a device explicitly creates a
connection between source device and derived device. We may derive a new device
of the source type from the derived device. 

e.g. 

The command below works 

$> ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -init_hw_device 
qsv=qsv@intel -hwaccel qsv -c:v h264_qsv -i input.h264 -vf
hwmap=derive_device=vaapi,scale_vaapi -f null -

but the command below doesn't work

$> ffmpeg -init_hw_device qsv=dev:hw_any,child_device=/dev/dri/renderD128
-hwaccel qsv -c:v h264_qsv -i ~/graphics/movies/720p.mp4 -vf
hwmap=derive_device=vaapi,scale_vaapi -f null -

(We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276695.html
 to create a connection between two devices too, but this change breaks 'make
fate-hwdevice V=2'). 

> 
> > 
> > /dev/dri/renderD128 is actually open for h264_qsv decoder without this
> > patch. After applying this patch, /dev/dri/card0 is used.
> > ---
> > v2: Rebase the patchset against the latest master branch and resolve a
> > conflict.
> > 
> >  fftools/Makefile     |   1 -
> >  fftools/ffmpeg.h     |   1 -
> >  fftools/ffmpeg_hw.c  |  12 +++++
> >  fftools/ffmpeg_opt.c |  54 +++++++++++++++++++--  fftools/ffmpeg_qsv.c
> > > 109 -------------------------------------------
> > 
> >  libavcodec/qsvdec.c  |  31 +++++++++++-
> >  6 files changed, 92 insertions(+), 116 deletions(-)  delete mode 100644
> > fftools/ffmpeg_qsv.c
> > 
> > diff --git a/fftools/Makefile b/fftools/Makefile index
> > 5affaa3f56..5234932ab0
> > 100644
> > --- a/fftools/Makefile
> > +++ b/fftools/Makefile
> > @@ -10,7 +10,6 @@ ALLAVPROGS   =
> > $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
> >  ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
> > 
> >  OBJS-ffmpeg                        += fftools/ffmpeg_opt.o
> > fftools/ffmpeg_filter.o
> > fftools/ffmpeg_hw.o
> > -OBJS-ffmpeg-$(CONFIG_LIBMFX)       += fftools/ffmpeg_qsv.o
> >  ifndef CONFIG_VIDEOTOOLBOX
> >  OBJS-ffmpeg-$(CONFIG_VDA)          += fftools/ffmpeg_videotoolbox.o
> >  endif
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index
> > d9c0628657..d2dd7ca092 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -61,7 +61,6 @@ enum HWAccelID {
> >      HWACCEL_AUTO,
> >      HWACCEL_GENERIC,
> >      HWACCEL_VIDEOTOOLBOX,
> > -    HWACCEL_QSV,
> >  };
> > 
> >  typedef struct HWAccel {
> > diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c index
> > fc4a5d31d6..043e4c5863 100644
> > --- a/fftools/ffmpeg_hw.c
> > +++ b/fftools/ffmpeg_hw.c
> > @@ -339,6 +339,18 @@ int hw_device_setup_for_decode(InputStream *ist)
> >          } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
> >              type = ist->hwaccel_device_type;
> >              dev = hw_device_get_by_type(type);
> > +
> > +            // When "-qsv_device device" is used, an internal QSV device
> > named
> > +            // as "__qsv_device" is created and another QSV device is
> > created
> > +            // if "-init_hw_device qsv=name@name" is used. There are 2 QSV
> > devices
> > +            // if both "-qsv_device device" and "-init_hw_device
> > qsv=name@name"
> > +            // are used, hw_device_get_by_type(AV_HWDEVICE_TYPE_QSV)
> > returns NULL.
> > +            // To keep back-compatibility with the removed ad-hoc libmfx
> > setup
> > code,
> > +            // call hw_device_get_by_name("__qsv_device") to select the
> > internal QSV
> > +            // device. This will be removed once -qsv_device is no longer
> > supported.
> > +            if (!dev && type == AV_HWDEVICE_TYPE_QSV)
> > +                dev = hw_device_get_by_name("__qsv_device");
> > +
> >              if (!dev)
> >                  err = hw_device_init_from_type(type, NULL, &dev);
> >          } else {
> > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index
> > 1b43bab9fc..4ddc6c939b 100644
> > --- a/fftools/ffmpeg_opt.c
> > +++ b/fftools/ffmpeg_opt.c
> > @@ -137,9 +137,6 @@ static const char *const opt_name_enc_time_bases[]
> > = {"enc_time_base"
> >  const HWAccel hwaccels[] = {
> >  #if CONFIG_VIDEOTOOLBOX
> >      { "videotoolbox", videotoolbox_init, HWACCEL_VIDEOTOOLBOX,
> > AV_PIX_FMT_VIDEOTOOLBOX }, -#endif -#if CONFIG_LIBMFX
> > -    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
> >  #endif
> >      { 0 },
> >  };
> > @@ -571,6 +568,49 @@ 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) { #if CONFIG_VAAPI
> > +    const char *prefix = "vaapi=__qsv_child_device:"; #elif
> > +CONFIG_DXVA2
> > +    const char *prefix = "dxva2=__qsv_child_device:"; #else
> > +    const char *prefix = NULL;
> > +#endif
> 
> Same like for the command line. You can directly initialize a QSV context 
> with a single operation.

According to the above explanation, I prefer to derive device explicitly. 

> 
> [...]
> 
> > +    if (avctx->hw_device_ctx && !avctx->hw_frames_ctx && ret ==
> > AV_PIX_FMT_QSV) {
> > +        AVHWFramesContext *hwframes_ctx;
> > +        AVQSVFramesContext *frames_hwctx;
> > +
> > +        avctx->hw_frames_ctx =
> > + av_hwframe_ctx_alloc(avctx->hw_device_ctx);
> > +
> > +        if (!avctx->hw_frames_ctx) {
> > +            av_log(avctx, AV_LOG_ERROR, "av_hwframe_ctx_alloc failed\n");
> > +            return AVERROR(ENOMEM);
> > +        }
> > +
> > +        hwframes_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx-
> > > data;
> > 
> > +        frames_hwctx = hwframes_ctx->hwctx;
> > +        hwframes_ctx->width             = FFALIGN(avctx->coded_width,  32);
> > +        hwframes_ctx->height            = FFALIGN(avctx->coded_height, 32);
> > +        hwframes_ctx->format            = AV_PIX_FMT_QSV;
> > +        hwframes_ctx->sw_format         = avctx->sw_pix_fmt;
> > +        hwframes_ctx->initial_pool_size = 64 + avctx->extra_hw_frames;
> 
> 
> I'm not sure whether 64 is a good default choice. I had to reduce this to 32
> when processing 4k video with OpenCL interop (memory sharing), otherwise I got
> OOM errors. (with video mem set to the maximum of 1G in the BIOS)
> 

64 is used in ffmpeg_qsv.c, we have another patchset to optimize the memory
usage in qsv path and will submit it once this patchset is merged. 

> 
> Besides the above comments, this looks good to me in general. It is the right
> step to go forward IMO.
> 
> Just the double device initialization should not be required (it isn't now)
> when it comes to the selection between D3D9 and D3D11. I am simply using a
> device parameter for this, and a command line looks like this:
> 
> ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1,dx11=1" -hwaccel qsv
> -c:v h264_qsv -i input.h264 -f null -
> 

According to the above explanation, you can still use this way for the selection
between D3D9 and D3D11. 

Thanks
Haihao


> 
> But let's get this done done first..
> softworkz
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Soft Works Aug. 2, 2021, 7:11 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Monday, 2 August 2021 07:53
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Haihao Xiang
> > > Sent: Thursday, 29 July 2021 09:04
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > HW_DEVICE_CTX method
> > >
> > > This allows user set hw_device_ctx instead of hw_frames_ctx for QSV
> > > decoders, hence we may remove the ad-hoc libmfx setup code from
> > > FFmpeg.
> > >
> > > "-hwaccel_output_format format" is applied to QSV decoders after
> > > removing the ad-hoc libmfx code. To keep compatibility with old
> > > commandlines, the default format is set to AV_PIX_FMT_QSV. User
> > > should set "- hwaccel_output_format qsv" explicitly if AV_PIX_FMT_QSV
> is expected.
> > >
> > > User may use the normal way to set device for QSV decoders now.
> > > "-qsv_device device" is deprecated and will be removed from FFmpeg.
> > >
> > > For example:
> > >
> > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0 -init_hw_device
> > > qsv=hw@va -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> >
> > Hi Haihao,
> >
> > Why so complicated with double device initialization?
> > The regular way is this:
> >
> > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> >
> > Or for Windows:
> >
> > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel qsv
> > -c:v h264_qsv -i input.h264 -f null -
> 
> After applying this patch, the above commands may work too, actually a child
> device is created internally in this way, then a QSV device is derived from the
> child device implicitly. However deriving a device explicitly creates a
> connection between source device and derived device. We may derive a
> new device of the source type from the derived device.

"may work too" is not sufficient. The existing and documented method must 
continue to work.

Besides that, it doesn't make sense to me to start advertising those overcomplicated
command lines with double device initialization as the default way for using QSV.

> 
> e.g.
> 
> The command below works
> 
> $> ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -init_hw_device
> qsv=qsv@intel -hwaccel qsv -c:v h264_qsv -i input.h264 -vf
> hwmap=derive_device=vaapi,scale_vaapi -f null -
> 
> but the command below doesn't work
> 
> $> ffmpeg -init_hw_device
> qsv=dev:hw_any,child_device=/dev/dri/renderD128
> -hwaccel qsv -c:v h264_qsv -i ~/graphics/movies/720p.mp4 -vf
> hwmap=derive_device=vaapi,scale_vaapi -f null -
> 
> (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> February/276695.html
>  to create a connection between two devices too, but this change breaks
> 'make fate-hwdevice V=2').

I had done a similar patch quite a while ago, just forgot about it,
so yes, that patch is crucial as well.

softworkz
Haihao Xiang Aug. 2, 2021, 8:25 a.m. UTC | #4
On Mon, 2021-08-02 at 07:11 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Monday, 2 August 2021 07:53
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > HW_DEVICE_CTX method
> > 
> > On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Haihao Xiang
> > > > Sent: Thursday, 29 July 2021 09:04
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > HW_DEVICE_CTX method
> > > > 
> > > > This allows user set hw_device_ctx instead of hw_frames_ctx for QSV
> > > > decoders, hence we may remove the ad-hoc libmfx setup code from
> > > > FFmpeg.
> > > > 
> > > > "-hwaccel_output_format format" is applied to QSV decoders after
> > > > removing the ad-hoc libmfx code. To keep compatibility with old
> > > > commandlines, the default format is set to AV_PIX_FMT_QSV. User
> > > > should set "- hwaccel_output_format qsv" explicitly if AV_PIX_FMT_QSV
> > 
> > is expected.
> > > > 
> > > > User may use the normal way to set device for QSV decoders now.
> > > > "-qsv_device device" is deprecated and will be removed from FFmpeg.
> > > > 
> > > > For example:
> > > > 
> > > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0 -init_hw_device
> > > > qsv=hw@va -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > 
> > > Hi Haihao,
> > > 
> > > Why so complicated with double device initialization?
> > > The regular way is this:
> > > 
> > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > 
> > > Or for Windows:
> > > 
> > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel qsv
> > > -c:v h264_qsv -i input.h264 -f null -
> > 
> > After applying this patch, the above commands may work too, actually a child
> > device is created internally in this way, then a QSV device is derived from
> > the
> > child device implicitly. However deriving a device explicitly creates a
> > connection between source device and derived device. We may derive a
> > new device of the source type from the derived device.
> 
> "may work too" is not sufficient. The existing and documented method must 
> continue to work.

Could you please provide an example which is broken by this patch? I don't seeregression in my testing. 

> 
> Besides that, it doesn't make sense to me to start advertising those
> overcomplicated
> command lines with double device initialization as the default way for using
> QSV.

This patch doesn't advertise that double device initialization is the default
way for QSV. I just use it as an example in the commit log, I may add another
example in the commit log if needed. 

> 
> > 
> > e.g.
> > 
> > The command below works
> > 
> > $> ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -init_hw_device
> > qsv=qsv@intel -hwaccel qsv -c:v h264_qsv -i input.h264 -vf
> > hwmap=derive_device=vaapi,scale_vaapi -f null -
> > 
> > but the command below doesn't work
> > 
> > $> ffmpeg -init_hw_device
> > qsv=dev:hw_any,child_device=/dev/dri/renderD128
> > -hwaccel qsv -c:v h264_qsv -i ~/graphics/movies/720p.mp4 -vf
> > hwmap=derive_device=vaapi,scale_vaapi -f null -
> > 
> > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > February/276695.html
> >  to create a connection between two devices too, but this change breaks
> > 'make fate-hwdevice V=2').
> 
> I had done a similar patch quite a while ago, just forgot about it,
> so yes, that patch is crucial as well.

Let's fix it in another patch if the above command is expected to work too. 

Thanks
Haihao
Soft Works Aug. 2, 2021, 10:27 a.m. UTC | #5
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Monday, 2 August 2021 10:25
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> On Mon, 2021-08-02 at 07:11 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Xiang, Haihao
> > > Sent: Monday, 2 August 2021 07:53
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > HW_DEVICE_CTX method
> > >
> > > On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> > > > > Of Haihao Xiang
> > > > > Sent: Thursday, 29 July 2021 09:04
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > > HW_DEVICE_CTX method
> > > > >
> > > > > This allows user set hw_device_ctx instead of hw_frames_ctx for
> > > > > QSV decoders, hence we may remove the ad-hoc libmfx setup code
> > > > > from FFmpeg.
> > > > >
> > > > > "-hwaccel_output_format format" is applied to QSV decoders after
> > > > > removing the ad-hoc libmfx code. To keep compatibility with old
> > > > > commandlines, the default format is set to AV_PIX_FMT_QSV. User
> > > > > should set "- hwaccel_output_format qsv" explicitly if
> > > > > AV_PIX_FMT_QSV
> > >
> > > is expected.
> > > > >
> > > > > User may use the normal way to set device for QSV decoders now.
> > > > > "-qsv_device device" is deprecated and will be removed from
> FFmpeg.
> > > > >
> > > > > For example:
> > > > >
> > > > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0
> > > > > -init_hw_device qsv=hw@va -hwaccel qsv -c:v h264_qsv -i
> > > > > input.h264 -f null -
> > > >
> > > > Hi Haihao,
> > > >
> > > > Why so complicated with double device initialization?
> > > > The regular way is this:
> > > >
> > > > ffmpeg -init_hw_device
> "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > >
> > > > Or for Windows:
> > > >
> > > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel
> > > > qsv -c:v h264_qsv -i input.h264 -f null -
> > >
> > > After applying this patch, the above commands may work too, actually
> > > a child device is created internally in this way, then a QSV device
> > > is derived from the child device implicitly. However deriving a
> > > device explicitly creates a connection between source device and
> > > derived device. We may derive a new device of the source type from
> > > the derived device.
> >
> > "may work too" is not sufficient. The existing and documented method
> > must continue to work.
> 
> Could you please provide an example which is broken by this patch? I don't
> seeregression in my testing.

What I wanted to point out is that "may work too" is not a good attitude 
towards functionality used by millions of users (most of which without 
even knowing).

Besides compatibility and command line complexity, there's also a technical
problem with your approach.

The following command line does not work:

ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd@dx -hwaccel qsv -c:v h264_qsv -i INPUT -filter_complex "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv" -c:v h264_qsv output.mp4

ERROR: A hardware device reference is required to upload frames to

It can be fixed by setting filter_hw_device:

ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd@dx -hwaccel qsv -c:v h264_qsv -i INPUT -filter_hw_device qd -filter_complex "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv" -c:v h264_qsv

This is not required when using the regular QSV initialization:

ffmpeg -init_hw_device qsv=qd1:hw2,child_device=1 -hwaccel qsv -c:v h264_qsv -i INPUT -filter_complex "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv" -c:v h264_qsv output.mp4


Needing to add filter_hw_device seems to be easy - but only when you know that you need to do that. And that's the kind of point where many users would give up.


But now let's look at the classic initialization using qsv_device:
(btw: I don't think that this should be deprecated at all; it's a more convenient way - easier to remember and not that much to type)

ffmpeg -qsv_device 1 -hwaccel qsv -c:v h264_qsv -i INPUT -filter_complex "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv" -c:v h264_qsv output.mp4

This doesn't work with your patch either, and it's not possible to set the filter device that way.
Though, it will work when you change your internal initialization to use the regular single-shot initialization.

It's not exactly a regression because it didn't work before, but that's how it should work (finally) thanks to this patch.

Kind regards,
softworkz 






The issues here are:

1. Wasting (previous) video memory

The above command causes the creation of 



> 
> >
> > Besides that, it doesn't make sense to me to start advertising those
> > overcomplicated command lines with double device initialization as the
> > default way for using QSV.
> 
> This patch doesn't advertise that double device initialization is the default
> way for QSV. I just use it as an example in the commit log, I may add another
> example in the commit log if needed.
> 
> >
> > >
> > > e.g.
> > >
> > > The command below works
> > >
> > > $> ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128
> > > -init_hw_device qsv=qsv@intel -hwaccel qsv -c:v h264_qsv -i
> > > input.h264 -vf hwmap=derive_device=vaapi,scale_vaapi -f null -
> > >
> > > but the command below doesn't work
> > >
> > > $> ffmpeg -init_hw_device
> > > qsv=dev:hw_any,child_device=/dev/dri/renderD128
> > > -hwaccel qsv -c:v h264_qsv -i ~/graphics/movies/720p.mp4 -vf
> > > hwmap=derive_device=vaapi,scale_vaapi -f null -
> > >
> > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > February/276695.html
> > >  to create a connection between two devices too, but this change
> > > breaks 'make fate-hwdevice V=2').
> >
> > I had done a similar patch quite a while ago, just forgot about it, so
> > yes, that patch is crucial as well.
> 
> Let's fix it in another patch if the above command is expected to work too.
> 
> Thanks
> Haihao
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Haihao Xiang Aug. 3, 2021, 5:21 a.m. UTC | #6
On Mon, 2021-08-02 at 10:27 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Monday, 2 August 2021 10:25
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > HW_DEVICE_CTX method
> > 
> > On Mon, 2021-08-02 at 07:11 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Xiang, Haihao
> > > > Sent: Monday, 2 August 2021 07:53
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > HW_DEVICE_CTX method
> > > > 
> > > > On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > 
> > Behalf
> > > > > > Of Haihao Xiang
> > > > > > Sent: Thursday, 29 July 2021 09:04
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > > > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > > > HW_DEVICE_CTX method
> > > > > > 
> > > > > > This allows user set hw_device_ctx instead of hw_frames_ctx for
> > > > > > QSV decoders, hence we may remove the ad-hoc libmfx setup code
> > > > > > from FFmpeg.
> > > > > > 
> > > > > > "-hwaccel_output_format format" is applied to QSV decoders after
> > > > > > removing the ad-hoc libmfx code. To keep compatibility with old
> > > > > > commandlines, the default format is set to AV_PIX_FMT_QSV. User
> > > > > > should set "- hwaccel_output_format qsv" explicitly if
> > > > > > AV_PIX_FMT_QSV
> > > > 
> > > > is expected.
> > > > > > 
> > > > > > User may use the normal way to set device for QSV decoders now.
> > > > > > "-qsv_device device" is deprecated and will be removed from
> > 
> > FFmpeg.
> > > > > > 
> > > > > > For example:
> > > > > > 
> > > > > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0
> > > > > > -init_hw_device qsv=hw@va -hwaccel qsv -c:v h264_qsv -i
> > > > > > input.h264 -f null -
> > > > > 
> > > > > Hi Haihao,
> > > > > 
> > > > > Why so complicated with double device initialization?
> > > > > The regular way is this:
> > > > > 
> > > > > ffmpeg -init_hw_device
> > 
> > "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > > > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > > > 
> > > > > Or for Windows:
> > > > > 
> > > > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel
> > > > > qsv -c:v h264_qsv -i input.h264 -f null -
> > > > 
> > > > After applying this patch, the above commands may work too, actually
> > > > a child device is created internally in this way, then a QSV device
> > > > is derived from the child device implicitly. However deriving a
> > > > device explicitly creates a connection between source device and
> > > > derived device. We may derive a new device of the source type from
> > > > the derived device.
> > > 
> > > "may work too" is not sufficient. The existing and documented method
> > > must continue to work.
> > 
> > Could you please provide an example which is broken by this patch? I don't
> > seeregression in my testing.
> 
> What I wanted to point out is that "may work too" is not a good attitude 
> towards functionality used by millions of users (most of which without 
> even knowing).
> 

Okay, I've gotten your point. I agree with you we should make FFmpeg/qsv more
user-friendly. My first step is to make QSV work with HW_DEVICE_CTX method
without breaking existing commands in this patch. Actually I have already taken 
the commands you mentioned below into account and submitted another patch (
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html)

> Besides compatibility and command line complexity, there's also a technical
> problem with your approach.
> 

> The following command line does not work:
> 
> ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd@dx -hwaccel qsv -c:v
> h264_qsv -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> ERROR: A hardware device reference is required to upload frames to

I'm aware of this issue. Without this patch, the above command and other similar
commands don't work too, so this issue is not caused by this patch. 

e.g, below is an example which doesn't use qsv decoder at all, it also doesn't
work

$> ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD128 -init_hw_device
qsv=hw@va -f lavfi -i yuvtestsrc  -vf
format=nv12,hwupload=extra_hw_frames=64 -c:v h264_qsv -y out.h264

The root cause is there is no default selection when there are multiple hw
devices, I submitted 
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html to make it worksa few weeks ago. 

> 
> It can be fixed by setting filter_hw_device:
> 
> ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd@dx -hwaccel qsv -c:v
> h264_qsv -i INPUT -filter_hw_device qd -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv
> 
> This is not required when using the regular QSV initialization:
> 
> ffmpeg -init_hw_device qsv=qd1:hw2,child_device=1 -hwaccel qsv -c:v h264_qsv
> -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> 
> Needing to add filter_hw_device seems to be easy - but only when you know that
> you need to do that. And that's the kind of point where many users would give
> up.
> 
> 
> But now let's look at the classic initialization using qsv_device:
> (btw: I don't think that this should be deprecated at all; it's a more
> convenient way - easier to remember and not that much to type)

Currently there are a few different options which can be used to create HW
devices for QSV, many user are confused with these options. e.g. We must use
qsv_device to specify the device for QSV decoder if we want to use a non-default 
device, however we have to use init_hw_device to initialize hw device for QSV
encoder, so I prefer to reduce options and use the same way for device
initialization and selection in the future. But I'm fine to keep qsv_device
option if you think it is a better choice. 

> 
> ffmpeg -qsv_device 1 -hwaccel qsv -c:v h264_qsv -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> This doesn't work with your patch either, and it's not possible to set the
> filter device that way.
> Though, it will work when you change your internal initialization to use the
> regular single-shot initialization.
> 
> It's not exactly a regression because it didn't work before, but that's how it
> should work (finally) thanks to this patch.

I'm aware of this issue too, and 
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html can fix it as
well.

Yes, the above command may work if using single-shot initialization, but the
following command does not work. However it works if using double device
initialization (http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html
is not needed for this case).

$> ffmpeg -qsv_device /dev/dri/renderD128 -hwaccel qsv -c:v h264_qsv -i
input.h264 -vf hwmap=derive_device=opencl,unsharp_opencl -f null -

So I don't think single-shot initialization is better than double device
initialization, however I am fine to use single-shot initialization if the above
command is not expected to work.

Thanks
Haihao
Soft Works Aug. 4, 2021, 5:43 a.m. UTC | #7
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Tuesday, 3 August 2021 07:22
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> On Mon, 2021-08-02 at 10:27 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Xiang, Haihao
> > > Sent: Monday, 2 August 2021 10:25
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > HW_DEVICE_CTX method
> > >
> > > On Mon, 2021-08-02 at 07:11 +0000, Soft Works wrote:
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> > > > > Of Xiang, Haihao
> > > > > Sent: Monday, 2 August 2021 07:53
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support
> > > > > for HW_DEVICE_CTX method
> > > > >
> > > > > On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > >
> > > Behalf
> > > > > > > Of Haihao Xiang
> > > > > > > Sent: Thursday, 29 July 2021 09:04
> > > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > > Cc: Haihao Xiang <haihao.xiang@intel.com>
> > > > > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support
> > > > > > > for HW_DEVICE_CTX method
> > > > > > >
> > > > > > > This allows user set hw_device_ctx instead of hw_frames_ctx
> > > > > > > for QSV decoders, hence we may remove the ad-hoc libmfx
> > > > > > > setup code from FFmpeg.
> > > > > > >
> > > > > > > "-hwaccel_output_format format" is applied to QSV decoders
> > > > > > > after removing the ad-hoc libmfx code. To keep compatibility
> > > > > > > with old commandlines, the default format is set to
> > > > > > > AV_PIX_FMT_QSV. User should set "- hwaccel_output_format
> > > > > > > qsv" explicitly if AV_PIX_FMT_QSV
> > > > >
> > > > > is expected.
> > > > > > >
> > > > > > > User may use the normal way to set device for QSV decoders
> now.
> > > > > > > "-qsv_device device" is deprecated and will be removed from
> > >
> > > FFmpeg.
> > > > > > >
> > > > > > > For example:
> > > > > > >
> > > > > > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0
> > > > > > > -init_hw_device qsv=hw@va -hwaccel qsv -c:v h264_qsv -i
> > > > > > > input.h264 -f null -
> > > > > >
> > > > > > Hi Haihao,
> > > > > >
> > > > > > Why so complicated with double device initialization?
> > > > > > The regular way is this:
> > > > > >
> > > > > > ffmpeg -init_hw_device
> > >
> > > "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > > > > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > > > >
> > > > > > Or for Windows:
> > > > > >
> > > > > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1"
> > > > > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > > >
> > > > > After applying this patch, the above commands may work too,
> > > > > actually a child device is created internally in this way, then
> > > > > a QSV device is derived from the child device implicitly.
> > > > > However deriving a device explicitly creates a connection
> > > > > between source device and derived device. We may derive a new
> > > > > device of the source type from the derived device.
> > > >
> > > > "may work too" is not sufficient. The existing and documented
> > > > method must continue to work.
> > >
> > > Could you please provide an example which is broken by this patch? I
> > > don't seeregression in my testing.
> >
> > What I wanted to point out is that "may work too" is not a good
> > attitude towards functionality used by millions of users (most of
> > which without even knowing).
> >
> 
> Okay, I've gotten your point. I agree with you we should make FFmpeg/qsv
> more user-friendly. My first step is to make QSV work with HW_DEVICE_CTX
> method without breaking existing commands in this patch.

Great, thanks! 

> Currently there are a few different options which can be used to create HW
> devices for QSV, many user are confused with these options. e.g. We must
> use qsv_device to specify the device for QSV decoder if we want to use a
> non-default device, however we have to use init_hw_device to initialize hw
> device for QSV encoder, so I prefer to reduce options and use the same way
> for device initialization and selection in the future. But I'm fine to keep
> qsv_device option if you think it is a better choice.

Yes, ideally _all_ of those command line variations should be working in the end. What I always found really unfortunate is that, despite the existence of a common syntax for working with hw accelerations, the individual developers have still managed to do things differently for each hw acceleration; there's no single/common command pattern that can be used with all hw accels. 
I think it would be desirable to get closer to a common method. That's why I meant that documenting the double device initialization as the preferred method would just lead into the opposite direction.

Otherwise, your patches are on the right track IMO.

softworkz


> 
> >
> > ffmpeg -qsv_device 1 -hwaccel qsv -c:v h264_qsv -i INPUT
> > -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overla
> y_qsv"
> > -c:v h264_qsv output.mp4
> >
> > This doesn't work with your patch either, and it's not possible to set
> > the filter device that way.
> > Though, it will work when you change your internal initialization to
> > use the regular single-shot initialization.
> >
> > It's not exactly a regression because it didn't work before, but
> > that's how it should work (finally) thanks to this patch.
> 
> I'm aware of this issue too, and
> http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html can fix it
> as well.
> 
> Yes, the above command may work if using single-shot initialization, but the
> following command does not work. However it works if using double device
> initialization (http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> June/281879.html
> is not needed for this case).
> 
> $> ffmpeg -qsv_device /dev/dri/renderD128 -hwaccel qsv -c:v h264_qsv -i
> input.h264 -vf hwmap=derive_device=opencl,unsharp_opencl -f null -
> 
> So I don't think single-shot initialization is better than double device
> initialization, however I am fine to use single-shot initialization if the above
> command is not expected to work.
> 
> Thanks
> Haihao
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Soft Works Aug. 4, 2021, 10:10 a.m. UTC | #8
Haihao,

I've just been looking into the source patch from February:

> > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > February/276695.html
> > >  to create a connection between two devices too, but this change
> > > breaks 'make fate-hwdevice V=2').

I've re-read the discussion about why it fails Fate and I actually _think_
that it might not fail anymore with your (this) patch applied.

To test, it would probably be required to submit a combined patch,
unless you'd have your own Fate server to test..

softworkz
Haihao Xiang Aug. 5, 2021, 2:19 a.m. UTC | #9
On Wed, 2021-08-04 at 10:10 +0000, Soft Works wrote:
> Haihao,
> 
> I've just been looking into the source patch from February:
> 
> > > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > February/276695.html
> > > >  to create a connection between two devices too, but this change
> > > > breaks 'make fate-hwdevice V=2').
> 
> I've re-read the discussion about why it fails Fate and I actually _think_
> that it might not fail anymore with your (this) patch applied.
> 
> To test, it would probably be required to submit a combined patch,
> unless you'd have your own Fate server to test..
> 

I also thought the issue should disappear after applying my patch and 
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276695.html, however my
testing shows 'make fate-hwdevice V=2' still fails. I ran the fate test on my
local machine.

Thanks
Haihao
Haihao Xiang Aug. 6, 2021, 8 a.m. UTC | #10
> On Wed, 2021-08-04 at 10:10 +0000, Soft Works wrote:
> > Haihao,
> > 
> > I've just been looking into the source patch from February:
> > 
> > > > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > February/276695.html
> > > > >  to create a connection between two devices too, but this change
> > > > > breaks 'make fate-hwdevice V=2').
> > 
> > I've re-read the discussion about why it fails Fate and I actually _think_
> > that it might not fail anymore with your (this) patch applied.
> > 
> > To test, it would probably be required to submit a combined patch,
> > unless you'd have your own Fate server to test..
> > 
> 
> I also thought the issue should disappear after applying my patch and 
> http://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276695.html, however my
> testing shows 'make fate-hwdevice V=2' still fails. I ran the fate test on my
> local machine.

Hi Softworks,

I think Mark's comment is not about fixing issue after applying #276695, instead
it is about fixing/workarounding the original issue in other ways. 

After applying #276695, user may derive a QSV device to a VAAPI device however
user can't derive this VAAPI device back to the original QSV device, so fate-
hwdevice fails. My patch is not related to hw device creation/derivation, fate-
hwdevice still fails even if applying my patch. 

If we can't apply #276695, do you agree to use double device initialization
instead of single-shot initialization to handle -qsv_device option ? 

Thanks
Haihao
Soft Works Aug. 6, 2021, 8:26 a.m. UTC | #11
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Friday, 6 August 2021 10:01
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> 
> > On Wed, 2021-08-04 at 10:10 +0000, Soft Works wrote:
> > > Haihao,
> > >
> > > I've just been looking into the source patch from February:
> > >
> > > > > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > > February/276695.html
> > > > > >  to create a connection between two devices too, but this
> > > > > > change breaks 'make fate-hwdevice V=2').
> > >
> > > I've re-read the discussion about why it fails Fate and I actually
> > > _think_ that it might not fail anymore with your (this) patch applied.
> > >
> > > To test, it would probably be required to submit a combined patch,
> > > unless you'd have your own Fate server to test..
> > >
> >
> > I also thought the issue should disappear after applying my patch and
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276695.html,
> > however my testing shows 'make fate-hwdevice V=2' still fails. I ran
> > the fate test on my local machine.
> 
> Hi Softworks,
> 
> I think Mark's comment is not about fixing issue after applying #276695,
> instead it is about fixing/workarounding the original issue in other ways.
> 
> After applying #276695, user may derive a QSV device to a VAAPI device
> however user can't derive this VAAPI device back to the original QSV device,
> so fate- hwdevice fails. My patch is not related to hw device
> creation/derivation, fate- hwdevice still fails even if applying my patch.
> 
> If we can't apply #276695, do you agree to use double device initialization
> instead of single-shot initialization to handle -qsv_device option ?

No, I don't. The patch setting the ->source value is essential for being able 
to derive an OpenCL from a QSV context, which is the reason why I had 
added it even before the patch from your colleague had been submitted.

One part of Marks comments was caused by the fact that the supplied
command line was wrong and the other part was about what Mark had
called "hacks" for QSV. Those "hacks" are removed by your patch now.
When there's still a Fate error we need to check out the reason, but I'm 
not familiar with running Fate.

I ran this: 

make fate-rsync SAMPLES=fate-suite/
make fate       SAMPLES=fate-suite/

It took a long time but it didn't return any error.

Then I ran this:

make fate-hwdevice V=2

Which built an executable: hwdevice.exe

Finally, running hwdevice.exe returned:

Attempted to test 5 device types: 5 passed, 0 failed, 0 skipped.


Either I've done something wrong in Fate execution or the error doesn't
exist in my code base for some reason.

I have already changed the code in qsvdec to single-shot initialization,
have you done the same?

softworkz
Soft Works Aug. 7, 2021, 3:30 a.m. UTC | #12
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Friday, 6 August 2021 10:26
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> HW_DEVICE_CTX method
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Friday, 6 August 2021 10:01
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > HW_DEVICE_CTX method
> >
> >
> > > On Wed, 2021-08-04 at 10:10 +0000, Soft Works wrote:
> > > > Haihao,
> > > >
> > > > I've just been looking into the source patch from February:
> > > >
> > > > > > > (We may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > > > February/276695.html
> > > > > > >  to create a connection between two devices too, but this
> > > > > > > change breaks 'make fate-hwdevice V=2').
> > > >
> > > > I've re-read the discussion about why it fails Fate and I actually
> > > > _think_ that it might not fail anymore with your (this) patch applied.
> > > >
> > > > To test, it would probably be required to submit a combined patch,
> > > > unless you'd have your own Fate server to test..
> > > >
> > >
> > > I also thought the issue should disappear after applying my patch
> > > and
> > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-February/276695.html,
> > > however my testing shows 'make fate-hwdevice V=2' still fails. I ran
> > > the fate test on my local machine.
> >
> > Hi Softworks,
> >
> > I think Mark's comment is not about fixing issue after applying
> > #276695, instead it is about fixing/workarounding the original issue in other
> ways.
> >
> > After applying #276695, user may derive a QSV device to a VAAPI device
> > however user can't derive this VAAPI device back to the original QSV
> > device, so fate- hwdevice fails. My patch is not related to hw device
> > creation/derivation, fate- hwdevice still fails even if applying my patch.
> >
> > If we can't apply #276695, do you agree to use double device
> > initialization instead of single-shot initialization to handle -qsv_device
> option ?
> 
> No, I don't. The patch setting the ->source value is essential for being able to
> derive an OpenCL from a QSV context, which is the reason why I had added it
> even before the patch from your colleague had been submitted.
> 
> One part of Marks comments was caused by the fact that the supplied
> command line was wrong and the other part was about what Mark had called
> "hacks" for QSV. Those "hacks" are removed by your patch now.
> When there's still a Fate error we need to check out the reason, but I'm not
> familiar with running Fate.
> 
> I ran this:
> 
> make fate-rsync SAMPLES=fate-suite/
> make fate       SAMPLES=fate-suite/
> 
> It took a long time but it didn't return any error.
> 
> Then I ran this:
> 
> make fate-hwdevice V=2
> 
> Which built an executable: hwdevice.exe
> 
> Finally, running hwdevice.exe returned:
> 
> Attempted to test 5 device types: 5 passed, 0 failed, 0 skipped.
> 
> 
> Either I've done something wrong in Fate execution or the error doesn't exist
> in my code base for some reason.
> 
> I have already changed the code in qsvdec to single-shot initialization, have
> you done the same?
> 

Hi Haihao,

The test passed in my case because I had another hack in place.

I have done and submitted a proper implementation now. It combines the patch 
from Guangxin Xu https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210222084504.339978-1-guangxin.xu@intel.com/
with better handling of device derivation:

https://patchwork.ffmpeg.org/project/ffmpeg/patch/BYAPR04MB597605B1168D1F04AD6D36A7BAF49@BYAPR04MB5976.namprd04.prod.outlook.com/

softworkz
diff mbox series

Patch

diff --git a/fftools/Makefile b/fftools/Makefile
index 5affaa3f56..5234932ab0 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -10,7 +10,6 @@  ALLAVPROGS   = $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
 ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
 
 OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
-OBJS-ffmpeg-$(CONFIG_LIBMFX)       += fftools/ffmpeg_qsv.o
 ifndef CONFIG_VIDEOTOOLBOX
 OBJS-ffmpeg-$(CONFIG_VDA)          += fftools/ffmpeg_videotoolbox.o
 endif
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d9c0628657..d2dd7ca092 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -61,7 +61,6 @@  enum HWAccelID {
     HWACCEL_AUTO,
     HWACCEL_GENERIC,
     HWACCEL_VIDEOTOOLBOX,
-    HWACCEL_QSV,
 };
 
 typedef struct HWAccel {
diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
index fc4a5d31d6..043e4c5863 100644
--- a/fftools/ffmpeg_hw.c
+++ b/fftools/ffmpeg_hw.c
@@ -339,6 +339,18 @@  int hw_device_setup_for_decode(InputStream *ist)
         } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
             type = ist->hwaccel_device_type;
             dev = hw_device_get_by_type(type);
+
+            // When "-qsv_device device" is used, an internal QSV device named
+            // as "__qsv_device" is created and another QSV device is created
+            // if "-init_hw_device qsv=name@name" is used. There are 2 QSV devices
+            // if both "-qsv_device device" and "-init_hw_device qsv=name@name"
+            // are used, hw_device_get_by_type(AV_HWDEVICE_TYPE_QSV) returns NULL.
+            // To keep back-compatibility with the removed ad-hoc libmfx setup code,
+            // call hw_device_get_by_name("__qsv_device") to select the internal QSV
+            // device. This will be removed once -qsv_device is no longer supported.
+            if (!dev && type == AV_HWDEVICE_TYPE_QSV)
+                dev = hw_device_get_by_name("__qsv_device");
+
             if (!dev)
                 err = hw_device_init_from_type(type, NULL, &dev);
         } else {
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 1b43bab9fc..4ddc6c939b 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -137,9 +137,6 @@  static const char *const opt_name_enc_time_bases[]            = {"enc_time_base"
 const HWAccel hwaccels[] = {
 #if CONFIG_VIDEOTOOLBOX
     { "videotoolbox", videotoolbox_init, HWACCEL_VIDEOTOOLBOX, AV_PIX_FMT_VIDEOTOOLBOX },
-#endif
-#if CONFIG_LIBMFX
-    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
 #endif
     { 0 },
 };
@@ -571,6 +568,49 @@  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)
+{
+#if CONFIG_VAAPI
+    const char *prefix = "vaapi=__qsv_child_device:";
+#elif CONFIG_DXVA2
+    const char *prefix = "dxva2=__qsv_child_device:";
+#else
+    const char *prefix = NULL;
+#endif
+    int err = 0;
+    char *tmp = NULL;
+
+    if (prefix) {
+        av_log(NULL, AV_LOG_WARNING,
+            "WARNING: Please use \"-init_hw_device args\" to initialise QSV "
+            "device and \"-hwaccel_device devicename\" to select QSV device "
+            "for QSV decoders. \"-qsv_device device\" is DEPRECATED and will "
+            "be removed in the future. Please do not use \"__qsv_child_device\" "
+            "or \"__qsv_device\" as device name when both \"-qsv_device device\" "
+            "and \"-init_hw_device args\" are used.\n");
+
+        tmp = av_asprintf("%s%s", prefix, arg);
+
+        if (!tmp)
+            return AVERROR(ENOMEM);
+
+        err = hw_device_init_from_string(tmp, NULL);
+
+        if (err)
+            goto error;
+
+        err = hw_device_init_from_string("qsv=__qsv_device@__qsv_child_device", NULL);
+    } else
+        err = AVERROR(ENOSYS);
+
+error:
+    av_free(tmp);
+    return err;
+}
+
+#endif
+
 static int opt_init_hw_device(void *optctx, const char *opt, const char *arg)
 {
     if (!strcmp(arg, "list")) {
@@ -898,6 +938,12 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
                     "with old commandlines. This behaviour is DEPRECATED and will be removed "
                     "in the future. Please explicitly set \"-hwaccel_output_format cuda\".\n");
                 ist->hwaccel_output_format = AV_PIX_FMT_CUDA;
+            } else if (!hwaccel_output_format && hwaccel && !strcmp(hwaccel, "qsv")) {
+                av_log(NULL, AV_LOG_WARNING,
+                    "WARNING: defaulting hwaccel_output_format to qsv for compatibility "
+                    "with old commandlines. This behaviour is DEPRECATED and will be removed "
+                    "in the future. Please explicitly set \"-hwaccel_output_format qsv\".\n");
+                ist->hwaccel_output_format = AV_PIX_FMT_QSV;
             } else if (hwaccel_output_format) {
                 ist->hwaccel_output_format = av_get_pix_fmt(hwaccel_output_format);
                 if (ist->hwaccel_output_format == AV_PIX_FMT_NONE) {
@@ -3825,7 +3871,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/fftools/ffmpeg_qsv.c b/fftools/ffmpeg_qsv.c
deleted file mode 100644
index 7bd999d310..0000000000
--- a/fftools/ffmpeg_qsv.c
+++ /dev/null
@@ -1,109 +0,0 @@ 
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include <mfx/mfxvideo.h>
-#include <stdlib.h>
-
-#include "libavutil/dict.h"
-#include "libavutil/hwcontext.h"
-#include "libavutil/hwcontext_qsv.h"
-#include "libavutil/opt.h"
-#include "libavcodec/qsv.h"
-
-#include "ffmpeg.h"
-
-static AVBufferRef *hw_device_ctx;
-char *qsv_device = NULL;
-
-static int qsv_get_buffer(AVCodecContext *s, AVFrame *frame, int flags)
-{
-    InputStream *ist = s->opaque;
-
-    return av_hwframe_get_buffer(ist->hw_frames_ctx, frame, 0);
-}
-
-static void qsv_uninit(AVCodecContext *s)
-{
-    InputStream *ist = s->opaque;
-    av_buffer_unref(&ist->hw_frames_ctx);
-}
-
-static int qsv_device_init(InputStream *ist)
-{
-    int err;
-    AVDictionary *dict = NULL;
-
-    if (qsv_device) {
-        err = av_dict_set(&dict, "child_device", qsv_device, 0);
-        if (err < 0)
-            return err;
-    }
-
-    err = av_hwdevice_ctx_create(&hw_device_ctx, AV_HWDEVICE_TYPE_QSV,
-                                 ist->hwaccel_device, dict, 0);
-    if (err < 0) {
-        av_log(NULL, AV_LOG_ERROR, "Error creating a QSV device\n");
-        goto err_out;
-    }
-
-err_out:
-    if (dict)
-        av_dict_free(&dict);
-
-    return err;
-}
-
-int qsv_init(AVCodecContext *s)
-{
-    InputStream *ist = s->opaque;
-    AVHWFramesContext *frames_ctx;
-    AVQSVFramesContext *frames_hwctx;
-    int ret;
-
-    if (!hw_device_ctx) {
-        ret = qsv_device_init(ist);
-        if (ret < 0)
-            return ret;
-    }
-
-    av_buffer_unref(&ist->hw_frames_ctx);
-    ist->hw_frames_ctx = av_hwframe_ctx_alloc(hw_device_ctx);
-    if (!ist->hw_frames_ctx)
-        return AVERROR(ENOMEM);
-
-    frames_ctx   = (AVHWFramesContext*)ist->hw_frames_ctx->data;
-    frames_hwctx = frames_ctx->hwctx;
-
-    frames_ctx->width             = FFALIGN(s->coded_width,  32);
-    frames_ctx->height            = FFALIGN(s->coded_height, 32);
-    frames_ctx->format            = AV_PIX_FMT_QSV;
-    frames_ctx->sw_format         = s->sw_pix_fmt;
-    frames_ctx->initial_pool_size = 64 + s->extra_hw_frames;
-    frames_hwctx->frame_type      = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
-
-    ret = av_hwframe_ctx_init(ist->hw_frames_ctx);
-    if (ret < 0) {
-        av_log(NULL, AV_LOG_ERROR, "Error initializing a QSV frame pool\n");
-        return ret;
-    }
-
-    ist->hwaccel_get_buffer = qsv_get_buffer;
-    ist->hwaccel_uninit     = qsv_uninit;
-
-    return 0;
-}
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index 622750927c..1ee9944aa2 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -99,7 +99,7 @@  static const AVCodecHWConfigInternal *const qsv_hw_configs[] = {
         .public = {
             .pix_fmt     = AV_PIX_FMT_QSV,
             .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
-                           AV_CODEC_HW_CONFIG_METHOD_AD_HOC,
+                           AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX,
             .device_type = AV_HWDEVICE_TYPE_QSV,
         },
         .hwaccel = NULL,
@@ -248,6 +248,35 @@  static int qsv_decode_preinit(AVCodecContext *avctx, QSVContext *q, enum AVPixel
         q->nb_ext_buffers = user_ctx->nb_ext_buffers;
     }
 
+    if (avctx->hw_device_ctx && !avctx->hw_frames_ctx && ret == AV_PIX_FMT_QSV) {
+        AVHWFramesContext *hwframes_ctx;
+        AVQSVFramesContext *frames_hwctx;
+
+        avctx->hw_frames_ctx = av_hwframe_ctx_alloc(avctx->hw_device_ctx);
+
+        if (!avctx->hw_frames_ctx) {
+            av_log(avctx, AV_LOG_ERROR, "av_hwframe_ctx_alloc failed\n");
+            return AVERROR(ENOMEM);
+        }
+
+        hwframes_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
+        frames_hwctx = hwframes_ctx->hwctx;
+        hwframes_ctx->width             = FFALIGN(avctx->coded_width,  32);
+        hwframes_ctx->height            = FFALIGN(avctx->coded_height, 32);
+        hwframes_ctx->format            = AV_PIX_FMT_QSV;
+        hwframes_ctx->sw_format         = avctx->sw_pix_fmt;
+        hwframes_ctx->initial_pool_size = 64 + avctx->extra_hw_frames;
+        frames_hwctx->frame_type        = MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
+
+        ret = av_hwframe_ctx_init(avctx->hw_frames_ctx);
+
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Error initializing a QSV frame pool\n");
+            av_buffer_unref(&avctx->hw_frames_ctx);
+            return ret;
+        }
+    }
+
     if (avctx->hw_frames_ctx) {
         AVHWFramesContext    *frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
         AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;