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

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 08:00:52AM -0800, Ganesh Ajjanagadde wrote:
> 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.

IMO its ok to revert ones own commits if they break something

its also ok to commit things that everyone is happy with
like for example if you forget a ";" adding it is perfectly ok without
patch, noone will be unhappy about that.
the more complex a patch is and the more close it is to other peoples
authorship/maitaince the more likely someone else will be unhappy

sending patches vs. directly commiting so that work and latency is
minimized and everyone is happy is not always trivial

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151227/4d371f11/attachment.sig>