[FFmpeg-devel] Recent regression in VA-API compatibility (assertion in H.264 encode)

On 01/12/17 16:48, Devin Heitmueller wrote:
> Hello,
> 
> It looks like a recent patch causes VA-API H.264 encode to stop working and an assertion to be thrown.  I ran a git bisect and narrowed it down to the following commit:
> 
> 32a618a948c20f18db102d0b0976790222a57105 is the first bad commit
> commit 32a618a948c20f18db102d0b0976790222a57105
> Author: Mark Thompson <sw@jkqxz.net>
> Date:   Wed Oct 18 19:46:53 2017 +0100
> 
>     vaapi_h264: Do not use deprecated header type
>     
>     SEI headers should be inserted as generic raw data (the old specific
>     type has been deprecated in libva2).
> 
> 
> When run with the above patch, I get the following output:
> 
> [h264_vaapi @ 0x37d0a20] Warning: some packed headers are not supported (want 0xd, got 0xb).
> [h264_vaapi @ 0x37d0a20] The encode compression level option is not supported with this VAAPI version.
> ffmpeg: i965_drv_video.c:352: va_enc_packed_type_to_idx: Assertion `0' failed.
> 
> Here’s the vainfo output which provides the version info for the driver, va-api version, etc.  This is on a Haswell system running Centos 7.
> 
> libva info: VA-API version 0.34.0
> libva info: va_getDriverName() returns 0
> libva info: Trying to open /usr/lib64/dri/i965_drv_video.so
> libva info: Found init function __vaDriverInit_0_34
> libva info: va_openDriver() returns 0
> vainfo: VA-API version: 0.34 (libva 1.2.1)
> vainfo: Driver version: Intel i965 driver - 1.2.2

Upgrading to a version less than four years old might be a plan - I admit we do notionally support that version because of old RHEL/CentOS, but it is not well tested (as you are finding).

> 
> I’m using the following command line for testing:
> 
> ./ffmpeg -y -vaapi_device /dev/dri/card0 -i /home/devin/inputfile.ts -vf 'format=nv12,hwupload' -c:v h264_vaapi out.mp4
> 
> Any suggestions that could be offered would be greatly appreciated.  Likewise please let me know if there is any other information I can provide that would assist in getting this resolved.

Try this?  (Not tested, hardware which can run a version that old isn't immediately to hand.)



(Probably wants a comment pointing at the driver code to explain why that condition, too.)

- Mark

Hi Mark,

>> 
>> Here’s the vainfo output which provides the version info for the driver, va-api version, etc.  This is on a Haswell system running Centos 7.
>> 
>> libva info: VA-API version 0.34.0
>> libva info: va_getDriverName() returns 0
>> libva info: Trying to open /usr/lib64/dri/i965_drv_video.so
>> libva info: Found init function __vaDriverInit_0_34
>> libva info: va_openDriver() returns 0
>> vainfo: VA-API version: 0.34 (libva 1.2.1)
>> vainfo: Driver version: Intel i965 driver - 1.2.2
> 
> Upgrading to a version less than four years old might be a plan - I admit we do notionally support that version because of old RHEL/CentOS, but it is not well tested (as you are finding).

This is actually what you get with the very latest release of Centos 7.4 (downloaded yesterday).  Hence while we could certainly argue that perhaps they’re shipping versions that are too old, it’s not like I’m running some archaic five-year-old copy of Centos I found a DVD for in the bottom of a drawer.  :-)

And don’t misunderstand, I’m not against saying “Centos is dumb and should bundle newer versions of the library/driver/whatever” - I’m just trying to make clear that this is what the experience will be of any non-technical user who just does a binary install from the most recent versions of one of the more popular distros.

> 
> Try this?  (Not tested, hardware which can run a version that old isn't immediately to hand.)
> 
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -261,7 +261,8 @@ static int vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>         return 0;
> 
> #if !CONFIG_VAAPI_1
> -    } else if (priv->sei_cbr_workaround_needed) {
> +    } else if (priv->sei_cbr_workaround_needed &&
> +               ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SLICE) {
>         // Insert a zero-length header using the old SEI type.  This is
>         // required to avoid triggering broken behaviour on Intel platforms
>         // in CBR mode where an invalid SEI message is generated by the
> 

Ok, will give this a try tonight and report back on my findings.

Thanks!

Devin

>> Try this?  (Not tested, hardware which can run a version that old isn't immediately to hand.)
>> 
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -261,7 +261,8 @@ static int vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>>        return 0;
>> 
>> #if !CONFIG_VAAPI_1
>> -    } else if (priv->sei_cbr_workaround_needed) {
>> +    } else if (priv->sei_cbr_workaround_needed &&
>> +               ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SLICE) {
>>        // Insert a zero-length header using the old SEI type.  This is
>>        // required to avoid triggering broken behaviour on Intel platforms
>>        // in CBR mode where an invalid SEI message is generated by the
>> 
> 
> Ok, will give this a try tonight and report back on my findings.

FYI:  this doesn’t appear to have had any effect - I still get the same assert message.

Devin

On 01/12/17 18:47, Devin Heitmueller wrote:
> 
>>> Try this?  (Not tested, hardware which can run a version that old isn't immediately to hand.)
>>>
>>> --- a/libavcodec/vaapi_encode_h264.c
>>> +++ b/libavcodec/vaapi_encode_h264.c
>>> @@ -261,7 +261,8 @@ static int vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>>>        return 0;
>>>
>>> #if !CONFIG_VAAPI_1
>>> -    } else if (priv->sei_cbr_workaround_needed) {
>>> +    } else if (priv->sei_cbr_workaround_needed &&
>>> +               ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SLICE) {
>>>        // Insert a zero-length header using the old SEI type.  This is
>>>        // required to avoid triggering broken behaviour on Intel platforms
>>>        // in CBR mode where an invalid SEI message is generated by the
>>>
>>
>> Ok, will give this a try tonight and report back on my findings.
> 
> FYI:  this doesn’t appear to have had any effect - I still get the same assert message.

Urgh, right.  It's the SEI it doesn't like rather than the workaround.

"-sei 0" will make it work, I think?  Something further would likely need a specific test for old versions of the Intel driver.

- Mark

On 01/12/17 18:37, Devin Heitmueller wrote:
> Hi Mark,
> 
>>>
>>> Here’s the vainfo output which provides the version info for the driver, va-api version, etc.  This is on a Haswell system running Centos 7.
>>>
>>> libva info: VA-API version 0.34.0
>>> libva info: va_getDriverName() returns 0
>>> libva info: Trying to open /usr/lib64/dri/i965_drv_video.so
>>> libva info: Found init function __vaDriverInit_0_34
>>> libva info: va_openDriver() returns 0
>>> vainfo: VA-API version: 0.34 (libva 1.2.1)
>>> vainfo: Driver version: Intel i965 driver - 1.2.2
>>
>> Upgrading to a version less than four years old might be a plan - I admit we do notionally support that version because of old RHEL/CentOS, but it is not well tested (as you are finding).
> 
> This is actually what you get with the very latest release of Centos 7.4 (downloaded yesterday).  Hence while we could certainly argue that perhaps they’re shipping versions that are too old, it’s not like I’m running some archaic five-year-old copy of Centos I found a DVD for in the bottom of a drawer.  :-)

Yeah, I'm aware that it is still current despite the archaic packages - if it weren't we would probably have dropped it already, at least for encode.

> And don’t misunderstand, I’m not against saying “Centos is dumb and should bundle newer versions of the library/driver/whatever” - I’m just trying to make clear that this is what the experience will be of any non-technical user who just does a binary install from the most recent versions of one of the more popular distros.

I'm not sure that's quite accurate - if you install on current hardware it will refuse to run at all because that version of the driver doesn't support anything newer than Haswell :P

But yes, it should definitely be fixed somehow.  At some point that "fix" is to drop support for it, though.

- Mark