On Thu, Nov 19, 2015 at 11:03 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Thu, Nov 19, 2015 at 10:53 AM, James Darnley <james.darn...@gmail.com>
> wrote:
>> On 2015-11-19 13:52, Ganesh Ajjanagadde wrote:
>>> diff --git a/libavfilter/af_dynaudnorm.c b/libavfilter/af_dynaudnorm.c
>>>> index 8f0c2d0..62a2653 100644
>>>> --- a/libavfilter/af_dynaudnorm.c
>>>> +++ b/libavfilter/af_dynaudnorm.c
>>>> @@ -227,8 +227,6 @@ static int cqueue_pop(cqueue *q)
>>>> return 0;
>>>> }
>>>>
>>>> -static const double s_pi =
>>>> 3.1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679;
>>>> -
>>>> static void init_gaussian_filter(DynamicAudioNormalizerContext *s)
>>>> {
>>>> double total_weight = 0.0;
>>>> @@ -238,7 +236,7 @@ static void
>>>> init_gaussian_filter(DynamicAudioNormalizerContext *s)
>>>>
>>>> // Pre-compute constants
>>>> const int offset = s->filter_size / 2;
>>>> - const double c1 = 1.0 / (sigma * sqrt(2.0 * s_pi));
>>>> + const double c1 = 1.0 / (sigma * sqrt(2.0 * M_PI));
>>>> const double c2 = 2.0 * pow(sigma, 2.0);
>>>>
>>
>> Paul asked you to drop this. I didn't see whether he replied that time
>> but when I asked why he didn't use M_PI when he first created the
>> filter, I got this reply.
>
> I never got the reply since unless I am mistaken, such a reply never
> came on the ML. It was in the absence of this info that I gave an
> additional day to give some benefit of doubt. Such replies belong on
> the ML IMHO, since they affect the review of the patch.
>
>>
>>>>> +static const double s_pi =
>>>>> 3.1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679;
>>>>
>>>> Why not use the standard M_PI define?
>>>
>>> Its too short. And I try hard to produce exact same output as
>>> reference implementation as possible, even if doubles are used.
>
> If it is too short, it is a bug with libm which is highly unlikely and
> needs to be substantiated with concrete evidence. Such evidence is
> easy to obtain via e.g av_double2int, or the hexadecimal format
> specifier (C99) for floats. The expansion here is of a ludicrous
> length and gives no value whatsoever.
Confirmed just now bit-exactness via the hex print:
0x1.921fb54442d18p+1
0x1.921fb54442d18p+1.
>
>>>
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
