[FFmpeg-devel] [PATCH v1] avcodec/utils: use av_rescale()

[Andreas Rheinhardt]

lance.lmwang at gmail.com:
> On Mon, Apr 20, 2020 at 07:00:55PM +0800, lance.lmwang at gmail.com wrote:
>> From: Limin Wang <lance.lmwang at gmail.com>
>>
>> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>> ---
>>  libavcodec/utils.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 26c038dfd9..005d596dfd 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2229,8 +2229,8 @@ int64_t ff_guess_coded_bitrate(AVCodecContext *avctx)
>>          const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
>>          bits_per_coded_sample = av_get_bits_per_pixel(desc);
>>      }
>> -    bitrate = (int64_t)bits_per_coded_sample * avctx->width * avctx->height *
>> -              framerate.num / framerate.den;
>> +    bitrate = av_rescale(avctx->width * avctx->height,
>> +                bits_per_coded_sample * framerate.num, framerate.den);

Is this supposed to be a cosmetic patch or do you have a testcase where
it leads to better results? Anyway, bits_per_coded_sample *
framerate.num might overflow (the function parameter is 64bit, but the
calculation will nevertheless be performed as int * int).
(I am not saying that the overflow checks for the code as-is are
perfect, as I don't know what checks have already been performed to make
sure that the calculation doesn't overflow an int64_t; but you would add
a new possibility for overflow.)

And your indentation is off: the second line (i.e.
bits_per_coded_sample) should be aligned with the first function
parameter in the first line (i.e. avctx->width).

- Andreas