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

[Mark Thompson]

On 03/02/17 22:44, Aman Gupta wrote:
> On Fri, Feb 3, 2017 at 12:19 PM, Mark Thompson  wrote:
> 
>> On 03/02/17 05:45, wm4 wrote:
>>> On Thu,  2 Feb 2017 09:29:13 -0800
>>> Aman Gupta  wrote:
>>>
 From: Aman Gupta 

 Program terminated with signal SIGSEGV, Segmentation fault.
 opts=opts@entry=0x0, flags=flags@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, , );
  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?
>>
> 
> I saw this when I was using libva master. vaInitialize() was failing in my
> environment (see https://github.com/01org/libva/issues/20) and after the
> failure ffmpeg crashed.
> 
> Here was the output from ffmpeg:
> 
> libva info: VA-API version 0.40.0
> libva info: va_getDriverName() returns 1
> libva error: va_getDriverName() failed with operation
> failed,driver_name=i965
> [AVHWDeviceContext @ 0x1b03d80] Failed to initialise VAAPI connection: 1
> (operation failed).
> Segmentation fault
> 
> And the backtrace:
> 
>   #0  0x00aff8a4 in vaTerminate ()
>   #1  0x00ae50ce in vaapi_device_free (ctx=) at
> libavutil/hwcontext_vaapi.c:882
>   #2  0x00ae1f9e in hwdevice_ctx_free (opaque=,
> data=) at libavutil/hwcontext.c:66
>   #3  0x00ad856f in buffer_replace (src=0x0, dst=0x7fffa26ef1b8) at
> libavutil/buffer.c:119
>   #4  av_buffer_unref (buf=buf@entry=0x7fffa26ef1f8) at
> libavutil/buffer.c:129
>   #5  0x00ae299f in av_hwdevice_ctx_create (pdevice_ref=0x170ac50
> , type=type@entry=AV_HWDEVICE_TYPE_VAAPI, device= out>,
>   opts=opts@entry=0x0, flags=flags@entry=0) at libavutil/hwcontext.c:494
>   #6  0x00400968 in vaapi_device_init (device=) at
> ffmpeg_vaapi.c:223
> 
> Definitely possible that this is a bug in libva instead, and that failure
> midway through vaInitialize() is not dealt with appropriately during
> vaTerminate().
> 
> Feel free to revert the commit.

Can you build libva with debug enabled and clarify exactly how and where it's 
failing there?  From your description on github I'm inclined to think it is 
some bad interaction in libva with running as root, but it would be good to be 
sure.  (And we should revert the change here.)

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Aman Gupta]

On Fri, Feb 3, 2017 at 12:19 PM, Mark Thompson  wrote:

> On 03/02/17 05:45, wm4 wrote:
> > On Thu,  2 Feb 2017 09:29:13 -0800
> > Aman Gupta  wrote:
> >
> >> From: Aman Gupta 
> >>
> >> Program terminated with signal SIGSEGV, Segmentation fault.
> >> opts=opts@entry=0x0, flags=flags@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, , );
> >>  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?
>

I saw this when I was using libva master. vaInitialize() was failing in my
environment (see https://github.com/01org/libva/issues/20) and after the
failure ffmpeg crashed.

Here was the output from ffmpeg:

libva info: VA-API version 0.40.0
libva info: va_getDriverName() returns 1
libva error: va_getDriverName() failed with operation
failed,driver_name=i965
[AVHWDeviceContext @ 0x1b03d80] Failed to initialise VAAPI connection: 1
(operation failed).
Segmentation fault

And the backtrace:

  #0  0x00aff8a4 in vaTerminate ()
  #1  0x00ae50ce in vaapi_device_free (ctx=) at
libavutil/hwcontext_vaapi.c:882
  #2  0x00ae1f9e in hwdevice_ctx_free (opaque=,
data=) at libavutil/hwcontext.c:66
  #3  0x00ad856f in buffer_replace (src=0x0, dst=0x7fffa26ef1b8) at
libavutil/buffer.c:119
  #4  av_buffer_unref (buf=buf@entry=0x7fffa26ef1f8) at
libavutil/buffer.c:129
  #5  0x00ae299f in av_hwdevice_ctx_create (pdevice_ref=0x170ac50
, type=type@entry=AV_HWDEVICE_TYPE_VAAPI, device=,
  opts=opts@entry=0x0, flags=flags@entry=0) at libavutil/hwcontext.c:494
  #6  0x00400968 in vaapi_device_init (device=) at
ffmpeg_vaapi.c:223

Definitely possible that this is a bug in libva instead, and that failure
midway through vaInitialize() is not dealt with appropriately during
vaTerminate().

Feel free to revert the commit.

Aman


> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [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  wrote:
> 
>> From: Aman Gupta 
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> opts=opts@entry=0x0, flags=flags@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, , );
>>  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Thu,  2 Feb 2017 09:29:13 -0800
Aman Gupta  wrote:

> From: Aman Gupta 
> 
> Program terminated with signal SIGSEGV, Segmentation fault.
> opts=opts@entry=0x0, flags=flags@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, , );
>  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Mark Thompson]

On 02/02/17 20:05, Aman Gupta wrote:
> On Thu, Feb 2, 2017 at 9:29 AM, Aman Gupta  wrote:
> 
>> From: Aman Gupta 
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> opts=opts@entry=0x0, flags=flags@entry=0) at libavutil/hwcontext.c:494
>>
> 
> Looks like my editor ate the gdb backtrace I had pasted. Will resubmit with
> the commit message fixed if no one has objects to the diff.

Patch LGTM (oops).  If you want to make a new version with the commit message 
adjusted to your satisfaction then it can be applied immediately.

Thanks,

- Mark

>> ---
>>  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, , );
>>  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);
>>
>> --
>> 2.10.1 (Apple Git-78)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Aman Gupta]

On Thu, Feb 2, 2017 at 9:29 AM, Aman Gupta  wrote:

> From: Aman Gupta 
>
> Program terminated with signal SIGSEGV, Segmentation fault.
> opts=opts@entry=0x0, flags=flags@entry=0) at libavutil/hwcontext.c:494
>

Looks like my editor ate the gdb backtrace I had pasted. Will resubmit with
the commit message fixed if no one has objects to the diff.


> ---
>  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, , );
>  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);
>
> --
> 2.10.1 (Apple Git-78)
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Aman Gupta]

From: Aman Gupta 

Program terminated with signal SIGSEGV, Segmentation fault.
opts=opts@entry=0x0, flags=flags@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, , );
 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);
 
-- 
2.10.1 (Apple Git-78)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel