| 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 | expand |
| 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 |
> -----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
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".
> -----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
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
> -----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".
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
> -----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".
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
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
> 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
> -----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
> -----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 --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;