| Message ID | 552e2be7-6687-679b-b5e5-05cb0057a3de@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 31/10/17 02:37, Jun Zhao wrote: > From d1e105057e93e7c2788d6d684292db9008fbf3ac Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.zhao@intel.com> > Date: Tue, 31 Oct 2017 10:19:08 +0800 > Subject: [PATCH 2/2] lavc/vaapi_encode_h264: correct bit_rate_scale setting. > > As H264 Spec 2012 E.2.2, bit_rate_scale means the max input bit rate. > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> > --- > libavcodec/vaapi_encode_h264.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c > index 1d43e934ef..27a810c64e 100644 > --- a/libavcodec/vaapi_encode_h264.c > +++ b/libavcodec/vaapi_encode_h264.c > @@ -406,7 +406,7 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) > // Try to scale these to a sensible range so that the > // golomb encode of the value is not overlong. > hrd->bit_rate_scale = > - av_clip_uintp2(av_log2(avctx->bit_rate) - 15 - 6, 4); > + av_clip_uintp2(av_log2(FFMAX(avctx->bit_rate, avctx->rc_max_rate)) - 15 - 6, 4); > hrd->bit_rate_value_minus1[0] = > (avctx->bit_rate >> hrd->bit_rate_scale + 6) - 1; > > -- > 2.14.1 > Can you offer some more justification for that? I think the change is wrong: consider under what circumstances overflow/underflow occurs. - Mark
On 2017/10/31 18:26, Mark Thompson wrote: > On 31/10/17 02:37, Jun Zhao wrote: >> From d1e105057e93e7c2788d6d684292db9008fbf3ac Mon Sep 17 00:00:00 2001 >> From: Jun Zhao <jun.zhao@intel.com> >> Date: Tue, 31 Oct 2017 10:19:08 +0800 >> Subject: [PATCH 2/2] lavc/vaapi_encode_h264: correct bit_rate_scale setting. >> >> As H264 Spec 2012 E.2.2, bit_rate_scale means the max input bit rate. >> >> Signed-off-by: Jun Zhao <jun.zhao@intel.com> >> Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> >> --- >> libavcodec/vaapi_encode_h264.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c >> index 1d43e934ef..27a810c64e 100644 >> --- a/libavcodec/vaapi_encode_h264.c >> +++ b/libavcodec/vaapi_encode_h264.c >> @@ -406,7 +406,7 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) >> // Try to scale these to a sensible range so that the >> // golomb encode of the value is not overlong. >> hrd->bit_rate_scale = >> - av_clip_uintp2(av_log2(avctx->bit_rate) - 15 - 6, 4); >> + av_clip_uintp2(av_log2(FFMAX(avctx->bit_rate, avctx->rc_max_rate)) - 15 - 6, 4); >> hrd->bit_rate_value_minus1[0] = >> (avctx->bit_rate >> hrd->bit_rate_scale + 6) - 1; avctx->bit_rate will change with FFMAX(avctx->bit_rate, avctx->rc_max_rate) when setting bit_rate_value_minus1. >> >> -- >> 2.14.1 >> > Can you offer some more justification for that? I think the change is wrong: consider under what circumstances overflow/underflow occurs. In VBR/CBR mode, I think max bit rate = FFMAX(bit_rate, rc_max_rate) for bit_rate_scale/bit_rate_value_minus1. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 31/10/17 15:18, Jun Zhao wrote: > On 2017/10/31 18:26, Mark Thompson wrote: >> On 31/10/17 02:37, Jun Zhao wrote: >>> From d1e105057e93e7c2788d6d684292db9008fbf3ac Mon Sep 17 00:00:00 2001 >>> From: Jun Zhao <jun.zhao@intel.com> >>> Date: Tue, 31 Oct 2017 10:19:08 +0800 >>> Subject: [PATCH 2/2] lavc/vaapi_encode_h264: correct bit_rate_scale setting. >>> >>> As H264 Spec 2012 E.2.2, bit_rate_scale means the max input bit rate. >>> >>> Signed-off-by: Jun Zhao <jun.zhao@intel.com> >>> Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> >>> --- >>> libavcodec/vaapi_encode_h264.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c >>> index 1d43e934ef..27a810c64e 100644 >>> --- a/libavcodec/vaapi_encode_h264.c >>> +++ b/libavcodec/vaapi_encode_h264.c >>> @@ -406,7 +406,7 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) >>> // Try to scale these to a sensible range so that the >>> // golomb encode of the value is not overlong. >>> hrd->bit_rate_scale = >>> - av_clip_uintp2(av_log2(avctx->bit_rate) - 15 - 6, 4); >>> + av_clip_uintp2(av_log2(FFMAX(avctx->bit_rate, avctx->rc_max_rate)) - 15 - 6, 4); >>> hrd->bit_rate_value_minus1[0] = >>> (avctx->bit_rate >> hrd->bit_rate_scale + 6) - 1; > > avctx->bit_rate will change with FFMAX(avctx->bit_rate, avctx->rc_max_rate) when setting bit_rate_value_minus1. The two values combine to specify the BitRate[0] value for this HRD instance. The same value has to be used for both the scale and the value parts. >>> >>> -- >>> 2.14.1 >>> >> Can you offer some more justification for that? I think the change is wrong: consider under what circumstances overflow/underflow occurs. > In VBR/CBR mode, I think max bit rate = FFMAX(bit_rate, rc_max_rate) for > bit_rate_scale/bit_rate_value_minus1. The bitrate here is how fast the CPB fills. It is not the same as the RC max rate, which will only affect how fast the CPB can be emptied. - Mark
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index 1d43e934ef..27a810c64e 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -406,7 +406,7 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) // Try to scale these to a sensible range so that the // golomb encode of the value is not overlong. hrd->bit_rate_scale = - av_clip_uintp2(av_log2(avctx->bit_rate) - 15 - 6, 4); + av_clip_uintp2(av_log2(FFMAX(avctx->bit_rate, avctx->rc_max_rate)) - 15 - 6, 4); hrd->bit_rate_value_minus1[0] = (avctx->bit_rate >> hrd->bit_rate_scale + 6) - 1;