[FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation

Ganesh Ajjanagadde gajjanag at mit.edu
Sun Dec 27 17:06:27 CET 2015


On Sun, Dec 27, 2015 at 7:44 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Sun, Dec 27, 2015 at 07:40:53AM -0500, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Sun, Dec 27, 2015 at 6:02 AM, Michael Niedermayer <michaelni at gmx.at>
>> wrote:
>>
>> > +    for (i = 0; i < 20; i++)
>> > +        c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32;
>> >      for (; i < 128; i++)
>> > +        c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01);
>> >
>>
>> To innocent bystanders, this is hard to understand. Let's keep it a habit
>> to document things (what and why), where the "what" portion is probably
>> "this code emulates pow(10, x) with ff_pow10(x - 0.01)". As for why (most
>> specifically, why the 0.01?), I'm going to assume that here, you're trying
>> to get unit integers to not go to unit.0000000[..]001, so you subtract 0.01
>> before the ceil so it works fine again to get the exact unit integer output
>> number. If that's correct, please add a comment saying that, and then lgtm.

In my view this change is not good for the exact reasons above. It is
a "magic constant" that is a pure hack. The commit I made made
accuracy more consistent across platforms (instead of relying on the
whims of libm's pow/exp2), and was commented so that intent was clear.
Nack from me.

>
> yes
> changed
> applied
> thx
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Concerning the gods, I have no means of knowing whether they exist or not
> or of what sort they may be, because of the obscurity of the subject, and
> the brevity of human life -- Protagoras
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list