[FFmpeg-devel] [PATCH] avfilter: add arbitrary audio FIR filter

[Muhammad Faiz]

On Mon, May 8, 2017 at 3:43 PM, Paul B Mahol <onemda at gmail.com> wrote:
> On 5/8/17, Muhammad Faiz <mfcc64 at gmail.com> wrote:
>> On Mon, May 8, 2017 at 1:22 AM, Paul B Mahol <onemda at gmail.com> wrote:
>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>> ---
>>>  configure                |   2 +
>>>  doc/filters.texi         |  30 +++
>>>  libavfilter/Makefile     |   1 +
>>>  libavfilter/af_afir.c    | 541
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  libavfilter/allfilters.c |   1 +
>>>  5 files changed, 575 insertions(+)
>>>  create mode 100644 libavfilter/af_afir.c
>>>
>>> + at item auto
>>> +Enable auto gain calculation of Impulse Response coefficients.
>>> +By default is enabled.
>>> + at end table
>>> +
>>
>> Probably, these options aren't required if algo is correct.
>
> Which ones? All of them or just last one?

IMHO:
auto isn't required. The filter should compute normalization factor correctly.
envelope will change the frequency response.
I can't see the difference between dry and wet. Basically it can be
replaced with volume filter.
length can be replaced by atrim.

Probably length and dry/wet will be OK to avoid using atrim and volume filter.


>
>>> +    for (i = 0; i < s->nb_partitions; i++) {
>>> +        const int coffset = i * (s->part_size + 1);
>>> +
>>> +        for (n = 0; n < s->part_size; n++) {
>>> +            const float cre = coeff[coffset + n].re;
>>> +            const float cim = coeff[coffset + n].im;
>>> +            const float tre = block[2 * n    ];
>>> +            const float tim = block[2 * n + 1];
>>> +
>>> +            sum[2 * n    ] += tre * cre - tim * cim;
>>> +            sum[2 * n + 1] += tre * cim + tim * cre;
>>> +        }
>>> +        sum[2 * n] += block[2 * n] * coeff[coffset + n].re;
>>> +    }
>>
>> This is still wrong.
>> As I read in articles, each ir partition is convoluted with different
>> data block.
>
> Hmm, could you re-check it? The code I looked doesn't do that.

Based on the article:
Consider 4096-taps FIR partitioned to 4 x 1024-taps using 2048-fft and
block size is 1024, using OLS. Negative index means zero padding
block.
fft_result(0) = fft(block(-1), block(0))  * fft(fir(0), 0)
              + fft(block(-2), block(-1)) * fft(fir(1), 0)
              + fft(block(-3), block(-2)) * fft(fir(2), 0)
              + fft(block(-4), block(-3)) * fft(fir(3), 0)
then result(0) = right half of ifft(fft_result(0))

fft_result(1) = fft(block(0), block(1))   * fft(fir(0), 0)
              + fft(block(-1), block(0))  * fft(fir(1), 0)
              + ... and so on

>
>> Test:
>>
>> aevalsrc        = 'if(n, 0, 1)',
>> firequalizer    =
>>     delay       = 0.023:
>>     fixed       = on:
>>     wfunc       = nuttall:
>>     gain        = 'if(between(f, 1000, 5000), -INF, 0)',
>> atrim = end_sample = 2048 [ir];
>
> The size of IR is too short. If I increase it to 12048 I get desired output.
>

Actually fir is only 2029-taps, zero padded to 2048-taps.
Using atrim = end_sample = 2048:
nb_partitions: 2
partition_size: 1024
Here second partition contains non-zero coefficients.

Using atrim = end_sample = 4096:
nb_partitions: 2
partition_size: 2048
Here second partition contains all-zero coefficients, and the
incorrect behaviour is hidden.

Thank's.