[FFmpeg-devel] [PATCH 4/9] avfilter/af_flanger: use rint instead of floor hack

[Ganesh Ajjanagadde]

On Wed, Dec 2, 2015 at 7:39 AM, Paul B Mahol <onemda at gmail.com> wrote:
> On 12/2/15, Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>> On Wed, Dec 2, 2015 at 1:18 AM, Paul B Mahol <onemda at gmail.com> wrote:
>>> On 12/2/15, Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>>> ---
>>>>  libavfilter/af_flanger.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavfilter/af_flanger.c b/libavfilter/af_flanger.c
>>>> index f8ec830..a92367c 100644
>>>> --- a/libavfilter/af_flanger.c
>>>> +++ b/libavfilter/af_flanger.c
>>>> @@ -130,7 +130,7 @@ static int config_input(AVFilterLink *inlink)
>>>>          return AVERROR(ENOMEM);
>>>>
>>>>      ff_generate_wave_table(s->wave_shape, AV_SAMPLE_FMT_FLT, s->lfo,
>>>> s->lfo_length,
>>>> -                           floor(s->delay_min * inlink->sample_rate +
>>>> 0.5),
>>>> +                           rint(s->delay_min * inlink->sample_rate),
>>>>                             s->max_samples - 2., 3 * M_PI_2);
>>>>
>>>>      return av_samples_alloc_array_and_samples(&s->delay_buffer, NULL,
>>>> --
>>>> 2.6.2
>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel at ffmpeg.org
>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>
>>> Have you checked that output is same?
>>
>> Well if is not, rint is more accurate than floor, this is the whole
>> point of the patch. What I can tell is that FATE passes.
>>
>> One can craft input so that floor(x + 0.5) is not identical to
>> rint(x), and that is the point of these patches - to be more accurate
>> when we can be. A simple example: what happens at half-integers, i.e
>> 1.5? Then, floor always returns the next up, e.g 2.0, while rint(x)
>> rounds to the nearest even integer in accord with IEEE-754. This is
>> done to reduce rounding biases on floating point numbers - think of a
>> large number of half integer samples, the floor hack results in
>> consistent upward bias, the rint (or llrint, lrint more generally)
>> avoids this.
>>
>> I care about technical purity of filters; you seem to care about
>> copying it over from some other place and matching another filter
>> exactly, regardless of the quality of such filters. In that case, I
>> think FFmpeg's monolithic filter design needs to be reconsidered; we
>> should allow seamless integration of external filters. These two goals
>> are at odds with each other, and I will always personally prefer the
>> first, since it actually allows greater flexibility for improvements.
>> Ultimately, I am not a maintainer for these things and I have no say
>> on the matter or personal interest in it.
>>
>
> On 2nd look, patch should be fine.

pushed, thanks.