[FFmpeg-devel] [PATCHv2] lavc/aacenc_utils: replace powf(x, y) by expf(logf(x), y)
Ganesh Ajjanagadde
gajjanag at gmail.com
Tue Mar 15 03:06:01 CET 2016
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).
[...]
More information about the ffmpeg-devel
mailing list