[vlc-devel] [PATCH] dxva2: fixed incorrect acquiring/releasing of hw surfaces

[Steve Lhomme]

On Thu, Jun 29, 2017 at 9:28 PM, Oliver Collyer <ovcollyer at mac.com> wrote:
> I found that after using DXVA2 in a custom libVLC application I could no longer reset my D3D9 device, because some surfaces hadn't been released.
>
> This was caused by two issues:
>
> 1) failure to release the surfaces in DxDestroyVideoDecoder that were created by DxCreateVideoDecoder
> 2) incorrectly calling AcquirePictureSys on the newly created (as opposed to copied) surfaces
>
> The result was that all the surfaces allocated had 2 references that were never released.
>
> ---
>  modules/codec/avcodec/dxva2.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/modules/codec/avcodec/dxva2.c b/modules/codec/avcodec/dxva2.c
> index 31c8f67afb..6c1351853a 100644
> --- a/modules/codec/avcodec/dxva2.c
> +++ b/modules/codec/avcodec/dxva2.c
> @@ -178,12 +178,12 @@ static void d3d9_pic_context_destroy(struct picture_context_t *opaque)
>      }
>  }
>
> -static struct va_pic_context *CreatePicContext(IDirect3DSurface9 *);
> +static struct va_pic_context *CreatePicContext(IDirect3DSurface9 *, bool);
>
>  static struct picture_context_t *d3d9_pic_context_copy(struct picture_context_t *ctx)
>  {
>      struct va_pic_context *src_ctx = (struct va_pic_context*)ctx;
> -    struct va_pic_context *pic_ctx = CreatePicContext(src_ctx->picsys.surface);
> +    struct va_pic_context *pic_ctx = CreatePicContext(src_ctx->picsys.surface, true);
>      if (unlikely(pic_ctx==NULL))
>          return NULL;
>      pic_ctx->va_surface = src_ctx->va_surface;
> @@ -191,7 +191,7 @@ static struct picture_context_t *d3d9_pic_context_copy(struct picture_context_t
>      return &pic_ctx->s;
>  }
>
> -static struct va_pic_context *CreatePicContext(IDirect3DSurface9 *surface)
> +static struct va_pic_context *CreatePicContext(IDirect3DSurface9 *surface, bool copying)
>  {
>      struct va_pic_context *pic_ctx = calloc(1, sizeof(*pic_ctx));
>      if (unlikely(pic_ctx==NULL))
> @@ -199,14 +199,15 @@ static struct va_pic_context *CreatePicContext(IDirect3DSurface9 *surface)
>      pic_ctx->s.destroy = d3d9_pic_context_destroy;
>      pic_ctx->s.copy    = d3d9_pic_context_copy;
>      pic_ctx->picsys.surface = surface;
> -    AcquirePictureSys(&pic_ctx->picsys);
> +    if (copying)
> +        AcquirePictureSys(&pic_ctx->picsys);
>      return pic_ctx;
>  }
>
>  static struct va_pic_context* NewSurfacePicContext(vlc_va_t *va, int surface_index)
>  {
>      directx_sys_t *dx_sys = &va->sys->dx_sys;
> -    return CreatePicContext(dx_sys->hw_surface[surface_index]);
> +    return CreatePicContext(dx_sys->hw_surface[surface_index], false);
>  }

Rather than changing the signature of CreateContext (which really
means creating a context and nothing else) you should modify
NewSurfacePicContext like it's done in d3d11va:

    if (unlikely(pic_ctx==NULL))
        return NULL;

    /* all the resources are acquired during surfaces init, and a second time in
     * CreatePicContext(), undo one of them otherwise we need an extra release
     * when the pool is emptied */
    ReleasePictureSys(&pic_ctx->picsys);
    return pic_ctx;


>  static int Get(vlc_va_t *va, picture_t *pic, uint8_t **data)
> @@ -749,6 +750,8 @@ static void DxDestroyVideoDecoder(vlc_va_t *va)
>      {
>          IDirectXVideoDecoder_Release(dx_sys->decoder);
>          dx_sys->decoder = NULL;
> +        for (unsigned i = 0; i < dx_sys->va_pool.surface_count; i++)
> +            IDirect3DSurface9_Release(dx_sys->hw_surface[i]);
>      }
>  }

Seems right. d3d11va has something equivalent.

By the way, do you have a good way to detect leaks in D3D9 ? In D3D11
there's a flag to debug such things and also one in DXGI. But I didn't
find such a thing for D3D9.

> --
> 2.11.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel