[FFmpeg-devel] [PATCH v2 2/4] libavfilter/qsvvpp: enabling d3d11va support

[Steve Lhomme]

Comments below.

On 2020-04-15 15:07, artem.galin at gmail.com wrote:
> From: Artem Galin <artem.galin at intel.com>
> 
> Adding DX11 relevant device type checks and adjusting callback with
> proper MediaSDK pair type support.
> 
> Signed-off-by: Artem Galin <artem.galin at intel.com>
> ---
>   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.

> +#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 ?

>   
>           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;
> +    }

handle_type is potentially used initialized after that. You probably 
want to return an error is the handle type is unknown.

> +
>       for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) {
> -        ret = MFXVideoCORE_GetHandle(device_hwctx->session, handle_types[i], &handle);
> -        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.

Also you don't need to do the for() loop since you only want to read the 
handle of the type you want.

The title of the patch is misleading. MFX_HANDLE_D3D11_DEVICE was 
already supported in the list of handles.

> +            ret = MFXVideoCORE_GetHandle(device_hwctx->session, handle_types[i], &handle);
> +            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 handle\n");
>           return AVERROR_UNKNOWN;
>       }
> -- 
> 2.26.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>