diff mbox series

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

Message ID 20200415130741.27263-2-artem.galin@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/4] fftools/qsv: enabling d3d11va/dxva2 device selection | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

galinart April 15, 2020, 1:07 p.m. UTC 
From: Artem Galin <artem.galin@intel.com>

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/qsvvpp.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

Comments

Steve Lhomme April 21, 2020, 10:25 a.m. UTC | #1 
Comments below.

On 2020-04-15 15:07, artem.galin@gmail.com wrote:
> From: Artem Galin <artem.galin@intel.com>
> 
> 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/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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
galinart April 22, 2020, 6:06 p.m. UTC | #2 
On Tue, 21 Apr 2020 at 11:25, Steve Lhomme <robux4@ycbcr.xyz> wrote:

> Comments below.
>
> On 2020-04-15 15:07, artem.galin@gmail.com wrote:
> > From: Artem Galin <artem.galin@intel.com>
> >
> > 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/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], &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.
>
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], &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@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
> _______________________________________________
> 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".
diff mbox series

Patch

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)
+#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;
 
         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;
+    }
+
     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) {
+            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;
     }