[Libva] [PATCH 1/4] i965_encoder: consistently represent framerate as a fraction

Xiang, Haihao haihao.xiang at intel.com
Wed Dec 21 04:21:28 UTC 2016


> Update references in both H.264 encoders (gen6_mfc and gen9_vdenc).
> 
> Signed-off-by: Mark Thompson <sw at jkqxz.net>
> ---
> New version.
> 
> Changes for the whole series:
> * Use a single field for framerate (adding a new struct to represent
> it), as recommended by Haihao.
> * Fix some missed cases where bitrate was read from the initial
> sequence parameters rather than the most recent RC parameters.
> 
> The new struct is called "struct intel_fraction" to be consistent
> with the namespacing in the rest of that file, it could go somewhere
> else if that is preferred.  Relatedly, the code might be nicer with
> some more convenience functions around it (notably comparison and
> fraction -> double, which are both used in a number of places), but
> I'm not sure where they would go or what they would be called so I
> have not done this.
> 
> Thanks,
> 
> - Mark
> 
> 
>  src/gen6_mfc_common.c | 11 +++++++----
>  src/gen9_vdenc.c      | 28 ++++++++++++++++------------
>  src/gen9_vdenc.h      |  2 +-
>  src/i965_encoder.c    | 49 ++++++++++++++++++++++++++++++++++-------
> --------
>  src/i965_encoder.h    |  8 +++++++-
>  5 files changed, 65 insertions(+), 33 deletions(-)
> 
> diff --git a/src/gen6_mfc_common.c b/src/gen6_mfc_common.c
> index 68d030e..15c0637 100644
> --- a/src/gen6_mfc_common.c
> +++ b/src/gen6_mfc_common.c
> @@ -119,16 +119,19 @@ static void intel_mfc_brc_init(struct
> encode_state *encode_state,
>  
>          if (i == 0) {
>              bitrate = encoder_context->brc.bits_per_second[0];
> -            framerate = (double)encoder_context-
> >brc.framerate_per_100s[0] / 100.0;
> +            framerate = (double)encoder_context-
> >brc.framerate[0].num / (double)encoder_context-
> >brc.framerate[0].den;
>          } else {
>              bitrate = (encoder_context->brc.bits_per_second[i] -
> encoder_context->brc.bits_per_second[i - 1]);
> -            framerate = (double)(encoder_context-
> >brc.framerate_per_100s[i] - encoder_context-
> >brc.framerate_per_100s[i - 1]) / 100.0;
> +            framerate = ((double)encoder_context-
> >brc.framerate[i].num / (double)encoder_context-
> >brc.framerate[i].den) -
> +                ((double)encoder_context->brc.framerate[i - 1].num /
> (double)encoder_context->brc.framerate[i - 1].den);
>          }
>  
>          if (i == encoder_context->layer.num_layers - 1)
>              factor = 1.0;
> -        else
> -            factor = (double)encoder_context-
> >brc.framerate_per_100s[i] / encoder_context-
> >brc.framerate_per_100s[encoder_context->layer.num_layers - 1];
> +        else {
> +            factor = ((double)encoder_context->brc.framerate[i].num
> / (double)encoder_context->brc.framerate[i].den) /
> +                ((double)encoder_context->brc.framerate[i - 1].num /
> (double)encoder_context->brc.framerate[i - 1].den);
> +        }
>  
>          hrd_factor = (double)bitrate / encoder_context-
> >brc.bits_per_second[encoder_context->layer.num_layers - 1];
>  
> diff --git a/src/gen9_vdenc.c b/src/gen9_vdenc.c
> index 8009b31..691faec 100644
> --- a/src/gen9_vdenc.c
> +++ b/src/gen9_vdenc.c
> @@ -851,7 +851,7 @@
> gen9_vdenc_update_misc_parameters(VADriverContextP ctx,
>      if (vdenc_context->internal_rate_mode != I965_BRC_CQP &&
>          encoder_context->brc.need_reset) {
>          /* So far, vdenc doesn't support temporal layer */
> -        vdenc_context->frames_per_100s = encoder_context-
> >brc.framerate_per_100s[0];
> +        vdenc_context->framerate = encoder_context-
> >brc.framerate[0];
>  
>          vdenc_context->vbv_buffer_size_in_bit = encoder_context-
> >brc.hrd_buffer_size;
>          vdenc_context->init_vbv_buffer_fullness_in_bit =
> encoder_context->brc.hrd_initial_buffer_fullness;
> @@ -927,7 +927,8 @@ gen9_vdenc_update_parameters(VADriverContextP
> ctx,
>           !vdenc_context->vbv_buffer_size_in_bit ||
>           !vdenc_context->max_bit_rate ||
>           !vdenc_context->target_bit_rate ||
> -         !vdenc_context->frames_per_100s))
> +         !vdenc_context->framerate.num ||
> +         !vdenc_context->framerate.den))
>          vdenc_context->brc_enabled = 0;
>  
>      if (!vdenc_context->brc_enabled) {
> @@ -1565,7 +1566,8 @@
> gen9_vdenc_get_profile_level_max_frame(VADriverContextP ctx,
>          tmpf = max_mbps / 172.0;
>  
>      max_byte_per_frame0 = (uint64_t)(tmpf * bits_per_mb);
> -    max_byte_per_frame1 = (uint64_t)(((double)max_mbps * 100) /
> vdenc_context->frames_per_100s *bits_per_mb);
> +    max_byte_per_frame1 = (uint64_t)(((double)max_mbps *
> vdenc_context->framerate.den) /
> +                                     (double)vdenc_context-
> >framerate.num * bits_per_mb);
>  
>      /* TODO: check VAEncMiscParameterTypeMaxFrameSize */
>      ret = (unsigned int)MIN(max_byte_per_frame0,
> max_byte_per_frame1);
> @@ -1586,12 +1588,12 @@
> gen9_vdenc_calculate_initial_qp(VADriverContextP ctx,
>  
>      frame_size = (vdenc_context->frame_width * vdenc_context-
> >frame_height * 3 / 2);
>      qp = (int)(1.0 / 1.2 * pow(10.0,
> -                               (log10(frame_size * 2.0 / 3.0 *
> ((float)vdenc_context->frames_per_100s) /
> -                                      ((float)(vdenc_context-
> >target_bit_rate * 1000) * 100)) - x0) *
> +                               (log10(frame_size * 2.0 / 3.0 *
> vdenc_context->framerate.num /
> +                                      ((double)vdenc_context-
> >target_bit_rate * 1000.0 * vdenc_context->framerate.den)) - x0) *
>                                 (y1 - y0) / (x1 - x0) + y0) + 0.5);
>      qp += 2;
> -    delat_qp = (int)(9 - (vdenc_context->vbv_buffer_size_in_bit *
> ((float)vdenc_context->frames_per_100s) /
> -                          ((float)(vdenc_context->target_bit_rate *
> 1000) * 100)));
> +    delat_qp = (int)(9 - (vdenc_context->vbv_buffer_size_in_bit *
> ((double)vdenc_context->framerate.num) /
> +                          ((double)vdenc_context->target_bit_rate *
> 1000.0 * vdenc_context->framerate.den)));
>      if (delat_qp > 0)
>          qp += delat_qp;
>  
> @@ -1615,7 +1617,8 @@
> gen9_vdenc_update_huc_brc_init_dmem(VADriverContextP ctx,
>      double input_bits_per_frame, bps_ratio;
>      int i;
>  
> -    vdenc_context->brc_init_reset_input_bits_per_frame =
> ((double)(vdenc_context->max_bit_rate * 1000) * 100) / vdenc_context-
> >frames_per_100s;
> +    vdenc_context->brc_init_reset_input_bits_per_frame =
> +        ((double)vdenc_context->max_bit_rate * 1000.0 *
> vdenc_context->framerate.den) / vdenc_context->framerate.num;
>      vdenc_context->brc_init_current_target_buf_full_in_bits =
> vdenc_context->brc_init_reset_input_bits_per_frame;
>      vdenc_context->brc_target_size = vdenc_context-
> >init_vbv_buffer_fullness_in_bit;
>  
> @@ -1645,8 +1648,8 @@
> gen9_vdenc_update_huc_brc_init_dmem(VADriverContextP ctx,
>      else if (vdenc_context->internal_rate_mode == I965_BRC_VBR)
>          dmem->brc_flag |= 0x20;
>  
> -    dmem->frame_rate_m = vdenc_context->frames_per_100s;
> -    dmem->frame_rate_d = 100;
> +    dmem->frame_rate_m = vdenc_context->framerate.num;
> +    dmem->frame_rate_d = vdenc_context->framerate.den;
>  
>      dmem->profile_level_max_frame =
> gen9_vdenc_get_profile_level_max_frame(ctx, encoder_context,
> seq_param->level_idc);
>  
> @@ -1656,8 +1659,9 @@
> gen9_vdenc_update_huc_brc_init_dmem(VADriverContextP ctx,
>      dmem->min_qp = 10;
>      dmem->max_qp = 51;
>  
> -    input_bits_per_frame = ((double)vdenc_context->max_bit_rate *
> 1000 * 100) / vdenc_context->frames_per_100s;
> -    bps_ratio = input_bits_per_frame / ((double)vdenc_context-
> >vbv_buffer_size_in_bit * 100 / vdenc_context->frames_per_100s);
> +    input_bits_per_frame = ((double)vdenc_context->max_bit_rate *
> 1000.0 * vdenc_context->framerate.den) / vdenc_context-
> >framerate.num;
> +    bps_ratio = input_bits_per_frame /
> +        ((double)vdenc_context->vbv_buffer_size_in_bit *
> vdenc_context->framerate.den / vdenc_context->framerate.num);
>  
>      if (bps_ratio < 0.1)
>          bps_ratio = 0.1;
> diff --git a/src/gen9_vdenc.h b/src/gen9_vdenc.h
> index e790497..41e4362 100644
> --- a/src/gen9_vdenc.h
> +++ b/src/gen9_vdenc.h
> @@ -741,7 +741,7 @@ struct gen9_vdenc_context
>      uint32_t    min_bit_rate;           /* in kbps */
>      uint64_t    init_vbv_buffer_fullness_in_bit;
>      uint64_t    vbv_buffer_size_in_bit;
> -    uint32_t    frames_per_100s;
> +    struct intel_fraction framerate;
>      uint32_t    gop_size;
>      uint32_t    ref_dist;
>      uint32_t    brc_target_size;
> diff --git a/src/i965_encoder.c b/src/i965_encoder.c
> index d874322..e5aff72 100644
> --- a/src/i965_encoder.c
> +++ b/src/i965_encoder.c
> @@ -42,6 +42,17 @@
>  
>  #include "i965_post_processing.h"
>  
> +static struct intel_fraction
> +reduce_fraction(struct intel_fraction f)
> +{
> +    unsigned int a = f.num, b = f.den, c;
> +    while (c = a % b) {
> +        a = b;
> +        b = c;
> +    }
> +    return (struct intel_fraction) { f.num / b, f.den / b };
> +}
> +
>  static VAStatus
>  clear_border(struct object_surface *obj_surface)
>  {
> @@ -306,8 +317,9 @@
> intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
>                                                  struct
> intel_encoder_context *encoder_context)
>  {
>      VAEncSequenceParameterBufferH264 *seq_param =
> (VAEncSequenceParameterBufferH264 *)encode_state->seq_param_ext-
> >buffer;
> +    struct intel_fraction framerate;
> +    unsigned int bits_per_second;
>      unsigned short num_pframes_in_gop, num_bframes_in_gop;
> -    unsigned int bits_per_second, framerate_per_100s;
>  
>      if (!encoder_context->is_new_sequence)
>          return VA_STATUS_SUCCESS;
> @@ -315,10 +327,13 @@
> intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
>      assert(seq_param);
>      bits_per_second = seq_param->bits_per_second; // for the highest
> layer
>  
> -    if (!seq_param->num_units_in_tick || !seq_param->time_scale)
> -        framerate_per_100s = 3000;
> -    else
> -        framerate_per_100s = seq_param->time_scale * 100 / (2 *
> seq_param->num_units_in_tick); // for the highest layer
> +    if (!seq_param->num_units_in_tick || !seq_param->time_scale) {
> +        framerate = (struct intel_fraction) { 30, 1 };
> +    } else {
> +        // for the highest layer
> +        framerate = (struct intel_fraction) { seq_param->time_scale, 
> 2 * seq_param->num_units_in_tick };
> +    }
> +    framerate = reduce_fraction(framerate);
>  
>      encoder_context->brc.num_iframes_in_gop = 1; // Always 1
>  
> @@ -326,7 +341,7 @@
> intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
>          if (seq_param->ip_period == 0)
>              goto error;
>  
> -        encoder_context->brc.gop_size = (unsigned
> int)(framerate_per_100s / 100.0 + 0.5); // fake
> +        encoder_context->brc.gop_size = (framerate.num +
> framerate.den - 1) / framerate.den; // fake
>          num_pframes_in_gop = (encoder_context->brc.gop_size +
>                                seq_param->ip_period - 1) / seq_param-
> >ip_period - 1;
>      } else if (seq_param->intra_period == 1) { // E.g. IDRIII...
> @@ -347,11 +362,12 @@
> intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
>      if (num_pframes_in_gop != encoder_context-
> >brc.num_pframes_in_gop ||
>          num_bframes_in_gop != encoder_context-
> >brc.num_bframes_in_gop ||
>          bits_per_second != encoder_context-
> >brc.bits_per_second[encoder_context->layer.num_layers - 1] ||
> -        framerate_per_100s != encoder_context-
> >brc.framerate_per_100s[encoder_context->layer.num_layers - 1]) {
> +        framerate.num != encoder_context-
> >brc.framerate[encoder_context->layer.num_layers - 1].num ||
> +        framerate.den != encoder_context-
> >brc.framerate[encoder_context->layer.num_layers - 1].den) {
>          encoder_context->brc.num_pframes_in_gop =
> num_pframes_in_gop;
>          encoder_context->brc.num_bframes_in_gop =
> num_bframes_in_gop;
>          encoder_context->brc.bits_per_second[encoder_context-
> >layer.num_layers - 1] = bits_per_second;
> -        encoder_context->brc.framerate_per_100s[encoder_context-
> >layer.num_layers - 1] = framerate_per_100s;
> +        encoder_context->brc.framerate[encoder_context-
> >layer.num_layers - 1] = framerate;
>          encoder_context->brc.need_reset = 1;
>      }
>  
> @@ -392,8 +408,9 @@
> intel_encoder_check_brc_vp8_sequence_parameter(VADriverContextP ctx,
>      num_pframes_in_gop = encoder_context->brc.gop_size - 1;
>      bits_per_second = seq_param->bits_per_second;       // for the
> highest layer
>  
> -    if (!encoder_context->brc.framerate_per_100s[encoder_context-
> >layer.num_layers - 1]) {
> -        encoder_context->brc.framerate_per_100s[encoder_context-
> >layer.num_layers - 1] = 3000;  // for the highest layer
> +    if (!encoder_context->brc.framerate[encoder_context-
> >layer.num_layers - 1].num) {
> +        // for the highest layer
> +        encoder_context->brc.framerate[encoder_context-
> >layer.num_layers - 1] = (struct intel_fraction) { 30, 1 };
>          encoder_context->brc.need_reset = 1;
>      }
>  
> @@ -480,7 +497,7 @@
> intel_encoder_check_framerate_parameter(VADriverContextP ctx,
>                                          struct intel_encoder_context
> *encoder_context,
>                                          VAEncMiscParameterFrameRate
> *misc)
>  {
> -    int framerate_per_100s;
> +    struct intel_fraction framerate;
>      int temporal_id = 0;
>  
>      if (encoder_context->layer.num_layers >= 2)
> @@ -490,12 +507,14 @@
> intel_encoder_check_framerate_parameter(VADriverContextP ctx,
>          return;
>  
>      if (misc->framerate & 0xffff0000)
> -        framerate_per_100s = (misc->framerate & 0xffff) * 100 /
> ((misc->framerate >> 16) & 0xffff);
> +        framerate = (struct intel_fraction) { misc->framerate >> 16
> & 0xffff, misc->framerate & 0xffff };


'misc->framerate >> 16 & 0xffff' is denominator, not numerator. 


>      else
> -        framerate_per_100s = misc->framerate * 100;
> +        framerate = (struct intel_fraction) { misc->framerate, 1 };
> +    framerate = reduce_fraction(framerate);
>  
> -    if (encoder_context->brc.framerate_per_100s[temporal_id] !=
> framerate_per_100s) {
> -        encoder_context->brc.framerate_per_100s[temporal_id] =
> framerate_per_100s;
> +    if (encoder_context->brc.framerate[temporal_id].num !=
> framerate.num ||
> +        encoder_context->brc.framerate[temporal_id].den !=
> framerate.den) {
> +        encoder_context->brc.framerate[temporal_id] = framerate;
>          encoder_context->brc.need_reset = 1;
>      }
>  }
> diff --git a/src/i965_encoder.h b/src/i965_encoder.h
> index 0b636d6..7016975 100644
> --- a/src/i965_encoder.h
> +++ b/src/i965_encoder.h
> @@ -55,6 +55,12 @@ struct intel_roi
>      char  value;
>  };
>  
> +struct intel_fraction
> +{
> +    unsigned int num;
> +    unsigned int den;
> +};
> +
>  struct intel_encoder_context
>  {
>      struct hw_context base;
> @@ -80,7 +86,7 @@ struct intel_encoder_context
>          unsigned short num_pframes_in_gop;
>          unsigned short num_bframes_in_gop;
>          unsigned int bits_per_second[MAX_TEMPORAL_LAYERS];
> -        unsigned int framerate_per_100s[MAX_TEMPORAL_LAYERS];
> +        struct intel_fraction framerate[MAX_TEMPORAL_LAYERS];
>          unsigned int mb_rate_control[MAX_TEMPORAL_LAYERS];
>          unsigned int target_percentage[MAX_TEMPORAL_LAYERS];
>          unsigned int hrd_buffer_size;

BTW could you refine your patch series to avoid compiler warning?


More information about the Libva mailing list