[FFmpeg-devel] [FFmpeg-cvslog] lavc/h264_levels: Avoid integer overflow in bitrate
sw at jkqxz.net
Thu Sep 27 01:31:29 EEST 2018
On 26/09/18 23:18, Carl Eugen Hoyos wrote:
> 2018-09-27 0:15 GMT+02:00, Mark Thompson <sw at jkqxz.net>:
>> On 26/09/18 22:43, Carl Eugen Hoyos wrote:
>>> 2018-09-25 0:16 GMT+02:00, Mark Thompson <git at videolan.org>:
>>>> ffmpeg | branch: master | Mark Thompson <sw at jkqxz.net> | Mon Sep 24
>>>> 2018 +0100| [581b4125aa187f2cf848d7a27e6128573c80dc64] | committer: Mark
>>>> lavc/h264_levels: Avoid integer overflow in bitrate
>>>> Fixes CID #1439656.
>>>> libavcodec/h264_levels.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> diff --git a/libavcodec/h264_levels.c b/libavcodec/h264_levels.c
>>>> index 6b4e18a914..737b7dcf06 100644
>>>> --- a/libavcodec/h264_levels.c
>>>> +++ b/libavcodec/h264_levels.c
>>>> @@ -105,7 +105,7 @@ const H264LevelDescriptor *ff_h264_guess_level(int
>>>> if (level->constraint_set3_flag && no_cs3f)
>>>> - if (bitrate > level->max_br * h264_get_br_factor(profile_idc))
>>>> + if (bitrate > (int64_t)level->max_br *
>>> If the overflow is possible at all (I doubt it), wouldn't it be cleaner
>>> to change the type of cpb_br_nal_factor to uint32_t?
>> Some of the largest cases overflow 32-bit signed int - e.g. level 6.2 in
>> High 10 allows up to 800000 * 3600 = 2880000000 bps. (High profile or lower
>> doesn't have a cpbBrNalFactor large enough to hit this case.)
> But max_br is already unsigned, what's wrong with making cpb_br_nal_factor
> also unsigned?
Making the whole calculation int64_t avoids the possibility that a future level or profile addition would cause this expression to overflow the 32-bit unsigned case as well and require further changes, the need for which would probably not be noticed immediately. (H.265 does have cases larger than 2^32 here.)
>> I used int64_t because that's the type of bitrate, on the other side of the
> Does that simplify things for the compiler?
It's might be slightly better on 64-bit machines (no need to zero-extend the 32-bit unsigned value) and slightly worse on 32-bit (additional handling of the top half), but I think the point above is more important.
More information about the ffmpeg-devel