Mark Thompson wrote:
On 26/01/17 18:26, Andy Furniss wrote:
Tested with ffmpeg and gst-vaapi. Without this bits per
frame is set way too low.
Signed-off-by: Andy Furniss <adf.li...@gmail.com>
---
src/gallium/state_trackers/va/picture.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/gallium/state_trackers/va/picture.c
b/src/gallium/state_trackers/va/picture.c
index 82584ea..a024437 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -119,14 +119,30 @@ getEncParamPreset(vlVaContext *context)
context->desc.h264enc.rate_ctrl.fill_data_enable = 1;
context->desc.h264enc.rate_ctrl.enforce_hrd = 1;
context->desc.h264enc.enable_vui = false;
- if (context->desc.h264enc.rate_ctrl.frame_rate_num == 0)
- context->desc.h264enc.rate_ctrl.frame_rate_num = 30;
- context->desc.h264enc.rate_ctrl.target_bits_picture =
- context->desc.h264enc.rate_ctrl.target_bitrate /
context->desc.h264enc.rate_ctrl.frame_rate_num;
- context->desc.h264enc.rate_ctrl.peak_bits_picture_integer =
- context->desc.h264enc.rate_ctrl.peak_bitrate /
context->desc.h264enc.rate_ctrl.frame_rate_num;
- context->desc.h264enc.rate_ctrl.peak_bits_picture_fraction = 0;
+ if (context->desc.h264enc.rate_ctrl.frame_rate_num == 0 ||
+ context->desc.h264enc.rate_ctrl.frame_rate_den == 0) {
+ context->desc.h264enc.rate_ctrl.frame_rate_num = 30;
+ context->desc.h264enc.rate_ctrl.frame_rate_den = 1;
+ }
+ if (context->desc.h264enc.rate_ctrl.frame_rate_den > 1) {
+ context->desc.h264enc.rate_ctrl.target_bits_picture =
+ context->desc.h264enc.rate_ctrl.target_bitrate /
+ (context->desc.h264enc.rate_ctrl.frame_rate_num /
+ context->desc.h264enc.rate_ctrl.frame_rate_den + 1);
This is still doing more rounding than necessary? (Consider 0.5fps as 1/2,
say.) Don't you want:
Hmm, sub 1 fps is going to be treated as 1 fps with my code but -
context->desc.h264enc.rate_ctrl.target_bits_picture =
(context->desc.h264enc.rate_ctrl.target_bitrate *
context->desc.h264enc.rate_ctrl.frame_rate_den) /
context->desc.h264enc.rate_ctrl.frame_rate_num;
(And no need for any extra +1 or conditional, because you've already made sure
that neither of them are zero.)
This will overflow, with high bitrate plus ntsc denom = 1001
Could use floats I guess like I did here.
https://cgit.freedesktop.org/mesa/mesa/commit/?id=a5993022275c20061ac025d9adc26c5f9d02afee
I don't know what the preference is between float and int.
+ context->desc.h264enc.rate_ctrl.peak_bits_picture_integer =
+ context->desc.h264enc.rate_ctrl.peak_bitrate /
+ (context->desc.h264enc.rate_ctrl.frame_rate_num /
+ context->desc.h264enc.rate_ctrl.frame_rate_den + 1);
Similarly this one.
+ } else {
+ context->desc.h264enc.rate_ctrl.target_bits_picture =
+ context->desc.h264enc.rate_ctrl.target_bitrate /
+ context->desc.h264enc.rate_ctrl.frame_rate_num;
+ context->desc.h264enc.rate_ctrl.peak_bits_picture_integer =
+ context->desc.h264enc.rate_ctrl.peak_bitrate /
+ context->desc.h264enc.rate_ctrl.frame_rate_num;
+ }
Doing that would also avoid needing the if at all.
+ context->desc.h264enc.rate_ctrl.peak_bits_picture_fraction = 0;
context->desc.h264enc.ref_pic_mode = 0x00000201;
}
@@ -362,7 +378,7 @@ handleVAEncSequenceParameterBufferType(vlVaDriver *drv,
vlVaContext *context, vl
context->gop_coeff = VL_VA_ENC_GOP_COEFF;
context->desc.h264enc.gop_size = h264->intra_idr_period *
context->gop_coeff;
context->desc.h264enc.rate_ctrl.frame_rate_num = h264->time_scale / 2;
- context->desc.h264enc.rate_ctrl.frame_rate_den = 1;
+ context->desc.h264enc.rate_ctrl.frame_rate_den = h264->num_units_in_tick;
return VA_STATUS_SUCCESS;
}
Otherwise LGTM :)
Thanks.
Thanks,
- Mark
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
