[FFmpeg-devel] [PATCH] avutil/hwcontext_vaapi: fix SEGV in vaTerminate when vaInitialize fails

[Mark Thompson]

On 03/02/17 05:45, wm4 wrote:
> On Thu,  2 Feb 2017 09:29:13 -0800
> Aman Gupta <ffmpeg at tmm1.net> wrote:
> 
>> From: Aman Gupta <aman at tmm1.net>
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>>     opts=opts at entry=0x0, flags=flags at entry=0) at libavutil/hwcontext.c:494
>> ---
>>  libavutil/hwcontext_vaapi.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
>> index 6176bdc..0051acb 100644
>> --- a/libavutil/hwcontext_vaapi.c
>> +++ b/libavutil/hwcontext_vaapi.c
>> @@ -961,14 +961,13 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device,
>>          return AVERROR(EINVAL);
>>      }
>>  
>> -    hwctx->display = display;
>> -
>>      vas = vaInitialize(display, &major, &minor);
>>      if (vas != VA_STATUS_SUCCESS) {
>>          av_log(ctx, AV_LOG_ERROR, "Failed to initialise VAAPI "
>>                 "connection: %d (%s).\n", vas, vaErrorStr(vas));
>>          return AVERROR(EIO);
>>      }
>> +    hwctx->display = display;
>>      av_log(ctx, AV_LOG_VERBOSE, "Initialised VAAPI connection: "
>>             "version %d.%d\n", major, minor);
>>  
> 
> Would that mean it doesn't free the display that was created with
> vaGetDisplay? Is that right?
> 
> In my experiments, calling vaTerminate right after vaGetDisplay works
> just fine.

Right, looking more carefully at libva that is exactly what you are meant to do, and the code there is careful to make it all work.  The segfault case I was thinking of here isn't exactly the same (and used the Intel proprietary driver, which should probably be considered dubious), so applying it was premature.

Aman, can you explain more about the case you saw this in?

- Mark