[FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Ganesh Ajjanagadde]

On Sun, Dec 27, 2015 at 4:31 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Sat, Dec 26, 2015 at 8:23 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
> wrote:
>>
>> On Sat, Dec 26, 2015 at 4:20 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
>> wrote:
>> > On Sat, Dec 26, 2015 at 4:08 PM, James Almer <jamrial at gmail.com> wrote:
>> >> On 12/23/2015 3:47 PM, Ganesh Ajjanagadde wrote:
>> >>> exp10, introduced recently, is superior for the purpose.
>> >>>
>> >>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >>> ---
>> >>>  libavcodec/on2avc.c | 5 +++--
>> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>> >>> index 04c8e41..0409b3e 100644
>> >>> --- a/libavcodec/on2avc.c
>> >>> +++ b/libavcodec/on2avc.c
>> >>> @@ -22,6 +22,7 @@
>> >>>
>> >>>  #include "libavutil/channel_layout.h"
>> >>>  #include "libavutil/float_dsp.h"
>> >>> +#include "libavutil/libm.h"
>> >>>  #include "avcodec.h"
>> >>>  #include "bytestream.h"
>> >>>  #include "fft.h"
>> >>> @@ -934,9 +935,9 @@ static av_cold int
>> >>> on2avc_decode_init(AVCodecContext *avctx)
>> >>>                 "Stereo mode support is not good, patch is
>> >>> welcome\n");
>> >>>
>> >>>      for (i = 0; i < 20; i++)
>> >>> -        c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 16) / 32;
>> >>> +        c->scale_tab[i] = ceil(exp10(i * 0.1) * 16) / 32;
>> >>>      for (; i < 128; i++)
>> >>> -        c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 0.5);
>> >>> +        c->scale_tab[i] = ceil(exp10(i * 0.1) * 0.5);
>> >>>
>> >>>      if (avctx->sample_rate < 32000 || avctx->channels == 1)
>> >>>          memcpy(c->long_win, ff_on2avc_window_long_24000,
>> >>
>> >> This apparently broke ICC
>> >>
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226215846&slot=x86_64-linux-gnu-icc-2011.4.191
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226235348&slot=x86_64-linux-gnu-icc-2011_sp1.13.367
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226203729&slot=x86_64-archlinux-icc-2013
>> >
>> > Thanks for the report. A couple of questions:
>> > 1. Is there an easy way to acquire icc so that I can reproduce?
>> > GCC/Clang are perfectly fine with it.
>> > 2. Do you know what the ICC platforms use for exp2?
>> >
>> > In the absence of remarks and my own inability to fix this, I will
>> > revert tonight.
>> > BTW, please let me know the general policy for this kind of breakage:
>> > i.e, how quickly do such regressions need to be fixed.
>>
>> Different fix pushed that also speeds up things.
>
>
> You shouldn't push things without review...

I assumed this does not apply to things that were broken by commits
made by one self, and also assumed that regressions should be fixed
asap, proper fixes coming in later as Michael posted now.

Guess I was wrong here, noted this for the future. Thanks.

>
> Ronald