Discussion:
[PATCH 1/3] Declare I010 support for QueryImageFormats
(too old to reply)
Mark Thompson
2016-12-05 17:49:43 UTC
Permalink
Signed-off-by: Mark Thompson <***@jkqxz.net>
---
It's already returned as a usable surface format for the video processor, but is missing from the list returned by vaQueryImageFormats().

src/i965_drv_video.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index 79a2aec..d83427c 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -268,6 +268,8 @@ i965_image_formats_map[I965_MAX_IMAGE_FORMATS + 1] = {
{ VA_FOURCC_BGRX, VA_LSB_FIRST, 32, 24, 0x00ff0000, 0x0000ff00, 0x000000ff } },
{ I965_SURFACETYPE_YUV,
{ VA_FOURCC_P010, VA_LSB_FIRST, 24, } },
+ { I965_SURFACETYPE_YUV,
+ { VA_FOURCC_I010, VA_LSB_FIRST, 24, } },
};

/* List of supported subpicture formats */
--
2.10.2
Mark Thompson
2016-12-05 17:54:58 UTC
Permalink
Signed-off-by: Mark Thompson <***@jkqxz.net>
---
The supported surface formats are already correct (i.e. only P010); this makes the render target format right as well (before this, it declares that it supports only YUV 4:2:0 8-bit).

src/i965_drv_video.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index d83427c..9e1c92a 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -880,6 +880,8 @@ i965_get_default_chroma_formats(VADriverContextP ctx, VAProfile profile,
break;

case VAProfileHEVCMain10:
+ if (HAS_HEVC10_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)
+ chroma_formats = VA_RT_FORMAT_YUV420_10BPP;
if (HAS_HEVC10_DECODING(i965) && entrypoint == VAEntrypointVLD)
chroma_formats |= i965->codec_info->hevc_dec_chroma_formats;
break;
--
2.10.2
Mark Thompson
2016-12-15 13:04:16 UTC
Permalink
Post by Mark Thompson
---
The supported surface formats are already correct (i.e. only P010); this makes the render target format right as well (before this, it declares that it supports only YUV 4:2:0 8-bit).
src/i965_drv_video.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index d83427c..9e1c92a 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -880,6 +880,8 @@ i965_get_default_chroma_formats(VADriverContextP ctx, VAProfile profile,
break;
+ if (HAS_HEVC10_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)
+ chroma_formats = VA_RT_FORMAT_YUV420_10BPP;
if (HAS_HEVC10_DECODING(i965) && entrypoint == VAEntrypointVLD)
chroma_formats |= i965->codec_info->hevc_dec_chroma_formats;
break;
Ping. (The other patches in this series have been considered separately.)

Thanks,

- Mark
Sean V Kelley
2016-12-15 18:05:58 UTC
Permalink
Post by Mark Thompson
---
The supported surface formats are already correct (i.e. only P010);
this makes the render target format right as well (before this, it
declares that it supports only YUV 4:2:0 8-bit).
 src/i965_drv_video.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index d83427c..9e1c92a 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -880,6 +880,8 @@
i965_get_default_chroma_formats(VADriverContextP ctx, VAProfile
profile,
         break;
 
+        if (HAS_HEVC10_ENCODING(i965) && entrypoint ==
VAEntrypointEncSlice)
+            chroma_formats = VA_RT_FORMAT_YUV420_10BPP;
         if (HAS_HEVC10_DECODING(i965) && entrypoint ==
VAEntrypointVLD)
             chroma_formats |= i965->codec_info-
Post by Mark Thompson
hevc_dec_chroma_formats;
         break;
Ping.  (The other patches in this series have been considered
separately.)
Yes this is a good catch.  It was overlooked with the addition of 10bit
support.  We may need to make further adjustment when we roll in new
encoder formats too.

lgtm.

applied.

Sean
Thanks,
- Mark
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Mark Thompson
2016-12-05 18:02:28 UTC
Permalink
Signed-off-by: Mark Thompson <***@jkqxz.net>
---
Tested on Kaby Lake. Someone who has access to the manuals should make sure that the framerate numerator/denominator actually works there as I am guessing it does.

Removes the frame_rate field in struct gen9_vp9_brc_curbe_param, because there is no longer a sensible value to set it to (and also it's not read anywhere). If it is needed, it wants to be replaced by the two parts of the fraction.

src/gen9_vp9_encoder.c | 45 ++++++++++++++++++++++++++++++++++-----------
src/gen9_vp9_encoder.h | 4 ++--
2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c
index 3ea1537..8ff1b9b 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate + VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->frame_rate_num;
+ cmd->dw11.frame_rated = vp9_state->frame_rate_den;

cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,7 +1235,7 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;

- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) / (vp9_state->frame_rate);
+ dInputBitsPerFrame = ((double)cmd->dw8.max_bit_rate * vp9_state->frame_rate_den) / (vp9_state->frame_rate_num);
dbps_ratio = dInputBitsPerFrame / ((double)(vp9_state->vbv_buffer_size_in_bit) / 30);
if (dbps_ratio < 0.1)
dbps_ratio = 0.1;
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;

vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -1523,7 +1522,6 @@ gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;

vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -3964,10 +3962,17 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param->data;

- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate >> 16 & 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate & 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}

/* RC misc will override HRD parameter */
@@ -3999,10 +4004,17 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param->data;

- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate >> 16 & 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate & 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}

if (vp9_state->brc_flag_check & VP9_BRC_RC) {
@@ -4031,14 +4043,25 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
/* If the parameter related with RC is changed. Reset BRC */
if (vp9_state->brc_flag_check & VP9_BRC_FR) {
VAEncMiscParameterFrameRate *misc_param_fr;
+ uint32_t num, den;

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) {
+ if (misc_param_fr->framerate & 0xffff0000) {
+ num = misc_param_fr->framerate >> 16 & 0xffff;
+ den = misc_param_fr->framerate & 0xffff;
+ } else {
+ num = misc_param_fr->framerate;
+ den = 1;
+ }
+
+ if (vp9_state->frame_rate_num != num ||
+ vp9_state->frame_rate_den != den) {
vp9_state->brc_reset = 1;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ vp9_state->frame_rate_num = num;
+ vp9_state->frame_rate_den = den;
}
}

diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h
index ad2d875..260b8d5 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};

@@ -1925,7 +1924,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t frame_rate_num;
+ uint32_t frame_rate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
--
2.10.2
Zhao Yakui
2016-12-06 01:49:30 UTC
Permalink
Good work.

This patch looks good to me.

Add: Reviewed-by: Zhao Yakui <***@intel.com>

Thanks
Post by Mark Thompson
---
Tested on Kaby Lake. Someone who has access to the manuals should make sure that the framerate numerator/denominator actually works there as I am guessing it does.
Removes the frame_rate field in struct gen9_vp9_brc_curbe_param, because there is no longer a sensible value to set it to (and also it's not read anywhere). If it is needed, it wants to be replaced by the two parts of the fraction.
src/gen9_vp9_encoder.c | 45 ++++++++++++++++++++++++++++++++++-----------
src/gen9_vp9_encoder.h | 4 ++--
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c
index 3ea1537..8ff1b9b 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate + VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->frame_rate_num;
+ cmd->dw11.frame_rated = vp9_state->frame_rate_den;
cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,7 +1235,7 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;
- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) / (vp9_state->frame_rate);
+ dInputBitsPerFrame = ((double)cmd->dw8.max_bit_rate * vp9_state->frame_rate_den) / (vp9_state->frame_rate_num);
dbps_ratio = dInputBitsPerFrame / ((double)(vp9_state->vbv_buffer_size_in_bit) / 30);
if (dbps_ratio< 0.1)
dbps_ratio = 0.1;
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -1523,7 +1522,6 @@ gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -3964,10 +3962,17 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param->data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate& 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate>> 16& 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate& 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
/* RC misc will override HRD parameter */
@@ -3999,10 +4004,17 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param->data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate& 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate>> 16& 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate& 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
if (vp9_state->brc_flag_check& VP9_BRC_RC) {
@@ -4031,14 +4043,25 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
/* If the parameter related with RC is changed. Reset BRC */
if (vp9_state->brc_flag_check& VP9_BRC_FR) {
VAEncMiscParameterFrameRate *misc_param_fr;
+ uint32_t num, den;
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) {
+ if (misc_param_fr->framerate& 0xffff0000) {
+ num = misc_param_fr->framerate>> 16& 0xffff;
+ den = misc_param_fr->framerate& 0xffff;
+ } else {
+ num = misc_param_fr->framerate;
+ den = 1;
+ }
+
+ if (vp9_state->frame_rate_num != num ||
+ vp9_state->frame_rate_den != den) {
vp9_state->brc_reset = 1;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ vp9_state->frame_rate_num = num;
+ vp9_state->frame_rate_den = den;
}
}
diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h
index ad2d875..260b8d5 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};
@@ -1925,7 +1924,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t frame_rate_num;
+ uint32_t frame_rate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
Xiang, Haihao
2016-12-07 05:48:18 UTC
Permalink
Now we have the same way for frame rate for each codec in i965_encoder.c, it would be better to use the result directly

Thanks
Haihao
-----Original Message-----
Thompson
Sent: Tuesday, December 6, 2016 2:02 AM
Subject: [Libva] [PATCH 3/3] VP9 encode: support fractional framerate
---
Tested on Kaby Lake. Someone who has access to the manuals should make
sure that the framerate numerator/denominator actually works there as I am
guessing it does.
Removes the frame_rate field in struct gen9_vp9_brc_curbe_param, because
there is no longer a sensible value to set it to (and also it's not read anywhere).
If it is needed, it wants to be replaced by the two parts of the fraction.
src/gen9_vp9_encoder.c | 45 ++++++++++++++++++++++++++++++++++---
--------
src/gen9_vp9_encoder.h | 4 ++--
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c index
3ea1537..8ff1b9b 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate +
VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->frame_rate_num;
+ cmd->dw11.frame_rated = vp9_state->frame_rate_den;
cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,7 +1235,7 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;
- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) / (vp9_state-
Post by Mark Thompson
frame_rate);
+ dInputBitsPerFrame = ((double)cmd->dw8.max_bit_rate *
+ vp9_state->frame_rate_den) / (vp9_state->frame_rate_num);
dbps_ratio = dInputBitsPerFrame / ((double)(vp9_state-
Post by Mark Thompson
vbv_buffer_size_in_bit) / 30);
if (dbps_ratio < 0.1)
dbps_ratio = 0.1;
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate >> 16 & 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate & 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate >> 16 & 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate & 0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
/* If the parameter related with RC is changed. Reset BRC */
if (vp9_state->brc_flag_check & VP9_BRC_FR) {
VAEncMiscParameterFrameRate *misc_param_fr;
+ uint32_t num, den;
misc_param = (VAEncMiscParameterBuffer *)
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- if (vp9_state->frame_rate != misc_param_fr->framerate) {
+ if (misc_param_fr->framerate & 0xffff0000) {
+ num = misc_param_fr->framerate >> 16 & 0xffff;
+ den = misc_param_fr->framerate & 0xffff;
+ } else {
+ num = misc_param_fr->framerate;
+ den = 1;
+ }
+
+ if (vp9_state->frame_rate_num != num ||
+ vp9_state->frame_rate_den != den) {
vp9_state->brc_reset = 1;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ vp9_state->frame_rate_num = num;
+ vp9_state->frame_rate_den = den;
}
}
diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h index
ad2d875..260b8d5 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};
@@ -1925,7 +1924,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t frame_rate_num;
+ uint32_t frame_rate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
--
2.10.2
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Sean V Kelley
2016-12-07 17:42:01 UTC
Permalink
Post by Xiang, Haihao
Now we have the same way for frame rate for each codec in i965_encoder.c,
it would be better to use the result directly
Agree, Yakui can you look into this?

Sean
Post by Xiang, Haihao
Thanks
Haihao
-----Original Message-----
Mark
Thompson
Sent: Tuesday, December 6, 2016 2:02 AM
Subject: [Libva] [PATCH 3/3] VP9 encode: support fractional framerate
---
Tested on Kaby Lake. Someone who has access to the manuals should make
sure that the framerate numerator/denominator actually works there as I am
guessing it does.
Removes the frame_rate field in struct gen9_vp9_brc_curbe_param, because
there is no longer a sensible value to set it to (and also it's not read
anywhere).
If it is needed, it wants to be replaced by the two parts of the fraction.
src/gen9_vp9_encoder.c | 45 ++++++++++++++++++++++++++++++++++---
--------
src/gen9_vp9_encoder.h | 4 ++--
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c index
3ea1537..8ff1b9b 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate +
VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->frame_rate_num;
+ cmd->dw11.frame_rated = vp9_state->frame_rate_den;
cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,7 +1235,7 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;
- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) /
(vp9_state-
Post by Mark Thompson
frame_rate);
+ dInputBitsPerFrame = ((double)cmd->dw8.max_bit_rate *
+ vp9_state->frame_rate_den) / (vp9_state->frame_rate_num);
dbps_ratio = dInputBitsPerFrame /
((double)(vp9_state-
Post by Mark Thompson
vbv_buffer_size_in_bit) / 30);
if (dbps_ratio < 0.1)
dbps_ratio = 0.1;
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag =
vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate
*)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num =
misc_param_fr->framerate >>
16 & 0xffff;
+ vp9_state->frame_rate_den =
misc_param_fr->framerate &
0xffff;
+ } else {
+ vp9_state->frame_rate_num =
misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate
*)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate & 0xffff0000) {
+ vp9_state->frame_rate_num =
misc_param_fr->framerate >>
16 & 0xffff;
+ vp9_state->frame_rate_den =
misc_param_fr->framerate &
0xffff;
+ } else {
+ vp9_state->frame_rate_num =
misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
/* If the parameter related with RC is changed. Reset BRC */
if (vp9_state->brc_flag_check & VP9_BRC_FR) {
VAEncMiscParameterFrameRate *misc_param_fr;
+ uint32_t num, den;
misc_param = (VAEncMiscParameterBuffer *)
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- if (vp9_state->frame_rate != misc_param_fr->framerate) {
+ if (misc_param_fr->framerate & 0xffff0000) {
+ num = misc_param_fr->framerate >> 16 & 0xffff;
+ den = misc_param_fr->framerate & 0xffff;
+ } else {
+ num = misc_param_fr->framerate;
+ den = 1;
+ }
+
+ if (vp9_state->frame_rate_num != num ||
+ vp9_state->frame_rate_den != den) {
vp9_state->brc_reset = 1;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ vp9_state->frame_rate_num = num;
+ vp9_state->frame_rate_den = den;
}
}
diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h index
ad2d875..260b8d5 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};
@@ -1925,7 +1924,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t frame_rate_num;
+ uint32_t frame_rate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
--
2.10.2
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
--
Sean V. Kelley <***@intel.com>
Open Source Technology Center / SSG
Intel Corp.
Zhao Yakui
2016-12-08 01:13:16 UTC
Permalink
Post by Xiang, Haihao
Now we have the same way for frame rate for each codec in i965_encoder.c, it would be better to use the result directly
Hi, Mark

Can you refresh your patch based on Haihao's suggestion?
Now the frame rate is parsed in the function of
intel_encoder_check_framerate_parameter. (It is defined in
i965_encoder.c) and it is stored as framerate_per_100s.
So the vp9 can use the framerate_per_100s for the frame_rate_m. The
frame_rate_d can be 100.

Thanks
Yakui
Post by Xiang, Haihao
Thanks
Haihao
-----Original Message-----
Thompson
Sent: Tuesday, December 6, 2016 2:02 AM
Subject: [Libva] [PATCH 3/3] VP9 encode: support fractional framerate
---
Tested on Kaby Lake. Someone who has access to the manuals should make
sure that the framerate numerator/denominator actually works there as I am
guessing it does.
Removes the frame_rate field in struct gen9_vp9_brc_curbe_param, because
there is no longer a sensible value to set it to (and also it's not read anywhere).
If it is needed, it wants to be replaced by the two parts of the fraction.
src/gen9_vp9_encoder.c | 45 ++++++++++++++++++++++++++++++++++---
--------
src/gen9_vp9_encoder.h | 4 ++--
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c index
3ea1537..8ff1b9b 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate +
VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->frame_rate_num;
+ cmd->dw11.frame_rated = vp9_state->frame_rate_den;
cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,7 +1235,7 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;
- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) / (vp9_state-
Post by Mark Thompson
frame_rate);
+ dInputBitsPerFrame = ((double)cmd->dw8.max_bit_rate *
+ vp9_state->frame_rate_den) / (vp9_state->frame_rate_num);
dbps_ratio = dInputBitsPerFrame / ((double)(vp9_state-
Post by Mark Thompson
vbv_buffer_size_in_bit) / 30);
if (dbps_ratio< 0.1)
dbps_ratio = 0.1;
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;
vme_context->pfn_set_curbe_brc(ctx, encode_state,
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate& 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate>>
16& 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate&
0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
gen9_encode_vp9_check_parameter(VADriverContextP ctx,
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ if (misc_param_fr->framerate& 0xffff0000) {
+ vp9_state->frame_rate_num = misc_param_fr->framerate>>
16& 0xffff;
+ vp9_state->frame_rate_den = misc_param_fr->framerate&
0xffff;
+ } else {
+ vp9_state->frame_rate_num = misc_param_fr->framerate;
+ vp9_state->frame_rate_den = 1;
+ }
} else {
/* Assign the default frame rate */
- vp9_state->frame_rate = 30;
+ vp9_state->frame_rate_num = 30;
+ vp9_state->frame_rate_den = 1;
}
/* If the parameter related with RC is changed. Reset BRC */
if (vp9_state->brc_flag_check& VP9_BRC_FR) {
VAEncMiscParameterFrameRate *misc_param_fr;
+ uint32_t num, den;
misc_param = (VAEncMiscParameterBuffer *)
encode_state-
Post by Mark Thompson
misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
misc_param_fr = (VAEncMiscParameterFrameRate *)misc_param-
Post by Mark Thompson
data;
- if (vp9_state->frame_rate != misc_param_fr->framerate) {
+ if (misc_param_fr->framerate& 0xffff0000) {
+ num = misc_param_fr->framerate>> 16& 0xffff;
+ den = misc_param_fr->framerate& 0xffff;
+ } else {
+ num = misc_param_fr->framerate;
+ den = 1;
+ }
+
+ if (vp9_state->frame_rate_num != num ||
+ vp9_state->frame_rate_den != den) {
vp9_state->brc_reset = 1;
- vp9_state->frame_rate = misc_param_fr->framerate;
+ vp9_state->frame_rate_num = num;
+ vp9_state->frame_rate_den = den;
}
}
diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h index
ad2d875..260b8d5 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};
@@ -1925,7 +1924,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t frame_rate_num;
+ uint32_t frame_rate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
--
2.10.2
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Mark Thompson
2016-12-08 22:54:53 UTC
Permalink
Post by Zhao Yakui
Post by Xiang, Haihao
Now we have the same way for frame rate for each codec in i965_encoder.c, it would be better to use the result directly
Hi, Mark
Can you refresh your patch based on Haihao's suggestion?
Now the frame rate is parsed in the function of intel_encoder_check_framerate_parameter. (It is defined in i965_encoder.c) and it is stored as framerate_per_100s.
So the vp9 can use the framerate_per_100s for the frame_rate_m. The frame_rate_d can be 100.
Right, I went a bit further than that. The following removes all use of any of the RC parameter buffers (RC, HRD, framerate) from the specific encoders, moving top-level RC code into i965_encoder.c.

Tested with VP8, VP9, H.264 and H.265 on Kaby Lake. I haven't tested the low-power H.264 encoder (gen9_vdenc.c) because it is only enabled for Skylake using CQP at the moment, though there are some edits there.

Thanks,

- Mark
Mark Thompson
2016-12-08 22:56:32 UTC
Permalink
Update references in both H.264 encoders (gen6_mfc and gen9_vdenc).

Signed-off-by: Mark Thompson <***@jkqxz.net>
---
src/gen6_mfc_common.c | 11 ++++++----
src/gen9_vdenc.c | 29 ++++++++++++++----------
src/gen9_vdenc.h | 3 ++-
src/i965_encoder.c | 61 +++++++++++++++++++++++++++++++++++++--------------
src/i965_encoder.h | 3 ++-
5 files changed, 72 insertions(+), 35 deletions(-)

diff --git a/src/gen6_mfc_common.c b/src/gen6_mfc_common.c
index 68d030e..c29817e 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_num[0] / (double)encoder_context->brc.framerate_den[0];
} 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_num[i] / (double)encoder_context->brc.framerate_den[i]) -
+ ((double)encoder_context->brc.framerate_num[i - 1] / (double)encoder_context->brc.framerate_den[i - 1]);
}

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_num[i] / (double)encoder_context->brc.framerate_den[i]) /
+ ((double)encoder_context->brc.framerate_num[i - 1] / (double)encoder_context->brc.framerate_den[i - 1]);
+ }

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..b2d0b21 100644
--- a/src/gen9_vdenc.c
+++ b/src/gen9_vdenc.c
@@ -851,7 +851,8 @@ 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_num = encoder_context->brc.framerate_num[0];
+ vdenc_context->framerate_den = encoder_context->brc.framerate_den[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 +928,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 +1567,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 +1589,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 +1618,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 +1649,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 +1660,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 * 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..5b80a4a 100644
--- a/src/gen9_vdenc.h
+++ b/src/gen9_vdenc.h
@@ -741,7 +741,8 @@ 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;
+ uint32_t framerate_num;
+ uint32_t framerate_den;
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..75a6a1f 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -42,6 +42,18 @@

#include "i965_post_processing.h"

+static void
+reduce_fraction(unsigned int *num, unsigned int *den)
+{
+ unsigned int a = *num, b = *den, c;
+ while (c = a % b) {
+ a = b;
+ b = c;
+ }
+ *num /= b;
+ *den /= b;
+}
+
static VAStatus
clear_border(struct object_surface *obj_surface)
{
@@ -307,7 +319,7 @@ intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
{
VAEncSequenceParameterBufferH264 *seq_param = (VAEncSequenceParameterBufferH264 *)encode_state->seq_param_ext->buffer;
unsigned short num_pframes_in_gop, num_bframes_in_gop;
- unsigned int bits_per_second, framerate_per_100s;
+ unsigned int bits_per_second, framerate_num, framerate_den;

if (!encoder_context->is_new_sequence)
return VA_STATUS_SUCCESS;
@@ -315,10 +327,15 @@ 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_num = 30;
+ framerate_den = 1;
+ } else {
+ // for the highest layer
+ framerate_num = seq_param->time_scale;
+ framerate_den = 2 * seq_param->num_units_in_tick;
+ }
+ reduce_fraction(&framerate_num, &framerate_den);

encoder_context->brc.num_iframes_in_gop = 1; // Always 1

@@ -326,7 +343,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 +364,13 @@ 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_num[encoder_context->layer.num_layers - 1] ||
+ framerate_den != encoder_context->brc.framerate_den[encoder_context->layer.num_layers - 1]) {
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_num[encoder_context->layer.num_layers - 1] = framerate_num;
+ encoder_context->brc.framerate_den[encoder_context->layer.num_layers - 1] = framerate_den;
encoder_context->brc.need_reset = 1;
}

@@ -392,8 +411,10 @@ 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_num[encoder_context->layer.num_layers - 1]) {
+ // for the highest layer
+ encoder_context->brc.framerate_num[encoder_context->layer.num_layers - 1] = 30;
+ encoder_context->brc.framerate_den[encoder_context->layer.num_layers - 1] = 1;
encoder_context->brc.need_reset = 1;
}

@@ -480,7 +501,7 @@ intel_encoder_check_framerate_parameter(VADriverContextP ctx,
struct intel_encoder_context *encoder_context,
VAEncMiscParameterFrameRate *misc)
{
- int framerate_per_100s;
+ unsigned int framerate_num, framerate_den;
int temporal_id = 0;

if (encoder_context->layer.num_layers >= 2)
@@ -489,13 +510,19 @@ intel_encoder_check_framerate_parameter(VADriverContextP ctx,
if (temporal_id >= encoder_context->layer.num_layers)
return;

- if (misc->framerate & 0xffff0000)
- framerate_per_100s = (misc->framerate & 0xffff) * 100 / ((misc->framerate >> 16) & 0xffff);
- else
- framerate_per_100s = misc->framerate * 100;
+ if (misc->framerate & 0xffff0000) {
+ framerate_num = misc->framerate >> 16 & 0xffff;
+ framerate_den = misc->framerate & 0xffff;
+ } else {
+ framerate_num = misc->framerate;
+ framerate_den = 1;
+ }
+ reduce_fraction(&framerate_num, &framerate_den);

- 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_num[temporal_id] != framerate_num ||
+ encoder_context->brc.framerate_den[temporal_id] != framerate_den) {
+ encoder_context->brc.framerate_num[temporal_id] = framerate_num;
+ encoder_context->brc.framerate_den[temporal_id] = framerate_den;
encoder_context->brc.need_reset = 1;
}
}
diff --git a/src/i965_encoder.h b/src/i965_encoder.h
index 0b636d6..4547849 100644
--- a/src/i965_encoder.h
+++ b/src/i965_encoder.h
@@ -80,7 +80,8 @@ 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];
+ unsigned int framerate_num[MAX_TEMPORAL_LAYERS];
+ unsigned int framerate_den[MAX_TEMPORAL_LAYERS];
unsigned int mb_rate_control[MAX_TEMPORAL_LAYERS];
unsigned int target_percentage[MAX_TEMPORAL_LAYERS];
unsigned int hrd_buffer_size;
--
2.10.2
Xiang, Haihao
2016-12-15 08:54:18 UTC
Permalink
Hi Mark,

Thanks for your patch, is there any benefit to use a fraction? using
100 as framerate_den works well in the driver.

Regards
Haihao
Post by Mark Thompson
Update references in both H.264 encoders (gen6_mfc and gen9_vdenc).
---
 src/gen6_mfc_common.c | 11 ++++++----
 src/gen9_vdenc.c      | 29 ++++++++++++++----------
 src/gen9_vdenc.h      |  3 ++-
 src/i965_encoder.c    | 61 +++++++++++++++++++++++++++++++++++++--
------------
 src/i965_encoder.h    |  3 ++-
 5 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/src/gen6_mfc_common.c b/src/gen6_mfc_common.c
index 68d030e..c29817e 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-
Post by Mark Thompson
brc.framerate_per_100s[0] / 100.0;
+            framerate = (double)encoder_context-
Post by Mark Thompson
brc.framerate_num[0] / (double)encoder_context-
brc.framerate_den[0];
         } else {
             bitrate = (encoder_context->brc.bits_per_second[i] -
encoder_context->brc.bits_per_second[i - 1]);
-            framerate = (double)(encoder_context-
Post by Mark Thompson
brc.framerate_per_100s[i] - encoder_context-
brc.framerate_per_100s[i - 1]) / 100.0;
+            framerate = ((double)encoder_context-
Post by Mark Thompson
brc.framerate_num[i] / (double)encoder_context-
brc.framerate_den[i]) -
+                ((double)encoder_context->brc.framerate_num[i - 1] /
(double)encoder_context->brc.framerate_den[i - 1]);
         }
 
         if (i == encoder_context->layer.num_layers - 1)
             factor = 1.0;
-        else
-            factor = (double)encoder_context-
Post by Mark Thompson
brc.framerate_per_100s[i] / encoder_context-
brc.framerate_per_100s[encoder_context->layer.num_layers - 1];
+        else {
+            factor = ((double)encoder_context->brc.framerate_num[i]
/ (double)encoder_context->brc.framerate_den[i]) /
+                ((double)encoder_context->brc.framerate_num[i - 1] /
(double)encoder_context->brc.framerate_den[i - 1]);
+        }
 
         hrd_factor = (double)bitrate / encoder_context-
Post by Mark Thompson
brc.bits_per_second[encoder_context->layer.num_layers - 1];
 
diff --git a/src/gen9_vdenc.c b/src/gen9_vdenc.c
index 8009b31..b2d0b21 100644
--- a/src/gen9_vdenc.c
+++ b/src/gen9_vdenc.c
@@ -851,7 +851,8 @@
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-
Post by Mark Thompson
brc.framerate_per_100s[0];
+        vdenc_context->framerate_num = encoder_context-
Post by Mark Thompson
brc.framerate_num[0];
+        vdenc_context->framerate_den = encoder_context-
Post by Mark Thompson
brc.framerate_den[0];
 
         vdenc_context->vbv_buffer_size_in_bit = encoder_context-
Post by Mark Thompson
brc.hrd_buffer_size;
         vdenc_context->init_vbv_buffer_fullness_in_bit =
encoder_context->brc.hrd_initial_buffer_fullness;
@@ -927,7 +928,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 +1567,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-
Post by Mark Thompson
framerate_num * bits_per_mb);
 
     /* TODO: check VAEncMiscParameterTypeMaxFrameSize */
     ret = (unsigned int)MIN(max_byte_per_frame0,
max_byte_per_frame1);
@@ -1586,12 +1589,12 @@
gen9_vdenc_calculate_initial_qp(VADriverContextP ctx,
 
     frame_size = (vdenc_context->frame_width * vdenc_context-
Post by Mark Thompson
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-
Post by Mark Thompson
target_bit_rate * 1000) * 100)) - x0) *
+                               (log10(frame_size * 2.0 / 3.0 *
vdenc_context->framerate_num /
+                                      ((double)vdenc_context-
Post by Mark Thompson
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 +1618,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-
Post by Mark Thompson
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-
Post by Mark Thompson
init_vbv_buffer_fullness_in_bit;
 
@@ -1645,8 +1649,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 +1660,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-
Post by Mark Thompson
vbv_buffer_size_in_bit * 100 / vdenc_context->frames_per_100s);
+    input_bits_per_frame = ((double)vdenc_context->max_bit_rate *
1000 * 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..5b80a4a 100644
--- a/src/gen9_vdenc.h
+++ b/src/gen9_vdenc.h
@@ -741,7 +741,8 @@ 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;
+    uint32_t    framerate_num;
+    uint32_t    framerate_den;
     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..75a6a1f 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -42,6 +42,18 @@
 
 #include "i965_post_processing.h"
 
+static void
+reduce_fraction(unsigned int *num, unsigned int *den)
+{
+    unsigned int a = *num, b = *den, c;
+    while (c = a % b) {
+        a = b;
+        b = c;
+    }
+    *num /= b;
+    *den /= b;
+}
+
 static VAStatus
 clear_border(struct object_surface *obj_surface)
 {
@@ -307,7 +319,7 @@
intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
 {
     VAEncSequenceParameterBufferH264 *seq_param =
(VAEncSequenceParameterBufferH264 *)encode_state->seq_param_ext-
Post by Mark Thompson
buffer;
     unsigned short num_pframes_in_gop, num_bframes_in_gop;
-    unsigned int bits_per_second, framerate_per_100s;
+    unsigned int bits_per_second, framerate_num, framerate_den;
 
     if (!encoder_context->is_new_sequence)
         return VA_STATUS_SUCCESS;
@@ -315,10 +327,15 @@
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_num = 30;
+        framerate_den = 1;
+    } else {
+        // for the highest layer
+        framerate_num = seq_param->time_scale;
+        framerate_den = 2 * seq_param->num_units_in_tick;
+    }
+    reduce_fraction(&framerate_num, &framerate_den);
 
     encoder_context->brc.num_iframes_in_gop = 1; // Always 1
 
@@ -326,7 +343,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-
Post by Mark Thompson
ip_period - 1;
     } else if (seq_param->intra_period == 1) { // E.g. IDRIII...
@@ -347,11 +364,13 @@
intel_encoder_check_brc_h264_sequence_parameter(VADriverContextP ctx,
     if (num_pframes_in_gop != encoder_context-
Post by Mark Thompson
brc.num_pframes_in_gop ||
         num_bframes_in_gop != encoder_context-
Post by Mark Thompson
brc.num_bframes_in_gop ||
         bits_per_second != encoder_context-
Post by Mark Thompson
brc.bits_per_second[encoder_context->layer.num_layers - 1] ||
-        framerate_per_100s != encoder_context-
Post by Mark Thompson
brc.framerate_per_100s[encoder_context->layer.num_layers - 1]) {
+        framerate_num != encoder_context-
Post by Mark Thompson
brc.framerate_num[encoder_context->layer.num_layers - 1] ||
+        framerate_den != encoder_context-
Post by Mark Thompson
brc.framerate_den[encoder_context->layer.num_layers - 1]) {
         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-
Post by Mark Thompson
layer.num_layers - 1] = bits_per_second;
-        encoder_context->brc.framerate_per_100s[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = framerate_per_100s;
+        encoder_context->brc.framerate_num[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = framerate_num;
+        encoder_context->brc.framerate_den[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = framerate_den;
         encoder_context->brc.need_reset = 1;
     }
 
@@ -392,8 +411,10 @@
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-
Post by Mark Thompson
layer.num_layers - 1]) {
-        encoder_context->brc.framerate_per_100s[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = 3000;  // for the highest layer
+    if (!encoder_context->brc.framerate_num[encoder_context-
Post by Mark Thompson
layer.num_layers - 1]) {
+        // for the highest layer
+        encoder_context->brc.framerate_num[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = 30;
+        encoder_context->brc.framerate_den[encoder_context-
Post by Mark Thompson
layer.num_layers - 1] = 1;
         encoder_context->brc.need_reset = 1;
     }
 
@@ -480,7 +501,7 @@
intel_encoder_check_framerate_parameter(VADriverContextP ctx,
                                         struct intel_encoder_context
*encoder_context,
                                         VAEncMiscParameterFrameRate
*misc)
 {
-    int framerate_per_100s;
+    unsigned int framerate_num, framerate_den;
     int temporal_id = 0;
 
     if (encoder_context->layer.num_layers >= 2)
@@ -489,13 +510,19 @@
intel_encoder_check_framerate_parameter(VADriverContextP ctx,
     if (temporal_id >= encoder_context->layer.num_layers)
         return;
 
-    if (misc->framerate & 0xffff0000)
-        framerate_per_100s = (misc->framerate & 0xffff) * 100 /
((misc->framerate >> 16) & 0xffff);
-    else
-        framerate_per_100s = misc->framerate * 100;
+    if (misc->framerate & 0xffff0000) {
+        framerate_num = misc->framerate >> 16 & 0xffff;
+        framerate_den = misc->framerate       & 0xffff;
+    } else {
+        framerate_num = misc->framerate;
+        framerate_den = 1;
+    }
+    reduce_fraction(&framerate_num, &framerate_den);
 
-    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_num[temporal_id] !=
framerate_num ||
+        encoder_context->brc.framerate_den[temporal_id] !=
framerate_den) {
+        encoder_context->brc.framerate_num[temporal_id] =
framerate_num;
+        encoder_context->brc.framerate_den[temporal_id] =
framerate_den;
         encoder_context->brc.need_reset = 1;
     }
 }
diff --git a/src/i965_encoder.h b/src/i965_encoder.h
index 0b636d6..4547849 100644
--- a/src/i965_encoder.h
+++ b/src/i965_encoder.h
@@ -80,7 +80,8 @@ 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];
+        unsigned int framerate_num[MAX_TEMPORAL_LAYERS];
+        unsigned int framerate_den[MAX_TEMPORAL_LAYERS];
         unsigned int mb_rate_control[MAX_TEMPORAL_LAYERS];
         unsigned int target_percentage[MAX_TEMPORAL_LAYERS];
         unsigned int hrd_buffer_size;
Mark Thompson
2016-12-15 12:26:13 UTC
Permalink
Post by Xiang, Haihao
Hi Mark,
Thanks for your patch, is there any benefit to use a fraction? using
100 as framerate_den works well in the driver.
Everything the driver interacts with directly, on both sides, uses a fraction:

* In VAAPI, VAEncMiscParameterFrameRate contains a 16 / 16 fraction.

* Newer hardware uses a fraction directly where it requires a framerate (gen9_vp9_encoder.c:1204, gen9_vdenc.c:1648).

* H.264 and H.265 streams contain num_units_in_tick / time_scale.

* gstreamer (GstVideoInfo.fps_[nd]), libyami (VideoFrameRate.frameRate{Num,Denom}) and libavcodec (AVCodecContext.{time_base,framerate} are fraction structures) all represent framerate as a fraction.

The driver should use a fraction as well to preserve the exact value and be consistent with everything it interacts with.

Do you have any other concerns about the patch? If there are additional references outside the core driver code which I have missed somehow then I would be happy to update them as well.

Thanks,

- Mark
Sean V Kelley
2016-12-15 17:22:25 UTC
Permalink
I'm fine with this change and it adds consistency.

lgtm

Thanks,

Sean
Post by Mark Thompson
Post by Xiang, Haihao
Hi Mark,
Thanks for your patch, is there any benefit to use a fraction? using
100 as framerate_den works well in the driver.
* In VAAPI, VAEncMiscParameterFrameRate contains a 16 / 16 fraction.
* Newer hardware uses a fraction directly where it requires a framerate
(gen9_vp9_encoder.c:1204, gen9_vdenc.c:1648).
* H.264 and H.265 streams contain num_units_in_tick / time_scale.
* gstreamer (GstVideoInfo.fps_[nd]), libyami (VideoFrameRate.frameRate{Num,Denom})
and libavcodec (AVCodecContext.{time_base,framerate} are fraction
structures) all represent framerate as a fraction.
The driver should use a fraction as well to preserve the exact value and
be consistent with everything it interacts with.
Do you have any other concerns about the patch? If there are additional
references outside the core driver code which I have missed somehow then I
would be happy to update them as well.
Thanks,
- Mark
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
--
Sean V. Kelley <***@intel.com>
Open Source Technology Center / SSG
Intel Corp.
Xiang, Haihao
2016-12-16 01:12:03 UTC
Permalink
Post by Mark Thompson
Post by Xiang, Haihao
Hi Mark,
Thanks for your patch, is there any benefit to use a fraction? using
100 as framerate_den works well in the driver.
* In VAAPI, VAEncMiscParameterFrameRate contains a 16 / 16 fraction.
* Newer hardware uses a fraction directly where it requires a
framerate (gen9_vp9_encoder.c:1204, gen9_vdenc.c:1648).
* H.264 and H.265 streams contain num_units_in_tick / time_scale.
* gstreamer (GstVideoInfo.fps_[nd]), libyami
(VideoFrameRate.frameRate{Num,Denom}) and libavcodec
(AVCodecContext.{time_base,framerate} are fraction structures) all
represent framerate as a fraction.
The driver should use a fraction as well to preserve the exact value
and be consistent with everything it interacts with.
Agree, the driver should use a fraction. What I mean is that
(frames_per_100s / 100) works well in practice, so I prefer 1 field
instead of 2 fields in the corresponding structure.

Thanks
Haihao
Post by Mark Thompson
Do you have any other concerns about the patch?  If there are
additional references outside the core driver code which I have
missed somehow then I would be happy to update them as well.
Thanks,
- Mark
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Mark Thompson
2016-12-08 22:57:19 UTC
Permalink
Signed-off-by: Mark Thompson <***@jkqxz.net>
---
src/gen9_mfc.h | 6 -----
src/gen9_mfc_hevc.c | 67 +++++----------------------------------------
src/i965_encoder.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 80 insertions(+), 71 deletions(-)

diff --git a/src/gen9_mfc.h b/src/gen9_mfc.h
index f7dc572..8e7d5ad 100644
--- a/src/gen9_mfc.h
+++ b/src/gen9_mfc.h
@@ -153,12 +153,6 @@ struct gen9_hcpe_context {
int target_frame_size[3]; // I,P,B
double bits_per_frame;
double qpf_rounding_accumulator;
-
- double saved_bps;
- double saved_fps;
- int saved_intra_period;
- int saved_ip_period;
- int saved_idr_period;
} brc;

struct {
diff --git a/src/gen9_mfc_hevc.c b/src/gen9_mfc_hevc.c
index be5666c..b4eca4f 100644
--- a/src/gen9_mfc_hevc.c
+++ b/src/gen9_mfc_hevc.c
@@ -2214,11 +2214,9 @@ static void intel_hcpe_brc_init(struct encode_state *encode_state,
{
struct gen9_hcpe_context *mfc_context = encoder_context->mfc_context;
VAEncSequenceParameterBufferHEVC *pSequenceParameter = (VAEncSequenceParameterBufferHEVC *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterHRD* pParameterHRD = NULL;
- VAEncMiscParameterBuffer* pMiscParamHRD = NULL;

double bitrate = pSequenceParameter->bits_per_second * 1.0;
- double framerate = (double)pSequenceParameter->vui_time_scale / (double)pSequenceParameter->vui_num_units_in_tick;
+ double framerate = (double)encoder_context->brc.framerate_num[0] / (double)encoder_context->brc.framerate_den[0];
int inum = 1, pnum = 0, bnum = 0; /* Gop structure: number of I, P, B frames in the Gop. */
int intra_period = pSequenceParameter->intra_period;
int ip_period = pSequenceParameter->ip_period;
@@ -2238,12 +2236,6 @@ static void intel_hcpe_brc_init(struct encode_state *encode_state,
qp1_size = qp1_size * bpp;
qp51_size = qp51_size * bpp;

- if (!encode_state->misc_param[VAEncMiscParameterTypeHRD][0] || !encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer)
- return;
-
- pMiscParamHRD = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer;
- pParameterHRD = (VAEncMiscParameterHRD*)pMiscParamHRD->data;
-
if (pSequenceParameter->ip_period) {
pnum = (intra_period + ip_period - 1) / ip_period - 1;
bnum = intra_period - inum - pnum;
@@ -2262,7 +2254,7 @@ static void intel_hcpe_brc_init(struct encode_state *encode_state,

bpf = mfc_context->brc.bits_per_frame = bitrate / framerate;

- if (!pParameterHRD || pParameterHRD->buffer_size <= 0)
+ if (!encoder_context->brc.hrd_buffer_size)
{
mfc_context->hrd.buffer_size = bitrate * ratio;
mfc_context->hrd.current_buffer_fullness =
@@ -2270,7 +2262,7 @@ static void intel_hcpe_brc_init(struct encode_state *encode_state,
bitrate * ratio/2 : mfc_context->hrd.buffer_size / 2.;
}else
{
- buffer_size = (double)pParameterHRD->buffer_size ;
+ buffer_size = (double)encoder_context->brc.hrd_buffer_size;
if(buffer_size < bitrate * ratio_min)
{
buffer_size = bitrate * ratio_min;
@@ -2279,11 +2271,11 @@ static void intel_hcpe_brc_init(struct encode_state *encode_state,
buffer_size = bitrate * ratio_max ;
}
mfc_context->hrd.buffer_size =buffer_size;
- if(pParameterHRD->initial_buffer_fullness > 0)
+ if(encoder_context->brc.hrd_initial_buffer_fullness)
{
mfc_context->hrd.current_buffer_fullness =
- (double)(pParameterHRD->initial_buffer_fullness < mfc_context->hrd.buffer_size) ?
- pParameterHRD->initial_buffer_fullness : mfc_context->hrd.buffer_size / 2.;
+ (double)(encoder_context->brc.hrd_initial_buffer_fullness < mfc_context->hrd.buffer_size) ?
+ encoder_context->brc.hrd_initial_buffer_fullness : mfc_context->hrd.buffer_size / 2.;
}else
{
mfc_context->hrd.current_buffer_fullness = mfc_context->hrd.buffer_size / 2.;
@@ -2519,51 +2511,6 @@ int intel_hcpe_interlace_check(VADriverContextP ctx,
return 1;
}

-/*
- * Check whether the parameters related with CBR are updated and decide whether
- * it needs to reinitialize the configuration related with CBR.
- * Currently it will check the following parameters:
- * bits_per_second
- * frame_rate
- * gop_configuration(intra_period, ip_period, intra_idr_period)
- */
-static bool intel_hcpe_brc_updated_check(struct encode_state *encode_state,
- struct intel_encoder_context *encoder_context)
-{
- /* to do */
- unsigned int rate_control_mode = encoder_context->rate_control_mode;
- struct gen9_hcpe_context *mfc_context = encoder_context->mfc_context;
- double cur_fps, cur_bitrate;
- VAEncSequenceParameterBufferHEVC *pSequenceParameter;
-
-
- if (rate_control_mode != VA_RC_CBR) {
- return false;
- }
-
- pSequenceParameter = (VAEncSequenceParameterBufferHEVC *)encode_state->seq_param_ext->buffer;
-
- cur_bitrate = pSequenceParameter->bits_per_second;
- cur_fps = (double)pSequenceParameter->vui_time_scale /
- (double)pSequenceParameter->vui_num_units_in_tick;
-
- if ((cur_bitrate == mfc_context->brc.saved_bps) &&
- (cur_fps == mfc_context->brc.saved_fps) &&
- (pSequenceParameter->intra_period == mfc_context->brc.saved_intra_period) &&
- (pSequenceParameter->intra_idr_period == mfc_context->brc.saved_idr_period) &&
- (pSequenceParameter->intra_period == mfc_context->brc.saved_intra_period)) {
- /* the parameters related with CBR are not updaetd */
- return false;
- }
-
- mfc_context->brc.saved_ip_period = pSequenceParameter->ip_period;
- mfc_context->brc.saved_intra_period = pSequenceParameter->intra_period;
- mfc_context->brc.saved_idr_period = pSequenceParameter->intra_idr_period;
- mfc_context->brc.saved_fps = cur_fps;
- mfc_context->brc.saved_bps = cur_bitrate;
- return true;
-}
-
void intel_hcpe_brc_prepare(struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
@@ -2574,7 +2521,7 @@ void intel_hcpe_brc_prepare(struct encode_state *encode_state,
bool brc_updated;
assert(encoder_context->codec != CODEC_MPEG2);

- brc_updated = intel_hcpe_brc_updated_check(encode_state, encoder_context);
+ brc_updated = encoder_context->brc.need_reset;

/*Programing bit rate control */
if ((mfc_context->bit_rate_control_context[HEVC_SLICE_I].MaxSizeInWord == 0) ||
diff --git a/src/i965_encoder.c b/src/i965_encoder.c
index 75a6a1f..fc2557a 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -436,19 +436,87 @@ intel_encoder_check_brc_vp8_sequence_parameter(VADriverContextP ctx,
}

static VAStatus
+intel_encoder_check_brc_hevc_sequence_parameter(VADriverContextP ctx,
+ struct encode_state *encode_state,
+ struct intel_encoder_context *encoder_context)
+{
+ VAEncSequenceParameterBufferHEVC *seq_param = (VAEncSequenceParameterBufferHEVC*)encode_state->seq_param_ext->buffer;
+ unsigned int framerate_num, framerate_den;
+ unsigned int gop_size, num_iframes_in_gop, num_pframes_in_gop, num_bframes_in_gop;
+
+ if (!encoder_context->is_new_sequence)
+ return VA_STATUS_SUCCESS;
+ if (!seq_param)
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+
+ if (!seq_param->vui_time_scale || !seq_param->vui_num_units_in_tick) {
+ framerate_num = 30;
+ framerate_den = 1;
+ } else {
+ framerate_num = seq_param->vui_time_scale;
+ framerate_den = seq_param->vui_num_units_in_tick;
+ }
+ reduce_fraction(&framerate_num, &framerate_den);
+
+ num_iframes_in_gop = 1;
+ if (seq_param->intra_period == 0) {
+ gop_size = -1;
+ num_pframes_in_gop = -1;
+ } else if (seq_param->intra_period == 1) {
+ gop_size = 1;
+ num_pframes_in_gop = 0;
+ } else {
+ gop_size = seq_param->intra_period;
+ num_pframes_in_gop = (seq_param->intra_period + seq_param->ip_period - 1) / seq_param->ip_period - 1;
+ }
+ num_bframes_in_gop = gop_size - num_iframes_in_gop - num_pframes_in_gop;
+
+ if (encoder_context->brc.framerate_num[0] != framerate_num ||
+ encoder_context->brc.framerate_den[0] != framerate_den) {
+ encoder_context->brc.framerate_num[0] = framerate_num;
+ encoder_context->brc.framerate_den[0] = framerate_den;
+ encoder_context->brc.need_reset = 1;
+ }
+
+ if (encoder_context->brc.gop_size != gop_size ||
+ encoder_context->brc.num_iframes_in_gop != num_iframes_in_gop ||
+ 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.gop_size = gop_size;
+ encoder_context->brc.num_iframes_in_gop = num_iframes_in_gop;
+ 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.need_reset = 1;
+ }
+
+ if (encoder_context->brc.bits_per_second[0] != seq_param->bits_per_second) {
+ encoder_context->brc.bits_per_second[0] = seq_param->bits_per_second;
+ encoder_context->brc.need_reset = 1;
+ }
+
+ return VA_STATUS_SUCCESS;
+}
+
+static VAStatus
intel_encoder_check_brc_sequence_parameter(VADriverContextP ctx,
struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
- if (encoder_context->codec == CODEC_H264 ||
- encoder_context->codec == CODEC_H264_MVC)
+ switch (encoder_context->codec) {
+ case CODEC_H264:
+ case CODEC_H264_MVC:
return intel_encoder_check_brc_h264_sequence_parameter(ctx, encode_state, encoder_context);

- if (encoder_context->codec == CODEC_VP8)
+ case CODEC_VP8:
return intel_encoder_check_brc_vp8_sequence_parameter(ctx, encode_state, encoder_context);

- // TODO: other codecs
- return VA_STATUS_SUCCESS;
+ case CODEC_HEVC:
+ return intel_encoder_check_brc_hevc_sequence_parameter(ctx, encode_state, encoder_context);
+
+ default:
+ // TODO: other codecs
+ return VA_STATUS_SUCCESS;
+ }
}

static void
--
2.10.2
Mark Thompson
2016-12-08 22:57:59 UTC
Permalink
Signed-off-by: Mark Thompson <***@jkqxz.net>
---
src/gen6_mfc.h | 6 ------
src/gen8_mfc.c | 64 ++++++++++++----------------------------------------------
2 files changed, 13 insertions(+), 57 deletions(-)

diff --git a/src/gen6_mfc.h b/src/gen6_mfc.h
index 290922b..7a5d940 100644
--- a/src/gen6_mfc.h
+++ b/src/gen6_mfc.h
@@ -235,12 +235,6 @@ struct gen6_mfc_context
double qpf_rounding_accumulator[MAX_TEMPORAL_LAYERS];
int bits_prev_frame[MAX_TEMPORAL_LAYERS];
int prev_slice_type[MAX_TEMPORAL_LAYERS];
-
- double saved_bps;
- double saved_fps;
- int saved_intra_period;
- int saved_ip_period;
- int saved_idr_period;
} brc;

struct {
diff --git a/src/gen8_mfc.c b/src/gen8_mfc.c
index d1de92c..b0e7b08 100644
--- a/src/gen8_mfc.c
+++ b/src/gen8_mfc.c
@@ -3324,12 +3324,8 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
{
struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer* misc_param_hrd = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer;
- VAEncMiscParameterHRD* param_hrd = (VAEncMiscParameterHRD*)misc_param_hrd->data;
- VAEncMiscParameterBuffer* misc_param_frame_rate_buffer = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate* param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buffer->data;
double bitrate = seq_param->bits_per_second;
- unsigned int frame_rate = param_frame_rate->framerate;
+ double framerate = (double)encoder_context->brc.framerate_num[0] / (double)encoder_context->brc.framerate_den[0];
int inum = 1, pnum = 0;
int intra_period = seq_param->intra_period;
int width_in_mbs = ALIGN(seq_param->frame_width, 16) / 16;
@@ -3340,14 +3336,14 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,

mfc_context->brc.mode = encoder_context->rate_control_mode;

- mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period)/frame_rate) /
+ mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period) / framerate) /
(double)(inum + BRC_PWEIGHT * pnum ));
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P] = BRC_PWEIGHT * mfc_context->brc.target_frame_size[0][SLICE_TYPE_I];

mfc_context->brc.gop_nums[0][SLICE_TYPE_I] = inum;
mfc_context->brc.gop_nums[0][SLICE_TYPE_P] = pnum;

- mfc_context->brc.bits_per_frame[0] = bitrate/frame_rate;
+ mfc_context->brc.bits_per_frame[0] = bitrate / framerate;

mfc_context->brc.qp_prime_y[0][SLICE_TYPE_I] = gen8_mfc_vp8_qindex_estimate(encode_state,
mfc_context,
@@ -3358,10 +3354,15 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P],
0);

- mfc_context->hrd.buffer_size[0] = (double)param_hrd->buffer_size;
- mfc_context->hrd.current_buffer_fullness[0] =
- (double)(param_hrd->initial_buffer_fullness < mfc_context->hrd.buffer_size[0])?
- param_hrd->initial_buffer_fullness: mfc_context->hrd.buffer_size[0]/2.;
+ if (encoder_context->brc.hrd_buffer_size)
+ mfc_context->hrd.buffer_size[0] = (double)encoder_context->brc.hrd_buffer_size;
+ else
+ mfc_context->hrd.buffer_size[0] = 2.0 * bitrate;
+ if (encoder_context->brc.hrd_initial_buffer_fullness &&
+ encoder_context->brc.hrd_initial_buffer_fullness < mfc_context->hrd.buffer_size[0])
+ mfc_context->hrd.current_buffer_fullness[0] = (double)encoder_context->brc.hrd_initial_buffer_fullness;
+ else
+ mfc_context->hrd.current_buffer_fullness[0] = mfc_context->hrd.buffer_size[0] / 2.0;
mfc_context->hrd.target_buffer_fullness[0] = (double)mfc_context->hrd.buffer_size[0]/2.;
mfc_context->hrd.buffer_capacity[0] = (double)mfc_context->hrd.buffer_size[0]/max_frame_size;
mfc_context->hrd.violation_noted = 0;
@@ -3509,45 +3510,6 @@ static void gen8_mfc_vp8_hrd_context_update(struct encode_state *encode_state,
mfc_context->vui_hrd.i_frame_number++;
}

-/*
- * Check whether the parameters related with CBR are updated and decide whether
- * it needs to reinitialize the configuration related with CBR.
- * Currently it will check the following parameters:
- * bits_per_second
- * frame_rate
- * gop_configuration(intra_period, ip_period, intra_idr_period)
- */
-static bool gen8_mfc_vp8_brc_updated_check(struct encode_state *encode_state,
- struct intel_encoder_context *encoder_context)
-{
- unsigned int rate_control_mode = encoder_context->rate_control_mode;
- struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
- double cur_fps, cur_bitrate;
- VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer *misc_param_frame_rate_buf = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate *param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buf->data;
- unsigned int frame_rate = param_frame_rate->framerate;
-
- if (rate_control_mode != VA_RC_CBR) {
- return false;
- }
-
- cur_bitrate = seq_param->bits_per_second;
- cur_fps = frame_rate;
-
- if ((cur_bitrate == mfc_context->brc.saved_bps) &&
- (cur_fps == mfc_context->brc.saved_fps) &&
- (seq_param->intra_period == mfc_context->brc.saved_intra_period)) {
- /* the parameters related with CBR are not updaetd */
- return false;
- }
-
- mfc_context->brc.saved_intra_period = seq_param->intra_period;
- mfc_context->brc.saved_fps = cur_fps;
- mfc_context->brc.saved_bps = cur_bitrate;
- return true;
-}
-
static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
@@ -3557,7 +3519,7 @@ static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
bool brc_updated;
assert(encoder_context->codec != CODEC_MPEG2);

- brc_updated = gen8_mfc_vp8_brc_updated_check(encode_state, encoder_context);
+ brc_updated = encoder_context->brc.need_reset;

/*Programing bit rate control */
if (brc_updated) {
--
2.10.2
Charles, Daniel
2016-12-09 22:21:14 UTC
Permalink
Post by Mark Thompson
---
src/gen6_mfc.h | 6 ------
src/gen8_mfc.c | 64 ++++++++++++----------------------------------------------
2 files changed, 13 insertions(+), 57 deletions(-)
diff --git a/src/gen6_mfc.h b/src/gen6_mfc.h
index 290922b..7a5d940 100644
--- a/src/gen6_mfc.h
+++ b/src/gen6_mfc.h
@@ -235,12 +235,6 @@ struct gen6_mfc_context
double qpf_rounding_accumulator[MAX_TEMPORAL_LAYERS];
int bits_prev_frame[MAX_TEMPORAL_LAYERS];
int prev_slice_type[MAX_TEMPORAL_LAYERS];
-
- double saved_bps;
- double saved_fps;
- int saved_intra_period;
- int saved_ip_period;
- int saved_idr_period;
} brc;
struct {
diff --git a/src/gen8_mfc.c b/src/gen8_mfc.c
index d1de92c..b0e7b08 100644
--- a/src/gen8_mfc.c
+++ b/src/gen8_mfc.c
@@ -3324,12 +3324,8 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
{
struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer* misc_param_hrd = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer;
- VAEncMiscParameterHRD* param_hrd = (VAEncMiscParameterHRD*)misc_param_hrd->data;
- VAEncMiscParameterBuffer* misc_param_frame_rate_buffer = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate* param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buffer->data;
double bitrate = seq_param->bits_per_second;
- unsigned int frame_rate = param_frame_rate->framerate;
+ double framerate = (double)encoder_context->brc.framerate_num[0] / (double)encoder_context->brc.framerate_den[0];
int inum = 1, pnum = 0;
int intra_period = seq_param->intra_period;
int width_in_mbs = ALIGN(seq_param->frame_width, 16) / 16;
@@ -3340,14 +3336,14 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
mfc_context->brc.mode = encoder_context->rate_control_mode;
- mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period)/frame_rate) /
+ mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period) / framerate) /
(double)(inum + BRC_PWEIGHT * pnum ));
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P] = BRC_PWEIGHT * mfc_context->brc.target_frame_size[0][SLICE_TYPE_I];
mfc_context->brc.gop_nums[0][SLICE_TYPE_I] = inum;
mfc_context->brc.gop_nums[0][SLICE_TYPE_P] = pnum;
- mfc_context->brc.bits_per_frame[0] = bitrate/frame_rate;
+ mfc_context->brc.bits_per_frame[0] = bitrate / framerate;
mfc_context->brc.qp_prime_y[0][SLICE_TYPE_I] = gen8_mfc_vp8_qindex_estimate(encode_state,
mfc_context,
@@ -3358,10 +3354,15 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P],
0);
- mfc_context->hrd.buffer_size[0] = (double)param_hrd->buffer_size;
- mfc_context->hrd.current_buffer_fullness[0] =
- (double)(param_hrd->initial_buffer_fullness < mfc_context->hrd.buffer_size[0])?
- param_hrd->initial_buffer_fullness: mfc_context->hrd.buffer_size[0]/2.;
+ if (encoder_context->brc.hrd_buffer_size)
+ mfc_context->hrd.buffer_size[0] = (double)encoder_context->brc.hrd_buffer_size;
+ else
+ mfc_context->hrd.buffer_size[0] = 2.0 * bitrate;
+ if (encoder_context->brc.hrd_initial_buffer_fullness &&
+ encoder_context->brc.hrd_initial_buffer_fullness < mfc_context->hrd.buffer_size[0])
+ mfc_context->hrd.current_buffer_fullness[0] = (double)encoder_context->brc.hrd_initial_buffer_fullness;
+ else
+ mfc_context->hrd.current_buffer_fullness[0] = mfc_context->hrd.buffer_size[0] / 2.0;
mfc_context->hrd.target_buffer_fullness[0] = (double)mfc_context->hrd.buffer_size[0]/2.;
mfc_context->hrd.buffer_capacity[0] = (double)mfc_context->hrd.buffer_size[0]/max_frame_size;
mfc_context->hrd.violation_noted = 0;
@@ -3509,45 +3510,6 @@ static void gen8_mfc_vp8_hrd_context_update(struct encode_state *encode_state,
mfc_context->vui_hrd.i_frame_number++;
}
-/*
- * Check whether the parameters related with CBR are updated and decide whether
- * it needs to reinitialize the configuration related with CBR.
- * bits_per_second
- * frame_rate
- * gop_configuration(intra_period, ip_period, intra_idr_period)
- */
-static bool gen8_mfc_vp8_brc_updated_check(struct encode_state *encode_state,
- struct intel_encoder_context *encoder_context)
-{
- unsigned int rate_control_mode = encoder_context->rate_control_mode;
- struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
- double cur_fps, cur_bitrate;
- VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer *misc_param_frame_rate_buf = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate *param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buf->data;
- unsigned int frame_rate = param_frame_rate->framerate;
-
- if (rate_control_mode != VA_RC_CBR) {
- return false;
- }
-
- cur_bitrate = seq_param->bits_per_second;
- cur_fps = frame_rate;
-
- if ((cur_bitrate == mfc_context->brc.saved_bps) &&
- (cur_fps == mfc_context->brc.saved_fps) &&
- (seq_param->intra_period == mfc_context->brc.saved_intra_period)) {
- /* the parameters related with CBR are not updaetd */
- return false;
- }
-
- mfc_context->brc.saved_intra_period = seq_param->intra_period;
- mfc_context->brc.saved_fps = cur_fps;
- mfc_context->brc.saved_bps = cur_bitrate;
- return true;
-}
-
static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
@@ -3557,7 +3519,7 @@ static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
bool brc_updated;
assert(encoder_context->codec != CODEC_MPEG2);
- brc_updated = gen8_mfc_vp8_brc_updated_check(encode_state, encoder_context);
+ brc_updated = encoder_context->brc.need_reset;
/*Programing bit rate control */
if (brc_updated) {
This patch fails to compile like this

i965_encoder_vp8.c:2768:68: error: ‘struct <anonymous>’ has no member
named ‘framerate_per_100s’
--
Daniel.
Post by Mark Thompson
--
2.10.2
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Charles, Daniel
2016-12-09 23:23:15 UTC
Permalink
On Fri, Dec 9, 2016 at 2:21 PM, Charles, Daniel
Post by Charles, Daniel
Post by Mark Thompson
---
src/gen6_mfc.h | 6 ------
src/gen8_mfc.c | 64 ++++++++++++----------------------------------------------
2 files changed, 13 insertions(+), 57 deletions(-)
diff --git a/src/gen6_mfc.h b/src/gen6_mfc.h
index 290922b..7a5d940 100644
--- a/src/gen6_mfc.h
+++ b/src/gen6_mfc.h
@@ -235,12 +235,6 @@ struct gen6_mfc_context
double qpf_rounding_accumulator[MAX_TEMPORAL_LAYERS];
int bits_prev_frame[MAX_TEMPORAL_LAYERS];
int prev_slice_type[MAX_TEMPORAL_LAYERS];
-
- double saved_bps;
- double saved_fps;
- int saved_intra_period;
- int saved_ip_period;
- int saved_idr_period;
} brc;
struct {
diff --git a/src/gen8_mfc.c b/src/gen8_mfc.c
index d1de92c..b0e7b08 100644
--- a/src/gen8_mfc.c
+++ b/src/gen8_mfc.c
@@ -3324,12 +3324,8 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
{
struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer* misc_param_hrd = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer;
- VAEncMiscParameterHRD* param_hrd = (VAEncMiscParameterHRD*)misc_param_hrd->data;
- VAEncMiscParameterBuffer* misc_param_frame_rate_buffer = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate* param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buffer->data;
double bitrate = seq_param->bits_per_second;
- unsigned int frame_rate = param_frame_rate->framerate;
+ double framerate = (double)encoder_context->brc.framerate_num[0] / (double)encoder_context->brc.framerate_den[0];
int inum = 1, pnum = 0;
int intra_period = seq_param->intra_period;
int width_in_mbs = ALIGN(seq_param->frame_width, 16) / 16;
@@ -3340,14 +3336,14 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
mfc_context->brc.mode = encoder_context->rate_control_mode;
- mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period)/frame_rate) /
+ mfc_context->brc.target_frame_size[0][SLICE_TYPE_I] = (int)((double)((bitrate * intra_period) / framerate) /
(double)(inum + BRC_PWEIGHT * pnum ));
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P] = BRC_PWEIGHT * mfc_context->brc.target_frame_size[0][SLICE_TYPE_I];
mfc_context->brc.gop_nums[0][SLICE_TYPE_I] = inum;
mfc_context->brc.gop_nums[0][SLICE_TYPE_P] = pnum;
- mfc_context->brc.bits_per_frame[0] = bitrate/frame_rate;
+ mfc_context->brc.bits_per_frame[0] = bitrate / framerate;
mfc_context->brc.qp_prime_y[0][SLICE_TYPE_I] = gen8_mfc_vp8_qindex_estimate(encode_state,
mfc_context,
@@ -3358,10 +3354,15 @@ static void gen8_mfc_vp8_brc_init(struct encode_state *encode_state,
mfc_context->brc.target_frame_size[0][SLICE_TYPE_P],
0);
- mfc_context->hrd.buffer_size[0] = (double)param_hrd->buffer_size;
- mfc_context->hrd.current_buffer_fullness[0] =
- (double)(param_hrd->initial_buffer_fullness < mfc_context->hrd.buffer_size[0])?
- param_hrd->initial_buffer_fullness: mfc_context->hrd.buffer_size[0]/2.;
+ if (encoder_context->brc.hrd_buffer_size)
+ mfc_context->hrd.buffer_size[0] = (double)encoder_context->brc.hrd_buffer_size;
+ else
+ mfc_context->hrd.buffer_size[0] = 2.0 * bitrate;
+ if (encoder_context->brc.hrd_initial_buffer_fullness &&
+ encoder_context->brc.hrd_initial_buffer_fullness < mfc_context->hrd.buffer_size[0])
+ mfc_context->hrd.current_buffer_fullness[0] = (double)encoder_context->brc.hrd_initial_buffer_fullness;
+ else
+ mfc_context->hrd.current_buffer_fullness[0] = mfc_context->hrd.buffer_size[0] / 2.0;
mfc_context->hrd.target_buffer_fullness[0] = (double)mfc_context->hrd.buffer_size[0]/2.;
mfc_context->hrd.buffer_capacity[0] = (double)mfc_context->hrd.buffer_size[0]/max_frame_size;
mfc_context->hrd.violation_noted = 0;
@@ -3509,45 +3510,6 @@ static void gen8_mfc_vp8_hrd_context_update(struct encode_state *encode_state,
mfc_context->vui_hrd.i_frame_number++;
}
-/*
- * Check whether the parameters related with CBR are updated and decide whether
- * it needs to reinitialize the configuration related with CBR.
- * bits_per_second
- * frame_rate
- * gop_configuration(intra_period, ip_period, intra_idr_period)
- */
-static bool gen8_mfc_vp8_brc_updated_check(struct encode_state *encode_state,
- struct intel_encoder_context *encoder_context)
-{
- unsigned int rate_control_mode = encoder_context->rate_control_mode;
- struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
- double cur_fps, cur_bitrate;
- VAEncSequenceParameterBufferVP8 *seq_param = (VAEncSequenceParameterBufferVP8 *)encode_state->seq_param_ext->buffer;
- VAEncMiscParameterBuffer *misc_param_frame_rate_buf = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer;
- VAEncMiscParameterFrameRate *param_frame_rate = (VAEncMiscParameterFrameRate*)misc_param_frame_rate_buf->data;
- unsigned int frame_rate = param_frame_rate->framerate;
-
- if (rate_control_mode != VA_RC_CBR) {
- return false;
- }
-
- cur_bitrate = seq_param->bits_per_second;
- cur_fps = frame_rate;
-
- if ((cur_bitrate == mfc_context->brc.saved_bps) &&
- (cur_fps == mfc_context->brc.saved_fps) &&
- (seq_param->intra_period == mfc_context->brc.saved_intra_period)) {
- /* the parameters related with CBR are not updaetd */
- return false;
- }
-
- mfc_context->brc.saved_intra_period = seq_param->intra_period;
- mfc_context->brc.saved_fps = cur_fps;
- mfc_context->brc.saved_bps = cur_bitrate;
- return true;
-}
-
static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
@@ -3557,7 +3519,7 @@ static void gen8_mfc_vp8_brc_prepare(struct encode_state *encode_state,
bool brc_updated;
assert(encoder_context->codec != CODEC_MPEG2);
- brc_updated = gen8_mfc_vp8_brc_updated_check(encode_state, encoder_context);
+ brc_updated = encoder_context->brc.need_reset;
/*Programing bit rate control */
if (brc_updated) {
This patch fails to compile like this
i965_encoder_vp8.c:2768:68: error: ‘struct <anonymous>’ has no member
named ‘framerate_per_100s’
Nevermind this report, this file has nothing to do here
--
Daniel.
Post by Charles, Daniel
--
Daniel.
Post by Mark Thompson
--
2.10.2
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Mark Thompson
2016-12-08 23:01:53 UTC
Permalink
Also adds support for fractional framerate.

Signed-off-by: Mark Thompson <***@jkqxz.net>
---
src/gen9_vp9_encoder.c | 240 +++++++++----------------------------------------
src/gen9_vp9_encoder.h | 11 +--
src/i965_drv_video.c | 10 +--
src/i965_encoder.c | 36 ++++++++
src/i965_encoder.h | 1 +
5 files changed, 80 insertions(+), 218 deletions(-)

diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c
index 3ea1537..533bf81 100644
--- a/src/gen9_vp9_encoder.c
+++ b/src/gen9_vp9_encoder.c
@@ -1201,8 +1201,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
VP9_BRC_KBPS;
cmd->dw9.min_bit_rate = (vp9_state->min_bit_rate + VP9_BRC_KBPS - 1) / VP9_BRC_KBPS *
VP9_BRC_KBPS;
- cmd->dw10.frame_ratem = vp9_state->frame_rate;
- cmd->dw11.frame_rated = 1;
+ cmd->dw10.frame_ratem = vp9_state->framerate_num;
+ cmd->dw11.frame_rated = vp9_state->framerate_den;

cmd->dw14.avbr_accuracy = 30;
cmd->dw14.avbr_convergence = 150;
@@ -1235,8 +1235,8 @@ void gen9_vp9_set_curbe_brc(VADriverContextP ctx,
cmd->dw17.enable_dynamic_scaling = vp9_state->dys_in_use;
cmd->dw17.brc_overshoot_cbr_pct = 150;

- dInputBitsPerFrame = (double)(cmd->dw8.max_bit_rate) / (vp9_state->frame_rate);
- dbps_ratio = dInputBitsPerFrame / ((double)(vp9_state->vbv_buffer_size_in_bit) / 30);
+ dInputBitsPerFrame = (double)cmd->dw8.max_bit_rate * (double)vp9_state->framerate_den / (double)vp9_state->framerate_num;
+ dbps_ratio = dInputBitsPerFrame / (double)vp9_state->vbv_buffer_size_in_bit / 30.0;
if (dbps_ratio < 0.1)
dbps_ratio = 0.1;
if (dbps_ratio > 3.5)
@@ -1423,7 +1423,6 @@ gen9_vp9_brc_init_reset_kernel(VADriverContextP ctx,
brc_initreset_curbe.initbrc = !vp9_state->brc_inited;
brc_initreset_curbe.mbbrc_enabled = 0;
brc_initreset_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_initreset_curbe.frame_rate = vp9_state->frame_rate;

vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -1523,7 +1522,6 @@ gen9_vp9_brc_intra_dist_kernel(VADriverContextP ctx,
brc_intra_dist_curbe.initbrc = !vp9_state->brc_inited;
brc_intra_dist_curbe.mbbrc_enabled = 0;
brc_intra_dist_curbe.ref_frame_flag = vp9_state->ref_frame_flag;
- brc_intra_dist_curbe.frame_rate = vp9_state->frame_rate;

vme_context->pfn_set_curbe_brc(ctx, encode_state,
gpe_context,
@@ -3926,168 +3924,51 @@ gen9_encode_vp9_check_parameter(VADriverContextP ctx,
return VA_STATUS_ERROR_UNIMPLEMENTED;

if (vp9_state->brc_enabled) {
- if (vp9_state->brc_flag_check & VP9_BRC_FAILURE) {
- WARN_ONCE("Rate control misc_parameter is required for BRC\n");
- return VA_STATUS_ERROR_INVALID_PARAMETER;
- }
-
- if (vp9_state->first_frame) {
- unsigned int brc_flag;
- VAEncMiscParameterBuffer *misc_param;
-
- brc_flag = VP9_BRC_SEQ | VP9_BRC_RC;
- if ((vp9_state->brc_flag_check & brc_flag) != brc_flag) {
- WARN_ONCE("SPS/RC misc is required for BRC\n");
- return VA_STATUS_ERROR_INVALID_PARAMETER;
- }
+ if (vp9_state->first_frame || vp9_state->picture_coding_type == KEY_FRAME) {
+ vp9_state->brc_reset = encoder_context->brc.need_reset || vp9_state->first_frame;

/* 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_num[0] || !encoder_context->brc.framerate_den[0] ||
+ !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_num = encoder_context->brc.framerate_num[0];
+ vp9_state->framerate_den = encoder_context->brc.framerate_den[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;
+
+ 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;
+
} 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;
- }
- }
+ if (!encoder_context->brc.framerate_num[0] || !encoder_context->brc.framerate_den[0] ||
+ !encoder_context->brc.bits_per_second[0] || !encoder_context->brc.target_percentage[0] ||
+ !encoder_context->brc.window_size)
+ return VA_STATUS_ERROR_INVALID_PARAMETER;

- /* 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->gop_size = encoder_context->brc.gop_size;
+
+ vp9_state->framerate_num = encoder_context->brc.framerate_num[0];
+ vp9_state->framerate_den = encoder_context->brc.framerate_den[0];

- /* 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;
- }
- }
+ 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;
}
}
}
@@ -5805,47 +5686,6 @@ static void
gen9_vp9_pak_brc_prepare(struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
{
- struct gen9_encoder_context_vp9 *pak_context = encoder_context->mfc_context;
- struct gen9_vp9_state *vp9_state;
-
- vp9_state = (struct gen9_vp9_state *)(encoder_context->enc_priv_state);
-
- if (!vp9_state || !pak_context)
- return;
-
- if (vp9_state->brc_enabled) {
- /* check the buffer related with BRC */
- vp9_state->brc_flag_check = 0;
- if (encode_state->seq_param_ext && encode_state->seq_param_ext->buffer) {
- vp9_state->brc_flag_check |= VP9_BRC_SEQ;
- }
-
- /* Frame_rate */
- if (encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0] &&
- encode_state->misc_param[VAEncMiscParameterTypeFrameRate][0]->buffer) {
- vp9_state->brc_flag_check |= VP9_BRC_FR;
- }
-
- /* HRD */
- if (encode_state->misc_param[VAEncMiscParameterTypeRateControl][0] &&
- encode_state->misc_param[VAEncMiscParameterTypeRateControl][0]->buffer) {
- vp9_state->brc_flag_check |= VP9_BRC_RC;
- }
-
- if (encode_state->misc_param[VAEncMiscParameterTypeHRD][0] &&
- encode_state->misc_param[VAEncMiscParameterTypeHRD][0]->buffer) {
- vp9_state->brc_flag_check |= VP9_BRC_HRD;
- }
-
- /*
- * If user-app doesn't pass the buffer related with BRC for the first
- * frame, the error flag is returned.
- */
- if (vp9_state->brc_flag_check == 0 && vp9_state->first_frame) {
- vp9_state->brc_flag_check |= VP9_BRC_FAILURE;
- }
- }
- return;
}

static void
diff --git a/src/gen9_vp9_encoder.h b/src/gen9_vp9_encoder.h
index ad2d875..3c47ba3 100644
--- a/src/gen9_vp9_encoder.h
+++ b/src/gen9_vp9_encoder.h
@@ -1552,7 +1552,6 @@ struct gen9_vp9_brc_curbe_param
int32_t brc_num_pak_passes;
bool multi_ref_qp_check;
int16_t frame_number;
- int32_t frame_rate;
VP9_MEDIA_STATE_TYPE media_state_type;
};

@@ -1649,12 +1648,6 @@ typedef struct _vp9_frame_status_
uint8_t intra_only;
} vp9_frame_status;

-#define VP9_BRC_SEQ 0x01
-#define VP9_BRC_HRD 0x02
-#define VP9_BRC_RC 0x04
-#define VP9_BRC_FR 0x08
-#define VP9_BRC_FAILURE (1 << 31)
-
struct gen9_hcpe_pipe_mode_select_param
{
uint32_t codec_mode;
@@ -1925,7 +1918,8 @@ struct gen9_vp9_state {
unsigned long init_vbv_buffer_fullness_in_bit;
unsigned long vbv_buffer_size_in_bit;
int frame_number;
- uint32_t frame_rate;
+ uint32_t framerate_num;
+ uint32_t framerate_den;
uint8_t ref_frame_flag;
uint8_t dys_ref_frame_flag;
uint8_t picture_coding_type;
@@ -1936,7 +1930,6 @@ struct gen9_vp9_state {
int target_percentage;
unsigned int mb_data_offset;
int curr_pak_pass;
- unsigned int brc_flag_check;
bool first_frame;
bool dys_enabled;
bool dys_in_use;
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index e602b4e..15920f5 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -2794,7 +2794,7 @@ i965_BeginPicture(VADriverContextP ctx,
struct object_surface *obj_surface = SURFACE(render_target);
struct object_config *obj_config;
VAStatus vaStatus = VA_STATUS_SUCCESS;
- int i, j;
+ int i;

ASSERT_RET(obj_context, VA_STATUS_ERROR_INVALID_CONTEXT);
ASSERT_RET(obj_surface, VA_STATUS_ERROR_INVALID_SURFACE);
@@ -2848,14 +2848,6 @@ i965_BeginPicture(VADriverContextP ctx,
i965_release_buffer_store(&obj_context->codec_state.encode.misc_param[VAEncMiscParameterTypeROI][0]);

i965_release_buffer_store(&obj_context->codec_state.encode.encmb_map);
-
- if (obj_config->profile == VAProfileVP9Profile0) {
- for (i = 0; i < ARRAY_ELEMS(obj_context->codec_state.encode.misc_param); i++)
- for (j = 0; j < ARRAY_ELEMS(obj_context->codec_state.encode.misc_param[0]); j++)
- i965_release_buffer_store(&obj_context->codec_state.encode.misc_param[i][j]);
-
- i965_release_buffer_store(&obj_context->codec_state.encode.seq_param_ext);
- }
} else {
obj_context->codec_state.decode.current_render_target = render_target;
i965_release_buffer_store(&obj_context->codec_state.decode.pic_param);
diff --git a/src/i965_encoder.c b/src/i965_encoder.c
index fc2557a..8af0df5 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -498,6 +498,34 @@ intel_encoder_check_brc_hevc_sequence_parameter(VADriverContextP ctx,
}

static VAStatus
+intel_encoder_check_brc_vp9_sequence_parameter(VADriverContextP ctx,
+ struct encode_state *encode_state,
+ struct intel_encoder_context *encoder_context)
+{
+ VAEncSequenceParameterBufferVP9 *seq_param = (VAEncSequenceParameterBufferVP9*)encode_state->seq_param_ext->buffer;
+ unsigned int gop_size;
+
+ if (!encoder_context->is_new_sequence)
+ return VA_STATUS_SUCCESS;
+ if (!seq_param)
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+
+ if (seq_param->intra_period == 0)
+ gop_size = -1; // Dummy value (infinity).
+ else
+ gop_size = seq_param->intra_period;
+
+ if (encoder_context->brc.bits_per_second[0] != seq_param->bits_per_second ||
+ encoder_context->brc.gop_size != gop_size) {
+ encoder_context->brc.bits_per_second[0] = seq_param->bits_per_second;
+ encoder_context->brc.gop_size = gop_size;
+ encoder_context->brc.need_reset = 1;
+ }
+
+ return VA_STATUS_SUCCESS;
+}
+
+static VAStatus
intel_encoder_check_brc_sequence_parameter(VADriverContextP ctx,
struct encode_state *encode_state,
struct intel_encoder_context *encoder_context)
@@ -513,6 +541,9 @@ intel_encoder_check_brc_sequence_parameter(VADriverContextP ctx,
case CODEC_HEVC:
return intel_encoder_check_brc_hevc_sequence_parameter(ctx, encode_state, encoder_context);

+ case CODEC_VP9:
+ return intel_encoder_check_brc_vp9_sequence_parameter(ctx, encode_state, encoder_context);
+
default:
// TODO: other codecs
return VA_STATUS_SUCCESS;
@@ -549,6 +580,11 @@ intel_encoder_check_rate_control_parameter(VADriverContextP ctx,
encoder_context->brc.target_percentage[temporal_id] = misc->target_percentage;
encoder_context->brc.need_reset = 1;
}
+
+ if (encoder_context->brc.window_size != misc->window_size) {
+ encoder_context->brc.window_size = misc->window_size;
+ encoder_context->brc.need_reset = 1;
+ }
}

static void
diff --git a/src/i965_encoder.h b/src/i965_encoder.h
index 4547849..f71031b 100644
--- a/src/i965_encoder.h
+++ b/src/i965_encoder.h
@@ -86,6 +86,7 @@ struct intel_encoder_context
unsigned int target_percentage[MAX_TEMPORAL_LAYERS];
unsigned int hrd_buffer_size;
unsigned int hrd_initial_buffer_fullness;
+ unsigned int window_size;
unsigned int need_reset;

unsigned int num_roi;
--
2.10.2
Zhao Yakui
2016-12-06 01:37:46 UTC
Permalink
Post by Mark Thompson
---
It's already returned as a usable surface format for the video processor, but is missing from the list returned by vaQueryImageFormats().
src/i965_drv_video.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index 79a2aec..d83427c 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -268,6 +268,8 @@ i965_image_formats_map[I965_MAX_IMAGE_FORMATS + 1] = {
{ VA_FOURCC_BGRX, VA_LSB_FIRST, 32, 24, 0x00ff0000, 0x0000ff00, 0x000000ff } },
{ I965_SURFACETYPE_YUV,
{ VA_FOURCC_P010, VA_LSB_FIRST, 24, } },
+ { I965_SURFACETYPE_YUV,
+ { VA_FOURCC_I010, VA_LSB_FIRST, 24, } },
};
The format returned by vaQueryImageFormats is mainly used for calling
vaCreateImage. That is to say: if we want to add the I010 to the
image_format list, we should firstly add the support in i965_CreateImage.

For the 10-bit surfaces(P010, I010), it will be preferred that it is
created by using vaCreateSurfaces. If the image is needed, the
vaDeriveImage can be used instead.

Is it OK to you?

Thanks
Yakui
Post by Mark Thompson
/* List of supported subpicture formats */
Mark Thompson
2016-12-06 21:26:26 UTC
Permalink
Post by Mark Thompson
---
It's already returned as a usable surface format for the video processor, but is missing from the list returned by vaQueryImageFormats().
src/i965_drv_video.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index 79a2aec..d83427c 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -268,6 +268,8 @@ i965_image_formats_map[I965_MAX_IMAGE_FORMATS + 1] = {
{ VA_FOURCC_BGRX, VA_LSB_FIRST, 32, 24, 0x00ff0000, 0x0000ff00, 0x000000ff } },
{ I965_SURFACETYPE_YUV,
{ VA_FOURCC_P010, VA_LSB_FIRST, 24, } },
+ { I965_SURFACETYPE_YUV,
+ { VA_FOURCC_I010, VA_LSB_FIRST, 24, } },
};
The format returned by vaQueryImageFormats is mainly used for calling vaCreateImage. That is to say: if we want to add the I010 to the image_format list, we should firstly add the support in i965_CreateImage.
For the 10-bit surfaces(P010, I010), it will be preferred that it is created by using vaCreateSurfaces. If the image is needed, the vaDeriveImage can be used instead.
Is it OK to you?
Right, testing a bit more I can see there is more support missing than I initially thought. It needs support throughout the VPP processing chain for vaGetImage() in particular to work, so the cases I was hoping for aren't yet possible.

The use-case I'm thinking of here is to be able to move data back and forth between software (typically LSB-packed three-plane YUV 4:2:0, here being called I010: e.g. libx264, libvpx, libavcodec) and hardware (all MSB-packed two-plane YUV 4:2:0, i.e. P010) without an additional shift/(de)interleave step in software. Since we want to avoid CPU involvement where possible, full image support is helpful because we can get the GPU to deal with the copying/conversion between normal memory and surfaces - vaDeriveImage() is not really sufficient because the CPU then has to do the copy (and also has to deal with the uncached memory requiring the streaming operations, making it even more awkward to use).

I'll drop this patch for now, since the changes required to make that work are somewhat more involved than I was hoping.

Thanks,

- Mark
Loading...