[vlc-devel] [PATCH] DxVA2: lock the buffer pool when getting/releasing a buffer

[Steve Lhomme]

On Thu, Apr 2, 2015 at 3:36 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le 2015-04-02 16:33, Steve Lhomme a écrit :
>>
>> in frame threading mode, the buffers can be requested from different
>> threads, resulting in a race condition
>>
>> Doesn't fix #11693
>> ---
>>  modules/codec/avcodec/dxva2.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/modules/codec/avcodec/dxva2.c b/modules/codec/avcodec/dxva2.c
>> index f4289ee..76a1ebb 100644
>> --- a/modules/codec/avcodec/dxva2.c
>> +++ b/modules/codec/avcodec/dxva2.c
>> @@ -265,6 +265,7 @@ typedef struct {
>>      LPDIRECT3DSURFACE9 d3d;
>>      int                refcount;
>>      unsigned int       order;
>> +    vlc_va_sys_t       *sys;
>>  } vlc_va_surface_t;
>>
>>  #define VA_DXVA2_MAX_SURFACE_COUNT (64)
>> @@ -274,6 +275,8 @@ struct vlc_va_sys_t
>>      int          width;
>>      int          height;
>>
>> +    vlc_mutex_t     surface_lock;
>> +
>>      /* DLL */
>>      HINSTANCE             hd3d9_dll;
>>      HINSTANCE             hdxva2_dll;
>> @@ -468,6 +471,8 @@ static int Get(vlc_va_t *va, void **opaque,
>> uint8_t **data)
>>          return VLC_EGENERIC;
>>      }
>>
>> +    vlc_mutex_lock( &sys->surface_lock );
>> +
>>      /* Grab an unused surface, in case none are, try the oldest
>>       * XXX using the oldest is a workaround in case a problem
>> happens with libavcodec */
>>      unsigned i, old;
>> @@ -489,6 +494,9 @@ static int Get(vlc_va_t *va, void **opaque,
>> uint8_t **data)
>>      surface->order = sys->surface_order++;
>>      *data = (void *)surface->d3d;
>>      *opaque = surface;
>> +
>> +    vlc_mutex_unlock( &sys->surface_lock );
>> +
>>      return VLC_SUCCESS;
>>  }
>>
>> @@ -496,8 +504,12 @@ static void Release(void *opaque, uint8_t *data)
>>  {
>>      vlc_va_surface_t *surface = opaque;
>>
>> +    vlc_mutex_lock( &surface->sys->surface_lock );
>> +
>>      surface->refcount--;
>>      (void) data;
>> +
>> +    vlc_mutex_unlock( &surface->sys->surface_lock );
>
>
> This could use atomic or interlocked operations.

The mutex in Get() protects the loop to look for the proper buffer to
use. So not sure this would be possible.

>>  }
>>
>>  static void Close(vlc_va_t *va, AVCodecContext *ctx)
>> @@ -515,6 +527,7 @@ static void Close(vlc_va_t *va, AVCodecContext *ctx)
>>          FreeLibrary(sys->hdxva2_dll);
>>      if (sys->hd3d9_dll)
>>          FreeLibrary(sys->hd3d9_dll);
>> +    vlc_mutex_destroy( &sys->surface_lock );
>>
>>      free((char *)va->description);
>>      free(sys);
>> @@ -530,6 +543,8 @@ static int Open(vlc_va_t *va, AVCodecContext
>> *ctx, const es_format_t *fmt)
>>      sys->codec_id = ctx->codec_id;
>>      (void) fmt;
>>
>> +    vlc_mutex_init( &sys->surface_lock );
>> +
>>      /* Load dll*/
>>      sys->hd3d9_dll = LoadLibrary(TEXT("D3D9.DLL"));
>>      if (!sys->hd3d9_dll) {
>> @@ -927,6 +942,7 @@ static int DxCreateVideoDecoder(vlc_va_t *va,
>>          surface->d3d = sys->hw_surface[i];
>>          surface->refcount = 0;
>>          surface->order = 0;
>> +        surface->sys = sys;
>>      }
>>      msg_Dbg(va, "IDirectXVideoAccelerationService_CreateSurface
>> succeed with %d surfaces (%dx%d)",
>>              sys->surface_count, fmt->i_width, fmt->i_height);
>
>
> --
> Rémi Denis-Courmont
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel