[FFmpeg-devel,2/2] lavc/vaapi_encode_h264: correct bit_rate_scale setting.

Submitted by Jun Zhao on Oct. 31, 2017, 2:37 a.m.

Details

Message ID 552e2be7-6687-679b-b5e5-05cb0057a3de@gmail.com
State New
Headers show

Commit Message

Jun Zhao Oct. 31, 2017, 2:37 a.m.
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(-)

Comments

Mark Thompson Oct. 31, 2017, 10:26 a.m.
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
Jun Zhao Oct. 31, 2017, 3:18 p.m.
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
Mark Thompson Oct. 31, 2017, 3:40 p.m.
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

Patch hide | download patch | download mbox

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;