On 21/12/16 04:46, Xiang, Haihao wrote:
> On Mon, 2016-12-19 at 23:01 +0000, Mark Thompson wrote:
>> Also adds support for fractional framerate.
>>
>> Signed-off-by: Mark Thompson <s...@jkqxz.net>
>> ---
>>  src/gen9_vp9_encoder.c | 240 
>> ++++++++-----------------------------------------
>>  src/gen9_vp9_encoder.h |  10 +--
>>  src/i965_drv_video.c   |  10 +--
>>  src/i965_encoder.c     |  36 ++++++++
>>  src/i965_encoder.h     |   1 +
>>  5 files changed, 77 insertions(+), 220 deletions(-)
>> ...
>>              /* check the corresponding BRC parameter for CBR and VBR */
>>              if (encoder_context->rate_control_mode == VA_RC_CBR) {
>> -                vp9_state->target_bit_rate = seq_param->bits_per_second;
>> -                vp9_state->gop_size = seq_param->intra_period;
>> -
>> -                if (vp9_state->brc_flag_check & VP9_BRC_HRD) {
>> -                    VAEncMiscParameterHRD *misc_param_hrd;
>> -
>> -                    misc_param = (VAEncMiscParameterBuffer *)
>> -                        
>> encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer;
>> -                    misc_param_hrd = (VAEncMiscParameterHRD 
>> *)misc_param->data;
>> -
>> -                    vp9_state->init_vbv_buffer_fullness_in_bit = 
>> misc_param_hrd->initial_buffer_fullness;
>> -                    vp9_state->vbv_buffer_size_in_bit = 
>> misc_param_hrd->buffer_size;
>> -                }
>> -
>> -                if (vp9_state->brc_flag_check & VP9_BRC_FR) {
>> -                    VAEncMiscParameterFrameRate *misc_param_fr;
>> -
>> -                    misc_param = (VAEncMiscParameterBuffer *)
>> -                        
>> encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
>> -                    misc_param_fr = (VAEncMiscParameterFrameRate 
>> *)misc_param->data;
>> -
>> -                    vp9_state->frame_rate = misc_param_fr->framerate;
>> -                } else {
>> -                    /* Assign the default frame rate */
>> -                    vp9_state->frame_rate = 30;
>> -                }
>> -
>> -                /* RC misc will override HRD parameter */
>> -                if (vp9_state->brc_flag_check & VP9_BRC_RC) {
>> -                    VAEncMiscParameterRateControl *misc_param_rc;
>> -
>> -                    misc_param = (VAEncMiscParameterBuffer *)
>> -                        
>> encode_state->misc_param[VAEncMiscParameterTypeRateControl][0]->buffer;
>> -                    misc_param_rc = (VAEncMiscParameterRateControl 
>> *)misc_param->data;
>> -
>> -                    vp9_state->target_bit_rate = 
>> misc_param_rc->bits_per_second;
>> -                    vp9_state->vbv_buffer_size_in_bit = 
>> (misc_param_rc->bits_per_second / 1000) *
>> -                                                 misc_param_rc->window_size;
>> -                    vp9_state->init_vbv_buffer_fullness_in_bit = 
>> vp9_state->vbv_buffer_size_in_bit / 2;
>> -                    vp9_state->window_size = misc_param_rc->window_size;
>> -                }
>> +                if (!encoder_context->brc.framerate[0].num || 
>> !encoder_context->brc.framerate[0].den ||
>> +                    !encoder_context->brc.bits_per_second[0])
>> +                    return VA_STATUS_ERROR_INVALID_PARAMETER;
>> +
>> +                vp9_state->gop_size = encoder_context->brc.gop_size;
>> +
>> +                vp9_state->framerate = encoder_context->brc.framerate[0];
>> +                vp9_state->target_bit_rate = 
>> encoder_context->brc.bits_per_second[0];
>>                  vp9_state->max_bit_rate = vp9_state->target_bit_rate;
>>                  vp9_state->min_bit_rate = vp9_state->target_bit_rate;
>> -            } else {
>> -                /* VBR mode */
>> -                brc_flag = VP9_BRC_SEQ | VP9_BRC_RC;
>> -                vp9_state->target_bit_rate = seq_param->bits_per_second;
>> -                vp9_state->gop_size = seq_param->intra_period;
>> -
>> -                if (vp9_state->brc_flag_check & VP9_BRC_FR) {
>> -                    VAEncMiscParameterFrameRate *misc_param_fr;
>> -
>> -                    misc_param = (VAEncMiscParameterBuffer *)
>> -                        
>> encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
>> -                    misc_param_fr = (VAEncMiscParameterFrameRate 
>> *)misc_param->data;
>> -
>> -                    vp9_state->frame_rate = misc_param_fr->framerate;
>> -                } else {
>> -                    /* Assign the default frame rate */
>> -                    vp9_state->frame_rate = 30;
>> -                }
>> -
>> -                if (vp9_state->brc_flag_check & VP9_BRC_RC) {
>> -                    VAEncMiscParameterRateControl *misc_param_rc;
>> -
>> -                    misc_param = (VAEncMiscParameterBuffer *)
>> -                        
>> encode_state->misc_param[VAEncMiscParameterTypeRateControl][0]->buffer;
>> -                    misc_param_rc = (VAEncMiscParameterRateControl 
>> *)misc_param->data;
>> -
>> -                    vp9_state->max_bit_rate = 
>> misc_param_rc->bits_per_second;
>> -                    vp9_state->vbv_buffer_size_in_bit = 
>> (misc_param_rc->bits_per_second / 1000) *
>> -                                                 misc_param_rc->window_size;
>> -                    vp9_state->init_vbv_buffer_fullness_in_bit = 
>> vp9_state->vbv_buffer_size_in_bit / 2;
>> -                    vp9_state->target_bit_rate = 
>> (misc_param_rc->bits_per_second / 100) *
>> -                                misc_param_rc->target_percentage;
>> -                    vp9_state->min_bit_rate = 
>> (misc_param_rc->bits_per_second / 100) *
>> -                         (2 * misc_param_rc->target_percentage - 100);
>> -                    vp9_state->target_percentage = 
>> misc_param_rc->target_percentage;
>> -                    vp9_state->window_size = misc_param_rc->window_size;
>> -                }
>> -            }
>> -        }
>> -        else if (vp9_state->picture_coding_type == KEY_FRAME){
>> -            VAEncMiscParameterBuffer *misc_param;
>> -            /* update the BRC parameter only when it is key-frame */
>> -            /* If the parameter related with RC is changed. Reset BRC */
>> -            if (vp9_state->brc_flag_check & VP9_BRC_FR) {
>> -               VAEncMiscParameterFrameRate *misc_param_fr;
>> -
>> -               misc_param = (VAEncMiscParameterBuffer *)
>> -                   
>> encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
>> -               misc_param_fr = (VAEncMiscParameterFrameRate 
>> *)misc_param->data;
>> -
>> -               if (vp9_state->frame_rate != misc_param_fr->framerate) {
>> -                   vp9_state->brc_reset = 1;
>> -                   vp9_state->frame_rate = misc_param_fr->framerate;
>> -               }
>> -            }
>>  
>> -            /* check the GOP size. And bit_per_second in SPS is ignored */
>> -            if (vp9_state->brc_flag_check & VP9_BRC_SEQ) {
>> -                if (vp9_state->gop_size != seq_param->intra_period) {
>> -                    vp9_state->brc_reset = 1;
>> -                    vp9_state->gop_size = seq_param->intra_period;
>> -                }
>> -            }
>> +                vp9_state->vbv_buffer_size_in_bit = 
>> encoder_context->brc.hrd_buffer_size;
>> +                vp9_state->init_vbv_buffer_fullness_in_bit = 
>> encoder_context->brc.hrd_initial_buffer_fullness;
> 
> vp9_state->init_vbv_buffer_fullness_in_bit may be overrode by the misc
> RC parameters (vp9_state->window_size) in the current code.

I think we want to ignore window_size in CBR mode?  The bitrate is intended to 
be constant, so the window to average over is necessarily zero.  The relevant 
parts (to my mind) of the RC parameter structure (really just bits_per_second) 
are already reflected in the generic code.

I'm happy to change it if you feel the window_size should be used there.  
Though, I would ask which one "wins" if both HRD parameters and window_size are 
set?

>>  
>> -            /* update the bit_per_second */
>> -            if (vp9_state->brc_flag_check & VP9_BRC_RC) {
>> -                VAEncMiscParameterRateControl *misc_param_rc;
>> -
>> -                misc_param = (VAEncMiscParameterBuffer *)
>> -                    
>> encode_state->misc_param[VAEncMiscParameterTypeRateControl][0]->buffer;
>> -                misc_param_rc = (VAEncMiscParameterRateControl 
>> *)misc_param->data;
>> -
>> -                if (encoder_context->rate_control_mode == VA_RC_CBR) {
>> -                    if (vp9_state->target_bit_rate != 
>> misc_param_rc->bits_per_second ||
>> -                        vp9_state->window_size != 
>> misc_param_rc->window_size) {
>> -                        vp9_state->target_bit_rate = 
>> misc_param_rc->bits_per_second;
>> -                        vp9_state->vbv_buffer_size_in_bit = 
>> (misc_param_rc->bits_per_second / 1000) *
>> -                                                 misc_param_rc->window_size;
>> -                        vp9_state->init_vbv_buffer_fullness_in_bit = 
>> vp9_state->vbv_buffer_size_in_bit * 2;
>> -                        vp9_state->window_size = misc_param_rc->window_size;
>> -                        vp9_state->max_bit_rate = 
>> vp9_state->target_bit_rate;
>> -                        vp9_state->min_bit_rate = 
>> vp9_state->target_bit_rate;
>> -                        vp9_state->brc_reset = 1;
>> -                    }
>> -                } else {
>> -                    /* VBR mode */
>> -                    if (vp9_state->max_bit_rate != 
>> misc_param_rc->bits_per_second ||
>> -                        vp9_state->target_percentage != 
>> misc_param_rc->target_percentage) {
>> -
>> -                        vp9_state->target_bit_rate = 
>> (misc_param_rc->bits_per_second / 100) *
>> -                                misc_param_rc->target_percentage;
>> -                        vp9_state->min_bit_rate = 
>> (misc_param_rc->bits_per_second / 100) *
>> -                             (2 * misc_param_rc->target_percentage - 100);
>> -                        vp9_state->max_bit_rate = 
>> misc_param_rc->bits_per_second;
>> -                        vp9_state->vbv_buffer_size_in_bit = 
>> (misc_param_rc->bits_per_second / 1000) *
>> -                                                 misc_param_rc->window_size;
>> -                        vp9_state->init_vbv_buffer_fullness_in_bit = 
>> vp9_state->vbv_buffer_size_in_bit / 2;
>> -                        vp9_state->target_percentage = 
>> misc_param_rc->target_percentage;
>> -                        vp9_state->window_size = misc_param_rc->window_size;
>> -                        vp9_state->brc_reset = 1;
>> -                    }
>> -                }
>> +            } else {
>> +                /* VBR mode */
>> +                if (!encoder_context->brc.framerate[0].num || 
>> !encoder_context->brc.framerate[0].den ||
>> +                    !encoder_context->brc.bits_per_second[0] || 
>> !encoder_context->brc.target_percentage[0] ||
>> +                    !encoder_context->brc.window_size)
>> +                    return VA_STATUS_ERROR_INVALID_PARAMETER;
>> +
>> +                vp9_state->gop_size = encoder_context->brc.gop_size;
>> +
>> +                vp9_state->framerate = encoder_context->brc.framerate[0];
>> +                vp9_state->max_bit_rate = 
>> encoder_context->brc.bits_per_second[0];
>> +                vp9_state->target_percentage = 
>> encoder_context->brc.target_percentage[0];
>> +                vp9_state->window_size = encoder_context->brc.window_size;
>> +
>> +                vp9_state->target_bit_rate = vp9_state->max_bit_rate * 
>> encoder_context->brc.target_percentage[0] / 100;
>> +                if (2 * vp9_state->target_bit_rate < 
>> vp9_state->max_bit_rate)
>> +                    vp9_state->min_bit_rate = 0;
>> +                else
>> +                    vp9_state->min_bit_rate = 2 * 
>> vp9_state->target_bit_rate - vp9_state->max_bit_rate;
>> +
>> +                vp9_state->vbv_buffer_size_in_bit = 
>> (vp9_state->max_bit_rate / 1000) * vp9_state->window_size;
>> +                vp9_state->init_vbv_buffer_fullness_in_bit = 
>> vp9_state->vbv_buffer_size_in_bit / 2;
> 
> This patch uses two different ways to get vp9_state->vbv_buffer_size_in_bit 
> for CBR and VBR. 
> I think we should use the same way. I prefer the way used for CBR in your 
> patch although it is not the same of the current code.

The VBR mode behaviour was intended to be the same as what is there currently, 
mainly because it is harder for me to verify the result than with CBR.  The 
current code for VBR ignores the HRD parameters and instead generates VBV 
values from the RC parameters, and I have preserved that behaviour.

I can change it to use the HRD parameters if they are set?  Or would you prefer 
that the two code paths are merged, with a branch on CBR vs. VBR only around 
the setting of min|max_bit_rate?

Thanks,

- Mark

_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva

Reply via email to