[FFmpeg-devel] [PATCH] avfilter: add audio pulsator filter

[Paul B Mahol]

On 12/2/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> +static void lfo_advance(SimpleLFO *lfo, unsigned count)
>> +{
>> +    lfo->phase = fabs(lfo->phase + count * lfo->freq / lfo->srate);
>> +    if (lfo->phase >= 1)
>> +        lfo->phase = fmod(lfo->phase, 1);
>> +}
>> +
>> +static double lfo_get_value(SimpleLFO *lfo)
>> +{
>> +    double phs = FFMIN(100, lfo->phase / FFMIN(1.99, FFMAX(0.01,
>> lfo->pwidth)) + lfo->offset);
>> +    double val;
>> +
>> +    if (phs > 1)
>> +        phs = fmod(phs, 1.);
>> +
>> +    switch (lfo->mode) {
>> +    case SINE:
>> +        val = sin(phs * 2 * M_PI);
>> +        break;
>> +    case TRIANGLE:
>> +        if (phs > 0.75)
>> +            val = (phs - 0.75) * 4 - 1;
>> +        else if (phs > 0.5)
>> +            val = (phs - 0.5) * 4 * -1;
>> +        else if (phs > 0.25)
>> +            val = 1 - (phs - 0.25) * 4;
>> +        else
>> +            val = phs * 4;
>
> Why didn't you remove the useless branch and replace with a simple
> -4*phs + 2 for 0.25 to 0.75 case?
>
> I guess you did not remove the branch because you believed (but have
> not substantiated) that some notion of "bit-exactness" (highly dubious
> since floating point is not reproducible exactly across all platforms)
> with respect to some reference is violated. This "reference" is copied
> over from some other source code with no link whatsoever in the commit
> message or for the reviewer; only a simple copyright notice, making it
> impossible for a reviewer to do a proper review.
>
> [...]

In both cases the output is different and that is for s16 output case.