sfstoolbox / sfs-matlab

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

Interpolation toa #101

Closed VeraE closed 8 years ago

VeraE commented 8 years ago

I propose a different interpolation method for HRIRs that calculates a linear interpolation in the frequency domain for magnitude and phase separately. This way, the time of arrival in the HRIRs is interpolated as it is to be expected. At the moment I added a new conf-parameter called conf.ir.interpolationmethod that can be 'simple' (= the old interpolation method samplewise in the time domain without considering the time of arrival) or 'freqdomain'. As the new method makes use of the FFT symmetries for real signals and interpolates only one half of the spectrum, it is only a bit slower than the old method (about 2-3% on my system). Should we still keep the old method? I could extend the new method to 2D interpolation as well.

fietew commented 8 years ago
  1. Does the unwrap of the phase really solve all the issues with the linear interpolation of the phase, i.e. does it consider all the "wrap around" effects w.r.t 2*pi?
  2. Maybe a reference for the algorithm should be added to the comments
fs446 commented 8 years ago

maybe this is inspiring:

http://iem.kug.ac.at/projects/workspace/2011/phasenabrollung-zur-interpolation-von-aussenohruebertragungsfunktionen-auf-der-kugel.html

On 28.06.2016 09:39, Fiete Winter wrote:

  1. Does the unwrap of the phase really solve all the issues with the linear interpolation of the phase, i.e. does it consider all the "wrap around" effects w.r.t 2*pi?
  2. Maybe a reference for the algorithm should be added to the comments

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/sfstoolbox/sfs/pull/101#issuecomment-228974956

VeraE commented 8 years ago

Thanks for your comments. I updated the interpolation so that phase aliasing is avoided and added references. The 2D interpolation for our case should work the same way but I would postpone that to when #76 is finished as I think this could affect the parameters that interpolate_ir() receives and possibly make the distinction in 1D and 2D unnecessary. Should the warning for backwards compatibility better be turned off? If this looks alright to you, I'll update the comment header.

hagenw commented 8 years ago

Looks good, thanks. Could you maybe create a function test_interpolation.m in the validation folder and add some code that compares the outcome of 'simple' with 'freqdomain'. Would be cool to have a nice comparison.

Regarding the warning for compatibility: I would decide on this at the end, if we see what are the differences etc.

hagenw commented 8 years ago

In SFS_config.m you are referencing the file test_interpolation_methods.m which should be placed in the validation folder, but this file does not exist. Could you add one?

VeraE commented 8 years ago

Sorry for the delay, I had to wait for publishing of the HRTFs with low frequency correction with a new license before I could finish the test file. I have added it now. Does this look alright to you?

hagenw commented 8 years ago

Cool, this looks nice now and I'm going to merge it.