Hi, On Mon, Mar 14, 2016 at 10:06 PM, Ganesh Ajjanagadde <gajja...@gmail.com> wrote:
> On Mon, Mar 14, 2016 at 8:56 AM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: > > Hi, > > > > On Sun, Mar 13, 2016 at 12:34 PM, Ganesh Ajjanagadde <gajja...@gmail.com > > > > wrote: > >> > >> On Sun, Mar 13, 2016 at 7:51 AM, Ronald S. Bultje <rsbul...@gmail.com> > >> wrote: > >> > Hi, > >> > > >> > On Sat, Mar 12, 2016 at 11:40 AM, Ganesh Ajjanagadde > >> > <gajja...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel