[FFmpeg-devel] d3d11va: use the proper slice index

The slice index expected by D3D11VA is the one from the texture not from the
array or texture/slices.

In VLC the slices we provide the decoder don't start from 0 and thus pictures
appear in bogus order. With possible crashes and corruptions when using an
invalid index.

--
* forgot to bump the minor version
* get rid of DXVA_CONTEXT_SURFACE
* bump the minor version, not the macro
---
 libavcodec/dxva2.c          | 12 +++++++++++-
 libavcodec/dxva2_internal.h |  3 ---
 libavcodec/version.h        |  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

Any comment on this ? We are waiting or this patch to proceed with the
related patch in VLC (because we need to rely on the proper revision).

It's been merged in libav with a slightly different commit text:

The decoding buffer index expected by D3D11VA is the one from the
ID3D11Texture2D not the one from the ID3D11VideoDecoderOutputView array
in AVD3D11VAContext.

Otherwise, when providing decoder slices that do not start from 0,
pictures appear in bogus order. For an invalid index crashes and
image corruption can occur.

On Wed, Oct 5, 2016 at 12:52 PM, Steve Lhomme <robux4@videolabs.io> wrote:
> The slice index expected by D3D11VA is the one from the texture not from the
> array or texture/slices.
>
> In VLC the slices we provide the decoder don't start from 0 and thus pictures
> appear in bogus order. With possible crashes and corruptions when using an
> invalid index.
>
> --
> * forgot to bump the minor version
> * get rid of DXVA_CONTEXT_SURFACE
> * bump the minor version, not the macro
> ---
>  libavcodec/dxva2.c          | 12 +++++++++++-
>  libavcodec/dxva2_internal.h |  3 ---
>  libavcodec/version.h        |  2 +-
>  3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
> index f68df86..e168233 100644
> --- a/libavcodec/dxva2.c
> +++ b/libavcodec/dxva2.c
> @@ -42,8 +42,18 @@ unsigned ff_dxva2_get_surface_index(const AVCodecContext *avctx,
>      unsigned i;
>
>      for (i = 0; i < DXVA_CONTEXT_COUNT(avctx, ctx); i++)
> -        if (DXVA_CONTEXT_SURFACE(avctx, ctx, i) == surface)
> +#if CONFIG_D3D11VA
> +        if (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD && ctx->d3d11va.surface[i] == surface)
> +        {
> +            D3D11_VIDEO_DECODER_OUTPUT_VIEW_DESC viewDesc;
> +            ID3D11VideoDecoderOutputView_GetDesc(ctx->d3d11va.surface[i], &viewDesc);
> +            return viewDesc.Texture2D.ArraySlice;
> +        }
> +#endif
> +#if CONFIG_DXVA2
> +        if (avctx->pix_fmt == AV_PIX_FMT_DXVA2_VLD && ctx->dxva2.surface[i] == surface)
>              return i;
> +#endif
>
>      assert(0);
>      return 0;
> diff --git a/libavcodec/dxva2_internal.h b/libavcodec/dxva2_internal.h
> index ad89f82..24adb99 100644
> --- a/libavcodec/dxva2_internal.h
> +++ b/libavcodec/dxva2_internal.h
> @@ -70,7 +70,6 @@ typedef union {
>  #if CONFIG_D3D11VA && CONFIG_DXVA2
>  #define DXVA_CONTEXT_WORKAROUND(avctx, ctx)     (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? ctx->d3d11va.workaround : ctx->dxva2.workaround)
>  #define DXVA_CONTEXT_COUNT(avctx, ctx)          (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? ctx->d3d11va.surface_count : ctx->dxva2.surface_count)
> -#define DXVA_CONTEXT_SURFACE(avctx, ctx, i)     (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? ctx->d3d11va.surface[i] : ctx->dxva2.surface[i])
>  #define DXVA_CONTEXT_DECODER(avctx, ctx)        (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? ctx->d3d11va.decoder : ctx->dxva2.decoder)
>  #define DXVA_CONTEXT_REPORT_ID(avctx, ctx)      (*(avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? &ctx->d3d11va.report_id : &ctx->dxva2.report_id))
>  #define DXVA_CONTEXT_CFG(avctx, ctx)            (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD ? ctx->d3d11va.cfg : ctx->dxva2.cfg)
> @@ -80,7 +79,6 @@ typedef union {
>  #elif CONFIG_DXVA2
>  #define DXVA_CONTEXT_WORKAROUND(avctx, ctx)     (ctx->dxva2.workaround)
>  #define DXVA_CONTEXT_COUNT(avctx, ctx)          (ctx->dxva2.surface_count)
> -#define DXVA_CONTEXT_SURFACE(avctx, ctx, i)     (ctx->dxva2.surface[i])
>  #define DXVA_CONTEXT_DECODER(avctx, ctx)        (ctx->dxva2.decoder)
>  #define DXVA_CONTEXT_REPORT_ID(avctx, ctx)      (*(&ctx->dxva2.report_id))
>  #define DXVA_CONTEXT_CFG(avctx, ctx)            (ctx->dxva2.cfg)
> @@ -90,7 +88,6 @@ typedef union {
>  #elif CONFIG_D3D11VA
>  #define DXVA_CONTEXT_WORKAROUND(avctx, ctx)     (ctx->d3d11va.workaround)
>  #define DXVA_CONTEXT_COUNT(avctx, ctx)          (ctx->d3d11va.surface_count)
> -#define DXVA_CONTEXT_SURFACE(avctx, ctx, i)     (ctx->d3d11va.surface[i])
>  #define DXVA_CONTEXT_DECODER(avctx, ctx)        (ctx->d3d11va.decoder)
>  #define DXVA_CONTEXT_REPORT_ID(avctx, ctx)      (*(&ctx->d3d11va.report_id))
>  #define DXVA_CONTEXT_CFG(avctx, ctx)            (ctx->d3d11va.cfg)
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index de7280f..a91595b 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>
>  #define LIBAVCODEC_VERSION_MAJOR  57
>  #define LIBAVCODEC_VERSION_MINOR  60
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MICRO 102
>
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> --
> 2.8.2
>

On Mon, Oct 10, 2016 at 11:48:20AM +0200, Steve Lhomme wrote:
> Any comment on this ? We are waiting or this patch to proceed with the
> related patch in VLC (because we need to rely on the proper revision).
> 
> It's been merged in libav with a slightly different commit text:
> 
> The decoding buffer index expected by D3D11VA is the one from the
> ID3D11Texture2D not the one from the ID3D11VideoDecoderOutputView array
> in AVD3D11VAContext.
> 
> Otherwise, when providing decoder slices that do not start from 0,
> pictures appear in bogus order. For an invalid index crashes and
> image corruption can occur.

applied

maybe our dxva2 maintainer wants a co maintainer?
if so would someone be interrested to help ?

Thanks

[...]

On Mon, Oct 10, 2016 at 4:01 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Mon, Oct 10, 2016 at 11:48:20AM +0200, Steve Lhomme wrote:
>> Any comment on this ? We are waiting or this patch to proceed with the
>> related patch in VLC (because we need to rely on the proper revision).
>>
>> It's been merged in libav with a slightly different commit text:
>>
>> The decoding buffer index expected by D3D11VA is the one from the
>> ID3D11Texture2D not the one from the ID3D11VideoDecoderOutputView array
>> in AVD3D11VAContext.
>>
>> Otherwise, when providing decoder slices that do not start from 0,
>> pictures appear in bogus order. For an invalid index crashes and
>> image corruption can occur.
>
> applied
>
> maybe our dxva2 maintainer wants a co maintainer?
> if so would someone be interrested to help ?
>

Unfortunately I have no way of testing the D3D11 stuff in there (and I
honestly wasn't a big fan of hacking it in like that in the first
place), so unless someone contributes ffmpeg_d3d11.c and
avutil/hwcontext_d3d11.c, there isn't much about a testing setup to
confirm whatever assumptions are made in the code.

- Hendrik

On Mon, 10 Oct 2016 17:07:06 +0200
Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Mon, Oct 10, 2016 at 4:01 PM, Michael Niedermayer
> >
> > maybe our dxva2 maintainer wants a co maintainer?
> > if so would someone be interrested to help ?
> >
> 
> Unfortunately I have no way of testing the D3D11 stuff in there (and I

do you want a windows box with a d3d11 card? there are many companies
now using ffmpeg and they want to get boxes into developer hands for
testing.

-compn

On Mon, Oct 10, 2016 at 11:34 PM, compn <tempn@mi.rr.com> wrote:
> On Mon, 10 Oct 2016 17:07:06 +0200
> Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
>> On Mon, Oct 10, 2016 at 4:01 PM, Michael Niedermayer
>> >
>> > maybe our dxva2 maintainer wants a co maintainer?
>> > if so would someone be interrested to help ?
>> >
>>
>> Unfortunately I have no way of testing the D3D11 stuff in there (and I
>
> do you want a windows box with a d3d11 card? there are many companies
> now using ffmpeg and they want to get boxes into developer hands for
> testing.
>

I have all the hardware, but I have no software that actually uses
d3d11, and ffmpeg itself also does not use it. Hence no testing
options.

- Hendrik

On Tue, 11 Oct 2016 10:18:06 +0200
Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Mon, Oct 10, 2016 at 11:34 PM, compn <tempn@mi.rr.com> wrote:
> > On Mon, 10 Oct 2016 17:07:06 +0200
> > Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> >  
> >> On Mon, Oct 10, 2016 at 4:01 PM, Michael Niedermayer  
> >> >
> >> > maybe our dxva2 maintainer wants a co maintainer?
> >> > if so would someone be interrested to help ?
> >> >  
> >>
> >> Unfortunately I have no way of testing the D3D11 stuff in there (and I  
> >
> > do you want a windows box with a d3d11 card? there are many companies
> > now using ffmpeg and they want to get boxes into developer hands for
> > testing.
> >  
> 
> I have all the hardware, but I have no software that actually uses
> d3d11, and ffmpeg itself also does not use it. Hence no testing
> options.

mpv supports it, if you need another API user than just vlc.