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

[Steve Lhomme]

On Fri, Jun 30, 2017 at 6:57 AM, Steve Lhomme <robux4 at gmail.com> wrote:
> 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.

Because in d3d11va the callback name is DxDestroySurfaces which is
better for a callback named pf_destroy_surfaces().

> 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