On Tue, 21 Apr 2020 at 11:25, Steve Lhomme wrote:
> Comments below.
>
> On 2020-04-15 15:07, artem.ga...@gmail.com wrote:
> > From: Artem Galin
> >
> > Adding DX11 relevant device type checks and adjusting callback with
> > proper MediaSDK pair type support.
> >
> > Signed-off-by: Artem Galin
> > ---
> > libavfilter/qsvvpp.c | 40 ++--
> > 1 file changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
> > index 8d5ff2eb65..8c6c878bac 100644
> > --- a/libavfilter/qsvvpp.c
> > +++ b/libavfilter/qsvvpp.c
> > @@ -32,10 +32,11 @@
> > #include "qsvvpp.h"
> > #include "video.h"
> >
> > -#define IS_VIDEO_MEMORY(mode) (mode &
> (MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET | \
> > +#define IS_VIDEO_MEMORY(mode) (mode &
> (MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET | \
> >
> MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET))
> > -#define IS_OPAQUE_MEMORY(mode) (mode & MFX_MEMTYPE_OPAQUE_FRAME)
> > -#define IS_SYSTEM_MEMORY(mode) (mode & MFX_MEMTYPE_SYSTEM_MEMORY)
> > +#define IS_OPAQUE_MEMORY(mode) (mode & MFX_MEMTYPE_OPAQUE_FRAME)
> > +#define IS_SYSTEM_MEMORY(mode) (mode & MFX_MEMTYPE_SYSTEM_MEMORY)
>
> Not sure changing these lines just to add a space is worth.
>
> Agree
> > +#define MFX_IMPL_VIA_MASK(impl) (0x0f00 & (impl))
> >
> > typedef struct QSVFrame {
> > AVFrame *frame;
> > @@ -129,7 +130,17 @@ static mfxStatus frame_unlock(mfxHDL pthis,
> mfxMemId mid, mfxFrameData *ptr)
> >
> > static mfxStatus frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
> > {
> > +#if CONFIG_VAAPI
> > *hdl = mid;
> > +#else
> > +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;
> > +#endif
> > return MFX_ERR_NONE;
> > }
> >
> > @@ -451,7 +462,7 @@ static int init_vpp_session(AVFilterContext *avctx,
> QSVVPPContext *s)
> >
> > s->out_mem_mode = IS_OPAQUE_MEMORY(s->in_mem_mode) ?
> > MFX_MEMTYPE_OPAQUE_FRAME :
> > - MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
> > + MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET |
> MFX_MEMTYPE_FROM_VPPOUT;
>
> What's the effect of this flag on VAAPI ?
>
This flag has to be set for both Linux and Windows
https://github.com/Intel-Media-SDK/MediaSDK/blob/master/samples/sample_encode/src/pipeline_encode.cpp#L1006
>
> >
> > out_frames_ctx = (AVHWFramesContext *)out_frames_ref->data;
> > out_frames_hwctx = out_frames_ctx->hwctx;
> > @@ -497,15 +508,24 @@ static int init_vpp_session(AVFilterContext
> *avctx, QSVVPPContext *s)
> > return AVERROR_UNKNOWN;
> > }
> >
> > +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 if (MFX_IMPL_VIA_VAAPI == MFX_IMPL_VIA_MASK(impl)) {
> > +handle_type = MFX_HANDLE_VA_DISPLAY;
> > +}
>
These lines needed to correctly extract device type and avoid some Windows
drivers issues.
>
> handle_type is potentially used initialized after that. You probably
> want to return an error is the handle type is unknown.
>
> Agree.
> > +
> > for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) {
> > -ret = MFXVideoCORE_GetHandle(device_hwctx->session,
> handle_types[i], );
> > -if (ret == MFX_ERR_NONE) {
> > -handle_type = handle_types[i];
> > -break;
> > +if (handle_types[i] == handle_type) {
>
> You're losing the possibility to pick whichever the system provides.
> Although in real life it may not be an issue if you can only have VAAPI
> or D3D9/D3D11 separately.
>
No, I just extract type of the given device session. No new devices
session, no new types were added in comparison with base.
>
> Also you don't need to do the for() loop since you only want to read the
> handle of the type you want.
>
Agree.
>
> The title of the patch is misleading. MFX_HANDLE_D3D11_DEVICE was
> already supported in the list of handles.
>
The base approach to extract device type by enumeration via GetHandle()
function call does not work properly on some Windows drivers and never
been tested with D3D11VA because of QSV works only via DXVA2 on Windows and
VAAPI on Linux.
> This patch does proper D3D11VA enabling that works.
>
> > +ret = MFXVideoCORE_GetHandle(device_hwctx->session,
> handle_types[i], );
> > +if (ret == MFX_ERR_NONE) {
> > +break;
> > +}
> > }
> > +handle = NULL;
> > }
> > -
> > -if (ret != MFX_ERR_NONE) {
> > +if (!handle) {
> > av_log(avctx, AV_LOG_ERROR, "Error getting the session
>
