diff mbox

[FFmpeg-devel] lavc/vaap_encode_h265: Init the refs with predefined value.

Message ID c32340e3-a674-9387-d9cf-2422b5104386@gmail.com
State New
Headers show

Commit Message

Jun Zhao Dec. 6, 2017, 8:20 a.m. UTC 
From 7519718216a2b98eb289ba20ba27685dff1cf686 Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>

Date: Wed, 6 Dec 2017 15:50:05 +0800
Subject: [PATCH] lavc/vaap_encode_h265: Init the refs with predefined value.

Give a explicit init value to ref0/ref1 list to avoid use default value
assgined by compiler, in iHD driver, it's will lead to HEVC ENC crash.
h264_vaapi encoder use this way too.

Signed-off-by: Jun Zhao <jun.zhao@intel.com>

---
 libavcodec/vaapi_encode_h265.c | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.14.1

Comments

Mark Thompson Dec. 6, 2017, 9:32 p.m. UTC | #1 
On 06/12/17 08:20, Jun Zhao wrote:
> 
> From 7519718216a2b98eb289ba20ba27685dff1cf686 Mon Sep 17 00:00:00 2001

> From: Jun Zhao <jun.zhao@intel.com>

> Date: Wed, 6 Dec 2017 15:50:05 +0800

> Subject: [PATCH] lavc/vaap_encode_h265: Init the refs with predefined value.

> 

> Give a explicit init value to ref0/ref1 list to avoid use default value

> assgined by compiler, in iHD driver, it's will lead to HEVC ENC crash.

> h264_vaapi encoder use this way too.

> 

> Signed-off-by: Jun Zhao <jun.zhao@intel.com>

> ---

>  libavcodec/vaapi_encode_h265.c | 6 ++++++

>  1 file changed, 6 insertions(+)

> 

> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c

> index 2fc94be632..db9a7163cc 100644

> --- a/libavcodec/vaapi_encode_h265.c

> +++ b/libavcodec/vaapi_encode_h265.c

> @@ -757,6 +757,12 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,

>      else

>          sh->slice_qp_delta = priv->fixed_qp_idr - (pps->init_qp_minus26 + 26);

>  

> +    for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {

> +        vslice->ref_pic_list0[i].picture_id = VA_INVALID_ID;

> +        vslice->ref_pic_list0[i].flags      = VA_PICTURE_HEVC_INVALID;

> +        vslice->ref_pic_list1[i].picture_id = VA_INVALID_ID;

> +        vslice->ref_pic_list1[i].flags      = VA_PICTURE_HEVC_INVALID;

> +    }

>  

>      *vslice = (VAEncSliceParameterBufferHEVC) {

>          .slice_segment_address = sh->slice_segment_address,

> -- 

> 2.14.1

> 


Um, this doesn't appear to do anything?

I haven't tried the new driver release yet, but I intend to soon.

- Mark
Mark Thompson Dec. 7, 2017, 12:29 a.m. UTC | #2 
On 06/12/17 21:32, Mark Thompson wrote:
> On 06/12/17 08:20, Jun Zhao wrote:
>> From 7519718216a2b98eb289ba20ba27685dff1cf686 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <jun.zhao@intel.com>
>> Date: Wed, 6 Dec 2017 15:50:05 +0800
>> Subject: [PATCH] lavc/vaap_encode_h265: Init the refs with predefined value.
>>
>> Give a explicit init value to ref0/ref1 list to avoid use default value
>> assgined by compiler, in iHD driver, it's will lead to HEVC ENC crash.
>> h264_vaapi encoder use this way too.
>>
>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
>> ---
>>  libavcodec/vaapi_encode_h265.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index 2fc94be632..db9a7163cc 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> @@ -757,6 +757,12 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
>>      else
>>          sh->slice_qp_delta = priv->fixed_qp_idr - (pps->init_qp_minus26 + 26);
>>  
>> +    for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {
>> +        vslice->ref_pic_list0[i].picture_id = VA_INVALID_ID;
>> +        vslice->ref_pic_list0[i].flags      = VA_PICTURE_HEVC_INVALID;
>> +        vslice->ref_pic_list1[i].picture_id = VA_INVALID_ID;
>> +        vslice->ref_pic_list1[i].flags      = VA_PICTURE_HEVC_INVALID;
>> +    }
>>  
>>      *vslice = (VAEncSliceParameterBufferHEVC) {
>>          .slice_segment_address = sh->slice_segment_address,
>> -- 
>> 2.14.1
>>
> 
> 
> Um, this doesn't appear to do anything?
> 
> I haven't tried the new driver release yet, but I intend to soon.

Well, I had a look.  Default build fails because of <https://github.com/intel/media-driver/issues/4>, but with that hacked around it does work for me on Skylake.

I made <https://github.com/intel/media-driver/pull/14> to add support for the MEM_TYPE_VA so that surface creation in ffmpeg can work without hacks.  With that, decode works for H.26[45] and VC-1, though VP8 hangs the GPU pretty much immediately (didn't investigate further).

For the surface attribute query stuff, I think it would probably be best just to implement it in the driver rather than adding further hacks to ffmpeg?  With the open-source version being released I don't think there is any reason for anyone to use the old binary-only form of it with VAAPI any more, so we might as well fix it properly.

Thanks,

- Mark
Jun Zhao Dec. 7, 2017, 12:41 a.m. UTC | #3 
On 2017/12/7 8:29, Mark Thompson wrote:
> On 06/12/17 21:32, Mark Thompson wrote:
>> On 06/12/17 08:20, Jun Zhao wrote:
>>> From 7519718216a2b98eb289ba20ba27685dff1cf686 Mon Sep 17 00:00:00 2001
>>> From: Jun Zhao <jun.zhao@intel.com>
>>> Date: Wed, 6 Dec 2017 15:50:05 +0800
>>> Subject: [PATCH] lavc/vaap_encode_h265: Init the refs with predefined value.
>>>
>>> Give a explicit init value to ref0/ref1 list to avoid use default value
>>> assgined by compiler, in iHD driver, it's will lead to HEVC ENC crash.
>>> h264_vaapi encoder use this way too.
>>>
>>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
>>> ---
>>>  libavcodec/vaapi_encode_h265.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>>> index 2fc94be632..db9a7163cc 100644
>>> --- a/libavcodec/vaapi_encode_h265.c
>>> +++ b/libavcodec/vaapi_encode_h265.c
>>> @@ -757,6 +757,12 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
>>>      else
>>>          sh->slice_qp_delta = priv->fixed_qp_idr - (pps->init_qp_minus26 + 26);
>>>  
>>> +    for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {
>>> +        vslice->ref_pic_list0[i].picture_id = VA_INVALID_ID;
>>> +        vslice->ref_pic_list0[i].flags      = VA_PICTURE_HEVC_INVALID;
>>> +        vslice->ref_pic_list1[i].picture_id = VA_INVALID_ID;
>>> +        vslice->ref_pic_list1[i].flags      = VA_PICTURE_HEVC_INVALID;
>>> +    }
>>>  
>>>      *vslice = (VAEncSliceParameterBufferHEVC) {
>>>          .slice_segment_address = sh->slice_segment_address,
>>> -- 
>>> 2.14.1
>>>
>>
>> Um, this doesn't appear to do anything?
>>
>> I haven't tried the new driver release yet, but I intend to soon.
> Well, I had a look.  Default build fails because of <https://github.com/intel/media-driver/issues/4>, but with that hacked around it does work for me on Skylake.
>
> I made <https://github.com/intel/media-driver/pull/14> to add support for the MEM_TYPE_VA so that surface creation in ffmpeg can work without hacks.  With that, decode works for H.26[45] and VC-1, though VP8 hangs the GPU pretty much immediately (didn't investigate further).
>
> For the surface attribute query stuff, I think it would probably be best just to implement it in the driver rather than adding further hacks to ffmpeg?  With the open-source version being released I don't think there is any reason for anyone to use the old binary-only form of it with VAAPI any more, so we might as well fix it properly.
>
> Thanks,
I agree need to add a fix in iHD open source driver, but I think FFmpeg 
AP-API need to add a explicit init for this.

BTW: if you want run FATE for H264/HEVC decoder, I think need to merge
the patch https://github.com/intel/media-driver/pull/10.

I will check VP8 DEC crash issue. Thanks.
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 2fc94be632..db9a7163cc 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -757,6 +757,12 @@  static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
     else
         sh->slice_qp_delta = priv->fixed_qp_idr - (pps->init_qp_minus26 + 26);
 
+    for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {
+        vslice->ref_pic_list0[i].picture_id = VA_INVALID_ID;
+        vslice->ref_pic_list0[i].flags      = VA_PICTURE_HEVC_INVALID;
+        vslice->ref_pic_list1[i].picture_id = VA_INVALID_ID;
+        vslice->ref_pic_list1[i].flags      = VA_PICTURE_HEVC_INVALID;
+    }
 
     *vslice = (VAEncSliceParameterBufferHEVC) {
         .slice_segment_address = sh->slice_segment_address,