On 13/04/17 13:34, Jun Zhao wrote:
> From 1fa48b45fe962d8c342d158d9c16ce24139ffd84 Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.z...@intel.com>
> Date: Thu, 13 Apr 2017 20:07:10 +0800
> Subject: [PATCH] vaapi_encode: Fix the vaapi h264 encoder VBR/CBR metadata
> setting error.
>
> before this fix, mediainfo check the bit rate control mode metadata info
> is wrong.
>
> Reproduce step:
> encode with CBR or VBR mode like this:
> ./ffmpeg -y -hwaccel vaapi -vaapi_device /dev/dri/renderD128 \
> -hwaccel_output_format vaapi -i input.mp4 -an -c:v h264_vaapi \
> -maxrate 5M -b:v 5M output_cbr.mp4
>
> ./ffmpeg -y -hwaccel vaapi -vaapi_device /dev/dri/renderD128 \
> -hwaccel_output_format vaapi -i input.mp4 -an -c:v h264_vaapi \
> -maxrate 6M -b:v 5M output_cbr.mp4
>
> then use the mediainfo check the bit rate control mode.
>
> Signed-off-by: Jun Zhao <jun.z...@intel.com>
Not yet generating the HRD and timing information in VBR mode was deliberate,
because I don't know enough about the conformance properties of the new
proprietary rate controller which the Intel driver has switched to. The old
CBR mode was straightforward to verify from the source code, and it seemed
reasonable to assume that the new CBR mode would be too given the constraints
on it (and also because existing versions of lavc couldn't be modified).
Can you say anything about the HRD conformance of the Intel driver here, or was
this patch purely for the metadata output (which may not actually match the
stream, hence my concern)?
Only the Intel driver is relevant to this as far as I know - the Mesa driver
does not support the packed headers necessary for this information to be
included in the stream.
> ---
> libavcodec/vaapi_encode_h264.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 92e2955..47d0c94 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -760,7 +760,7 @@ static int
> vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
> char tmp[256];
> size_t header_len;
>
> - if (index == 0 && ctx->va_rc_mode == VA_RC_CBR) {
> + if (index == 0 && (ctx->va_rc_mode == VA_RC_CBR || ctx->va_rc_mode ==
> VA_RC_VBR)) {
> *type = VAEncPackedHeaderH264_SEI;
>
> init_put_bits(&pbc, tmp, sizeof(tmp));
> @@ -868,7 +868,7 @@ static int
> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
> mseq->fixed_frame_rate_flag = 0;
> }
>
> - if (ctx->va_rc_mode == VA_RC_CBR) {
> + if (ctx->va_rc_mode == VA_RC_CBR || ctx->va_rc_mode == VA_RC_VBR) {
> priv->send_timing_sei = 1;
> mseq->nal_hrd_parameters_present_flag = 1;
>
> @@ -886,8 +886,7 @@ static int
> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
> mseq->cpb_size_value_minus1[0] =
> (ctx->hrd_params.hrd.buffer_size >> mseq->cpb_size_scale +
> 4) - 1;
>
> - // CBR mode isn't actually available here, despite naming.
> - mseq->cbr_flag[0] = 0;
> + mseq->cbr_flag[0] = (ctx->va_rc_mode == VA_RC_CBR ? 1 : 0);
As the comment notes, the necessary CBR mode isn't actually available. Drivers
only offer "soft" CBR, which can result in HRD overflow; this specifies "hard"
CBR, so if you want to set this flag you would need to insert filler NAL units
as well.
Thanks,
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
