| Message ID | 20201103184557.18444-8-artem.galin@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v7,1/8] libavcodec/qsv: enabling d3d11va support, added mfxhdlpair | 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 > Artem Galin > Sent: Tuesday, November 3, 2020 7:46 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Artem Galin <artem.galin@intel.com> > Subject: [FFmpeg-devel] [PATCH v7 8/8] libavfilter/vf_deinterlace_qsv: > enabling d3d11va support, added mfxhdlpair > > Adding DX11 relevant device type checks and adjusting callback with proper > MediaSDK pair type support. > Hi Artem, I have a few more notes regarding the patch: The switch to using mfxhdlpair will cause a regression in case of VAAPI/OpenCL Interop. In hwcontext_opencl.cs, function opencl_map_from_qsv, you need to cast the MemId to mfxHDLPair for getting the surface id. OpenCL interop on Windows is a whole different story, but it's not a small thing like the above, so you better handle that at a later time. It's not a regression anyway because this functionality doesn't exist yet (for D3D11). At the same time, that's another reason for NOT changing the default to D3D11 because somebody using QSV<=>OpenCL interop would see his commands failing when you change the default to D3D11. I don't know what others here are thinking (because nobody says anything), but IMO, a patch that doesn't switch the default impl would have less impact and probably better chances to get merged. Kind regards, softworkz
On Wed, 24 Feb 2021 at 17:50, Soft Works <softworkz@hotmail.com> wrote: > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Artem Galin > > Sent: Tuesday, November 3, 2020 7:46 PM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Artem Galin <artem.galin@intel.com> > > Subject: [FFmpeg-devel] [PATCH v7 8/8] libavfilter/vf_deinterlace_qsv: > > enabling d3d11va support, added mfxhdlpair > > > > Adding DX11 relevant device type checks and adjusting callback with > proper > > MediaSDK pair type support. > > > > Hi Artem, > > I have a few more notes regarding the patch: > > The switch to using mfxhdlpair will cause a regression in case of > VAAPI/OpenCL > Interop. In hwcontext_opencl.cs, function opencl_map_from_qsv, you need to > cast the MemId to mfxHDLPair for getting the surface id. > > OpenCL interop on Windows is a whole different story, but it's not a small > thing > like the above, so you better handle that at a later time. It's not a > regression > anyway because this functionality doesn't exist yet (for D3D11). > > At the same time, that's another reason for NOT changing the default to > D3D11 > because somebody using QSV<=>OpenCL interop would see his commands > failing when you change the default to D3D11. > > I don't know what others here are thinking (because nobody says anything), > but IMO, a patch that doesn't switch the default impl would have less > impact and probably better chances to get merged. > > Kind regards, > softworkz > > Hi Softworkz, D3D11VA works with more variety of HW configurations and there is no DX9 drop in the patch. Feel free to send your version of the patch with interop fix. Thanks, Artem. > > > _______________________________________________ > 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 > Artem Galin > Sent: Thursday, February 25, 2021 12:34 PM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v7 8/8] libavfilter/vf_deinterlace_qsv: > enabling d3d11va support, added mfxhdlpair > > On Wed, 24 Feb 2021 at 17:50, Soft Works <softworkz@hotmail.com> wrote: > > > > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Artem Galin > > > Sent: Tuesday, November 3, 2020 7:46 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Artem Galin <artem.galin@intel.com> > > > Subject: [FFmpeg-devel] [PATCH v7 8/8] libavfilter/vf_deinterlace_qsv: > > > enabling d3d11va support, added mfxhdlpair > > > > > > Adding DX11 relevant device type checks and adjusting callback with > > proper > > > MediaSDK pair type support. > > > > > > > Hi Artem, > > > > I have a few more notes regarding the patch: > > > > The switch to using mfxhdlpair will cause a regression in case of > > VAAPI/OpenCL Interop. In hwcontext_opencl.cs, function > > opencl_map_from_qsv, you need to cast the MemId to mfxHDLPair for > > getting the surface id. > > > > OpenCL interop on Windows is a whole different story, but it's not a > > small thing like the above, so you better handle that at a later time. > > It's not a regression anyway because this functionality doesn't exist > > yet (for D3D11). > > > > At the same time, that's another reason for NOT changing the default > > to > > D3D11 > > because somebody using QSV<=>OpenCL interop would see his commands > > failing when you change the default to D3D11. > > > > I don't know what others here are thinking (because nobody says > > anything), but IMO, a patch that doesn't switch the default impl would > > have less impact and probably better chances to get merged. > > > > Kind regards, > > softworkz > > > > > Hi Softworkz, > > D3D11VA works with more variety of HW configurations and there is no DX9 > drop in the patch. Feel free to send your version of the patch with interop > fix. Hi Artem, " there is no DX9 drop in the patch" > sure, D3D9 is not dropped, but you are changing the default to D3D11, which brings a lot of differences. Command lines that are working today won't work anymore - and not just for a single reason. There's a bunch of cases as I had already laid out earlier. The missing OpenCL+D3D11 interop is just an additional bullet to that list. In addition to that, there are at least two bugs in your v7 patch as I've outlined above. Kind regards, Softworkz
diff --git a/libavfilter/vf_deinterlace_qsv.c b/libavfilter/vf_deinterlace_qsv.c index 80217c8419..f7f9d916db 100644 --- a/libavfilter/vf_deinterlace_qsv.c +++ b/libavfilter/vf_deinterlace_qsv.c @@ -42,6 +42,8 @@ #include "internal.h" #include "video.h" +#define MFX_IMPL_VIA_MASK(impl) (0x0f00 & (impl)) + enum { QSVDEINT_MORE_OUTPUT = 1, QSVDEINT_MORE_INPUT, @@ -157,16 +159,16 @@ static mfxStatus frame_unlock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr) static mfxStatus frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl) { - *hdl = mid; + mfxHDLPair *pair_dst = (mfxHDLPair*)hdl; + mfxHDLPair *pair_src = (mfxHDLPair*)mid; + + pair_dst->first = pair_src->first; + + if (pair_src->second != (mfxMemId)MFX_INFINITE) + pair_dst->second = pair_src->second; return MFX_ERR_NONE; } -static const mfxHandleType handle_types[] = { - MFX_HANDLE_VA_DISPLAY, - MFX_HANDLE_D3D9_DEVICE_MANAGER, - MFX_HANDLE_D3D11_DEVICE, -}; - static int init_out_session(AVFilterContext *ctx) { @@ -183,26 +185,30 @@ static int init_out_session(AVFilterContext *ctx) mfxIMPL impl; mfxVideoParam par; mfxStatus err; - int i; + int ret, i; /* extract the properties of the "master" session given to us */ - err = MFXQueryIMPL(device_hwctx->session, &impl); - if (err == MFX_ERR_NONE) - err = MFXQueryVersion(device_hwctx->session, &ver); - if (err != MFX_ERR_NONE) { + ret = MFXQueryIMPL(device_hwctx->session, &impl); + if (ret == MFX_ERR_NONE) + ret = MFXQueryVersion(device_hwctx->session, &ver); + if (ret != MFX_ERR_NONE) { av_log(ctx, AV_LOG_ERROR, "Error querying the session attributes\n"); return AVERROR_UNKNOWN; } - for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) { - err = MFXVideoCORE_GetHandle(device_hwctx->session, handle_types[i], &handle); - if (err == MFX_ERR_NONE) { - handle_type = handle_types[i]; - break; - } + if (MFX_IMPL_VIA_VAAPI == MFX_IMPL_VIA_MASK(impl)) { + handle_type = MFX_HANDLE_VA_DISPLAY; + } else if (MFX_IMPL_VIA_D3D11 == MFX_IMPL_VIA_MASK(impl)) { + handle_type = MFX_HANDLE_D3D11_DEVICE; + } else if (MFX_IMPL_VIA_D3D9 == MFX_IMPL_VIA_MASK(impl)) { + handle_type = MFX_HANDLE_D3D9_DEVICE_MANAGER; + } else { + av_log(ctx, AV_LOG_ERROR, "Error unsupported handle type\n"); + return AVERROR_UNKNOWN; } - if (err != MFX_ERR_NONE) { + ret = MFXVideoCORE_GetHandle(device_hwctx->session, handle_type, &handle); + if (ret != MFX_ERR_NONE) { av_log(ctx, AV_LOG_ERROR, "Error getting the session handle\n"); return AVERROR_UNKNOWN; }
Adding DX11 relevant device type checks and adjusting callback with proper MediaSDK pair type support. Signed-off-by: Artem Galin <artem.galin@intel.com> --- libavfilter/vf_deinterlace_qsv.c | 44 ++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 19 deletions(-)