[FFmpeg-devel] [PATCH 1/3] omadec: fix overflows during bit rate calculation

[Hendrik Leppkes]

On Wed, Dec 14, 2016 at 1:02 AM, Andreas Cadhalpun
<andreas.cadhalpun at googlemail.com> wrote:
> On 13.12.2016 08:11, Paul B Mahol wrote:
>> On 12/13/16, Andreas Cadhalpun <andreas.cadhalpun at googlemail.com> wrote:
>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>> ---
>>>  libavformat/omadec.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/omadec.c b/libavformat/omadec.c
>>> index 6e476db..e7751d0 100644
>>> --- a/libavformat/omadec.c
>>> +++ b/libavformat/omadec.c
>>> @@ -365,7 +365,7 @@ static int oma_read_header(AVFormatContext *s)
>>>          st->codecpar->channels    = 2;
>>>          st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO;
>>>          st->codecpar->sample_rate = samplerate;
>>> -        st->codecpar->bit_rate    = st->codecpar->sample_rate * framesize *
>>> 8 / 1024;
>>> +        st->codecpar->bit_rate    = st->codecpar->sample_rate * framesize /
>>> 128;
>>>
>>>          /* fake the ATRAC3 extradata
>>>           * (wav format, makes stream copy to wav work) */
>>> @@ -398,7 +398,7 @@ static int oma_read_header(AVFormatContext *s)
>>>              return AVERROR_INVALIDDATA;
>>>          }
>>>          st->codecpar->sample_rate = samplerate;
>>> -        st->codecpar->bit_rate    = samplerate * framesize * 8 / 2048;
>>> +        st->codecpar->bit_rate    = samplerate * framesize / 256;
>>>          avpriv_set_pts_info(st, 64, 1, samplerate);
>>>          break;
>>>      case OMA_CODECID_MP3:
>>
>> Shouldn't using 8LL or similar be more future-proof?
>
> Why multiply with 8 when dividing by a multiple of 8 directly afterwards?
> That's just a waste of computational resources.
> If sample_rate and/or framesize get larger in the future, a cast can be added.
>

The compiler should be able to optimize such things out and it may add
clarity to what the code is doing.

- Hendrik