[FFmpeg-devel] [PATCH] lavc/vaapi_encode: fix PoC negative issue

[Mark Thompson]

On 23/02/17 00:44, Jun Zhao wrote:
> Ping?  I know the commit "eefa4b" give a fix, but I think this one more better for this issue :)

Would you care to offer any reasoning for why you think this is better?


> On 2017/2/8 15:39, Jun Zhao wrote:
>> From e37b2598d372b790c0a496c7b750802a1aa102be Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <jun.zhao at intel.com>
>> Date: Wed, 8 Feb 2017 15:25:18 +0800
>> Subject: [PATCH] lavc/vaapi_encode: fix PoC negative issue.
>> 
>> the issue occurs when setting a large b frames number with option "bf",
>> it results from hard coding the sps.log2_max_pic_order_cnt_lsb_minus4 with
>> 0/8 in h264/hevc vaapi encoder.
>> 
>> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
>> Signed-off-by: Yi A Wang <yi.a.wang at intel.com>
>> ---
>>  libavcodec/vaapi_encode_h264.c | 7 +++++++
>>  libavcodec/vaapi_encode_h265.c | 7 ++++++-
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 00d8e6a..946f8b9 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -784,6 +784,8 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>      VAAPIEncodeH264Context            *priv = ctx->priv_data;
>>      VAAPIEncodeH264MiscSequenceParams *mseq = &priv->misc_sequence_params;
>>      int i;
>> +    int max_delta_poc;
>> +    int log2_max_poc_lsb = 4;
>>  
>>      {
>>          vseq->seq_parameter_set_id = 0;
>> @@ -801,6 +803,11 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>          vseq->seq_fields.bits.log2_max_frame_num_minus4 = 4;
>>          vseq->seq_fields.bits.pic_order_cnt_type = 0;
>>  
>> +        max_delta_poc = (avctx->max_b_frames + 2) * 2;
>> +        while ((1 << log2_max_poc_lsb) <= max_delta_poc * 2)
>> +            log2_max_poc_lsb++;
>> +        vseq->seq_fields.bits.log2_max_pic_order_cnt_lsb_minus4 = log2_max_poc_lsb - 4;
>> +

I prefer the closed form used in the existing fix.

>>          if (avctx->width  != ctx->surface_width ||
>>              avctx->height != ctx->surface_height) {
>>              vseq->frame_cropping_flag = 1;
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index c5589ed..2fb8a3f 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> @@ -786,6 +786,8 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>      VAAPIEncodeH265Context            *priv = ctx->priv_data;
>>      VAAPIEncodeH265MiscSequenceParams *mseq = &priv->misc_sequence_params;
>>      int i;
>> +    int max_delta_poc;
>> +    int log2_max_poc_lsb = 8;
>>  
>>      {
>>          // general_profile_space == 0.
>> @@ -895,7 +897,10 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>          mseq->general_frame_only_constraint_flag = 1;
>>          mseq->general_inbld_flag = 0;
>>  
>> -        mseq->log2_max_pic_order_cnt_lsb_minus4 = 8;
>> +        max_delta_poc = (avctx->max_b_frames + 2) * 2;
>> +        while ((1 << log2_max_poc_lsb) <= max_delta_poc)
>> +            log2_max_poc_lsb++;
>> +        mseq->log2_max_pic_order_cnt_lsb_minus4 = log2_max_poc_lsb - 4;

Right, I didn't consider that H.265 might have the same problem.

Can H.265 actually have the >2^7 frame delay necessary to exceed the current bounds?

This is probably sensible anyway to save a few bits in the slice header, when suitably modified to not be bounded below at 8.  I would prefer the closed form rather than this loop, though.

>>          mseq->vps_sub_layer_ordering_info_present_flag = 0;
>>          mseq->vps_max_dec_pic_buffering_minus1[0] = (avctx->max_b_frames > 0) + 1;
>>          mseq->vps_max_num_reorder_pics[0]         = (avctx->max_b_frames > 0);
>> -- 
>> 2.9.3
>> 

Thanks,

- Mark