[Libva] [PATCH 4/4] VP9 encoder: use generic rate control parameters

Xiang, Haihao haihao.xiang at intel.com
Thu Dec 22 02:11:23 UTC 2016


> 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 <sw at 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?

Agree, we should use the HRD parameters.


> 
> > >  
> > > -            /* 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?

User should provide the HRD parameters to the driver. The problem with
the current implementation for VP9 is user can't change the buffer
fullness any more if the misc RC parameters are provided

> 
> Thanks,
> 
> - Mark
> 


More information about the Libva mailing list