sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
98 stars 39 forks source link

WFS FIR prefilter length hard-coded #52

Closed trettberg closed 8 years ago

trettberg commented 8 years ago

The number of taps for the WFS FIR prefilter is currently hard-coded in wfs_fir_prefilter() to Nfilt = 128. If one is interested e.g. in the delay introduced by WFS reproduction, the filter length appears as a magic number.

I'd suggest to store the filter length in conf.wfs, maybe as hpreFIRnTaps (N) or hpreFIROrder (N-1).

(Feel free to assign the issue to me.)

hagenw commented 8 years ago

That is a good idea and will make it more consistent. At the moment we have conf.wfs.hpreIIRorder, see in SFS_Config. This would suggest to name it conf.wfs.hpreFIRorder. The only downside is that for a non expert it is easier to understand if we specify the filter length in samples instead. But maybe we add just a line of comment, explaining what an order of e.g. 127 means.

There is another place where the actual hard coded filter length of 128 samples is considered, compare line 99 in sound_field_imp_wfs(), where the length is compensated for in order to have a consistent behavior of using the time t as input argument. The same holds for local WFS in sound_field_imp_localwfs().

Could you start to implement the changes in a new branch and create a pull request after pushing the branch? This would be nice.

If you are familiar with IIR, you might also have a look at #29.