[Mesa-dev] [PATCH] gallium: Add support for frame_cropping_flag of VAEncSequenceParameterBufferH264

[Sahu, Satyajit]

On 4/9/2019 12:34 AM, Liu, Leo wrote:
> On 4/8/19 1:17 AM, Sahu, Satyajit wrote:
>> From: suresh guttula <suresh.guttula at amd.com>
>>
>> This patch will add support for frame_cropping when the input size is not
>> matched with aligned size. Currently vaapi driver ignores frame cropping
>> values provided by client. This change will update SPS nalu with proper
>> cropping values.
>>
>> Signed-off-by: suresh guttula <suresh.guttula at amd.com>
>> Signed-off-by: Satyajit Sahu <satyajit.sahu at amd.com>
>> ---
>>    src/gallium/drivers/radeon/radeon_vce_52.c       | 8 ++++++--
>>    src/gallium/include/pipe/p_video_state.h         | 5 +++++
>>    src/gallium/state_trackers/va/picture_h264_enc.c | 8 ++++++++
>>    3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeon/radeon_vce_52.c b/src/gallium/drivers/radeon/radeon_vce_52.c
>> index fc7ddc62a90..593730756d5 100644
>> --- a/src/gallium/drivers/radeon/radeon_vce_52.c
>> +++ b/src/gallium/drivers/radeon/radeon_vce_52.c
>> @@ -81,8 +81,12 @@ static void get_pic_control_param(struct rvce_encoder *enc, struct pipe_h264_enc
>>    	unsigned encNumMBsPerSlice;
>>    	encNumMBsPerSlice = align(enc->base.width, 16) / 16;
>>    	encNumMBsPerSlice *= align(enc->base.height, 16) / 16;
>> -	enc->enc_pic.pc.enc_crop_right_offset = (align(enc->base.width, 16) - enc->base.width) >> 1;
>> -	enc->enc_pic.pc.enc_crop_bottom_offset = (align(enc->base.height, 16) - enc->base.height) >> 1;
> Shouldn't we need to keep this code path when the cropping info is not
> available, since VA spec says it's optional.
>
> Regards,
>
> Leo

As per H264 standard

When frame_cropping_flag is equal to 0, the values of 
frame_crop_left_offset, frame_crop_right_offset, frame_crop_top_offset, 
and frame_crop_bottom_offset shall be inferred to be equal to 0. 
(https://www.itu.int/rec/T-REC-H.264)

So I think it should be under frame_cropping_flag condition.

Regards,

Satyajit

>
>
>
>> +	if (pic->pic_ctrl.enc_frame_cropping_flag) {
>> +		enc->enc_pic.pc.enc_crop_left_offset = pic->pic_ctrl.enc_frame_crop_left_offset;
>> +		enc->enc_pic.pc.enc_crop_right_offset = pic->pic_ctrl.enc_frame_crop_right_offset;
>> +		enc->enc_pic.pc.enc_crop_top_offset = pic->pic_ctrl.enc_frame_crop_top_offset;
>> +		enc->enc_pic.pc.enc_crop_bottom_offset = pic->pic_ctrl.enc_frame_crop_bottom_offset;
>> +	}
>>    	enc->enc_pic.pc.enc_num_mbs_per_slice = encNumMBsPerSlice;
>>    	enc->enc_pic.pc.enc_b_pic_pattern = MAX2(enc->base.max_references, 1) - 1;
>>    	enc->enc_pic.pc.enc_number_of_reference_frames = MIN2(enc->base.max_references, 2);
>> diff --git a/src/gallium/include/pipe/p_video_state.h b/src/gallium/include/pipe/p_video_state.h
>> index 05855a36e23..1369f1a8ca6 100644
>> --- a/src/gallium/include/pipe/p_video_state.h
>> +++ b/src/gallium/include/pipe/p_video_state.h
>> @@ -395,6 +395,11 @@ struct pipe_h264_enc_pic_control
>>    {
>>       unsigned enc_cabac_enable;
>>       unsigned enc_constraint_set_flags;
>> +   unsigned enc_frame_cropping_flag;
>> +   unsigned enc_frame_crop_left_offset;
>> +   unsigned enc_frame_crop_right_offset;
>> +   unsigned enc_frame_crop_top_offset;
>> +   unsigned enc_frame_crop_bottom_offset;
>>    };
>>    
>>    struct pipe_h264_enc_picture_desc
>> diff --git a/src/gallium/state_trackers/va/picture_h264_enc.c b/src/gallium/state_trackers/va/picture_h264_enc.c
>> index abfd39633de..f46b3425566 100644
>> --- a/src/gallium/state_trackers/va/picture_h264_enc.c
>> +++ b/src/gallium/state_trackers/va/picture_h264_enc.c
>> @@ -127,6 +127,14 @@ vlVaHandleVAEncSequenceParameterBufferTypeH264(vlVaDriver *drv, vlVaContext *con
>>       context->desc.h264enc.rate_ctrl.frame_rate_num = h264->time_scale / 2;
>>       context->desc.h264enc.rate_ctrl.frame_rate_den = h264->num_units_in_tick;
>>       context->desc.h264enc.pic_order_cnt_type = h264->seq_fields.bits.pic_order_cnt_type;
>> +
>> +   if (h264->frame_cropping_flag) {
>> +      context->desc.h264enc.pic_ctrl.enc_frame_cropping_flag = h264->frame_cropping_flag;
>> +      context->desc.h264enc.pic_ctrl.enc_frame_crop_left_offset = h264->frame_crop_left_offset;
>> +      context->desc.h264enc.pic_ctrl.enc_frame_crop_right_offset = h264->frame_crop_right_offset;
>> +      context->desc.h264enc.pic_ctrl.enc_frame_crop_top_offset = h264->frame_crop_top_offset;
>> +      context->desc.h264enc.pic_ctrl.enc_frame_crop_bottom_offset = h264->frame_crop_bottom_offset;
>> +   }
>>       return VA_STATUS_SUCCESS;
>>    }
>>