> Update references in both H.264 encoders (gen6_mfc and gen9_vdenc).
>
> Signed-off-by: Mark Thompson <s...@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?
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva
