[FFmpeg-devel] [PATCHv2] lavc/aacenc_utils: replace powf(x, y) by expf(logf(x), y)

[Ronald S. Bultje]

Hi,

On Mon, Mar 14, 2016 at 10:06 PM, Ganesh Ajjanagadde <gajjanag at gmail.com>
wrote:

> On Mon, Mar 14, 2016 at 8:56 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > Hi,
> >
> > On Sun, Mar 13, 2016 at 12:34 PM, Ganesh Ajjanagadde <gajjanag at gmail.com
> >
> > wrote:
> >>
> >> On Sun, Mar 13, 2016 at 7:51 AM, Ronald S. Bultje <rsbultje at gmail.com>
> >> wrote:
> >> > Hi,
> >> >
> >> > On Sat, Mar 12, 2016 at 11:40 AM, Ganesh Ajjanagadde
> >> > <gajjanag at gmail.com>
> >> > wrote:
> >> >>
> >> >> diff --git a/libavutil/internal.h b/libavutil/internal.h
> >> >> index da76ca2..aa43754 100644
> >> >> --- a/libavutil/internal.h
> >> >> +++ b/libavutil/internal.h
> >> >> @@ -315,6 +315,22 @@ static av_always_inline float ff_exp10f(float x)
> >> >>  }
> >> >>
> >> >>  /**
> >> >> + * Compute x^y for floating point x, y. Note: this function is
> faster
> >> >> than the
> >> >> + * libm variant due to mainly 2 reasons:
> >> >> + * 1. It does not handle any edge cases. In particular, this is only
> >> >> guaranteed
> >> >> + * to work correctly for x > 0.
> >> >> + * 2. It is not as accurate as a standard nearly "correctly rounded"
> >> >> libm
> >> >> variant.
> >> >> + * @param x base
> >> >> + * @param y exponent
> >> >> + * @return x^y
> >> >> + */
> >> >> +static av_always_inline float ff_fast_pow(float x, float y)
> >> >> +{
> >> >> +    return expf(logf(x) * y);
> >> >> +}
> >> >
> >> >
> >> > Thanks, mostly OK. Small comments:
> >> >
> >> > - I wonder if this should move to a separate file, e.g. it seems more
> >> > fitting in mathematics.h or libm.h. internal.h seems like a strange
> >> > choice.
> >> > I don't know which is better, I'd personally probably go for libm.h
> but
> >> > I
> >> > can see why some people wouldn't like it since it slightly changes the
> >> > meaning of that file.
> >>
> >> I don't like moving it to libm either for this reason.
> >>
> >> I chose lavu/internal for 2 reasons:
> >> 1. simply to group with other similar things, (ff_exp10, ff_exp10f)
> >> that also went into lavu/internal.
> >> 2. more fundamentally, it appears lavu/mathematics.h is a public
> >> header. I am quite strongly against making ff_exp10, ff_fast_powf etc
> >> public.
> >
> >
> > internal.h is quickly becoming a dumping ground for everything that
> doesn't
> > have a specific place yet.
> >
> > Let's create that place instead. exp10/f should probably go there also.
> If
> > we want libm.h and mathematics.h to be outside the realm, maybe create a
> new
> > ffmath.h or fastmath.h header? I probably prefer ffmath.h since that
> makes
> > the goal slightly broader.
>
> I like ffmath.h, nonetheless it is more convenient for me to move in a
> separate patch (submitted just now).


Ok, no further comments from me then. I'd wait up to 24hrs for other
comments and if none, feel free to push.

Ronald