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

[Oliver Collyer]

> On 30 Jun 2017, at 07:57, Steve Lhomme <robux4 at gmail.com> wrote:
> 
> On Thu, Jun 29, 2017 at 9:28 PM, Oliver Collyer <ovcollyer at mac.com <mailto: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;
> 
> 

Great, I've sent a new patch.

>> 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.
> 

Unfortunately I couldn't find a way to do this.

I had to resort to old-school "printf" style debugging in dxva2 and studying the trace :)

>> --
>> 2.11.0
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel <https://mailman.videolan.org/listinfo/vlc-devel>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170630/879d2764/attachment.html>